Merge pull request #551 from NYPL-Simplified/analytics-in-db

Moved analytics config into the database and added it to the admin interface

api/admin/controller.py

@@ -92,6 +92,9 @@
 from api.novelist import NoveListAPI
 from core.opds_import import MetadataWranglerOPDSLookup
 
+from api.google_analytics_provider import GoogleAnalyticsProvider
+from core.local_analytics_provider import LocalAnalyticsProvider
+
 def setup_admin_controllers(manager):
     """Set up all the controllers that will be used by the admin parts of the web app."""
     if not manager.testing:
@@ -1248,12 +1251,12 @@ def admin_auth_services(self):
         auth_service = ExternalIntegration.admin_authentication(self._db)
         if auth_service:
             if id and id != auth_service.id:
-                return MISSING_ADMIN_AUTH_SERVICE
+                return MISSING_SERVICE
             if protocol != auth_service.protocol:
                 return CANNOT_CHANGE_PROTOCOL
         else:
             if id:
-                return MISSING_ADMIN_AUTH_SERVICE
+                return MISSING_SERVICE
 
             if protocol:
                 auth_service, is_new = get_one_or_create(
@@ -1366,7 +1369,7 @@ def patron_auth_services(self):
         if id:
             auth_service = get_one(self._db, ExternalIntegration, id=id, goal=ExternalIntegration.PATRON_AUTH_GOAL)
             if not auth_service:
-                return MISSING_PATRON_AUTH_SERVICE
+                return MISSING_SERVICE
             if protocol != auth_service.protocol:
                 return CANNOT_CHANGE_PROTOCOL
         else:
@@ -1480,7 +1483,7 @@ def metadata_services(self):
         if id:
             service = get_one(self._db, ExternalIntegration, id=id, goal=ExternalIntegration.METADATA_GOAL)
             if not service:
-                return MISSING_METADATA_SERVICE
+                return MISSING_SERVICE
             if protocol != service.protocol:
                 return CANNOT_CHANGE_PROTOCOL
         else:
@@ -1511,3 +1514,85 @@ def metadata_services(self):
         else:
             return Response(unicode(_("Success")), 200)
 
+    def analytics_services(self):
+        provider_apis = [GoogleAnalyticsProvider,
+                         LocalAnalyticsProvider,
+                        ]
+        protocols = self._get_integration_protocols(provider_apis)
+
+        if flask.request.method == 'GET':
+            services = []
+            for service in self._db.query(ExternalIntegration).filter(
+                ExternalIntegration.goal==ExternalIntegration.ANALYTICS_GOAL):
+
+                [protocol] = [p for p in protocols if p.get("name") == service.protocol]
+                libraries = []
+                for library in service.libraries:
+                    library_info = dict(short_name=library.short_name)
+                    for setting in protocol.get("library_settings", []):
+                        key = setting.get("key")
+                        value = ConfigurationSetting.for_library_and_externalintegration(
+                            self._db, key, library, service
+                        ).value
+                        if value:
+                            library_info[key] = value
+                    libraries.append(library_info)
+
+                settings = { setting.key: setting.value for setting in service.settings if setting.library_id == None}
+
+                services.append(
+                    dict(
+                        id=service.id,
+                        name=service.name,
+                        protocol=service.protocol,
+                        settings=settings,
+                        libraries=libraries,
+                    )
+                )
+
+            return dict(
+                analytics_services=services,
+                protocols=protocols,
+            )
+
+        id = flask.request.form.get("id")
+
+        protocol = flask.request.form.get("protocol")
+        if protocol and protocol not in [p.get("name") for p in protocols]:
+            return UNKNOWN_PROTOCOL
+
+        is_new = False
+        if id:
+            service = get_one(self._db, ExternalIntegration, id=id, goal=ExternalIntegration.ANALYTICS_GOAL)
+            if not service:
+                return MISSING_SERVICE
+            if protocol != service.protocol:
+                return CANNOT_CHANGE_PROTOCOL
+        else:
+            if protocol:
+                service, is_new = create(
+                    self._db, ExternalIntegration, protocol=protocol,
+                    goal=ExternalIntegration.ANALYTICS_GOAL
+                )
+            else:
+                return NO_PROTOCOL_FOR_NEW_SERVICE
+
+        name = flask.request.form.get("name")
+        if name:
+            if service.name != name:
+                service_with_name = get_one(self._db, ExternalIntegration, name=name)
+                if service_with_name:
+                    self._db.rollback()
+                    return INTEGRATION_NAME_ALREADY_IN_USE
+            service.name = name
+
+        [protocol] = [p for p in protocols if p.get("name") == protocol]
+        result = self._set_integration_settings_and_libraries(service, protocol)
+        if isinstance(result, ProblemDetail):
+            return result
+
+        if is_new:
+            return Response(unicode(_("Success")), 201)
+        else:
+            return Response(unicode(_("Success")), 200)
+

api/admin/package.json

@@ -9,6 +9,6 @@
   "author": "NYPL",
   "license": "Apache-2.0",
   "dependencies": {
-    "simplified-circulation-web": "0.0.26"
+    "simplified-circulation-web": "0.0.27"
   }
 }

api/admin/problem_details.py

@@ -198,18 +198,11 @@
     detail=_("You tried to store a password for an individual admin, but the database does not have the pgcrypto extension installed."),
 )
 
-MISSING_ADMIN_AUTH_SERVICE = pd(
-    "http://librarysimplified.org/terms/problem/missing-admin-auth-service",
+MISSING_SERVICE = pd(
+    "http://librarysimplified.org/terms/problem/missing-service",
     status_code=404,
-    title=_("Missing admin authentication service"),
-    detail=_("The specified admin authentication service does not exist."),
-)
-
-MISSING_PATRON_AUTH_SERVICE = pd(
-    "http://librarysimplified.org/terms/problem/missing-patron-auth-service",
-    status_code=404,
-    title=_("Missing patron authentication service"),
-    detail=_("The specified patron authentication service does not exist."),
+    title=_("Missing service"),
+    detail=_("The specified service does not exist."),
 )
 
 INVALID_CONFIGURATION_OPTION = pd(
@@ -246,10 +239,3 @@
     title=_("Missing sitewide setting value"),
     detail=_("A value is required to change a sitewide setting."),
 )
-
-MISSING_METADATA_SERVICE = pd(
-    "http://librarysimplified.org/terms/problem/missing-metadata-service",
-    status_code=404,
-    title=_("Missing metadata service"),
-    detail=_("The specified metadata service does not exist."),
-)

api/admin/routes.py

@@ -351,6 +351,18 @@ def metadata_services():
         return data
     return flask.jsonify(**data)
 
+@app.route("/admin/analytics_services", methods=['GET', 'POST'])
+@returns_problem_detail
+@requires_admin
+@requires_csrf_token
+def analytics_services():
+    data = app.manager.admin_settings_controller.analytics_services()
+    if isinstance(data, ProblemDetail):
+        return data
+    if isinstance(data, Response):
+        return data
+    return flask.jsonify(**data)
+
 @app.route("/admin/sitewide_settings", methods=['GET', 'POST'])
 @returns_problem_detail
 @requires_admin
@@ -383,6 +395,7 @@ def admin_sign_in_again():
 @app.route('/admin/web/<path:etc>') # catchall for single-page URLs
 def admin_view(collection=None, book=None, **kwargs):
     setting_up = (app.manager.admin_sign_in_controller.auth == None)
+    home_url = None
     if not setting_up:
         admin = app.manager.admin_sign_in_controller.authenticated_admin_from_request()
         if isinstance(admin, ProblemDetail):
@@ -404,8 +417,6 @@ def admin_view(collection=None, book=None, **kwargs):
         if libraries:
             library = libraries[0]
             home_url = app.manager.url_for('acquisition_groups', library_short_name=library.short_name)
-        else:
-            home_url = None
 
     csrf_token = flask.request.cookies.get("csrf_token") or app.manager.admin_sign_in_controller.generate_csrf_token()
 

api/authenticator.py

@@ -20,7 +20,6 @@
     json as pd_json,
 )
 from core.util.opds_authentication_document import OPDSAuthenticationDocument
-from core.analytics import Analytics
 from sqlalchemy.ext.hybrid import hybrid_property
 from problem_details import *
 from util.patron import PatronUtility
@@ -247,7 +246,7 @@ def set_value(self, patron, field_name, value):
             value = None
         setattr(patron, field_name, value)
 
-    def get_or_create_patron(self, _db, library_id):
+    def get_or_create_patron(self, _db, library_id, analytics=None):
         """Create a Patron with this information.
 
         TODO: I'm concerned in the general case with race
@@ -272,6 +271,9 @@ def get_or_create_patron(self, _db, library_id):
 
         :param library_id: Database ID of the Library with which this
             patron is associated.
+
+        :param analytics: Analytics instance to track the new patron
+            creation event.
         """
 
         # We must be very careful when checking whether the patron
@@ -293,10 +295,10 @@ def get_or_create_patron(self, _db, library_id):
         __transaction = _db.begin_nested()
         patron, is_new = get_one_or_create(_db, Patron, **search_by)
 
-        if is_new:
+        if is_new and analytics:
             # Send out an analytics event to record the fact
             # that a new patron was created.
-            Analytics.collect_event(_db, None,
+            analytics.collect_event(patron.library, None,
                                     CirculationEvent.NEW_PATRON)
 
         # This makes sure the Patron is brought into sync with the
@@ -344,11 +346,11 @@ class Authenticator(object):
     """Route requests to the appropriate LibraryAuthenticator.
     """
 
-    def __init__(self, _db):
+    def __init__(self, _db, analytics=None):
         self.library_authenticators = {}
 
         for library in _db.query(Library):
-            self.library_authenticators[library.short_name] = LibraryAuthenticator.from_config(_db, library)
+            self.library_authenticators[library.short_name] = LibraryAuthenticator.from_config(_db, library, analytics)
 
     def invoke_authenticator_method(self, method_name, *args, **kwargs):
         short_name = flask.request.library.short_name
@@ -374,7 +376,7 @@ class LibraryAuthenticator(object):
     """    
 
     @classmethod
-    def from_config(cls, _db, library):
+    def from_config(cls, _db, library, analytics=None):
         """Initialize an Authenticator for the given Library based on its
         configured ExternalIntegrations.
         """
@@ -393,7 +395,7 @@ def from_config(cls, _db, library):
         # AuthenticationProvider.
         for integration in integrations:
             try:
-                authenticator.register_provider(integration)
+                authenticator.register_provider(integration, analytics)
             except (ImportError, CannotLoadConfiguration), e:
                 # These are the two types of error that might be caused
                 # by misconfiguration, as opposed to bad code.
@@ -459,7 +461,7 @@ def assert_ready_for_oauth(self):
                 "OAuth providers are configured, but secret for signing bearer tokens is not."
             )
 
-    def register_provider(self, integration):
+    def register_provider(self, integration, analytics=None):
         """Turn an ExternalIntegration object into an AuthenticationProvider
         object, and register it.
 
@@ -489,7 +491,7 @@ def register_provider(self, integration):
             raise CannotLoadConfiguration(
                 "Loaded module %s but could not find a class called AuthenticationProvider inside." % module_name
             )
-        provider = provider_class(self.library, integration)
+        provider = provider_class(self.library, integration, analytics)
         if issubclass(provider_class, BasicAuthenticationProvider):
             self.register_basic_auth_provider(provider)
             # TODO: Run a self-test, or at least check that we have
@@ -742,7 +744,7 @@ class AuthenticationProvider(object):
         }
     ]
 
-    def __init__(self, library, integration):
+    def __init__(self, library, integration, analytics=None):
         """Basic constructor.
         
         :param library: Patrons authenticated through this provider
@@ -766,6 +768,7 @@ def __init__(self, library, integration):
 
         self.library_id = library.id
         self.log = logging.getLogger(self.NAME)
+        self.analytics = analytics
         # If there's a regular expression that maps authorization
         # identifier to external type, find it now.
         _db = Session.object_session(library)
@@ -1025,7 +1028,7 @@ class BasicAuthenticationProvider(AuthenticationProvider):
     # indicates that no value should be used.)
     class_default = object()
 
-    def __init__(self, library, integration):
+    def __init__(self, library, integration, analytics=None):
         """Create a BasicAuthenticationProvider.
 
         :param library: Patrons authenticated through this provider
@@ -1038,7 +1041,7 @@ def __init__(self, library, integration):
         object! It's associated with a scoped database session. Just
         pull normal Python objects out of it.
         """
-        super(BasicAuthenticationProvider, self).__init__(library, integration)
+        super(BasicAuthenticationProvider, self).__init__(library, integration, analytics)
         identifier_regular_expression = integration.setting(
             self.IDENTIFIER_REGULAR_EXPRESSION
         ).value or self.DEFAULT_IDENTIFIER_REGULAR_EXPRESSION
@@ -1147,7 +1150,7 @@ def authenticate(self, _db, credentials):
             # We have a PatronData from the ILS that does not
             # correspond to any local Patron. Create the local Patron.
             patron, is_new = patrondata.get_or_create_patron(
-                _db, self.library_id
+                _db, self.library_id, analytics=self.analytics
             )
 
         # The lookup failed in the first place either because the
@@ -1358,7 +1361,7 @@ def bearer_token_signing_secret(cls, _db):
             _db, cls.BEARER_TOKEN_SIGNING_SECRET
         )
 
-    def __init__(self, library, integration):
+    def __init__(self, library, integration, analytics=None):
         """Initialize this OAuthAuthenticationProvider.
 
         :param library: Patrons authenticated through this provider
@@ -1378,7 +1381,7 @@ def __init__(self, library, integration):
             process again.
         """
         super(OAuthAuthenticationProvider, self).__init__(
-            library, integration
+            library, integration, analytics
         )
         self.client_id = integration.username
         self.client_secret = integration.password
@@ -1496,7 +1499,7 @@ def oauth_callback(self, _db, code):
 
         # Convert the PatronData into a Patron object.
         patron, is_new = patrondata.get_or_create_patron(
-            _db, self.library_id
+            _db, self.library_id, analytics=self.analytics
         )
 
         # Create a credential for the Patron.