Fix certificates count as proxy for course completions

figures/metrics.py

@@ -26,7 +26,7 @@
 import math
 
 from django.contrib.auth import get_user_model
-from django.db.models import Avg, Max
+from django.db.models import Avg, Max, Sum
 
 from courseware.courses import get_course_by_id  # pylint: disable=import-error
 from courseware.models import StudentModule  # pylint: disable=import-error
@@ -403,24 +403,52 @@ def calc_from_course_enrollments():
         return calc_from_site_daily_metrics()
 
 
-def get_total_course_completions_for_time_period(site, start_date, end_date):
+def total_site_certificates_as_of_date(site, date_for):
+    """Get the total site certificates granted as of the given date
+
+    This function queries CourseDailyMetrics:
+
+    First, see if we have any records. If so, then get a record with the most
+    recent date
+
+    If we do, then get all records for that date and return the sum of
+    "num_learners_completed"
+
+    This implementation is a workaround until John can dig in and find out why
+    The following does not work
+
+    ```
+    site_cdm = CourseDailyMetrics.objects.filter(site=site,
+        date_for__lte=date_for)
+    recs = site_cdm.order_by('course_id').values('course_id').annotate(
+        latest_date=Max('date_for')).order_by('course_id')
+    data = recs.aggregate(Sum('num_learners_completed'))
+    return data['num_learners_completed__sum']
+    ```
     """
-    This metric is not currently captured in SiteDailyMetrics, so retrieving from
-    course dailies instead
+    qs = CourseDailyMetrics.objects.filter(
+        site=site,
+        date_for__lte=date_for).order_by('-date_for')
+    if qs:
+        latest_date = qs[0].date_for
+        recs = CourseDailyMetrics.objects.filter(site=site,
+                                                 date_for=latest_date)
+        data = recs.aggregate(Sum('num_learners_completed'))
+
+        return data['num_learners_completed__sum']
+    else:
+        return 0
+
+
+def get_total_course_completions_for_time_period(site, end_date, **_kwargs):
     """
-    def calc_from_course_daily_metrics():
-        filter_args = dict(
-            site=site,
-            date_for__gt=prev_day(start_date),
-            date_for__lt=next_day(end_date),
-        )
-        qs = CourseDailyMetrics.objects.filter(**filter_args)
-        if qs:
-            return qs.aggregate(maxval=Max('num_learners_completed'))['maxval']
-        else:
-            return 0
+    We're keeping the method signature for now because there is significant
+    enough rework that is out of scope of this fix
 
-    return calc_from_course_daily_metrics()
+    We want to rework this to just get the total certificates as of the
+    given date (so just one date not a date range)
+    """
+    return total_site_certificates_as_of_date(site=site, date_for=end_date)
 
 
 # -------------------------

figures/models.py

@@ -54,6 +54,9 @@ class CourseDailyMetrics(TimeStampedModel):
         )
 
     average_days_to_complete = models.IntegerField(blank=True, null=True)
+
+    # As of Figures 0.3.13, this is the total number of certificates granted
+    # for the course as of the "date_for"
     num_learners_completed = models.IntegerField()
 
     class Meta:

figures/pipeline/course_daily_metrics.py

@@ -185,6 +185,14 @@ def get_average_days_to_complete(course_id, date_for):
 
 
 def get_num_learners_completed(course_id, date_for):
+    """
+    Get the total number of certificates generated for the course up to the
+    'date_for' date
+
+    We will need to relabel this to "certificates"
+
+    We may want to get the number of certificates granted in the given day
+    """
     certificates = GeneratedCertificate.objects.filter(
         course_id=as_course_key(course_id),
         created_date__lt=as_datetime(next_day(date_for)))

tests/metrics/test_certificate_metrics.py

@@ -0,0 +1,111 @@
+"""
+
+# Background
+
+Figures originally calculated completions as the certificates generated
+
+
+As of mid 2020, we are reworking metrics so that course completsions are based
+off of gradable sections likely followed by using or adapting the completion
+aggregator
+
+We need to rename our current "completion" to indicate certificates and not
+completsions
+
+However, Figures UI and API still have the "completions" label for certificates
+
+
+# This Test Module
+
+This test module tests certificate metrics. If you read "completions" in this
+test module, think "certificates" until we do our relabelling
+
+"""
+
+import pytest
+from datetime import date
+from dateutil.relativedelta import relativedelta
+from freezegun import freeze_time
+
+from figures.helpers import days_in_month
+from figures.metrics import get_total_course_completions_for_time_period
+from figures.models import CourseDailyMetrics
+
+from tests.factories import (
+    CourseDailyMetricsFactory,
+    CourseOverviewFactory,
+    OrganizationFactory,
+    OrganizationCourseFactory,
+    SiteFactory
+)
+
+from tests.helpers import organizations_support_sites
+
+
+@pytest.fixture
+@pytest.mark.django_db
+def cdm_test_data(db, settings):
+    """Build CourseDailyMetrics data to test certificate counts
+    """
+    our_site = SiteFactory()
+    mock_today = date(year=2020, month=6, day=7)
+    last_month = mock_today - relativedelta(months=1)
+    courses = [CourseOverviewFactory() for i in range(2)]
+    # Create data for previous month. Just need one record
+    # purpose is to make sure it is not included in our production code request
+    prev_month_cdm = [CourseDailyMetrics(site=our_site,
+                                         course_id=str(courses[0].id),
+                                         date_for=last_month)]
+
+    # Create data for our current month
+    curr_month_cdm = []
+    cdm_data = [
+        dict(day=1, course_id=str(courses[0].id), num_learners_completed=1),
+        dict(day=6, course_id=str(courses[0].id), num_learners_completed=10),
+        dict(day=1, course_id=str(courses[1].id), num_learners_completed=2),
+        dict(day=6, course_id=str(courses[1].id), num_learners_completed=20),
+    ]
+
+    expected_cert_count = 30
+    for rec in cdm_data:
+        date_for = date(year=mock_today.year, month=mock_today.month, day=rec['day'])
+        cdm = CourseDailyMetricsFactory(
+            site=our_site,
+            course_id=rec['course_id'],
+            date_for=date_for,
+            num_learners_completed=rec['num_learners_completed'])
+        curr_month_cdm.append(cdm)
+
+    if organizations_support_sites():
+        settings.FEATURES['FIGURES_IS_MULTISITE'] = True
+        our_org = OrganizationFactory(sites=[our_site])
+        for course in courses:
+            OrganizationCourseFactory(organization=our_org,
+                                      course_id=str(course.id))
+    return dict(
+        mock_today=mock_today,
+        our_site=our_site,
+        courses=courses,
+        prev_month_cdm=prev_month_cdm,
+        curr_month_cdm=curr_month_cdm,
+        expected_cert_count=expected_cert_count,
+    )
+
+
+def test_certificate_metrics(cdm_test_data):
+    date_for = cdm_test_data['mock_today']
+    site = cdm_test_data['our_site']
+    expected_cert_count = cdm_test_data['expected_cert_count']
+    freezer = freeze_time(date_for)
+    freezer.start()
+
+    start_date = date(year=date_for.year, month=date_for.month, day=1)
+    end_date = date(year=date_for.year,
+                    month=date_for.month,
+                    day=days_in_month(date_for))
+
+    count = get_total_course_completions_for_time_period(site=site,
+                                                         start_date=start_date,
+                                                         end_date=end_date)
+    freezer.stop()
+    assert count == expected_cert_count

tests/metrics/test_metrics.py

@@ -366,20 +366,6 @@ def test_get_total_site_courses_for_time_period(self):
 
         assert count == self.site_daily_metrics[-1].course_count
 
-    def test_get_total_course_completions_for_time_period(self):
-        '''
-        We're incrementing values for test data, so the last SiteDailyMetrics
-        record will have the max value
-        '''
-
-        cdm = create_course_daily_metrics_data(
-            site=self.site,
-            start_date=self.data_start_date,
-            end_date=self.data_end_date)
-        count = get_total_course_completions_for_time_period(site=self.site,
-                                                             start_date=self.data_start_date,
-                                                             end_date=self.data_end_date)
-        assert count == cdm[-1].num_learners_completed
 
     def test_get_monthly_site_metrics(self):
         '''
@@ -508,21 +494,6 @@ def test_get_total_site_courses_for_time_period(self):
                                                        end_date=self.data_end_date)
         assert count == self.alpha_site_daily_metrics[-1].course_count
 
-    def test_get_total_course_completions_for_time_period(self):
-        '''
-        We're incrementing values for test data, so the last SiteDailyMetrics
-        record will have the max value
-        '''
-
-        cdm = create_course_daily_metrics_data(
-            site=self.alpha_site,
-            start_date=self.data_start_date,
-            end_date=self.data_end_date)
-        count = get_total_course_completions_for_time_period(
-            site=self.alpha_site,
-            start_date=self.data_start_date,
-            end_date=self.data_end_date)
-        assert count == cdm[-1].num_learners_completed
 
     def test_get_monthly_site_metrics(self):
         '''