diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index d4a38880fe456452738891be0752a70d375f03e5..b75e04359981744f21cf9a5210585c2468ad44c3 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -929,12 +929,18 @@ class CourseEnrollmentManager(models.Manager): return is_course_full - def users_enrolled_in(self, course_id): - """Return a queryset of User for every user enrolled in the course.""" - return User.objects.filter( - courseenrollment__course_id=course_id, - courseenrollment__is_active=True - ) + def users_enrolled_in(self, course_id, include_inactive=False): + """ + Return a queryset of User for every user enrolled in the course. If + `include_inactive` is True, returns both active and inactive enrollees + for the course. Otherwise returns actively enrolled users only. + """ + filter_kwargs = { + 'courseenrollment__course_id': course_id, + } + if not include_inactive: + filter_kwargs['courseenrollment__is_active'] = True + return User.objects.filter(**filter_kwargs) def enrollment_counts(self, course_id): """ diff --git a/common/djangoapps/student/tests/test_models.py b/common/djangoapps/student/tests/test_models.py index 42efec8f80ec64b08d7dc8cfe335a4c4448edc69..aaf8ac595f65b6a42f456b59b0566d313ed49c1d 100644 --- a/common/djangoapps/student/tests/test_models.py +++ b/common/djangoapps/student/tests/test_models.py @@ -21,6 +21,7 @@ class CourseEnrollmentTests(SharedModuleStoreTestCase): def setUp(self): super(CourseEnrollmentTests, self).setUp() self.user = UserFactory.create() + self.user_2 = UserFactory.create() def test_enrollment_status_hash_cache_key(self): username = 'test-user' @@ -82,3 +83,23 @@ class CourseEnrollmentTests(SharedModuleStoreTestCase): # Modifying enrollments should delete the cached value. CourseEnrollmentFactory.create(user=self.user) self.assertIsNone(cache.get(CourseEnrollment.enrollment_status_hash_cache_key(self.user))) + + def test_users_enrolled_in_active_only(self): + """CourseEnrollment.users_enrolled_in should return only Users with active enrollments when + `include_inactive` has its default value (False).""" + CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True) + CourseEnrollmentFactory.create(user=self.user_2, course_id=self.course.id, is_active=False) + + active_enrolled_users = list(CourseEnrollment.objects.users_enrolled_in(self.course.id)) + self.assertEqual([self.user], active_enrolled_users) + + def test_users_enrolled_in_all(self): + """CourseEnrollment.users_enrolled_in should return active and inactive users when + `include_inactive` is True.""" + CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True) + CourseEnrollmentFactory.create(user=self.user_2, course_id=self.course.id, is_active=False) + + all_enrolled_users = list( + CourseEnrollment.objects.users_enrolled_in(self.course.id, include_inactive=True) + ) + self.assertListEqual([self.user, self.user_2], all_enrolled_users) diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py index 69963b001a6e06e55f2dab4f7ed008acb845dea9..b2cdcc69611c36849eaeeb868492f6b681a5a105 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/grades.py +++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py @@ -272,7 +272,8 @@ class CourseGradeReport(object): def grouper(iterable, chunk_size=self.USER_BATCH_SIZE, fillvalue=None): args = [iter(iterable)] * chunk_size return izip_longest(*args, fillvalue=fillvalue) - users = CourseEnrollment.objects.users_enrolled_in(context.course_id) + + users = CourseEnrollment.objects.users_enrolled_in(context.course_id, include_inactive=True) users = users.select_related('profile__allow_certificate') return grouper(users) @@ -412,7 +413,7 @@ class ProblemGradeReport(object): start_time = time() start_date = datetime.now(UTC) status_interval = 100 - enrolled_students = CourseEnrollment.objects.users_enrolled_in(course_id) + enrolled_students = CourseEnrollment.objects.users_enrolled_in(course_id, include_inactive=True) task_progress = TaskProgress(action_name, enrolled_students.count(), start_time) # This struct encapsulates both the display names of each static item in the diff --git a/lms/djangoapps/instructor_task/tests/test_base.py b/lms/djangoapps/instructor_task/tests/test_base.py index 7192c67cf5c8beb428673034df5b925203627e28..949f6e64d82d52f1cd2ea3afbf063e213c8f27a3 100644 --- a/lms/djangoapps/instructor_task/tests/test_base.py +++ b/lms/djangoapps/instructor_task/tests/test_base.py @@ -162,21 +162,21 @@ class InstructorTaskCourseTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase) self.login(user_email, "test") self.current_user = username - def _create_user(self, username, email=None, is_staff=False, mode='honor'): + def _create_user(self, username, email=None, is_staff=False, mode='honor', enrollment_active=True): """Creates a user and enrolls them in the test course.""" if email is None: email = InstructorTaskCourseTestCase.get_user_email(username) thisuser = UserFactory.create(username=username, email=email, is_staff=is_staff) - CourseEnrollmentFactory.create(user=thisuser, course_id=self.course.id, mode=mode) + CourseEnrollmentFactory.create(user=thisuser, course_id=self.course.id, mode=mode, is_active=enrollment_active) return thisuser def create_instructor(self, username, email=None): """Creates an instructor for the test course.""" return self._create_user(username, email, is_staff=True) - def create_student(self, username, email=None, mode='honor'): + def create_student(self, username, email=None, mode='honor', enrollment_active=True): """Creates a student for the test course.""" - return self._create_user(username, email, is_staff=False, mode=mode) + return self._create_user(username, email, is_staff=False, mode=mode, enrollment_active=enrollment_active) @staticmethod def get_task_status(task_id): diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index bac4fb960d72df4b87f401213e761d3c9d97443b..2f4d5fde7c4a2cd2d30860266f507f4e6972e162 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -398,6 +398,25 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase): with self.assertNumQueries(41): CourseGradeReport.generate(None, None, course.id, None, 'graded') + def test_inactive_enrollments(self): + """ + Test that students with inactive enrollments are included in report. + """ + self.create_student('active-student', 'active@example.com') + self.create_student('inactive-student', 'inactive@example.com', enrollment_active=False) + + self.current_task = Mock() + self.current_task.update_state = Mock() + + with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task') as mock_current_task: + mock_current_task.return_value = self.current_task + result = CourseGradeReport.generate(None, None, self.course.id, None, 'graded') + + expected_students = 2 + self.assertDictContainsSubset( + {'attempted': expected_students, 'succeeded': expected_students, 'failed': 0}, result + ) + class TestTeamGradeReport(InstructorGradeReportTestCase): """ Test that teams appear correctly in the grade report when it is enabled for the course. """ @@ -760,6 +779,55 @@ class TestProblemGradeReport(TestReportMixin, InstructorTaskModuleTestCase): } ]) + @patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task') + def test_inactive_enrollment_included(self, _get_current_task): + """ + Students with inactive enrollments in a course should be included in Problem Grade Report. + """ + inactive_student = self.create_student('inactive-student', 'inactive@example.com', enrollment_active=False) + vertical = ItemFactory.create( + parent_location=self.problem_section.location, + category='vertical', + metadata={'graded': True}, + display_name='Problem Vertical' + ) + self.define_option_problem(u'Problem1', parent=vertical) + + self.submit_student_answer(self.student_1.username, u'Problem1', ['Option 1']) + result = ProblemGradeReport.generate(None, None, self.course.id, None, 'graded') + self.assertDictContainsSubset({'action_name': 'graded', 'attempted': 3, 'succeeded': 3, 'failed': 0}, result) + problem_name = u'Homework 1: Subsection - Problem1' + header_row = self.csv_header_row + [problem_name + ' (Earned)', problem_name + ' (Possible)'] + self.verify_rows_in_csv([ + dict(zip( + header_row, + [ + unicode(self.student_1.id), + self.student_1.email, + self.student_1.username, + '0.01', '1.0', '2.0', + ] + )), + dict(zip( + header_row, + [ + unicode(self.student_2.id), + self.student_2.email, + self.student_2.username, + '0.0', u'Not Attempted', '2.0', + ] + )), + dict(zip( + header_row, + [ + unicode(inactive_student.id), + inactive_student.email, + inactive_student.username, + '0.0', u'Not Attempted', '2.0', + ] + )) + ]) + @attr(shard=3) class TestProblemReportSplitTestContent(TestReportMixin, TestConditionalContent, InstructorTaskModuleTestCase):