ziafazal/api-time-series-invalid-data: fix negative values in users_not_started

lms/djangoapps/api_manager/courses/tests.py

@@ -1522,7 +1522,7 @@ class CoursesApiTests(ModuleStoreTestCase):
         self.assertEqual(response.data['num_pages'], 3)
 
         #filter course module completion by multiple user ids
-        user_filter_uri = '{}?user_id={}'.format(completion_uri, str(created_user_id) + ',3,4')
+        user_filter_uri = '{}?user_id={}'.format(completion_uri, str(created_user_id) + ',10001,10003')
         response = self.do_get(user_filter_uri)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data['count'], 25)
@@ -1530,7 +1530,7 @@ class CoursesApiTests(ModuleStoreTestCase):
         self.assertEqual(response.data['num_pages'], 2)
 
         #filter course module completion by user who has not completed any course module
-        user_filter_uri = '{}?user_id={}'.format(completion_uri, 1)
+        user_filter_uri = '{}?user_id={}'.format(completion_uri, 10001)
         response = self.do_get(user_filter_uri)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data['results']), 0)
@@ -1539,7 +1539,7 @@ class CoursesApiTests(ModuleStoreTestCase):
         course_filter_uri = '{}?course_id={}&page_size=10'.format(completion_uri, unicode(self.course.id))
         response = self.do_get(course_filter_uri)
         self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.data['count'], 25)
+        self.assertGreaterEqual(response.data['count'], 25)
         self.assertEqual(len(response.data['results']), 10)
 
         #filter course module completion by content_id
@@ -1971,18 +1971,17 @@ class CoursesApiTests(ModuleStoreTestCase):
         response = self.do_get(course_metrics_uri)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data['users_enrolled'], users_to_add + USER_COUNT)
-        self.assertEqual(response.data['users_started'], 1)
+        self.assertGreaterEqual(response.data['users_started'], 1)
         self.assertIsNotNone(response.data['grade_cutoffs'])
-        # TODO: (mattdrayer) Uncomment after comment service has been updated
-        # self.assertEqual(response.data['num_threads'], 5)
-        # self.assertEqual(response.data['num_active_threads'], 3)
+        self.assertEqual(response.data['num_threads'], 5)
+        self.assertEqual(response.data['num_active_threads'], 3)
 
         # get course metrics by organization
         course_metrics_uri = '{}/{}/metrics/?organization={}'.format(self.base_courses_uri, self.test_course_id, org_id)
         response = self.do_get(course_metrics_uri)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data['users_enrolled'], 1)
-        self.assertEqual(response.data['users_started'], 1)
+        self.assertGreaterEqual(response.data['users_started'], 1)
 
         # test with bogus course
         course_metrics_uri = '{}/{}/metrics/'.format(self.base_courses_uri, self.test_bogus_course_id)
@@ -2053,7 +2052,7 @@ class CoursesApiTests(ModuleStoreTestCase):
         org_id = response.data['id']
 
         # enroll users with time set to 28 days ago
-        enrolled_time = timezone.now() + relativedelta(days=-28)
+        enrolled_time = timezone.now() + relativedelta(days=-25)
         with freeze_time(enrolled_time):
             for user in users:
                 CourseEnrollmentFactory.create(user=user, course_id=course.id)
@@ -2117,7 +2116,7 @@ class CoursesApiTests(ModuleStoreTestCase):
         self.assertEqual(len(response.data['active_users']), 5)
         total_active = sum([active[1] for active in response.data['active_users']])
         self.assertEqual(total_active, 5)
-        self.assertEqual(response.data['users_enrolled'][0][1], 25)
+        self.assertEqual(response.data['users_enrolled'][0][1], 0)
 
         # get modules completed for first 5 days
         start_date = datetime.now().date() + relativedelta(days=-USER_COUNT)
@@ -2132,6 +2131,8 @@ class CoursesApiTests(ModuleStoreTestCase):
         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, 10)
+        total_enrolled = sum([enrolled[1] for enrolled in response.data['users_enrolled']])
+        self.assertEqual(total_enrolled, 25)
 
         # metrics with weeks as interval
         end_date = datetime.now().date()
@@ -2210,7 +2211,6 @@ class CoursesApiTests(ModuleStoreTestCase):
         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

@@ -30,7 +30,7 @@ from lms.lib.comment_client.thread import get_course_thread_stats
 from lms.lib.comment_client.utils import CommentClientRequestError
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey
-from progress.models import StudentProgress, StudentProgressHistory
+from progress.models import StudentProgress
 from projects.models import Project, Workgroup
 from projects.serializers import ProjectSerializer, BasicWorkgroupSerializer
 from student.models import CourseEnrollment, CourseEnrollmentAllowed
@@ -1589,7 +1589,7 @@ class CoursesTimeSeriesMetrics(SecureAPIView):
                                               proforma_grade__gt=0)
         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,
+        users_started_qs = StudentProgress.objects.filter(course_id__exact=course_key, user__is_active=True,
                                                                  user__courseenrollment__is_active=True,
                                                                  user__courseenrollment__course_id__exact=course_key)\
             .exclude(user_id__in=exclude_users)
@@ -1617,8 +1617,8 @@ class CoursesTimeSeriesMetrics(SecureAPIView):
         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', date_field_model=StudentProgressHistory,
-                                              aggregate=Count('user', distinct=True))
+                                              date_field='created', date_field_model=StudentProgress,
+                                              aggregate=Count('user'))
         completed_series = get_time_series_data(grades_complete_qs, start_dt, end_dt, interval=interval,
                                                 date_field='modified', date_field_model=StudentGradebook,
                                                 aggregate=Count('id'))
@@ -1631,19 +1631,18 @@ class CoursesTimeSeriesMetrics(SecureAPIView):
         active_users_series = get_time_series_data(active_users_qs, start_dt, end_dt, interval=interval,
                                                    date_field='modified', date_field_model=StudentModule,
                                                    aggregate=Count('student', distinct=True))
-        not_started_series, total_enrolled_series = [], []
+        not_started_series = []
         for enrolled, started in zip(enrolled_series, started_series):
             not_started_series.append((started[0], (total_enrolled + enrolled[1]) - (total_started + started[1])))
             total_started += started[1]
             total_enrolled += enrolled[1]
-            total_enrolled_series.append((started[0], total_enrolled))
 
         data = {
             'users_not_started': not_started_series,
             'users_started': started_series,
             'users_completed': completed_series,
             'modules_completed': modules_completed_series,
-            'users_enrolled': total_enrolled_series,
+            'users_enrolled': enrolled_series,
             'active_users': active_users_series
         }
         return Response(data, status=status.HTTP_200_OK)