diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index d460d85f1f..0491471de1 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1016,12 +1016,16 @@ class CourseEnrollmentManager(models.Manager): def users_enrolled_in(self, course_id, include_inactive=False, verified_only=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. - If 'verified_only' is True, returns report only for verified enrollees. + Return a queryset of User for every user enrolled in the course. + + Arguments: + course_id (CourseLocator): course_id to return enrollees for. + include_inactive (boolean): is a boolean when True, returns both active and inactive enrollees + verified_only (boolean): is a boolean when True, returns only verified enrollees. + + Returns: + Returns a User queryset. """ - # enrolled filter_kwargs = { 'courseenrollment__course_id': course_id, } diff --git a/lms/djangoapps/instructor_task/config/waffle.py b/lms/djangoapps/instructor_task/config/waffle.py index 51bde4b971..6d353b4622 100644 --- a/lms/djangoapps/instructor_task/config/waffle.py +++ b/lms/djangoapps/instructor_task/config/waffle.py @@ -4,7 +4,7 @@ waffle switches for the instructor_task app. """ -from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag, WaffleFlagNamespace, WaffleSwitchNamespace +from openedx.core.djangoapps.waffle_utils import WaffleFlagNamespace, WaffleSwitchNamespace WAFFLE_NAMESPACE = u'instructor_task' INSTRUCTOR_TASK_WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name=WAFFLE_NAMESPACE) @@ -12,46 +12,14 @@ WAFFLE_SWITCHES = WaffleSwitchNamespace(name=WAFFLE_NAMESPACE) # Waffle switches OPTIMIZE_GET_LEARNERS_FOR_COURSE = u'optimize_get_learners_for_course' - -# 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' +GENERATE_GRADE_REPORT_VERIFIED_ONLY = u'generate_grade_report_for_verified_only' def waffle_flags(): """ Returns the namespaced, cached, audited Waffle flags dictionary for Grades. """ - return { - PROBLEM_GRADE_REPORT_VERIFIED_ONLY: CourseWaffleFlag( - waffle_namespace=INSTRUCTOR_TASK_WAFFLE_FLAG_NAMESPACE, - 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, - ), - } - - -def problem_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) - - -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) + return {} def optimize_get_learners_switch_enabled(): @@ -59,3 +27,11 @@ def optimize_get_learners_switch_enabled(): Returns True if optimize get learner switch is enabled, otherwise False. """ return WAFFLE_SWITCHES.is_enabled(OPTIMIZE_GET_LEARNERS_FOR_COURSE) + + +def generate_grade_report_for_verified_only(): + """ + Returns True if waffle switch is enabled that indicates generate grading reports only for + verified learners. + """ + return WAFFLE_SWITCHES.is_enabled(GENERATE_GRADE_REPORT_VERIFIED_ONLY) diff --git a/lms/djangoapps/instructor_task/tasks_helper/grades.py b/lms/djangoapps/instructor_task/tasks_helper/grades.py index fae35afc51..805064ce16 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/grades.py +++ b/lms/djangoapps/instructor_task/tasks_helper/grades.py @@ -29,9 +29,8 @@ 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, - optimize_get_learners_switch_enabled, - problem_grade_report_verified_only + generate_grade_report_for_verified_only, + optimize_get_learners_switch_enabled ) from lms.djangoapps.teams.models import CourseTeamMembership from lms.djangoapps.verify_student.services import IDVerificationService @@ -311,8 +310,9 @@ class CourseGradeReport(object): """ Get enrolled learners in a course. - verified_only(bool): It indicates if we need only the verified - enrollments or all enrollments. + Arguments: + course_id (CourseLocator): course_id to return enrollees for. + verified_only (boolean): is a boolean when True, returns only verified enrollees. """ if optimize_get_learners_switch_enabled(): TASK_LOG.info(u'%s, Creating Course Grade with optimization', task_log_message) @@ -365,8 +365,8 @@ class CourseGradeReport(object): 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) - return get_enrolled_learners_for_course(course_id, verified_users_only) + report_for_verified_only = generate_grade_report_for_verified_only() + return get_enrolled_learners_for_course(course_id=course_id, verified_only=report_for_verified_only) def _user_grades(self, course_grade, context): """ @@ -543,10 +543,11 @@ class ProblemGradeReport(object): status_interval = 100 task_id = _xmodule_instance_args.get('task_id') if _xmodule_instance_args is not None else None + report_for_verified_only = generate_grade_report_for_verified_only() enrolled_students = CourseEnrollment.objects.users_enrolled_in( - course_id, + course_id=course_id, include_inactive=True, - verified_only=problem_grade_report_verified_only(course_id), + verified_only=report_for_verified_only, ) task_progress = TaskProgress(action_name, enrolled_students.count(), start_time) diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 37a091294a..7199277edc 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -15,36 +15,30 @@ import tempfile from contextlib import contextmanager from datetime import datetime, timedelta +from six import text_type +from six.moves import range, zip +from six.moves.urllib.parse import quote # pylint: disable=import-error + import ddt +import openedx.core.djangoapps.user_api.course_tag.api as course_tag_api import unicodecsv +from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory +from course_modes.models import CourseMode +from course_modes.tests.factories import CourseModeFactory from django.conf import settings from django.test.utils import override_settings from django.urls import reverse from edx_django_utils.cache import RequestCache from freezegun import freeze_time -from mock import ANY, MagicMock, Mock, patch -from pytz import UTC -from six import text_type -from six.moves import range, zip -from six.moves.urllib.parse import quote # pylint: disable=import-error - -import openedx.core.djangoapps.user_api.course_tag.api as course_tag_api -from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory -from course_modes.models import CourseMode -from course_modes.tests.factories import CourseModeFactory -from lms.djangoapps.courseware.tests.factories import InstructorFactory from lms.djangoapps.certificates.models import CertificateStatuses, GeneratedCertificate from lms.djangoapps.certificates.tests.factories import CertificateWhitelistFactory, GeneratedCertificateFactory +from lms.djangoapps.courseware.tests.factories import InstructorFactory from lms.djangoapps.grades.course_data import CourseData -from lms.djangoapps.grades.models import ( - PersistentCourseGrade, - PersistentSubsectionGradeOverride, -) +from lms.djangoapps.grades.models import PersistentCourseGrade, PersistentSubsectionGradeOverride from lms.djangoapps.grades.subsection_grade import CreateSubsectionGrade from lms.djangoapps.grades.transformer import GradesTransformer from lms.djangoapps.instructor_analytics.basic import UNAVAILABLE, list_problem_responses from lms.djangoapps.instructor_task.tasks_helper.certs import generate_students_certificates -# from lms.djangoapps.instructor_task.config.waffle import problem_grade_report_verified_only from lms.djangoapps.instructor_task.tasks_helper.enrollments import ( upload_enrollment_report, upload_exec_summary_report, @@ -70,12 +64,14 @@ from lms.djangoapps.instructor_task.tests.test_base import ( ) from lms.djangoapps.teams.tests.factories import CourseTeamFactory, CourseTeamMembershipFactory from lms.djangoapps.verify_student.tests.factories import SoftwareSecurePhotoVerificationFactory +from mock import ANY, MagicMock, Mock, patch from openedx.core.djangoapps.course_groups.models import CohortMembership, CourseUserGroupPartitionGroup from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory from openedx.core.djangoapps.credit.tests.factories import CreditCourseFactory from openedx.core.djangoapps.user_api.partition_schemes import RandomUserPartitionScheme from openedx.core.djangoapps.util.testing import ContentGroupTestCase, TestConditionalContent from openedx.core.lib.teams_config import TeamsConfig +from pytz import UTC from shoppingcart.models import ( Coupon, CourseRegistrationCode, @@ -88,19 +84,21 @@ from shoppingcart.models import ( from student.models import ALLOWEDTOENROLL_TO_ENROLLED, CourseEnrollment, CourseEnrollmentAllowed, ManualEnrollmentAudit from student.tests.factories import CourseEnrollmentFactory, UserFactory from survey.models import SurveyAnswer, SurveyForm +from waffle.testutils import override_switch from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls from xmodule.partitions.partitions import Group, UserPartition +from ..config.waffle import GENERATE_GRADE_REPORT_VERIFIED_ONLY from ..models import ReportStore from ..tasks_helper.utils import UPDATE_STATUS_FAILED, UPDATE_STATUS_SUCCEEDED - _TEAMS_CONFIG = TeamsConfig({ 'max_size': 2, 'topics': [{'id': 'topic', 'name': 'Topic', 'description': 'A Topic'}], }) +SWITCH_GENERATE_GRADE_REPORT_VERIFIED_ONLY = '.'.join(['instructor_task', GENERATE_GRADE_REPORT_VERIFIED_ONLY]) class InstructorGradeReportTestCase(TestReportMixin, InstructorTaskCourseTestCase): @@ -422,7 +420,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase): RequestCache.clear_all_namespaces() - expected_query_count = 51 + expected_query_count = 50 with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'): with check_mongo_calls(mongo_count): with self.assertNumQueries(expected_query_count): @@ -945,26 +943,23 @@ class TestProblemGradeReport(TestReportMixin, InstructorTaskModuleTestCase): ]) @patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task') + @override_switch(SWITCH_GENERATE_GRADE_REPORT_VERIFIED_ONLY, True) def test_single_problem_verified_student_only(self, _get_current_task): - with patch( - 'lms.djangoapps.instructor_task.tasks_helper.grades.problem_grade_report_verified_only', - return_value=True, - ): - 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) + 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(self.student_1.username, u'Problem1', ['Option 1']) - self.submit_student_answer(student_verified.username, u'Problem1', ['Option 1']) - result = ProblemGradeReport.generate(None, None, self.course.id, None, 'graded') - self.assertDictContainsSubset( - {'action_name': 'graded', 'attempted': 1, 'succeeded': 1, 'failed': 0}, result - ) + self.submit_student_answer(self.student_1.username, u'Problem1', ['Option 1']) + self.submit_student_answer(student_verified.username, u'Problem1', ['Option 1']) + result = ProblemGradeReport.generate(None, None, self.course.id, None, 'graded') + self.assertDictContainsSubset( + {'action_name': 'graded', 'attempted': 1, 'succeeded': 1, 'failed': 0}, result + ) @patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task') @patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.iter') @@ -2039,31 +2034,28 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): ) @patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task') + @override_switch(SWITCH_GENERATE_GRADE_REPORT_VERIFIED_ONLY, True) 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) + 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 - ) + 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):