John/merge master to juniper upgrade

.travis.yml

@@ -20,6 +20,7 @@ script:
   - tox
 
 env:
+  - TOXENV=py27-ginkgo
   - TOXENV=py27-hawthorn_community
   - TOXENV=py27-hawthorn_multisite
   - TOXENV=lint

README.rst

@@ -8,6 +8,35 @@ Reporting and data retrieval app for `Open edX <https://open.edx.org/>`__.
 
 .. _notice_section:
 
+26 Oct 2020 - Figures release 0.3.18
+====================================
+
+* FIX - Removed dependency on 'packaging.versions'
+
+  * https://github.com/appsembler/figures/pull/272
+  * NOTE: This PR updates a previous commit that required the `packaging` package
+
+
+16 Oct 2020 - Figures release 0.3.17
+====================================
+
+* Reworked SiteMonthlyMetrics registered users metric. This was causing the `/figures/api/site-monthly-metrics/registered_users` endpoint to timeout with a 500 error
+
+  * https://github.com/appsembler/figures/pull/268
+
+* Fixed Ginkgo (Django Filter 0.11.0) Backward compatibility issues
+
+  * https://github.com/appsembler/figures/pull/266
+  * https://github.com/appsembler/figures/pull/269
+
+* UI Bug fix: Add success feedback to csv export dialog
+
+  * https://github.com/appsembler/figures/pull/265
+
+* Bump http-proxy from 1.18.0 to 1.18.1 in /frontend
+
+  * https://github.com/appsembler/figures/pull/254
+
 
 28 Sep 2020 - Figures release 0.3.16
 ====================================

figures/filters.py

@@ -19,7 +19,6 @@
 """
 
 from __future__ import absolute_import
-from packaging import version
 from django.contrib.auth import get_user_model
 from django.contrib.sites.models import Site
 from django.db.models import F
@@ -42,13 +41,104 @@
 )
 
 
-def char_method_filter(method):
+def hack_get_version(version_string):
+    """Get the parsed version string as a list of ints [major, minor, point]
+
+    This works with packages that use only numbers in the format of
+    "major.minor.point"
+
+    This is hopefully a temporary hack. We cannot expect the Open edX deployment
+    to have `packaging` installed, so the original implementation here which
+    used `packaging.versions` to check the version is not supported without
+    requiring the extra step of installing `packaging`
     """
-    "method" is the method name string
-    First check for old style (pre  version 1 Django Filters)
+    return [int(val) for val in version_string.split('.')]
+
+
+DJANGO_FILTERS_VERSION = hack_get_version(django_filters.__version__)
+
+
+def char_filter(field_name, lookup_expr):
+    """For backwards compatibility.
+
+    We require both `field_name` and `lookup_expr` to minimize the work this
+    function needs to do by not needing to conditionally check for the
+    `field_name` parameter.
+
+    Adapted from this PR:
+    https://github.com/appsembler/figures/pull/264/files#diff-ccfc20c64a04dae3fe94285d727a3aa2R79
+
+    And we'll need to replace the code in PR 264 with this function
     """
-    if hasattr(django_filters, 'MethodFilter'):
-        return django_filters.MethodFilter(action=method)  # pylint: disable=no-member
+    if DJANGO_FILTERS_VERSION[0] < 1:
+        return django_filters.CharFilter(name=field_name, lookup_type=lookup_expr)
+    elif DJANGO_FILTERS_VERSION[0] < 2:
+        return django_filters.CharFilter(name=field_name, lookup_expr=lookup_expr)
+    else:
+        return django_filters.CharFilter(field_name=field_name, lookup_expr=lookup_expr)
+
+
+def char_method_filter(method):
+    """This function exists to address breaking changes in Django Filter
+
+    Parameters:
+        "method" is the method name string.
+
+    First check for old style (pre  version 1 Django Filter) to see if the
+    `MethodFilter` class exists.
+
+    IIf so, use that, else use `CharFilter`
+
+    First, check if Django Filter is pre 1.0. If so then use our custom method
+    filter class shim, 'CompatMethodFilter'
+    Otherwise, use version 1.0+ `CharFilter` class
+
+    TODO: Check that the versions stated are accurate. Meaning that the breaking
+    changes are at the major version for the differences we observer from 0.11.0
+    to 1.0.4 to 2.2.0 of Django Filter.
+
+    Pre Django Filter 1.0 uses the class, `MethodFilter`. Afterward, it uses the
+    class `CharFilter` with custom method handling.
+
+    With Django Filter 1.0, a new parameter, `name` was introduced and required
+    as a parameter in the custom filter methods.
+
+    Therefore, as a quick fix, we copied and modified the `MethodFilter` class
+    from Django Filter 0.11.0 and added it above in this module.
+
+    Before 1.0, the custom method signature is:
+
+    `(self, queryset, value)`
+
+    With 1.0, the method signature is:
+
+    `(self, queryset, name, value)`
+
+    Version 1.x replaces `MethodFilter` class with `FilterMethod`
+    Version 2.x changes the `name` parameter to `field_name`
+    """
+    if DJANGO_FILTERS_VERSION[0] < 1:
+        class CompatMethodFilter(django_filters.MethodFilter):  # pylint: disable=no-member
+            def filter(self, qs, value):
+                '''
+                This filter method will act as a proxy for the actual method we want to
+                call.
+
+                It will try to find the method on the parent filterset,
+                if not it attempts to search for the method `field_{{attribute_name}}`.
+                Otherwise it defaults to just returning the queryset.
+                '''
+                parent = getattr(self, 'parent', None)
+                parent_filter_method = getattr(parent, self.parent_action, None)
+                if not parent_filter_method:
+                    func_str = 'filter_{0}'.format(self.name)
+                    parent_filter_method = getattr(parent, func_str, None)
+
+                if parent_filter_method is not None:
+                    return parent_filter_method(qs, self.name, value)
+                return qs
+
+        return CompatMethodFilter(action=method)
     else:
         return django_filters.CharFilter(method=method)
 
@@ -70,28 +160,28 @@ def date_from_range_filter(field_name):
     https://django-filter.readthedocs.io/en/master/guide/migration.html#filter-name-renamed-to-filter-field-name-792
     First check for old style (pre version 2 Django Filters)
     """
-    if version.parse(django_filters.__version__) < version.parse('2.0.0'):
+    if DJANGO_FILTERS_VERSION[0] < 2:
         return django_filters.DateFromToRangeFilter(name=field_name)
     else:
         return django_filters.DateFromToRangeFilter(field_name=field_name)
 
 
 def char_filter(field_name, lookup_expr):
-    if version.parse(django_filters.__version__) < version.parse('2.0.0'):
+    if DJANGO_FILTERS_VERSION[0] < 2:
         return django_filters.CharFilter(name=field_name, lookup_expr=lookup_expr)
     else:
         return django_filters.CharFilter(field_name=field_name, lookup_expr=lookup_expr)
 
 
 def boolean_filter(field_name):
-    if version.parse(django_filters.__version__) < version.parse('2.0.0'):
+    if DJANGO_FILTERS_VERSION[0] < 2:
         return django_filters.BooleanFilter(name=field_name)
     else:
         return django_filters.BooleanFilter(field_name=field_name)
 
 
 class CourseOverviewFilter(django_filters.FilterSet):
-    '''Provides filtering for CourseOverview model objects
+    """Provides filtering for CourseOverview model objects
 
     Filters to consider adding:
         description: search/icontains
@@ -107,15 +197,15 @@ class CourseOverviewFilter(django_filters.FilterSet):
             AssertionError: <course id string repr> is not an instance of
             <class 'opaque_keys.edx.keys.CourseKey'>
 
-    '''
-
-    display_name = django_filters.CharFilter(lookup_expr='icontains')
-    org = char_filter(
-        field_name='display_org_with_default', lookup_expr='iexact')
-    number = char_filter(
-        field_name='display_number_with_default', lookup_expr='iexact')
-    number_contains = char_filter(
-        field_name='display_number_with_default', lookup_expr='icontains')
+    """
+    display_name = char_filter(field_name='display_name',
+                               lookup_expr='icontains')
+    org = char_filter(field_name='display_org_with_default',
+                      lookup_expr='iexact')
+    number = char_filter(field_name='display_number_with_default',
+                         lookup_expr='iexact')
+    number_contains = char_filter(field_name='display_number_with_default',
+                                  lookup_expr='icontains')
 
     class Meta:
         model = CourseOverview
@@ -219,23 +309,21 @@ def filter_exclude_completed(self, queryset, name, value):  # pylint: disable=un
 
 
 class UserFilterSet(django_filters.FilterSet):
-    '''Provides filtering for User model objects
+    """Provides filtering for User model objects
 
     Note: User has a 1:1 relationship with the edx-platform LMS
     student.models.UserProfile model
 
     We're starting with a few fields and will add as we find we want/need them
 
-    '''
+    """
     is_active = boolean_filter(field_name='is_active')
     is_staff = boolean_filter(field_name='is_staff')
     is_superuser = boolean_filter(field_name='is_superuser')
-    username = django_filters.CharFilter(lookup_expr='icontains')
-    email = django_filters.CharFilter(lookup_expr='icontains')
-    name = char_filter(lookup_expr='icontains', field_name='profile__name')
-
-    country = char_filter(
-        field_name='profile__country', lookup_expr='iexact')
+    username = char_filter(field_name='username', lookup_expr='icontains')
+    email = char_filter(field_name='email', lookup_expr='icontains')
+    name = char_filter(field_name='profile__name', lookup_expr='icontains')
+    country = char_filter(field_name='profile__country', lookup_expr='iexact')
     user_ids = char_method_filter(method='filter_user_ids')
     enrolled_in_course_id = char_method_filter(method='filter_enrolled_in_course_id')
 
@@ -346,8 +434,8 @@ class SiteFilterSet(django_filters.FilterSet):
     """
     Note: The Site filter has no knowledge of a default site, nor should it
     """
-    domain = django_filters.CharFilter(lookup_expr='icontains')
-    name = django_filters.CharFilter(lookup_expr='icontains')
+    domain = char_filter(field_name='domain', lookup_expr='icontains')
+    name = char_filter(field_name='name', lookup_expr='icontains')
 
     class Meta:
         model = Site

figures/metrics.py

@@ -293,7 +293,7 @@ def get_active_users_for_time_period(site, start_date, end_date, course_ids=None
         **filter_args).values('student__id').distinct().count()
 
 
-def get_total_site_users_for_time_period(site, start_date, end_date, **kwargs):
+def get_total_site_users_for_time_period(site, start_date, end_date, **_kwargs):
     """
     Returns the maximum number of users who joined before or on the end date
 
@@ -304,28 +304,15 @@ def get_total_site_users_for_time_period(site, start_date, end_date, **kwargs):
     TODO: Consider first trying to get the data from the SiteDailyMetrics
     model. If there are no records, then get the data from the User model
     """
-    def calc_from_user_model():
-        filter_args = dict(
-            date_joined__lt=as_datetime(next_day(end_date)),
-        )
-        users = figures.sites.get_users_for_site(site)
-        return users.filter(**filter_args).count()
-
-    def calc_from_site_daily_metrics():
-        filter_args = dict(
-            site=site,
-            date_for__gt=prev_day(start_date),
-            date_for__lt=next_day(end_date))
-        qs = SiteDailyMetrics.objects.filter(**filter_args)
-        if qs:
-            return qs.aggregate(maxval=Max('total_user_count'))['maxval']
-        else:
-            return 0
 
-    if kwargs.get('calc_from_sdm'):
-        return calc_from_site_daily_metrics()
+    filter_args = dict(site=site,
+                       date_for__gt=prev_day(start_date),
+                       date_for__lt=next_day(end_date))
+    qs = SiteDailyMetrics.objects.filter(**filter_args)
+    if qs:
+        return qs.aggregate(maxval=Max('total_user_count'))['maxval']
     else:
-        return calc_from_user_model()
+        return 0
 
 
 def get_total_site_users_joined_for_time_period(site, start_date, end_date,
@@ -334,6 +321,7 @@ def get_total_site_users_joined_for_time_period(site, start_date, end_date,
 
     NOTE: Untested and not yet used in the general site metrics, but we'll want to add it
     TODO: Rename this function to be "new_users" for consistency with the API endpoint
+    TODO: When we implement this, add data to Figures model space for performance
     """
     def calc_from_user_model():
         filter_args = dict(

frontend/src/views/ProgressOverview.js

@@ -314,8 +314,14 @@ class ProgressOverview extends Component {
         </HeaderAreaLayout>
         {this.state.csvExportProgress ? (
           <div className={cx({ 'container': true, 'csv-export-content': true})}>
-            <h2>Exporting your CSV data...</h2>
-            <p>Please don't close this browser tab.</p>
+            {(this.state.csvExportProgress < 1) ? [
+              <h2>Exporting your CSV data...</h2>,
+              <p>Please don't close this browser tab.</p>
+            ] : [
+              <h2>Export successful!</h2>,
+              <p>Depending on your browser settings, you will either be prompted with a prompt to save the generated file, or the file will be automatically downloaded into your default Downloads folder.</p>,
+              <p>It is now safe to close the exporter.</p>,
+            ]}
             <div className={styles['progress-bar']}>
               <span className={styles['progress-bar-inner']} style={{ width: this.state.csvExportProgress * 100 + '%'}}></span>
             </div>

frontend/src/views/_progress-overview-content.scss

@@ -229,6 +229,13 @@ button.export-the-csv-button {
     margin: 0 0 calcRem(20);
   }
 
+  p {
+    font-size: calcRem(14);
+    max-width: calcRem(800);
+    margin-left: auto;
+    margin-right: auto;
+  }
+
   .progress-bar {
     width: 100%;
     max-width: calcRem(500);
@@ -264,6 +271,7 @@ button.close-csv-button {
   outline: none;
   border-radius: 50px;
   transition: all 0.3s ease-in-out;
+  font-size: calcRem(16);
 
   &:hover {
     cursor: pointer;