Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Jun 29, 2017
93 changes: 89 additions & 4 deletions api/admin/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)

2 changes: 1 addition & 1 deletion api/admin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
"author": "NYPL",
"license": "Apache-2.0",
"dependencies": {
"simplified-circulation-web": "0.0.26"
"simplified-circulation-web": "0.0.27"
}
}
22 changes: 4 additions & 18 deletions api/admin/problem_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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."),
)
12 changes: 12 additions & 0 deletions api/admin/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 20 additions & 17 deletions api/authenticator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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.
"""
Expand All @@ -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.
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down
Loading