Skip to content

Commit

Permalink
Merge pull request #410 from appsembler/req_site_backend
Browse files Browse the repository at this point in the history
get_requested_site: configurable backend to get the site for an API call
  • Loading branch information
OmarIthawi authored Jan 27, 2022
2 parents 596df83 + 6ececda commit d8c76f9
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 32 deletions.
55 changes: 55 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,61 @@ From the `figures` repository root directory:

If all goes well, the Figures unit tests will all complete succesfully


-------------
Configuration
-------------

Figures can be configured via Django settings' ``FIGURES`` key. Open edX reads configuration from
the ``/edx/etc/lms.yml`` file both in devstack and production servers. In releases before Juniper it
was the ``lms.env.json`` file.

A Figures configuration may look like the following:


::

FEATURES: # The standard Open edX feature flags
# ... other features goes here ...
FIGURES_IS_MULTISITE: True
# ... more features goes there ...

FIGURES: # Other Figures configurations
SITES_BACKEND: 'openedx.core.djangoapps.appsembler.sites.utils:get_active_sites'
REQUESTED_SITE_BACKEND: 'tahoe_figures_plugins.sites:get_current_site_or_by_uuid'
FIGURES_PIPELINE_TASKS_ROUTING_KEY: 'edx.lms.core.high'
DAILY_METRICS_IMPORT_HOUR: 13
DAILY_METRICS_IMPORT_MINUTE: 0


Settings like ``SITES_BACKEND`` require a path to a Python function or class. The path is consists of two parts:
a Python module e.g. ``my_plugin_package.helpers`` and an object e.g ``my_helper`` separated by a colon e.g.
``my_plugin_package.helpers:my_helper``.

This object would be imported by the ``import_from_path`` helper in the
`figures/helpers.py <https://github.com/appsembler/figures/blob/932eeab84c469a34dfcb94232bbe6f7c08146b3f/figures/helpers.py#L84-L98>`__ module.

.....................
Configuration options
.....................


* ``FEATURES.FIGURES_IS_MULTISITE`` (default ``False``): Boolean feature flag to run Figures in a single-site mode by
default (when set to ``False``) most popular Open edX installation option.
The multisite mode requires a custom ``edx-organizations`` fork that is used for
Appsembler Tahoe clusters.

* ``FIGURES.SITES_BACKEND`` (default ``None``): A Python path to function to list figures sites.
For example, this is useful to customize which sites are processed and which are considered inactive.
By default (when ``None`` is used) all sites are listed in the multi-site mode.

* ``REQUESTED_SITE_BACKEND`` (default ``None``): Python path to a function that gets the current site.
For example it can be used for API purposes to pass a Site ID to get a different site.
By default (when ``None`` is used) the Django's ``get_current_site()`` helper is used.


**TBD:** Document ``FIGURES_PIPELINE_TASKS_ROUTING_KEY``, ``DAILY_METRICS_IMPORT_HOUR`` and ``DAILY_METRICS_IMPORT_MINUTE``.

------
Future
------
Expand Down
19 changes: 19 additions & 0 deletions figures/sites.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

from __future__ import absolute_import
from django.contrib.auth import get_user_model
from django.contrib.sites import shortcuts as sites_shortcuts
from django.contrib.sites.models import Site
from django.conf import settings

Expand Down Expand Up @@ -304,6 +305,24 @@ def _get_all_sites():
return Site.objects.all()


def get_requested_site(request):
"""
From a request return the corresponding site.
This functions makes use of the `REQUESTED_SITE_BACKEND` setting if configured, otherwise
it defaults to Django's get_current_site().
:return Site
"""
backend_path = settings.ENV_TOKENS['FIGURES'].get('REQUESTED_SITE_BACKEND')
if backend_path:
requested_site_backend = import_from_path(backend_path)
requested_site = requested_site_backend(request)
else:
requested_site = sites_shortcuts.get_current_site(request)
return requested_site


def get_sites():
"""
Get a list of sites for Figures purposes in a configurable manner.
Expand Down
63 changes: 31 additions & 32 deletions figures/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from django.conf import settings
from django.contrib.auth import get_user_model
from django.contrib.auth.decorators import login_required, user_passes_test
import django.contrib.sites.shortcuts
from django.contrib.sites.models import Site
from django.http import HttpResponseRedirect
from django.shortcuts import get_object_or_404, render
Expand Down Expand Up @@ -170,7 +169,7 @@ class CourseOverviewViewSet(CommonAuthMixin, viewsets.ReadOnlyModelViewSet):
lookup_value_regex = settings.COURSE_ID_PATTERN

def get_queryset(self):
site = django.contrib.sites.shortcuts.get_current_site(self.request)
site = figures.sites.get_requested_site(self.request)
queryset = figures.sites.get_courses_for_site(site)
return queryset

Expand All @@ -194,7 +193,7 @@ def get_course_key(self, course_id_str):
def retrieve(self, request, *args, **kwargs):
course_key = self.get_course_key(
kwargs.get('pk', ''))
site = django.contrib.sites.shortcuts.get_current_site(request)
site = figures.sites.get_requested_site(self.request)
if figures.helpers.is_multisite():
if site != figures.sites.get_site_for_course(course_key):
# Raising NotFound instead of PermissionDenied
Expand Down Expand Up @@ -243,7 +242,7 @@ class UserIndexViewSet(CommonAuthMixin, viewsets.ReadOnlyModelViewSet):
filter_class = UserFilterSet

def get_queryset(self):
site = django.contrib.sites.shortcuts.get_current_site(self.request)
site = figures.sites.get_requested_site(self.request)
queryset = figures.sites.get_users_for_site(site)
return queryset

Expand All @@ -256,7 +255,7 @@ class CourseEnrollmentViewSet(CommonAuthMixin, viewsets.ReadOnlyModelViewSet):
filter_class = CourseEnrollmentFilter

def get_queryset(self):
site = django.contrib.sites.shortcuts.get_current_site(self.request)
site = figures.sites.get_requested_site(self.request)
queryset = figures.sites.get_course_enrollments_for_site(site)
return queryset

Expand All @@ -270,7 +269,7 @@ class CourseDailyMetricsViewSet(CommonAuthMixin, viewsets.ModelViewSet):
filter_class = CourseDailyMetricsFilter

def get_queryset(self):
site = django.contrib.sites.shortcuts.get_current_site(self.request)
site = figures.sites.get_requested_site(self.request)
queryset = CourseDailyMetrics.objects.filter(site=site)
return queryset

Expand All @@ -284,7 +283,7 @@ class SiteDailyMetricsViewSet(CommonAuthMixin, viewsets.ModelViewSet):
filter_class = SiteDailyMetricsFilter

def get_queryset(self):
site = django.contrib.sites.shortcuts.get_current_site(self.request)
site = figures.sites.get_requested_site(self.request)
queryset = SiteDailyMetrics.objects.filter(site=site)
return queryset

Expand Down Expand Up @@ -316,7 +315,7 @@ def get(self, request, format=None): # pylint: disable=redefined-builtin
'''
Does not yet support multi-tenancy
'''
site = django.contrib.sites.shortcuts.get_current_site(request)
site = figures.sites.get_requested_site(request)
date_for = request.query_params.get('date_for')
data = self.metrics_method(site=site, date_for=date_for)

Expand Down Expand Up @@ -348,7 +347,7 @@ class GeneralUserDataViewSet(CommonAuthMixin, viewsets.ReadOnlyModelViewSet):
ordering_fields = ['username', 'email', 'profile__name', 'is_active', 'date_joined']

def get_queryset(self):
site = django.contrib.sites.shortcuts.get_current_site(self.request)
site = figures.sites.get_requested_site(self.request)
queryset = figures.sites.get_users_for_site(site)
return queryset

Expand All @@ -361,13 +360,13 @@ class LearnerDetailsViewSet(CommonAuthMixin, viewsets.ReadOnlyModelViewSet):
filter_class = UserFilterSet

def get_queryset(self):
site = django.contrib.sites.shortcuts.get_current_site(self.request)
site = figures.sites.get_requested_site(self.request)
queryset = figures.sites.get_users_for_site(site)
return queryset

def get_serializer_context(self):
context = super(LearnerDetailsViewSet, self).get_serializer_context()
context['site'] = django.contrib.sites.shortcuts.get_current_site(self.request)
context['site'] = figures.sites.get_requested_site(self.request)
return context


Expand Down Expand Up @@ -426,7 +425,7 @@ def get_queryset(self):
* If no valid course keys are found, then an empty list is returned from
this view
"""
site = django.contrib.sites.shortcuts.get_current_site(self.request)
site = figures.sites.get_requested_site(self.request)
course_keys = figures.sites.get_course_keys_for_site(site)
try:
param_course_keys = self.query_param_course_keys()
Expand All @@ -441,7 +440,7 @@ def get_queryset(self):

def get_serializer_context(self):
context = super(LearnerMetricsViewSetV1, self).get_serializer_context()
context['site'] = django.contrib.sites.shortcuts.get_current_site(self.request)
context['site'] = figures.sites.get_requested_site(self.request)
context['course_keys'] = self.query_param_course_keys()
return context

Expand Down Expand Up @@ -493,13 +492,13 @@ def get_queryset(self):
* If no valid course keys are found, then an empty list is returned from
this view
"""
site = django.contrib.sites.shortcuts.get_current_site(self.request)
site = figures.sites.get_requested_site(self.request)
course_ids = self.query_param_course_ids()
return site_users_enrollment_data(site=site, course_ids=course_ids)

def get_serializer_context(self):
context = super(LearnerMetricsViewSetV2, self).get_serializer_context()
context['site'] = django.contrib.sites.shortcuts.get_current_site(self.request)
context['site'] = figures.sites.get_requested_site(self.request)
context['course_ids'] = self.query_param_course_ids()
return context

Expand All @@ -521,7 +520,7 @@ class EnrollmentMetricsViewSet(CommonAuthMixin, viewsets.ReadOnlyModelViewSet):
filter_class = EnrollmentMetricsFilter

def get_queryset(self):
site = django.contrib.sites.shortcuts.get_current_site(self.request)
site = figures.sites.get_requested_site(self.request)
queryset = LearnerCourseGradeMetrics.objects.filter(site=site)
return queryset

Expand All @@ -534,7 +533,7 @@ def completed_ids(self, request):
The default router does not support hyphen in the custom action, so
we need to use the underscore until we implement a custom router
"""
site = django.contrib.sites.shortcuts.get_current_site(request)
site = figures.sites.get_requested_site(request)
qs = self.model.objects.completed_ids_for_site(site=site)
page = self.paginate_queryset(qs)
if page is not None:
Expand All @@ -552,7 +551,7 @@ def completed(self, request):
Return matching LearnerCourseGradeMetric rows that have completed
enrollments
"""
site = django.contrib.sites.shortcuts.get_current_site(request)
site = figures.sites.get_requested_site(request)
qs = self.model.objects.completed_for_site(site=site)
page = self.paginate_queryset(qs)
if page is not None:
Expand Down Expand Up @@ -582,7 +581,7 @@ def site_course_helper(self, pk):
except InvalidKeyError:
raise NotFound()

site = django.contrib.sites.shortcuts.get_current_site(self.request)
site = figures.sites.get_requested_site(self.request)
if figures.helpers.is_multisite():
if site != figures.sites.get_site_for_course(course_id):
raise NotFound()
Expand All @@ -609,7 +608,7 @@ def list(self, request):
TODO: NEXT Add query params to get data from previous months
TODO: Add paginagation
"""
site = django.contrib.sites.shortcuts.get_current_site(request)
site = figures.sites.get_requested_site(request)
course_keys = figures.sites.get_course_keys_for_site(site)
date_for = datetime.utcnow().date()
month_for = '{}/{}'.format(date_for.month, date_for.year)
Expand Down Expand Up @@ -711,13 +710,13 @@ def list(self, request):
Returns site metrics data for current month
"""

site = django.contrib.sites.shortcuts.get_current_site(request)
site = figures.sites.get_requested_site(request)
data = metrics.get_current_month_site_metrics(site)
return Response(data)

@list_route()
def registered_users(self, request):
site = django.contrib.sites.shortcuts.get_current_site(request)
site = figures.sites.get_requested_site(request)
date_for = datetime.utcnow().date()
months_back = 6

Expand All @@ -735,7 +734,7 @@ def new_users(self, request):
"""
TODO: Rename the metrics module function to "new_users" to match this
"""
site = django.contrib.sites.shortcuts.get_current_site(request)
site = figures.sites.get_requested_site(request)
date_for = datetime.utcnow().date()
months_back = 6

Expand All @@ -750,7 +749,7 @@ def new_users(self, request):

@list_route()
def course_completions(self, request):
site = django.contrib.sites.shortcuts.get_current_site(request)
site = figures.sites.get_requested_site(request)
date_for = datetime.utcnow().date()
months_back = 6

Expand All @@ -765,7 +764,7 @@ def course_completions(self, request):

@list_route()
def course_enrollments(self, request):
site = django.contrib.sites.shortcuts.get_current_site(request)
site = figures.sites.get_requested_site(request)
date_for = datetime.utcnow().date()
months_back = 6

Expand All @@ -780,7 +779,7 @@ def course_enrollments(self, request):

@list_route()
def site_courses(self, request):
site = django.contrib.sites.shortcuts.get_current_site(request)
site = figures.sites.get_requested_site(request)
date_for = datetime.utcnow().date()
months_back = 6

Expand All @@ -795,7 +794,7 @@ def site_courses(self, request):

@list_route()
def active_users(self, request):
site = django.contrib.sites.shortcuts.get_current_site(request)
site = figures.sites.get_requested_site(request)
months_back = 6
active_users = metrics.get_site_mau_history_metrics(site=site,
months_back=months_back)
Expand All @@ -816,7 +815,7 @@ def get_queryset(self):
def retrieve(self, request, **kwargs):
course_id_str = kwargs.get('pk', '')
course_key = CourseKey.from_string(course_id_str.replace(' ', '+'))
site = django.contrib.sites.shortcuts.get_current_site(request)
site = figures.sites.get_requested_site(request)

if figures.helpers.is_multisite():
if site != figures.sites.get_site_for_course(course_key):
Expand All @@ -827,7 +826,7 @@ def retrieve(self, request, **kwargs):
return Response(serializer.data)

def list(self, request):
site = django.contrib.sites.shortcuts.get_current_site(request)
site = figures.sites.get_requested_site(request)
course_overviews = figures.sites.get_courses_for_site(site)
data = []
for co in course_overviews:
Expand Down Expand Up @@ -858,7 +857,7 @@ def list(self, request):
We use list instead of retrieve because retrieve requires a resource
identifier, like a PK
"""
site = django.contrib.sites.shortcuts.get_current_site(request)
site = figures.sites.get_requested_site(request)
data = retrieve_live_site_mau_data(site)
serializer = self.serializer_class(data)
return Response(serializer.data)
Expand All @@ -872,7 +871,7 @@ class CourseMauMetricsViewSet(CommonAuthMixin, viewsets.ReadOnlyModelViewSet):
lookup_value_regex = settings.COURSE_ID_PATTERN

def get_queryset(self):
site = django.contrib.sites.shortcuts.get_current_site(self.request)
site = figures.sites.get_requested_site(self.request)
queryset = CourseMauMetrics.objects.filter(site=site)
return queryset

Expand All @@ -885,7 +884,7 @@ class SiteMauMetricsViewSet(CommonAuthMixin, viewsets.ReadOnlyModelViewSet):
filter_class = SiteMauMetricsFilter

def get_queryset(self):
site = django.contrib.sites.shortcuts.get_current_site(self.request)
site = figures.sites.get_requested_site(self.request)
queryset = SiteMauMetrics.objects.filter(site=site)
return queryset

Expand Down
Loading

0 comments on commit d8c76f9

Please sign in to comment.