diff --git a/lms/djangoapps/instructor_task/config/waffle.py b/lms/djangoapps/instructor_task/config/waffle.py index 7eebe17d0f..c5872c573d 100644 --- a/lms/djangoapps/instructor_task/config/waffle.py +++ b/lms/djangoapps/instructor_task/config/waffle.py @@ -11,6 +11,7 @@ INSTRUCTOR_TASK_WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name=u'instructor_ta # Course-specific flags PROBLEM_GRADE_REPORT_VERIFIED_ONLY = u'problem_grade_report_verified_only' +COURSE_GRADE_REPORT_VERIFIED_ONLY = u'course_grade_report_verified_only' def waffle_flags(): @@ -23,6 +24,11 @@ def waffle_flags(): flag_name=PROBLEM_GRADE_REPORT_VERIFIED_ONLY, flag_undefined_default=False, ), + COURSE_GRADE_REPORT_VERIFIED_ONLY: CourseWaffleFlag( + waffle_namespace=INSTRUCTOR_TASK_WAFFLE_FLAG_NAMESPACE, + flag_name=COURSE_GRADE_REPORT_VERIFIED_ONLY, + flag_undefined_default=False, + ), } @@ -33,3 +39,12 @@ def problem_grade_report_verified_only(course_id): False otherwise. """ return waffle_flags()[PROBLEM_GRADE_REPORT_VERIFIED_ONLY].is_enabled(course_id) + + +def course_grade_report_verified_only(course_id): + """ + Returns True if problem grade reports should only + return rows for verified students in the given course, + False otherwise. + """ + return waffle_flags()[PROBLEM_GRADE_REPORT_VERIFIED_ONLY].is_enabled(course_id) diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py index 2f358e3218..b25d2aac5b 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/grades.py +++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py @@ -14,23 +14,27 @@ import six from django.conf import settings from django.contrib.auth import get_user_model from lazy import lazy -from opaque_keys.edx.keys import UsageKey from pytz import UTC from six import text_type from six.moves import zip, zip_longest from course_blocks.api import get_course_blocks +from course_modes.models import CourseMode +from lms.djangoapps.certificates.models import CertificateWhitelist, GeneratedCertificate, certificate_info_for_user from lms.djangoapps.courseware.courses import get_course_by_id from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient -from lms.djangoapps.instructor_analytics.basic import list_problem_responses -from lms.djangoapps.instructor_analytics.csvs import format_dictlist -from lms.djangoapps.instructor_task.config.waffle import problem_grade_report_verified_only -from lms.djangoapps.certificates.models import CertificateWhitelist, GeneratedCertificate, certificate_info_for_user from lms.djangoapps.grades.api import CourseGradeFactory from lms.djangoapps.grades.api import context as grades_context from lms.djangoapps.grades.api import prefetch_course_and_subsection_grades +from lms.djangoapps.instructor_analytics.basic import list_problem_responses +from lms.djangoapps.instructor_analytics.csvs import format_dictlist +from lms.djangoapps.instructor_task.config.waffle import ( + course_grade_report_verified_only, + problem_grade_report_verified_only +) from lms.djangoapps.teams.models import CourseTeamMembership from lms.djangoapps.verify_student.services import IDVerificationService +from opaque_keys.edx.keys import UsageKey from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache from openedx.core.djangoapps.course_groups.cohorts import bulk_cache_cohorts, get_cohort, is_course_cohorted from openedx.core.djangoapps.user_api.course_tag.api import BulkCourseTags @@ -307,7 +311,7 @@ class CourseGradeReport(object): args = [iter(iterable)] * chunk_size return zip_longest(*args, fillvalue=fillvalue) - def users_for_course(course_id): + def users_for_course(course_id, verified_only=False): """ Get all the enrolled users in a course. @@ -315,11 +319,15 @@ class CourseGradeReport(object): out-of-memory errors in large courses. This method will be removed when `OPTIMIZE_GET_LEARNERS_FOR_COURSE` waffle flag is removed. """ - users = CourseEnrollment.objects.users_enrolled_in(course_id, include_inactive=True) + users = CourseEnrollment.objects.users_enrolled_in( + course_id, + include_inactive=True, + verified_only=verified_only, + ) users = users.select_related('profile') return grouper(users) - def users_for_course_v2(course_id): + def users_for_course_v2(course_id, verified_only=False): """ Get all the enrolled users in a course chunk by chunk. @@ -329,6 +337,8 @@ class CourseGradeReport(object): filter_kwargs = { 'courseenrollment__course_id': course_id, } + if verified_only: + filter_kwargs['courseenrollment__mode'] = CourseMode.VERIFIED user_ids_list = get_user_model().objects.filter(**filter_kwargs).values_list('id', flat=True).order_by('id') user_chunks = grouper(user_ids_list) @@ -343,13 +353,15 @@ class CourseGradeReport(object): ).select_related('profile') yield users + course_id = context.course_id task_log_message = u'{}, Task type: {}'.format(context.task_info_string, context.action_name) + verified_users_only = course_grade_report_verified_only(course_id) if WAFFLE_SWITCHES.is_enabled(OPTIMIZE_GET_LEARNERS_FOR_COURSE): TASK_LOG.info(u'%s, Creating Course Grade with optimization', task_log_message) - return users_for_course_v2(context.course_id) + return users_for_course_v2(course_id, verified_only=verified_users_only) TASK_LOG.info(u'%s, Creating Course Grade without optimization', task_log_message) - batch_users = users_for_course(context.course_id) + batch_users = users_for_course(course_id, verified_only=verified_users_only) return batch_users def _user_grades(self, course_grade, context): diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index ea8bc75638..4778b6be2b 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -422,7 +422,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase): RequestCache.clear_all_namespaces() - expected_query_count = 49 + expected_query_count = 51 with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'): with check_mongo_calls(mongo_count): with self.assertNumQueries(expected_query_count): @@ -2038,6 +2038,33 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): ignore_other_columns=True, ) + @patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task') + def test_course_grade_with_verified_student_only(self, _get_current_task): + """ + Tests that course grade report has expected data when it is generated only for + verified learners. + """ + with patch( + 'lms.djangoapps.instructor_task.tasks_helper.grades.course_grade_report_verified_only', + return_value=True, + ): + student_1 = self.create_student(u'user_honor') + student_verified = self.create_student(u'user_verified', mode='verified') + 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(student_1.username, u'Problem1', ['Option 1']) + self.submit_student_answer(student_verified.username, u'Problem1', ['Option 1']) + result = CourseGradeReport.generate(None, None, self.course.id, None, 'graded') + self.assertDictContainsSubset( + {'action_name': 'graded', 'attempted': 1, 'succeeded': 1, 'failed': 0}, result + ) + @ddt.data(True, False) def test_fast_generation(self, create_non_zero_grade): if create_non_zero_grade: