Skip to content

Commit

Permalink
Merge pull request #321 from edx-solutions/ziafazal/api-time-series-i…
Browse files Browse the repository at this point in the history 
…nvalid-data

ziafazal/api-time-series-invalid-data: fix negative values in users_not_started
  • Loading branch information
@mattdrayer
mattdrayer committed Nov 26, 2014
2 parents b6b044c + 5ed17e5 commit 58ee5e4
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 18 deletions.
22 changes: 11 additions & 11 deletions lms/djangoapps/api_manager/courses/tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1526,15 +1526,15 @@ def test_coursemodulecompletions_filters(self):
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)
self.assertEqual(len(response.data['results']), 20)
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)
Expand All @@ -1543,7 +1543,7 @@ def test_coursemodulecompletions_filters(self):
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
Expand Down Expand Up @@ -1975,18 +1975,17 @@ def test_courses_data_metrics(self):
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)
Expand Down Expand Up @@ -2057,7 +2056,7 @@ def test_courses_data_time_series_metrics(self):
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)
Expand Down Expand Up @@ -2121,7 +2120,7 @@ def test_courses_data_time_series_metrics(self):
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)
Expand All @@ -2136,6 +2135,8 @@ def test_courses_data_time_series_metrics(self):
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()
Expand Down Expand Up @@ -2214,7 +2215,6 @@ def test_courses_data_time_series_metrics(self):
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
Expand Down
13 changes: 6 additions & 7 deletions lms/djangoapps/api_manager/courses/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
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
Expand Down Expand Up @@ -1589,7 +1589,7 @@ def get(self, request, course_id): # pylint: disable=W0613
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)
Expand Down Expand Up @@ -1617,8 +1617,8 @@ def get(self, request, course_id): # pylint: disable=W0613
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'))
Expand All @@ -1631,19 +1631,18 @@ def get(self, request, course_id): # pylint: disable=W0613
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)
Expand Down

0 comments on commit 58ee5e4

Please sign in to comment.