Merge pull request #280 from edx-solutions/ziafazal/api-exclude-unenrolled-from-counts

ziafazal/api-exclude-unenrolled-from-counts:exclude un-enrolled users from course metrics/counts

lms/djangoapps/api_manager/courses/tests.py

@@ -2104,7 +2104,6 @@ class CoursesApiTests(TestCase):
         self.assertEqual(total_active, 5)
         self.assertEqual(response.data['users_enrolled'][0][1], 25)
 
-
         # get modules completed for first 5 days
         start_date = datetime.now().date() + relativedelta(days=-USER_COUNT)
         end_date = datetime.now().date() + relativedelta(days=-(USER_COUNT - 4))
@@ -2166,6 +2165,37 @@ class CoursesApiTests(TestCase):
         response = self.do_get(course_metrics_uri)
         self.assertEqual(response.status_code, 400)
 
+        # Test after un-enrolling some users
+        test_uri = self.base_courses_uri + '/' + test_course_id + '/users'
+        for j, user in enumerate(users[-5:]):
+            response = self.do_delete('{}/{}'.format(test_uri, user.id))
+            self.assertEqual(response.status_code, 204)
+        end_date = datetime.now().date()
+        start_date = end_date + relativedelta(days=-4)
+        course_metrics_uri = '{}/{}/time-series-metrics/?start_date={}&end_date={}'\
+            .format(self.base_courses_uri,
+                    test_course_id,
+                    start_date,
+                    end_date)
+
+        response = self.do_get(course_metrics_uri)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(response.data['users_not_started']), 5)
+        total_not_started = sum([not_started[1] for not_started in response.data['users_not_started']])
+        self.assertEqual(total_not_started, 0)
+        self.assertEqual(len(response.data['users_started']), 5)
+        total_started = sum([started[1] for started in response.data['users_started']])
+        self.assertEqual(total_started, 0)
+        self.assertEqual(len(response.data['users_completed']), 5)
+        total_completed = sum([completed[1] for completed in response.data['users_completed']])
+        self.assertEqual(total_completed, 0)
+        self.assertEqual(len(response.data['modules_completed']), 5)
+        total_modules_completed = sum([completed[1] for completed in response.data['modules_completed']])
+        self.assertEqual(total_modules_completed, 0)
+        self.assertEqual(len(response.data['active_users']), 5)
+        total_active = sum([active[1] for active in response.data['active_users']])
+        self.assertEqual(total_active, 0)
+        self.assertEqual(response.data['users_enrolled'][0][1], 20)
 
     def test_course_workgroups_list(self):
         projects_uri = self.base_projects_uri

lms/djangoapps/api_manager/courses/views.py

@@ -1452,7 +1452,9 @@ class CoursesMetricsGradesList(SecureListAPIView):
             return Response({}, status=status.HTTP_404_NOT_FOUND)
         course_key = get_course_key(course_id)
         exclude_users = _get_aggregate_exclusion_user_ids(course_key)
-        queryset = StudentGradebook.objects.filter(course_id__exact=course_key).exclude(user__in=exclude_users)
+        queryset = StudentGradebook.objects.filter(course_id__exact=course_key,
+                                                   user__courseenrollment__is_active=True)\
+            .exclude(user__in=exclude_users)
 
         upper_bound = getattr(settings, 'API_LOOKUP_UPPER_BOUND', 200)
         user_ids = self.request.QUERY_PARAMS.get('user_id', None)
@@ -1589,18 +1591,22 @@ class CoursesTimeSeriesMetrics(SecureAPIView):
         course_key = get_course_key(course_id)
         exclude_users = _get_aggregate_exclusion_user_ids(course_key)
         grade_complete_match_range = getattr(settings, 'GRADEBOOK_GRADE_COMPLETE_PROFORMA_MATCH_RANGE', 0.01)
-        grades_qs = StudentGradebook.objects.filter(course_id__exact=course_key, user__is_active=True).\
+        grades_qs = StudentGradebook.objects.filter(course_id__exact=course_key, user__is_active=True,
+                                                    user__courseenrollment__is_active=True).\
             exclude(user_id__in=exclude_users)
         grades_complete_qs = grades_qs.filter(proforma_grade__lte=F('grade') + grade_complete_match_range,
                                               proforma_grade__gt=0)
-        enrolled_qs = CourseEnrollment.objects.filter(course_id__exact=course_key, user__is_active=True)\
-            .exclude(user_id__in=exclude_users)
-        users_started_qs = StudentProgressHistory.objects.filter(course_id__exact=course_key, user__is_active=True)\
+        enrolled_qs = CourseEnrollment.objects.filter(course_id__exact=course_key, user__is_active=True,
+                                                      is_active=True).exclude(user_id__in=exclude_users)
+        users_started_qs = StudentProgressHistory.objects.filter(course_id__exact=course_key, user__is_active=True,
+                                                                 user__courseenrollment__is_active=True)\
             .exclude(user_id__in=exclude_users)
         modules_completed_qs = CourseModuleCompletion.get_actual_completions().filter(course_id__exact=course_key,
+                                                                                      user__courseenrollment__is_active=True,
                                                                                       user__is_active=True)\
             .exclude(user_id__in=exclude_users)
-        active_users_qs = StudentModule.objects.filter(course_id__exact=course_key, student__is_active=True)\
+        active_users_qs = StudentModule.objects.filter(course_id__exact=course_key, student__is_active=True,
+                                                       student__courseenrollment__is_active=True)\
             .exclude(student_id__in=exclude_users)
 
         organization = request.QUERY_PARAMS.get('organization', None)
@@ -1616,18 +1622,21 @@ class CoursesTimeSeriesMetrics(SecureAPIView):
         total_started = total_started_count['user__count'] or 0
         enrolled_series = get_time_series_data(enrolled_qs, start_dt, end_dt, interval=interval,
                                                date_field='created', aggregate=Count('id'))
-
         started_series = get_time_series_data(users_started_qs, start_dt, end_dt, interval=interval,
-                                              date_field='created', aggregate=Count('user', distinct=True))
+                                              date_field='created', date_field_model=StudentProgressHistory,
+                                              aggregate=Count('user', distinct=True))
         completed_series = get_time_series_data(grades_complete_qs, start_dt, end_dt, interval=interval,
-                                                date_field='modified', aggregate=Count('id'))
+                                                date_field='modified', date_field_model=StudentGradebook,
+                                                aggregate=Count('id'))
         modules_completed_series = get_time_series_data(modules_completed_qs, start_dt, end_dt, interval=interval,
-                                                        date_field='created', aggregate=Count('id'))
+                                                        date_field='created', date_field_model=CourseModuleCompletion,
+                                                        aggregate=Count('id'))
         # active users are those who accessed course in last 24 hours
         start_dt = start_dt + relativedelta(hours=-24)
         end_dt = end_dt + relativedelta(hours=-24)
         active_users_series = get_time_series_data(active_users_qs, start_dt, end_dt, interval=interval,
-                                                   date_field='modified', aggregate=Count('student', distinct=True))
+                                                   date_field='modified', date_field_model=StudentModule,
+                                                   aggregate=Count('student', distinct=True))
         not_started_series, total_enrolled_series = [], []
         for enrolled, started in zip(enrolled_series, started_series):
             not_started_series.append((started[0], (total_enrolled + enrolled[1]) - (total_started + started[1])))

lms/djangoapps/api_manager/utils.py

@@ -142,7 +142,8 @@ def detect_db_engine():
     return engine
 
 
-def get_time_series_data(queryset, start, end, interval='days', date_field='created', aggregate=None):
+def get_time_series_data(queryset, start, end, interval='days', date_field='created', date_field_model=None,
+                         aggregate=None):
     """
     Aggregate over time intervals to compute time series representation of data
     """
@@ -150,23 +151,26 @@ def get_time_series_data(queryset, start, end, interval='days', date_field='crea
     start, _ = get_interval_bounds(start, interval.rstrip('s'))
     _, end = get_interval_bounds(end, interval.rstrip('s'))
 
+    if date_field_model:
+        date_field = '"{}"."{}"'.format(date_field_model._meta.db_table, date_field)
+
     sql = {
         'mysql': {
-            'days': "DATE_FORMAT(`{}`, '%%Y-%%m-%%d')".format(date_field),
-            'weeks': "DATE_FORMAT(DATE_SUB(`{}`, INTERVAL(WEEKDAY(`{}`)) DAY), '%%Y-%%m-%%d')".format(date_field,
-                                                                                                      date_field),
-            'months': "DATE_FORMAT(`{}`, '%%Y-%%m-01')".format(date_field)
+            'days': "DATE_FORMAT({}, '%%Y-%%m-%%d')".format(date_field),
+            'weeks': "DATE_FORMAT(DATE_SUB({}, INTERVAL(WEEKDAY({})) DAY), '%%Y-%%m-%%d')".format(date_field,
+                                                                                                  date_field),
+            'months': "DATE_FORMAT({}, '%%Y-%%m-01')".format(date_field)
         },
         'sqlite': {
-            'days': "strftime('%%Y-%%m-%%d', `{}`)".format(date_field),
-            'weeks': "strftime('%%Y-%%m-%%d', julianday(`{}`) - strftime('%%w', `{}`) + 1)".format(date_field,
-                                                                                                   date_field),
-            'months': "strftime('%%Y-%%m-01', `{}`)".format(date_field)
+            'days': "strftime('%%Y-%%m-%%d', {})".format(date_field),
+            'weeks': "strftime('%%Y-%%m-%%d', julianday({}) - strftime('%%w', {}) + 1)".format(date_field,
+                                                                                               date_field),
+            'months': "strftime('%%Y-%%m-01', {})".format(date_field)
         }
     }
     interval_sql = sql[engine][interval]
-    kwargs = {'{}__range'.format(date_field): (start, end)}
-    aggregate_data = queryset.extra(select={'d': interval_sql}).filter(**kwargs).order_by().values('d').\
+    where_clause = '{} BETWEEN "{}" and "{}"'.format(date_field, start, end)
+    aggregate_data = queryset.extra(select={'d': interval_sql}, where=[where_clause]).order_by().values('d').\
         annotate(agg=aggregate)
 
     today = strip_time(now())

lms/djangoapps/gradebook/models.py

@@ -69,7 +69,8 @@ class StudentGradebook(TimeStampedModel):
         if total_user_count:
             # Generate the base data set we're going to work with
             queryset = StudentGradebook.objects.select_related('user')\
-                .filter(course_id__exact=course_key, user__is_active=True, user__in=enrolled_users_not_excluded)
+                .filter(course_id__exact=course_key, user__is_active=True, user__courseenrollment__is_active=True,
+                        user__in=enrolled_users_not_excluded)
             gradebook_user_count = len(queryset)
 
             if gradebook_user_count:

lms/djangoapps/progress/models.py

@@ -29,7 +29,8 @@ class StudentProgress(TimeStampedModel):
         """
         Returns count of completions for a given course.
         """
-        queryset = cls.objects.filter(course_id__exact=course_key, user__is_active=True)\
+        queryset = cls.objects.filter(course_id__exact=course_key, user__is_active=True,
+                                      user__courseenrollment__is_active=True)\
             .exclude(user__id__in=exclude_users)
         if org_ids:
             queryset = queryset.filter(user__organizations__in=org_ids)
@@ -42,7 +43,8 @@ class StudentProgress(TimeStampedModel):
         """
         Returns count of users who completed at least one module.
         """
-        queryset = cls.objects.filter(course_id__exact=course_key, user__is_active=True)\
+        queryset = cls.objects.filter(course_id__exact=course_key, user__is_active=True,
+                                      user__courseenrollment__is_active=True)\
             .exclude(user__id__in=exclude_users)
         if org_ids:
             queryset = queryset.filter(user__organizations__in=org_ids)
@@ -87,7 +89,8 @@ class StudentProgress(TimeStampedModel):
 
         """
         queryset = cls.objects\
-            .filter(course_id__exact=course_key, user__is_active=True).exclude(user__id__in=exclude_users)
+            .filter(course_id__exact=course_key, user__is_active=True, user__courseenrollment__is_active=True)\
+            .exclude(user__id__in=exclude_users)
         if org_ids:
             queryset = queryset.filter(user__organizations__in=org_ids)
         queryset = queryset.values(