diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index b2ff11fcd8..6ef96812f0 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -115,7 +115,7 @@ class CourseEndingTest(TestCase): ) cert_status = {'status': 'generating', 'grade': '67', 'mode': 'honor'} - with patch('lms.djangoapps.grades.new.course_grade.CourseGradeFactory.get_persisted') as patch_persisted_grade: + with patch('lms.djangoapps.grades.new.course_grade_factory.CourseGradeFactory.read') as patch_persisted_grade: patch_persisted_grade.return_value = Mock(percent=100) self.assertEqual( _cert_info(user, course, cert_status, course_mode), diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 8572193178..d806335655 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -68,7 +68,7 @@ from certificates.api import ( # pylint: disable=import-error get_certificate_url, has_html_certificates_enabled, ) -from lms.djangoapps.grades.new.course_grade import CourseGradeFactory +from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from xmodule.modulestore.django import modulestore from opaque_keys import InvalidKeyError @@ -423,7 +423,7 @@ def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disa ) if status in {'generating', 'ready', 'notpassing', 'restricted', 'auditing', 'unverified'}: - persisted_grade = CourseGradeFactory().get_persisted(user, course_overview) + persisted_grade = CourseGradeFactory().read(user, course=course_overview) if persisted_grade is not None: status_dict['grade'] = unicode(persisted_grade.percent) elif 'grade' in cert_status: diff --git a/common/test/acceptance/pages/lms/progress.py b/common/test/acceptance/pages/lms/progress.py index a08559b04c..e09807dee4 100644 --- a/common/test/acceptance/pages/lms/progress.py +++ b/common/test/acceptance/pages/lms/progress.py @@ -14,7 +14,7 @@ class ProgressPage(CoursePage): def is_browser_on_page(self): is_present = ( self.q(css='.course-info').present and - self.q(css='#grade-detail-graph').present + self.q(css='.grade-detail-graph').present ) return is_present @@ -115,7 +115,7 @@ class ProgressPage(CoursePage): Return the CSS index of the chapter with `title`. Returns `None` if it cannot find such a chapter. """ - chapter_css = '.chapters section .hd' + chapter_css = '.chapters>section h3' chapter_titles = self.q(css=chapter_css).map(lambda el: el.text.lower().strip()).results try: diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index e2eecea374..339a10b853 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -231,18 +231,18 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): # # of sql queries to default, # # of mongo queries, # ) - ('no_overrides', 1, True, False): (24, 6), - ('no_overrides', 2, True, False): (24, 6), - ('no_overrides', 3, True, False): (24, 6), - ('ccx', 1, True, False): (24, 6), - ('ccx', 2, True, False): (24, 6), - ('ccx', 3, True, False): (24, 6), - ('no_overrides', 1, False, False): (24, 6), - ('no_overrides', 2, False, False): (24, 6), - ('no_overrides', 3, False, False): (24, 6), - ('ccx', 1, False, False): (24, 6), - ('ccx', 2, False, False): (24, 6), - ('ccx', 3, False, False): (24, 6), + ('no_overrides', 1, True, False): (25, 1), + ('no_overrides', 2, True, False): (25, 1), + ('no_overrides', 3, True, False): (25, 1), + ('ccx', 1, True, False): (25, 1), + ('ccx', 2, True, False): (25, 1), + ('ccx', 3, True, False): (25, 1), + ('no_overrides', 1, False, False): (25, 1), + ('no_overrides', 2, False, False): (25, 1), + ('no_overrides', 3, False, False): (25, 1), + ('ccx', 1, False, False): (25, 1), + ('ccx', 2, False, False): (25, 1), + ('ccx', 3, False, False): (25, 1), } @@ -254,19 +254,19 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): __test__ = True TEST_DATA = { - ('no_overrides', 1, True, False): (24, 3), - ('no_overrides', 2, True, False): (24, 3), - ('no_overrides', 3, True, False): (24, 3), - ('ccx', 1, True, False): (24, 3), - ('ccx', 2, True, False): (24, 3), - ('ccx', 3, True, False): (24, 3), - ('ccx', 1, True, True): (25, 3), - ('ccx', 2, True, True): (25, 3), - ('ccx', 3, True, True): (25, 3), - ('no_overrides', 1, False, False): (24, 3), - ('no_overrides', 2, False, False): (24, 3), - ('no_overrides', 3, False, False): (24, 3), - ('ccx', 1, False, False): (24, 3), - ('ccx', 2, False, False): (24, 3), - ('ccx', 3, False, False): (24, 3), + ('no_overrides', 1, True, False): (25, 3), + ('no_overrides', 2, True, False): (25, 3), + ('no_overrides', 3, True, False): (25, 3), + ('ccx', 1, True, False): (25, 3), + ('ccx', 2, True, False): (25, 3), + ('ccx', 3, True, False): (25, 3), + ('ccx', 1, True, True): (26, 3), + ('ccx', 2, True, True): (26, 3), + ('ccx', 3, True, True): (26, 3), + ('no_overrides', 1, False, False): (25, 3), + ('no_overrides', 2, False, False): (25, 3), + ('no_overrides', 3, False, False): (25, 3), + ('ccx', 1, False, False): (25, 3), + ('ccx', 2, False, False): (25, 3), + ('ccx', 3, False, False): (25, 3), } diff --git a/lms/djangoapps/ccx/views.py b/lms/djangoapps/ccx/views.py index 4d4396bb8c..b8880accc5 100644 --- a/lms/djangoapps/ccx/views.py +++ b/lms/djangoapps/ccx/views.py @@ -33,7 +33,7 @@ from courseware.field_overrides import disable_overrides from django_comment_common.models import FORUM_ROLE_ADMINISTRATOR, assign_role from django_comment_common.utils import seed_permissions_roles from edxmako.shortcuts import render_to_response -from lms.djangoapps.grades.new.course_grade import CourseGradeFactory +from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from opaque_keys.edx.keys import CourseKey from ccx_keys.locator import CCXLocator from student.roles import CourseCcxCoachRole diff --git a/lms/djangoapps/certificates/management/commands/fix_ungraded_certs.py b/lms/djangoapps/certificates/management/commands/fix_ungraded_certs.py index 09e4c2d5a9..a1c01e5df8 100644 --- a/lms/djangoapps/certificates/management/commands/fix_ungraded_certs.py +++ b/lms/djangoapps/certificates/management/commands/fix_ungraded_certs.py @@ -7,7 +7,7 @@ from optparse import make_option from certificates.models import GeneratedCertificate from courseware import courses -from lms.djangoapps.grades.new.course_grade import CourseGradeFactory +from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory log = logging.getLogger(__name__) diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index 005b161d4a..11b1452b7a 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -11,7 +11,7 @@ from django.conf import settings from django.core.urlresolvers import reverse from requests.auth import HTTPBasicAuth -from lms.djangoapps.grades.new.course_grade import CourseGradeFactory +from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from xmodule.modulestore.django import modulestore from capa.xqueue_interface import XQueueInterface from capa.xqueue_interface import make_xheader, make_hashkey @@ -271,7 +271,7 @@ class XQueueCertInterface(object): self.request.session = {} is_whitelisted = self.whitelist.filter(user=student, course_id=course_id, whitelist=True).exists() - grade = CourseGradeFactory().create(student, course).summary + course_grade = CourseGradeFactory().create(student, course) enrollment_mode, __ = CourseEnrollment.enrollment_mode_for_user(student, course_id) mode_is_verified = enrollment_mode in GeneratedCertificate.VERIFIED_CERTS_MODES user_is_verified = SoftwareSecurePhotoVerification.user_is_verified(student) @@ -295,8 +295,6 @@ class XQueueCertInterface(object): else: # honor code and audit students template_pdf = "certificate-template-{id.org}-{id.course}.pdf".format(id=course_id) - if forced_grade: - grade['grade'] = forced_grade LOGGER.info( ( @@ -317,13 +315,13 @@ class XQueueCertInterface(object): cert.mode = cert_mode cert.user = student - cert.grade = grade['percent'] + cert.grade = course_grade.percent cert.course_id = course_id cert.name = profile_name cert.download_url = '' # Strip HTML from grade range label - grade_contents = grade.get('grade', None) + grade_contents = forced_grade or course_grade.letter_grade try: grade_contents = lxml.html.fromstring(grade_contents).text_content() passing = True diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index a9bafbd7d3..61a8d5e773 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -22,7 +22,7 @@ from capa.tests.response_xml_factory import ( from course_modes.models import CourseMode from courseware.models import StudentModule, BaseStudentModuleHistory from courseware.tests.helpers import LoginEnrollmentTestCase -from lms.djangoapps.grades.new.course_grade import CourseGradeFactory +from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from openedx.core.djangoapps.credit.api import ( set_credit_requirements, get_credit_requirement_status ) diff --git a/lms/djangoapps/courseware/tests/test_view_authentication.py b/lms/djangoapps/courseware/tests/test_view_authentication.py index eac6e4ec3e..0b3d644b63 100644 --- a/lms/djangoapps/courseware/tests/test_view_authentication.py +++ b/lms/djangoapps/courseware/tests/test_view_authentication.py @@ -86,7 +86,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase): # The student progress tab is not accessible to a student # before launch, so the instructor view-as-student feature - # should return a 403. + # should return a 404. # TODO (vshnayder): If this is not the behavior we want, will need # to make access checking smarter and understand both the effective # user (the student), and the requesting user (the prof) @@ -97,7 +97,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase): 'student_id': self.enrolled_user.id, } ) - self.assert_request_status_code(403, url) + self.assert_request_status_code(404, url) # The courseware url should redirect, not 200 url = self._reverse_urls(['courseware'], course)[0] diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 1ec5d8e39a..13054bbbab 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -46,6 +46,7 @@ from courseware.tests.factories import StudentModuleFactory, GlobalStaffFactory from courseware.url_helpers import get_redirect_url from courseware.user_state_client import DjangoXBlockUserStateClient from lms.djangoapps.commerce.utils import EcommerceService # pylint: disable=import-error +from lms.djangoapps.grades.config.waffle import waffle as grades_waffle, ASSUME_ZERO_GRADE_IF_ABSENT from milestones.tests.utils import MilestonesTestCaseMixin from openedx.core.djangoapps.catalog.tests.factories import CourseFactory as CatalogCourseFactory from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory, CourseRunFactory @@ -1440,19 +1441,25 @@ class ProgressPageTests(ModuleStoreTestCase): """Test that query counts remain the same for self-paced and instructor-paced courses.""" SelfPacedConfiguration(enabled=self_paced_enabled).save() self.setup_course(self_paced=self_paced) - with self.assertNumQueries(41), check_mongo_calls(4): + with self.assertNumQueries(42), check_mongo_calls(1): self._get_progress_page() - def test_progress_queries(self): + @ddt.data( + (False, 42, 28), + (True, 35, 24) + ) + @ddt.unpack + def test_progress_queries(self, enable_waffle, initial, subsequent): self.setup_course() - with self.assertNumQueries(41), check_mongo_calls(4): - self._get_progress_page() - - # subsequent accesses to the progress page require fewer queries. - for _ in range(2): - with self.assertNumQueries(27), check_mongo_calls(4): + with grades_waffle().override_in_model(ASSUME_ZERO_GRADE_IF_ABSENT, active=enable_waffle): + with self.assertNumQueries(initial), check_mongo_calls(1): self._get_progress_page() + # subsequent accesses to the progress page require fewer queries. + for _ in range(2): + with self.assertNumQueries(subsequent), check_mongo_calls(1): + self._get_progress_page() + @patch( 'lms.djangoapps.grades.new.course_grade.CourseGrade.summary', PropertyMock(return_value={'grade': 'Pass', 'percent': 0.75, 'section_breakdown': [], 'grade_breakdown': {}}) diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index aaf3036d90..c3f3784cb0 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -26,7 +26,7 @@ import urllib import waffle from lms.djangoapps.gating.api import get_entrance_exam_score_ratio, get_entrance_exam_usage_key -from lms.djangoapps.grades.new.course_grade import CourseGradeFactory +from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY from openedx.core.djangoapps.user_api.preferences.api import get_user_preference diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 2b657063ec..25dc1a1d07 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -40,7 +40,7 @@ from opaque_keys.edx.keys import CourseKey, UsageKey from rest_framework import status from lms.djangoapps.instructor.views.api import require_global_staff from lms.djangoapps.ccx.utils import prep_course_for_grading -from lms.djangoapps.grades.new.course_grade import CourseGradeFactory +from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from lms.djangoapps.instructor.enrollment import uses_shib from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException @@ -832,8 +832,7 @@ def _progress(request, course_key, student_id): except ValueError: raise Http404 - course = get_course_with_access(request.user, 'load', course_key, depth=None, check_if_enrolled=True) - prep_course_for_grading(course, request) + course = get_course_with_access(request.user, 'load', course_key) # check to see if there is a required survey that must be taken before # the user can access the course. @@ -861,12 +860,16 @@ def _progress(request, course_key, student_id): except User.DoesNotExist: raise Http404 - # NOTE: To make sure impersonation by instructor works, use - # student instead of request.user in the rest of the function. - # The pre-fetching of groups is done to make auth checks not require an # additional DB lookup (this kills the Progress page in particular). student = User.objects.prefetch_related("groups").get(id=student.id) + if request.user.id != student.id: + # refetch the course as the assumed student + course = get_course_with_access(student, 'load', course_key, check_if_enrolled=True) + prep_course_for_grading(course, request) + + # NOTE: To make sure impersonation by instructor works, use + # student instead of request.user in the rest of the function. course_grade = CourseGradeFactory().create(student, course) courseware_summary = course_grade.chapter_grades.values() diff --git a/lms/djangoapps/gating/api.py b/lms/djangoapps/gating/api.py index baf6888c08..cc9287fde4 100644 --- a/lms/djangoapps/gating/api.py +++ b/lms/djangoapps/gating/api.py @@ -78,7 +78,7 @@ def evaluate_entrance_exam(course_grade, user): minimum score required, the dependent milestones will be marked fulfilled for the user. """ - course = course_grade.course + course = course_grade.course_data.course if milestones_helpers.is_entrance_exams_enabled() and getattr(course, 'entrance_exam_enabled', False): if get_entrance_exam_content(user, course): exam_chapter_key = get_entrance_exam_usage_key(course) diff --git a/lms/djangoapps/gating/tests/test_integration.py b/lms/djangoapps/gating/tests/test_integration.py index 4bfa8c5df5..5f8aba1108 100644 --- a/lms/djangoapps/gating/tests/test_integration.py +++ b/lms/djangoapps/gating/tests/test_integration.py @@ -7,7 +7,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from lms.djangoapps.courseware.access import has_access from lms.djangoapps.grades.tests.utils import answer_problem -from lms.djangoapps.grades.new.course_grade import CourseGradeFactory +from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from milestones import api as milestones_api from milestones.tests.utils import MilestonesTestCaseMixin from openedx.core.djangolib.testing.utils import get_mock_request diff --git a/lms/djangoapps/grades/api/views.py b/lms/djangoapps/grades/api/views.py index 5590f051c5..c211204149 100644 --- a/lms/djangoapps/grades/api/views.py +++ b/lms/djangoapps/grades/api/views.py @@ -14,7 +14,7 @@ from courseware.access import has_access from lms.djangoapps.ccx.utils import prep_course_for_grading from lms.djangoapps.courseware import courses from lms.djangoapps.grades.api.serializers import GradingPolicySerializer -from lms.djangoapps.grades.new.course_grade import CourseGradeFactory +from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from openedx.core.lib.api.authentication import OAuth2AuthenticationAllowInactiveUser from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes from student.roles import CourseStaffRole diff --git a/lms/djangoapps/grades/config/__init__.py b/lms/djangoapps/grades/config/__init__.py index e69de29bb2..e946280209 100644 --- a/lms/djangoapps/grades/config/__init__.py +++ b/lms/djangoapps/grades/config/__init__.py @@ -0,0 +1,16 @@ +from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag +from lms.djangoapps.grades.config.waffle import waffle, ASSUME_ZERO_GRADE_IF_ABSENT + + +def assume_zero_if_absent(course_key): + """ + Returns whether an absent grade should be assumed to be zero. + """ + return should_persist_grades(course_key) and waffle().is_enabled(ASSUME_ZERO_GRADE_IF_ABSENT) + + +def should_persist_grades(course_key): + """ + Returns whether grades should be persisted. + """ + return PersistentGradesEnabledFlag.feature_enabled(course_key) diff --git a/lms/djangoapps/grades/config/waffle.py b/lms/djangoapps/grades/config/waffle.py new file mode 100644 index 0000000000..097902e08a --- /dev/null +++ b/lms/djangoapps/grades/config/waffle.py @@ -0,0 +1,20 @@ +""" +This module contains various configuration settings via +waffle switches for the Grades app. +""" +from openedx.core.djangolib.waffle_utils import WaffleSwitchPlus + + +# Namespace +WAFFLE_NAMESPACE = u'grades' + +# Switches +WRITE_ONLY_IF_ENGAGED = u'write_only_if_engaged' +ASSUME_ZERO_GRADE_IF_ABSENT = u'assume_zero_grade_if_absent' + + +def waffle(): + """ + Returns the namespaced, cached, audited Waffle class for Grades. + """ + return WaffleSwitchPlus(namespace=WAFFLE_NAMESPACE, log_prefix=u'Grades: ') diff --git a/lms/djangoapps/grades/management/commands/compute_grades.py b/lms/djangoapps/grades/management/commands/compute_grades.py index 3d3c02d996..bc0dfc6421 100644 --- a/lms/djangoapps/grades/management/commands/compute_grades.py +++ b/lms/djangoapps/grades/management/commands/compute_grades.py @@ -101,7 +101,7 @@ class Command(BaseCommand): kwargs=kwargs, options=task_options, ) - log.info("Persistent grades: Created {task_name}[{task_id}] with arguments {kwargs}".format( + log.info("Grades: Created {task_name}[{task_id}] with arguments {kwargs}".format( task_name=tasks.compute_grades_for_course.name, task_id=result.task_id, kwargs=kwargs, diff --git a/lms/djangoapps/grades/management/commands/get_grades.py b/lms/djangoapps/grades/management/commands/get_grades.py index c6b5525a74..fcdff39e21 100644 --- a/lms/djangoapps/grades/management/commands/get_grades.py +++ b/lms/djangoapps/grades/management/commands/get_grades.py @@ -7,7 +7,7 @@ from django.core.management.base import BaseCommand, CommandError import os from lms.djangoapps.courseware import courses from lms.djangoapps.certificates.models import GeneratedCertificate -from lms.djangoapps.grades.new.course_grade import CourseGradeFactory +from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey diff --git a/lms/djangoapps/grades/new/course_data.py b/lms/djangoapps/grades/new/course_data.py new file mode 100644 index 0000000000..b2d59ac069 --- /dev/null +++ b/lms/djangoapps/grades/new/course_data.py @@ -0,0 +1,102 @@ +from lms.djangoapps.course_blocks.api import get_course_blocks +from xmodule.modulestore.django import modulestore +from ..transformer import GradesTransformer + + +class CourseData(object): + """ + Utility access layer to intelligently get and cache the + requested course data as long as at least one property is + provided upon initialization. + + This is an in-memory object that maintains its own internal + cache during its lifecycle. + """ + def __init__(self, user, course=None, collected_block_structure=None, structure=None, course_key=None): + if not any([course, collected_block_structure, structure, course_key]): + raise ValueError( + "You must specify one of course, collected_block_structure, structure, or course_key to this method." + ) + self.user = user + self._collected_block_structure = collected_block_structure + self._structure = structure + self._course = course + self._course_key = course_key + self._location = None + + @property + def course_key(self): + if not self._course_key: + if self._course: + self._course_key = self._course.id + else: + structure = self.effective_structure + self._course_key = structure.root_block_usage_key.course_key + return self._course_key + + @property + def location(self): + if not self._location: + structure = self.effective_structure + if structure: + self._location = structure.root_block_usage_key + elif self._course: + self._location = self._course.location + else: + self._location = modulestore().make_course_usage_key(self.course_key) + return self._location + + @property + def structure(self): + if not self._structure: + self._structure = get_course_blocks( + self.user, + self.location, + collected_block_structure=self._collected_block_structure, + ) + return self._structure + + @property + def course(self): + if not self._course: + self._course = modulestore().get_course(self.course_key) + return self._course + + @property + def grading_policy_hash(self): + structure = self.effective_structure + if structure: + return structure.get_transformer_block_field( + structure.root_block_usage_key, + GradesTransformer, + 'grading_policy_hash', + ) + else: + return GradesTransformer.grading_policy_hash(self.course) + + @property + def version(self): + structure = self.effective_structure + course_block = structure[self.location] if structure else self.course + return getattr(course_block, 'course_version', None) + + @property + def edited_on(self): + # get course block from structure only; subtree_edited_on field on modulestore's course block isn't optimized. + course_block = self.structure[self.location] + return getattr(course_block, 'subtree_edited_on', None) + + @property + def effective_structure(self): + return self._structure or self._collected_block_structure + + def __unicode__(self): + return u'Course: course_key: {}'.format(self.course_key) + + def full_string(self): + if self.effective_structure: + return u'Course: course_key: {}, version: {}, edited_on: {}, grading_policy: {}'.format( + self.course_key, self.version, self.edited_on, self.grading_policy_hash, + ) + else: + return u'Course: course_key: {}, empty course structure'.format(self.course_key) diff --git a/lms/djangoapps/grades/new/course_grade.py b/lms/djangoapps/grades/new/course_grade.py index 03531116db..d358875e45 100644 --- a/lms/djangoapps/grades/new/course_grade.py +++ b/lms/djangoapps/grades/new/course_grade.py @@ -1,46 +1,39 @@ """ CourseGrade Class """ - -from collections import defaultdict, namedtuple, OrderedDict -from logging import getLogger - +from abc import abstractmethod +from collections import defaultdict, OrderedDict from django.conf import settings -from django.core.exceptions import PermissionDenied -import dogstats_wrapper as dog_stats_api from lazy import lazy -from lms.djangoapps.course_blocks.api import get_course_blocks -from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag -from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager -from openedx.core.djangoapps.signals.signals import COURSE_GRADE_CHANGED from xmodule import block_metadata_utils - -from ..models import PersistentCourseGrade -from .subsection_grade import SubsectionGradeFactory -from ..transformer import GradesTransformer +from .subsection_grade_factory import SubsectionGradeFactory +from .subsection_grade import ZeroSubsectionGrade -log = getLogger(__name__) - - -class CourseGrade(object): +class CourseGradeBase(object): """ - Course Grade class + Base class for Course Grades. """ - def __init__(self, student, course, course_structure): - self.student = student - self.course = course - self._percent = None - self._letter_grade = None + def __init__(self, user, course_data, percent=0, letter_grade=None, passed=False): + self.user = user + self.course_data = course_data - self.course_structure = course_structure - if self.course_structure: - course_block = course_structure[course.location] - self.course_version = getattr(course_block, 'course_version', None) - self.course_edited_timestamp = getattr(course_block, 'subtree_edited_on', None) + self.percent = percent + self.letter_grade = letter_grade + self.passed = passed - self._subsection_grade_factory = SubsectionGradeFactory(self.student, self.course, self.course_structure) + def __unicode__(self): + return u'Course Grade: percent: %s, letter_grade: %s, passed: %s'.format( + unicode(self.percent), self.letter_grade, self.passed, + ) + + def attempted(self): + """ + Returns whether at least one problem was attempted + by the user in the course. + """ + return False @lazy def graded_subsections_by_format(self): @@ -57,6 +50,20 @@ class CourseGrade(object): subsections_by_format[subsection_grade.format][subsection_grade.location] = subsection_grade return subsections_by_format + @lazy + def chapter_grades(self): + """ + Returns a dictionary of dictionaries. + The primary dictionary is keyed by the chapter's usage_key. + The secondary dictionary contains the chapter's + subsection grades, display name, and url name. + """ + course_structure = self.course_data.structure + return { + chapter_key: self._get_chapter_grade_info(course_structure[chapter_key], course_structure) + for chapter_key in course_structure.get_children(self.course_data.location) + } + @lazy def locations_to_scores(self): """ @@ -68,127 +75,6 @@ class CourseGrade(object): locations_to_scores.update(subsection_grade.locations_to_scores) return locations_to_scores - @lazy - def grade_value(self): - """ - Helper function to extract the grade value as calculated by the course's grader. - """ - # Grading policy might be overriden by a CCX, need to reset it - self.course.set_grading_policy(self.course.grading_policy) - grade_value = self.course.grader.grade( - self.graded_subsections_by_format, - generate_random_scores=settings.GENERATE_PROFILE_SCORES - ) - # can't use the existing properties due to recursion issues caused by referencing self.grade_value - percent = self._calc_percent(grade_value) - letter_grade = self._compute_letter_grade(percent) - self._log_event(log.warning, u"grade_value, percent: {0}, grade: {1}".format(percent, letter_grade)) - return grade_value - - @lazy - def chapter_grades(self): - """ - Returns a dictionary of dictionaries. - The primary dictionary is keyed by the chapter's usage_key. - The secondary dictionary contains the chapter's - subsection grades, display name, and url name. - """ - chapter_grades = OrderedDict() - for chapter_key in self.course_structure.get_children(self.course.location): - chapter = self.course_structure[chapter_key] - chapter_subsection_grades = [] - children = self.course_structure.get_children(chapter_key) - for subsection_key in children: - chapter_subsection_grades.append( - self._subsection_grade_factory.create(self.course_structure[subsection_key], read_only=True) - ) - - chapter_grades[chapter_key] = { - 'display_name': block_metadata_utils.display_name_with_default_escaped(chapter), - 'url_name': block_metadata_utils.url_name_for_block(chapter), - 'sections': chapter_subsection_grades - } - return chapter_grades - - @property - def percent(self): - """ - Returns a rounded percent from the overall grade. - """ - if self._percent is None: - self._percent = self._calc_percent(self.grade_value) - return self._percent - - @property - def letter_grade(self): - """ - Returns a letter representing the grade. - """ - if self._letter_grade is None: - self._letter_grade = self._compute_letter_grade(self.percent) - return self._letter_grade - - @property - def passed(self): - """ - Check user's course passing status. Return True if passed. - """ - nonzero_cutoffs = [cutoff for cutoff in self.course.grade_cutoffs.values() if cutoff > 0] - success_cutoff = min(nonzero_cutoffs) if nonzero_cutoffs else None - return success_cutoff and self.percent >= success_cutoff - - @property - def summary(self): - """ - Returns the grade summary as calculated by the course's grader. - """ - grade_summary = self.grade_value - grade_summary['percent'] = self.percent - grade_summary['grade'] = self.letter_grade - - return grade_summary - - def compute_and_update(self, read_only=False): - """ - Computes the grade for the given student and course. - - If read_only is True, doesn't save any updates to the grades. - """ - subsections_total = sum(len(chapter['sections']) for chapter in self.chapter_grades.itervalues()) - - total_graded_subsections = sum(len(x) for x in self.graded_subsections_by_format.itervalues()) - subsections_created = len(self._subsection_grade_factory._unsaved_subsection_grades) # pylint: disable=protected-access - subsections_read = subsections_total - subsections_created - blocks_total = len(self.locations_to_scores) - - if not read_only: - if PersistentGradesEnabledFlag.feature_enabled(self.course.id): - self._subsection_grade_factory.bulk_create_unsaved() - grading_policy_hash = self.get_grading_policy_hash(self.course.location, self.course_structure) - PersistentCourseGrade.update_or_create_course_grade( - user_id=self.student.id, - course_id=self.course.id, - course_version=self.course_version, - course_edited_timestamp=self.course_edited_timestamp, - grading_policy_hash=grading_policy_hash, - percent_grade=self.percent, - letter_grade=self.letter_grade or "", - passed=self.passed, - ) - self._signal_listeners_when_grade_computed() - - self._log_event( - log.warning, - u"compute_and_update, read_only: {0}, subsections read/created: {1}/{2}, blocks accessed: {3}, total " - u"graded subsections: {4}".format( - read_only, - subsections_read, - subsections_created, - blocks_total, - total_graded_subsections, - ) - ) - def score_for_chapter(self, chapter_key): """ Returns the aggregate weighted score for the given chapter. @@ -206,7 +92,6 @@ class CourseGrade(object): """ Calculate the aggregate weighted score for any location in the course. This method returns a tuple containing (earned_score, possible_score). - If the location is of 'problem' type, this method will return the possible and earned scores for that problem. If the location refers to a composite module (a vertical or section ) the scores will be the sums of @@ -215,7 +100,7 @@ class CourseGrade(object): if location in self.locations_to_scores: score = self.locations_to_scores[location] return score.earned, score.possible - children = self.course_structure.get_children(location) + children = self.course_data.structure.get_children(location) earned, possible = 0.0, 0.0 for child in children: child_earned, child_possible = self.score_for_module(child) @@ -223,229 +108,136 @@ class CourseGrade(object): possible += child_possible return earned, possible - @staticmethod - def get_grading_policy_hash(course_location, course_structure): + @lazy + def grader_result(self): """ - Gets the grading policy of the course at the given location - in the given course structure. + Returns the result from the course grader. """ - return course_structure.get_transformer_block_field( - course_location, - GradesTransformer, - 'grading_policy_hash' + course = self.course_data.course + course.set_grading_policy(course.grading_policy) + return course.grader.grade( + self.graded_subsections_by_format, + generate_random_scores=settings.GENERATE_PROFILE_SCORES, ) - @classmethod - def load_persisted_grade(cls, user, course, course_structure): + @property + def summary(self): """ - Initializes a CourseGrade object, filling its members with persisted values from the database. - - If the grading policy is out of date, recomputes the grade. - - If no persisted values are found, returns None. + Returns the grade summary as calculated by the course's grader. + DEPRECATED: To be removed as part of TNL-5291. """ - try: - persistent_grade = PersistentCourseGrade.read_course_grade(user.id, course.id) - except PersistentCourseGrade.DoesNotExist: - return None - course_grade = CourseGrade(user, course, course_structure) + # TODO(TNL-5291) Remove usages of this deprecated property. + grade_summary = self.grader_result + grade_summary['percent'] = self.percent + grade_summary['grade'] = self.letter_grade + return grade_summary - current_grading_policy_hash = course_grade.get_grading_policy_hash(course.location, course_structure) - if current_grading_policy_hash != persistent_grade.grading_policy_hash: - return None - else: - course_grade._percent = persistent_grade.percent_grade # pylint: disable=protected-access - course_grade._letter_grade = persistent_grade.letter_grade # pylint: disable=protected-access - course_grade.course_version = persistent_grade.course_version - course_grade.course_edited_timestamp = persistent_grade.course_edited_timestamp - - course_grade._log_event(log.info, u"load_persisted_grade") # pylint: disable=protected-access - - return course_grade - - @classmethod - def get_persisted_grade(cls, user, course): + def _get_chapter_grade_info(self, chapter, course_structure): """ - Gets the persisted grade in the database, without checking - whether it is up-to-date with the course's grading policy. - For read use only. + Helper that returns a dictionary of chapter grade information. """ - try: - persistent_grade = PersistentCourseGrade.read_course_grade(user.id, course.id) - except PersistentCourseGrade.DoesNotExist: - return None - else: - course_grade = CourseGrade(user, course, None) # no course structure needed - course_grade._percent = persistent_grade.percent_grade # pylint: disable=protected-access - course_grade._letter_grade = persistent_grade.letter_grade # pylint: disable=protected-access - course_grade.course_version = persistent_grade.course_version - course_grade.course_edited_timestamp = persistent_grade.course_edited_timestamp - return course_grade + chapter_subsection_grades = self._get_subsection_grades(course_structure, chapter.location) + return { + 'display_name': block_metadata_utils.display_name_with_default_escaped(chapter), + 'url_name': block_metadata_utils.url_name_for_block(chapter), + 'sections': chapter_subsection_grades, + } + + def _get_subsection_grades(self, course_structure, chapter_key): + """ + Returns a list of subsection grades for the given chapter. + """ + return [ + self._get_subsection_grade(course_structure[subsection_key]) + for subsection_key in course_structure.get_children(chapter_key) + ] + + @abstractmethod + def _get_subsection_grade(self, subsection): + """ + Abstract method to be implemented by subclasses for returning + the grade of the given subsection. + """ + raise NotImplementedError + + +class ZeroCourseGrade(CourseGradeBase): + """ + Course Grade class for Zero-value grades when no problems were + attempted in the course. + """ + def __init__(self, user, course_data): + super(ZeroCourseGrade, self).__init__(user, course_data) + + def _get_subsection_grade(self, subsection): + return ZeroSubsectionGrade(subsection, self.course_data) + + +class CourseGrade(CourseGradeBase): + """ + Course Grade class when grades are updated or read from storage. + """ + def __init__(self, user, course_data, *args, **kwargs): + super(CourseGrade, self).__init__(user, course_data, *args, **kwargs) + self._subsection_grade_factory = SubsectionGradeFactory(user, course_data=course_data) + + def update(self): + """ + Updates the grade for the course. + """ + grade_cutoffs = self.course_data.course.grade_cutoffs + self.percent = self._compute_percent(self.grader_result) + self.letter_grade = self._compute_letter_grade(grade_cutoffs, self.percent) + self.passed = self._compute_passed(grade_cutoffs, self.percent) + + @lazy + def attempted(self): + """ + Returns whether any of the subsections in this course + have been attempted by the student. + """ + for chapter in self.chapter_grades.itervalues(): + for subsection_grade in chapter['sections']: + if subsection_grade.attempted: + return True + return False + + def _get_subsection_grade(self, subsection): + # Pass read_only here so the subsection grades can be persisted in bulk at the end. + return self._subsection_grade_factory.create(subsection, read_only=True) @staticmethod - def _calc_percent(grade_value): + def _compute_percent(grader_result): """ - Helper for percent calculation. + Computes and returns the grade percentage from the given + result from the grader. """ - return round(grade_value['percent'] * 100 + 0.05) / 100 + return round(grader_result['percent'] * 100 + 0.05) / 100 - def _compute_letter_grade(self, percentage): + @staticmethod + def _compute_letter_grade(grade_cutoffs, percent): """ - Returns a letter grade as defined in grading_policy (e.g. 'A' 'B' 'C' for 6.002x) or None. - - Arguments - - grade_cutoffs is a dictionary mapping a grade to the lowest - possible percentage to earn that grade. - - percentage is the final percent across all problems in a course + Computes and returns the course letter grade given the + inputs, as defined in the grading_policy (e.g. 'A' 'B' 'C') + or None if not passed. """ - letter_grade = None - grade_cutoffs = self.course.grade_cutoffs # Possible grades, sorted in descending order of score descending_grades = sorted(grade_cutoffs, key=lambda x: grade_cutoffs[x], reverse=True) for possible_grade in descending_grades: - if percentage >= grade_cutoffs[possible_grade]: + if percent >= grade_cutoffs[possible_grade]: letter_grade = possible_grade break return letter_grade - def _signal_listeners_when_grade_computed(self): + @staticmethod + def _compute_passed(grade_cutoffs, percent): """ - Signal all listeners when grades are computed. + Computes and returns whether the given percent value + is a passing grade according to the given grade cutoffs. """ - responses = COURSE_GRADE_CHANGED.send_robust( - sender=None, - user=self.student, - course_grade=self, - course_key=self.course.id, - deadline=self.course.end - ) - - for receiver, response in responses: - log.debug( - 'Signal fired when student grade is calculated. Receiver: %s. Response: %s', - receiver, response - ) - - def _log_event(self, log_func, log_statement): - """ - Logs the given statement, for this instance. - """ - log_func(u"Persistent Grades: CourseGrade.{0}, course: {1}, user: {2}".format( - log_statement, - self.course.id, - self.student.id - )) - - -class CourseGradeFactory(object): - """ - Factory class to create Course Grade objects - """ - def create(self, student, course, collected_block_structure=None, read_only=True): - """ - Returns the CourseGrade object for the given student and course. - - If read_only is True, doesn't save any updates to the grades. - Raises a PermissionDenied if the user does not have course access. - """ - course_structure = get_course_blocks( - student, - course.location, - collected_block_structure=collected_block_structure, - ) - - # if user does not have access to this course, throw an exception - if not self._user_has_access_to_course(course_structure): - raise PermissionDenied("User does not have access to this course") - - return ( - self._get_saved_grade(student, course, course_structure) or - self._compute_and_update_grade(student, course, course_structure, read_only) - ) - - GradeResult = namedtuple('GradeResult', ['student', 'course_grade', 'err_msg']) - - def iter(self, course, students, read_only=True): - """ - Given a course and an iterable of students (User), yield a GradeResult - for every student enrolled in the course. GradeResult is a named tuple of: - - (student, course_grade, err_msg) - - If an error occurred, course_grade will be None and err_msg will be an - exception message. If there was no error, err_msg is an empty string. - """ - # Pre-fetch the collected course_structure so: - # 1. Correctness: the same version of the course is used to - # compute the grade for all students. - # 2. Optimization: the collected course_structure is not - # retrieved from the data store multiple times. - - collected_block_structure = get_block_structure_manager(course.id).get_collected() - for student in students: - with dog_stats_api.timer('lms.grades.CourseGradeFactory.iter', tags=[u'action:{}'.format(course.id)]): - try: - course_grade = CourseGradeFactory().create(student, course, collected_block_structure, read_only=read_only) - yield self.GradeResult(student, course_grade, "") - - except Exception as exc: # pylint: disable=broad-except - # Keep marching on even if this student couldn't be graded for - # some reason, but log it for future reference. - log.exception( - 'Cannot grade student %s (%s) in course %s because of exception: %s', - student.username, - student.id, - course.id, - exc.message - ) - yield self.GradeResult(student, None, exc.message) - - def update(self, student, course, course_structure): - """ - Updates the CourseGrade for this Factory's student. - """ - self._compute_and_update_grade(student, course, course_structure) - - def get_persisted(self, student, course): - """ - Returns the saved grade for the given course and student, - irrespective of whether the saved grade is up-to-date. - """ - if not PersistentGradesEnabledFlag.feature_enabled(course.id): - return None - - return CourseGrade.get_persisted_grade(student, course) - - def _get_saved_grade(self, student, course, course_structure): - """ - Returns the saved grade for the given course and student. - """ - if not PersistentGradesEnabledFlag.feature_enabled(course.id): - return None - - return CourseGrade.load_persisted_grade( - student, - course, - course_structure - ) - - def _compute_and_update_grade(self, student, course, course_structure, read_only=False): - """ - Freshly computes and updates the grade for the student and course. - - If read_only is True, doesn't save any updates to the grades. - """ - course_grade = CourseGrade(student, course, course_structure) - course_grade.compute_and_update(read_only) - return course_grade - - def _user_has_access_to_course(self, course_structure): - """ - Given a course structure, returns whether the user - for whom that course structure was retrieved - has access to the course. - """ - return len(course_structure) > 0 + nonzero_cutoffs = [cutoff for cutoff in grade_cutoffs.values() if cutoff > 0] + success_cutoff = min(nonzero_cutoffs) if nonzero_cutoffs else None + return success_cutoff and percent >= success_cutoff diff --git a/lms/djangoapps/grades/new/course_grade_factory.py b/lms/djangoapps/grades/new/course_grade_factory.py new file mode 100644 index 0000000000..f71f86b3c1 --- /dev/null +++ b/lms/djangoapps/grades/new/course_grade_factory.py @@ -0,0 +1,186 @@ +from collections import namedtuple +import dogstats_wrapper as dog_stats_api +from logging import getLogger + +from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager +from openedx.core.djangoapps.signals.signals import COURSE_GRADE_CHANGED + +from ..config import assume_zero_if_absent, should_persist_grades +from ..config.waffle import waffle, WRITE_ONLY_IF_ENGAGED +from ..models import PersistentCourseGrade +from .course_data import CourseData +from .course_grade import CourseGrade, ZeroCourseGrade + + +log = getLogger(__name__) + + +class CourseGradeFactory(object): + """ + Factory class to create Course Grade objects. + """ + GradeResult = namedtuple('GradeResult', ['student', 'course_grade', 'err_msg']) + + def create(self, user, course=None, collected_block_structure=None, course_structure=None, course_key=None): + """ + Returns the CourseGrade for the given user in the course. + Reads the value from storage and validates that the grading + policy hasn't changed since the grade was last computed. + If not in storage, returns a ZeroGrade if ASSUME_ZERO_GRADE_IF_ABSENT. + Else, if changed or not in storage, computes and returns a new value. + + At least one of course, collected_block_structure, course_structure, + or course_key should be provided. + """ + course_data = CourseData(user, course, collected_block_structure, course_structure, course_key) + try: + course_grade, read_policy_hash = self._read(user, course_data) + if read_policy_hash == course_data.grading_policy_hash: + return course_grade + read_only = False # update the persisted grade since the policy changed; TODO(TNL-6786) remove soon + except PersistentCourseGrade.DoesNotExist: + if assume_zero_if_absent(course_data.course_key): + return self._create_zero(user, course_data) + read_only = True # keep the grade un-persisted; TODO(TNL-6786) remove once all grades are backfilled + + return self._update(user, course_data, read_only) + + def read(self, user, course=None, collected_block_structure=None, course_structure=None, course_key=None): + """ + Returns the CourseGrade for the given user in the course as + persisted in storage. Does NOT verify whether the grading + policy is still valid since the grade was last computed. + If not in storage, returns a ZeroGrade if ASSUME_ZERO_GRADE_IF_ABSENT + else returns None. + + At least one of course, collected_block_structure, course_structure, + or course_key should be provided. + """ + course_data = CourseData(user, course, collected_block_structure, course_structure, course_key) + try: + course_grade, _ = self._read(user, course_data) + return course_grade + except PersistentCourseGrade.DoesNotExist: + if assume_zero_if_absent(course_data.course_key): + return self._create_zero(user, course_data) + else: + return None + + def update(self, user, course=None, collected_block_structure=None, course_structure=None, course_key=None): + """ + Computes, updates, and returns the CourseGrade for the given + user in the course. + + At least one of course, collected_block_structure, course_structure, + or course_key should be provided. + """ + course_data = CourseData(user, course, collected_block_structure, course_structure, course_key) + return self._update(user, course_data, read_only=False) + + def iter(self, course, students, force_update=False): + """ + Given a course and an iterable of students (User), yield a GradeResult + for every student enrolled in the course. GradeResult is a named tuple of: + + (student, course_grade, err_msg) + + If an error occurred, course_grade will be None and err_msg will be an + exception message. If there was no error, err_msg is an empty string. + """ + # Pre-fetch the collected course_structure so: + # 1. Correctness: the same version of the course is used to + # compute the grade for all students. + # 2. Optimization: the collected course_structure is not + # retrieved from the data store multiple times. + + collected_block_structure = get_block_structure_manager(course.id).get_collected() + for student in students: + with dog_stats_api.timer('lms.grades.CourseGradeFactory.iter', tags=[u'action:{}'.format(course.id)]): + try: + operation = CourseGradeFactory().update if force_update else CourseGradeFactory().create + course_grade = operation(student, course, collected_block_structure) + yield self.GradeResult(student, course_grade, "") + + except Exception as exc: # pylint: disable=broad-except + # Keep marching on even if this student couldn't be graded for + # some reason, but log it for future reference. + log.exception( + 'Cannot grade student %s (%s) in course %s because of exception: %s', + student.username, + student.id, + course.id, + exc.message + ) + yield self.GradeResult(student, None, exc.message) + + @staticmethod + def _create_zero(user, course_data): + """ + Returns a ZeroCourseGrade object for the given user and course. + """ + log.info(u'Grades: CreateZero, %s, User: %s', unicode(course_data), user.id) + return ZeroCourseGrade(user, course_data) + + @staticmethod + def _read(user, course_data): + """ + Returns a CourseGrade object based on stored grade information + for the given user and course. + """ + if not should_persist_grades(course_data.course_key): + raise PersistentCourseGrade.DoesNotExist + + persistent_grade = PersistentCourseGrade.read_course_grade(user.id, course_data.course_key) + course_grade = CourseGrade( + user, + course_data, + persistent_grade.percent_grade, + persistent_grade.letter_grade, + persistent_grade.passed_timestamp is not None, + ) + log.info(u'Grades: Read, %s, User: %s, %s', unicode(course_data), user.id, persistent_grade) + + return course_grade, persistent_grade.grading_policy_hash + + @staticmethod + def _update(user, course_data, read_only): + """ + Computes, saves, and returns a CourseGrade object for the + given user and course. + Sends a COURSE_GRADE_CHANGED signal to listeners. + """ + course_grade = CourseGrade(user, course_data) + course_grade.update() + + should_persist = ( + not read_only and # TODO(TNL-6786) Remove the read_only boolean once all grades are back-filled. + should_persist_grades(course_data.course_key) and + not waffle().is_enabled(WRITE_ONLY_IF_ENGAGED) or course_grade.attempted + ) + if should_persist: + course_grade._subsection_grade_factory.bulk_create_unsaved() + PersistentCourseGrade.update_or_create_course_grade( + user_id=user.id, + course_id=course_data.course_key, + course_version=course_data.version, + course_edited_timestamp=course_data.edited_on, + grading_policy_hash=course_data.grading_policy_hash, + percent_grade=course_grade.percent, + letter_grade=course_grade.letter_grade or "", + passed=course_grade.passed, + ) + + COURSE_GRADE_CHANGED.send_robust( + sender=None, + user=user, + course_grade=course_grade, + course_key=course_data.course_key, + deadline=course_data.course.end, + ) + + log.info( + u'Grades: Update, %s, User: %s, %s, persisted: %s', + course_data.full_string(), user.id, course_grade, should_persist, + ) + + return course_grade diff --git a/lms/djangoapps/grades/new/subsection_grade.py b/lms/djangoapps/grades/new/subsection_grade.py index 2c2df98d9f..8bdf6e3f4a 100644 --- a/lms/djangoapps/grades/new/subsection_grade.py +++ b/lms/djangoapps/grades/new/subsection_grade.py @@ -4,21 +4,18 @@ SubsectionGrade Class from collections import OrderedDict from lazy import lazy from logging import getLogger -from courseware.model_data import ScoresClient from lms.djangoapps.grades.scores import get_score, possibly_scored from lms.djangoapps.grades.models import BlockRecord, PersistentSubsectionGrade -from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag -from openedx.core.lib.grade_utils import is_score_higher_or_equal -from student.models import anonymous_id_for_user -from submissions import api as submissions_api from xmodule import block_metadata_utils, graders from xmodule.graders import AggregatedScore +from ..config.waffle import waffle, WRITE_ONLY_IF_ENGAGED + log = getLogger(__name__) -class SubsectionGrade(object): +class SubsectionGradeBase(object): """ Class for Subsection Grades. """ @@ -36,7 +33,6 @@ class SubsectionGrade(object): self.graded_total = None # aggregated grade for all graded problems self.all_total = None # aggregated grade for all problems, regardless of whether they are graded - self.locations_to_scores = OrderedDict() # dict of problem locations to ProblemScore @property def scores(self): @@ -58,6 +54,45 @@ class SubsectionGrade(object): ) return self.all_total.attempted + +class ZeroSubsectionGrade(SubsectionGradeBase): + """ + Class for Subsection Grades with Zero values. + """ + def __init__(self, subsection, course_data): + super(ZeroSubsectionGrade, self).__init__(subsection) + self.graded_total = AggregatedScore(tw_earned=0, tw_possible=None, graded=False, attempted=False) + self.all_total = AggregatedScore(tw_earned=0, tw_possible=None, graded=self.graded, attempted=False) + self.course_data = course_data + + @lazy + def locations_to_scores(self): + """ + Overrides the locations_to_scores member variable in order + to return empty scores for all scorable problems in the + course. + """ + locations = OrderedDict() # dict of problem locations to ProblemScore + for block_key in self.course_data.structure.post_order_traversal( + filter_func=possibly_scored, + start_node=self.location, + ): + block = self.course_data.structure[block_key] + if getattr(block, 'has_score', False): + locations[block_key] = get_score( + submissions_scores={}, csm_scores={}, persisted_block=None, block=block, + ) + return locations + + +class SubsectionGrade(SubsectionGradeBase): + """ + Class for Subsection Grades. + """ + def __init__(self, subsection): + super(SubsectionGrade, self).__init__(subsection) + self.locations_to_scores = OrderedDict() # dict of problem locations to ProblemScore + def init_from_structure(self, student, course_structure, submissions_scores, csm_scores): """ Compute the grade of this subsection for the given student and course. @@ -99,6 +134,7 @@ class SubsectionGrade(object): """ Saves the subsection grade in a persisted model. """ + subsection_grades = filter(lambda subs_grade: subs_grade._should_persist_per_attempted, subsection_grades) return PersistentSubsectionGrade.bulk_create_grades( [subsection_grade._persisted_model_params(student) for subsection_grade in subsection_grades], # pylint: disable=protected-access course_key, @@ -108,15 +144,25 @@ class SubsectionGrade(object): """ Saves the subsection grade in a persisted model. """ - self._log_event(log.debug, u"create_model", student) - return PersistentSubsectionGrade.create_grade(**self._persisted_model_params(student)) + if self._should_persist_per_attempted: + self._log_event(log.debug, u"create_model", student) + return PersistentSubsectionGrade.create_grade(**self._persisted_model_params(student)) def update_or_create_model(self, student): """ Saves or updates the subsection grade in a persisted model. """ - self._log_event(log.debug, u"update_or_create_model", student) - return PersistentSubsectionGrade.update_or_create_grade(**self._persisted_model_params(student)) + if self._should_persist_per_attempted: + self._log_event(log.debug, u"update_or_create_model", student) + return PersistentSubsectionGrade.update_or_create_grade(**self._persisted_model_params(student)) + + @property + def _should_persist_per_attempted(self): + """ + Returns whether the SubsectionGrade's model should be + persisted based on settings and attempted status. + """ + return not waffle().is_enabled(WRITE_ONLY_IF_ENGAGED) or self.attempted def _compute_block_score( self, @@ -182,7 +228,7 @@ class SubsectionGrade(object): Logs the given statement, for this instance. """ log_func( - u"Persistent Grades: SG.{}, subsection: {}, course: {}, " + u"Grades: SG.{}, subsection: {}, course: {}, " u"version: {}, edit: {}, user: {}," u"total: {}/{}, graded: {}/{}".format( log_statement, @@ -197,149 +243,3 @@ class SubsectionGrade(object): self.graded_total.possible, ) ) - - -class SubsectionGradeFactory(object): - """ - Factory for Subsection Grades. - """ - def __init__(self, student, course, course_structure): - self.student = student - self.course = course - self.course_structure = course_structure - - self._cached_subsection_grades = None - self._unsaved_subsection_grades = [] - - def create(self, subsection, read_only=False): - """ - Returns the SubsectionGrade object for the student and subsection. - - If read_only is True, doesn't save any updates to the grades. - """ - self._log_event( - log.debug, u"create, read_only: {0}, subsection: {1}".format(read_only, subsection.location), subsection, - ) - - subsection_grade = self._get_bulk_cached_grade(subsection) - if not subsection_grade: - subsection_grade = SubsectionGrade(subsection).init_from_structure( - self.student, self.course_structure, self._submissions_scores, self._csm_scores, - ) - if PersistentGradesEnabledFlag.feature_enabled(self.course.id): - if read_only: - self._unsaved_subsection_grades.append(subsection_grade) - else: - grade_model = subsection_grade.create_model(self.student) - self._update_saved_subsection_grade(subsection.location, grade_model) - return subsection_grade - - def bulk_create_unsaved(self): - """ - Bulk creates all the unsaved subsection_grades to this point. - """ - SubsectionGrade.bulk_create_models(self.student, self._unsaved_subsection_grades, self.course.id) - self._unsaved_subsection_grades = [] - - def update(self, subsection, only_if_higher=None): - """ - Updates the SubsectionGrade object for the student and subsection. - """ - # Save ourselves the extra queries if the course does not persist - # subsection grades. - self._log_event(log.warning, u"update, subsection: {}".format(subsection.location), subsection) - - calculated_grade = SubsectionGrade(subsection).init_from_structure( - self.student, self.course_structure, self._submissions_scores, self._csm_scores, - ) - - if PersistentGradesEnabledFlag.feature_enabled(self.course.id): - if only_if_higher: - try: - grade_model = PersistentSubsectionGrade.read_grade(self.student.id, subsection.location) - except PersistentSubsectionGrade.DoesNotExist: - pass - else: - orig_subsection_grade = SubsectionGrade(subsection).init_from_model( - self.student, grade_model, self.course_structure, self._submissions_scores, self._csm_scores, - ) - if not is_score_higher_or_equal( - orig_subsection_grade.graded_total.earned, - orig_subsection_grade.graded_total.possible, - calculated_grade.graded_total.earned, - calculated_grade.graded_total.possible, - ): - return orig_subsection_grade - - grade_model = calculated_grade.update_or_create_model(self.student) - self._update_saved_subsection_grade(subsection.location, grade_model) - - return calculated_grade - - @lazy - def _csm_scores(self): - """ - Lazily queries and returns all the scores stored in the user - state (in CSM) for the course, while caching the result. - """ - scorable_locations = [block_key for block_key in self.course_structure if possibly_scored(block_key)] - return ScoresClient.create_for_locations(self.course.id, self.student.id, scorable_locations) - - @lazy - def _submissions_scores(self): - """ - Lazily queries and returns the scores stored by the - Submissions API for the course, while caching the result. - """ - anonymous_user_id = anonymous_id_for_user(self.student, self.course.id) - return submissions_api.get_scores(unicode(self.course.id), anonymous_user_id) - - def _get_bulk_cached_grade(self, subsection): - """ - Returns the student's SubsectionGrade for the subsection, - while caching the results of a bulk retrieval for the - course, for future access of other subsections. - Returns None if not found. - """ - if not PersistentGradesEnabledFlag.feature_enabled(self.course.id): - return - - saved_subsection_grades = self._get_bulk_cached_subsection_grades() - subsection_grade = saved_subsection_grades.get(subsection.location) - if subsection_grade: - return SubsectionGrade(subsection).init_from_model( - self.student, subsection_grade, self.course_structure, self._submissions_scores, self._csm_scores, - ) - - def _get_bulk_cached_subsection_grades(self): - """ - Returns and caches (for future access) the results of - a bulk retrieval of all subsection grades in the course. - """ - if self._cached_subsection_grades is None: - self._cached_subsection_grades = { - record.full_usage_key: record - for record in PersistentSubsectionGrade.bulk_read_grades(self.student.id, self.course.id) - } - return self._cached_subsection_grades - - def _update_saved_subsection_grade(self, subsection_usage_key, subsection_model): - """ - Updates (or adds) the subsection grade for the given - subsection usage key in the local cache, iff the cache - is populated. - """ - if self._cached_subsection_grades is not None: - self._cached_subsection_grades[subsection_usage_key] = subsection_model - - def _log_event(self, log_func, log_statement, subsection): - """ - Logs the given statement, for this instance. - """ - log_func(u"Persistent Grades: SGF.{}, course: {}, version: {}, edit: {}, user: {}".format( - log_statement, - self.course.id, - getattr(subsection, 'course_version', None), - getattr(subsection, 'subtree_edited_on', None), - self.student.id, - )) diff --git a/lms/djangoapps/grades/new/subsection_grade_factory.py b/lms/djangoapps/grades/new/subsection_grade_factory.py new file mode 100644 index 0000000000..2960b5d626 --- /dev/null +++ b/lms/djangoapps/grades/new/subsection_grade_factory.py @@ -0,0 +1,162 @@ +from lazy import lazy +from logging import getLogger + +from courseware.model_data import ScoresClient +from openedx.core.lib.grade_utils import is_score_higher_or_equal +from student.models import anonymous_id_for_user +from submissions import api as submissions_api + +from lms.djangoapps.grades.config import should_persist_grades, assume_zero_if_absent +from lms.djangoapps.grades.models import PersistentSubsectionGrade +from lms.djangoapps.grades.scores import possibly_scored +from .course_data import CourseData +from .subsection_grade import SubsectionGrade, ZeroSubsectionGrade + + +log = getLogger(__name__) + + +class SubsectionGradeFactory(object): + """ + Factory for Subsection Grades. + """ + def __init__(self, student, course=None, course_structure=None, course_data=None): + self.student = student + self.course_data = course_data or CourseData(student, course=course, structure=course_structure) + + self._cached_subsection_grades = None + self._unsaved_subsection_grades = [] + + def create(self, subsection, read_only=False): + """ + Returns the SubsectionGrade object for the student and subsection. + + If read_only is True, doesn't save any updates to the grades. + """ + self._log_event( + log.debug, u"create, read_only: {0}, subsection: {1}".format(read_only, subsection.location), subsection, + ) + + subsection_grade = self._get_bulk_cached_grade(subsection) + if not subsection_grade: + if assume_zero_if_absent(self.course_data.course_key): + subsection_grade = ZeroSubsectionGrade(subsection, self.course_data) + else: + subsection_grade = SubsectionGrade(subsection).init_from_structure( + self.student, self.course_data.structure, self._submissions_scores, self._csm_scores, + ) + if should_persist_grades(self.course_data.course_key): + if read_only: + self._unsaved_subsection_grades.append(subsection_grade) + else: + grade_model = subsection_grade.create_model(self.student) + self._update_saved_subsection_grade(subsection.location, grade_model) + return subsection_grade + + def bulk_create_unsaved(self): + """ + Bulk creates all the unsaved subsection_grades to this point. + """ + SubsectionGrade.bulk_create_models(self.student, self._unsaved_subsection_grades, self.course_data.course_key) + self._unsaved_subsection_grades = [] + + def update(self, subsection, only_if_higher=None): + """ + Updates the SubsectionGrade object for the student and subsection. + """ + # Save ourselves the extra queries if the course does not persist + # subsection grades. + self._log_event(log.warning, u"update, subsection: {}".format(subsection.location), subsection) + + calculated_grade = SubsectionGrade(subsection).init_from_structure( + self.student, self.course_data.structure, self._submissions_scores, self._csm_scores, + ) + + if should_persist_grades(self.course_data.course_key): + if only_if_higher: + try: + grade_model = PersistentSubsectionGrade.read_grade(self.student.id, subsection.location) + except PersistentSubsectionGrade.DoesNotExist: + pass + else: + orig_subsection_grade = SubsectionGrade(subsection).init_from_model( + self.student, grade_model, self.course_data.structure, self._submissions_scores, self._csm_scores, + ) + if not is_score_higher_or_equal( + orig_subsection_grade.graded_total.earned, + orig_subsection_grade.graded_total.possible, + calculated_grade.graded_total.earned, + calculated_grade.graded_total.possible, + ): + return orig_subsection_grade + + grade_model = calculated_grade.update_or_create_model(self.student) + self._update_saved_subsection_grade(subsection.location, grade_model) + + return calculated_grade + + @lazy + def _csm_scores(self): + """ + Lazily queries and returns all the scores stored in the user + state (in CSM) for the course, while caching the result. + """ + scorable_locations = [block_key for block_key in self.course_data.structure if possibly_scored(block_key)] + return ScoresClient.create_for_locations(self.course_data.course_key, self.student.id, scorable_locations) + + @lazy + def _submissions_scores(self): + """ + Lazily queries and returns the scores stored by the + Submissions API for the course, while caching the result. + """ + anonymous_user_id = anonymous_id_for_user(self.student, self.course_data.course_key) + return submissions_api.get_scores(str(self.course_data.course_key), anonymous_user_id) + + def _get_bulk_cached_grade(self, subsection): + """ + Returns the student's SubsectionGrade for the subsection, + while caching the results of a bulk retrieval for the + course, for future access of other subsections. + Returns None if not found. + """ + if should_persist_grades(self.course_data.course_key): + saved_subsection_grades = self._get_bulk_cached_subsection_grades() + subsection_grade = saved_subsection_grades.get(subsection.location) + if subsection_grade: + return SubsectionGrade(subsection).init_from_model( + self.student, subsection_grade, self.course_data.structure, self._submissions_scores, self._csm_scores, + ) + + def _get_bulk_cached_subsection_grades(self): + """ + Returns and caches (for future access) the results of + a bulk retrieval of all subsection grades in the course. + """ + if self._cached_subsection_grades is None: + self._cached_subsection_grades = { + record.full_usage_key: record + for record in PersistentSubsectionGrade.bulk_read_grades(self.student.id, self.course_data.course_key) + } + return self._cached_subsection_grades + + def _update_saved_subsection_grade(self, subsection_usage_key, subsection_model): + """ + Updates (or adds) the subsection grade for the given + subsection usage key in the local cache, iff the cache + is populated. + """ + if self._cached_subsection_grades is not None: + self._cached_subsection_grades[subsection_usage_key] = subsection_model + + def _log_event(self, log_func, log_statement, subsection): + """ + Logs the given statement, for this instance. + """ + log_func(u"Grades: SGF.{}, course: {}, version: {}, edit: {}, user: {}".format( + log_statement, + self.course_data.course_key, + getattr(subsection, 'course_version', None), + getattr(subsection, 'subtree_edited_on', None), + self.student.id, + )) diff --git a/lms/djangoapps/grades/scores.py b/lms/djangoapps/grades/scores.py index 305b61ce8e..e9f5f4999c 100644 --- a/lms/djangoapps/grades/scores.py +++ b/lms/djangoapps/grades/scores.py @@ -5,7 +5,6 @@ from logging import getLogger from openedx.core.lib.cache_utils import memoized from xblock.core import XBlock -from xmodule.block_metadata_utils import display_name_with_default_escaped from xmodule.graders import ProblemScore from .transformer import GradesTransformer diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index d82557bc5a..48326feadf 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -26,7 +26,7 @@ from .signals import ( SCORE_PUBLISHED, ) from ..constants import ScoreDatabaseTableEnum -from ..new.course_grade import CourseGradeFactory +from ..new.course_grade_factory import CourseGradeFactory from ..scores import weighted_score from ..tasks import recalculate_subsection_grade_v3, RECALCULATE_GRADE_DELAY @@ -238,7 +238,7 @@ def recalculate_course_grade(sender, course, course_structure, user, **kwargs): """ Updates a saved course grade. """ - CourseGradeFactory().update(user, course, course_structure) + CourseGradeFactory().update(user, course=course, course_structure=course_structure) def _emit_problem_submitted_event(kwargs): diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index c7d0704168..7c30fae1de 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -31,8 +31,8 @@ from util.date_utils import from_timestamp from xmodule.modulestore.django import modulestore from .constants import ScoreDatabaseTableEnum -from .new.subsection_grade import SubsectionGradeFactory -from .new.course_grade import CourseGradeFactory +from .new.subsection_grade_factory import SubsectionGradeFactory +from .new.course_grade_factory import CourseGradeFactory from .signals.signals import SUBSECTION_SCORE_CHANGED from .transformer import GradesTransformer @@ -73,11 +73,7 @@ def compute_grades_for_course(course_key, offset, batch_size): course = courses.get_course_by_id(CourseKey.from_string(course_key)) enrollments = CourseEnrollment.objects.filter(course_id=course.id).order_by('created') student_iter = (enrollment.user for enrollment in enrollments[offset:offset + batch_size]) - list(CourseGradeFactory().iter( - course, - students=student_iter, - read_only=False, - )) + list(CourseGradeFactory().iter(course, students=student_iter, force_update=True)) @task(bind=True, base=_BaseTask, default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY) @@ -182,7 +178,7 @@ def _has_db_updated_with_new_score(self, scored_block_usage_key, **kwargs): if not db_is_updated: log.info( - u"Persistent Grades: tasks._has_database_updated_with_new_score is False. Task ID: {}. Kwargs: {}. Found " + u"Grades: tasks._has_database_updated_with_new_score is False. Task ID: {}. Kwargs: {}. Found " u"modified time: {}".format( self.request.id, kwargs, diff --git a/lms/djangoapps/grades/tests/integration/test_access.py b/lms/djangoapps/grades/tests/integration/test_access.py index d73d94fd42..cdc97638f5 100644 --- a/lms/djangoapps/grades/tests/integration/test_access.py +++ b/lms/djangoapps/grades/tests/integration/test_access.py @@ -15,7 +15,7 @@ from student.models import CourseEnrollment from student.tests.factories import UserFactory from xmodule.modulestore import ModuleStoreEnum -from ...new.subsection_grade import SubsectionGradeFactory +from ...new.subsection_grade_factory import SubsectionGradeFactory class GradesAccessIntegrationTest(ProblemSubmissionTestMixin, SharedModuleStoreTestCase): diff --git a/lms/djangoapps/grades/tests/test_course_data.py b/lms/djangoapps/grades/tests/test_course_data.py new file mode 100644 index 0000000000..8d98a9bbff --- /dev/null +++ b/lms/djangoapps/grades/tests/test_course_data.py @@ -0,0 +1,55 @@ +""" +Tests for CourseData utility class. +""" +from lms.djangoapps.course_blocks.api import get_course_blocks +from mock import patch +from student.tests.factories import UserFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory +from ..new.course_data import CourseData + + +class CourseDataTest(ModuleStoreTestCase): + """ + Simple tests to ensure CourseData works as advertised. + """ + + def setUp(self): + super(CourseDataTest, self).setUp() + self.course = CourseFactory.create() + self.user = UserFactory.create() + self.one_true_structure = get_course_blocks(self.user, self.course.location) + self.expected_results = { + 'course': self.course, + 'collected_block_structure': self.one_true_structure, + 'structure': self.one_true_structure, + 'course_key': self.course.id, + 'location': self.course.location, + } + + @patch('lms.djangoapps.grades.new.course_data.get_course_blocks') + def test_fill_course_data(self, mock_get_blocks): + """ + Tests to ensure that course data is fully filled with just a single input. + """ + mock_get_blocks.return_value = self.one_true_structure + for kwarg in self.expected_results: # We iterate instead of ddt due to dependence on 'self' + if kwarg == 'location': + continue # This property is purely output; it's never able to be used as input + kwargs = {kwarg: self.expected_results[kwarg]} + course_data = CourseData(self.user, **kwargs) + for arg in self.expected_results: + # No point validating the data we used as input, and c_b_s is input-only + if arg != kwarg and arg != "collected_block_structure": + expected = self.expected_results[arg] + actual = getattr(course_data, arg) + self.assertEqual(expected, actual) + + def test_no_data(self): + """ + Tests to ensure ??? happens when none of the data are provided. + + Maybe a dict pairing asked-for properties to resulting exceptions? Or an exception on init? + """ + with self.assertRaises(ValueError): + _ = CourseData(self.user) diff --git a/lms/djangoapps/grades/tests/test_grades.py b/lms/djangoapps/grades/tests/test_grades.py index 8caecfc573..84ae2b3a43 100644 --- a/lms/djangoapps/grades/tests/test_grades.py +++ b/lms/djangoapps/grades/tests/test_grades.py @@ -19,8 +19,8 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from .utils import answer_problem -from ..new.course_grade import CourseGradeFactory -from ..new.subsection_grade import SubsectionGradeFactory +from ..new.course_grade_factory import CourseGradeFactory +from ..new.subsection_grade_factory import SubsectionGradeFactory @attr(shard=1) @@ -78,7 +78,7 @@ class TestGradeIteration(SharedModuleStoreTestCase): self.assertIsNone(course_grade.letter_grade) self.assertEqual(course_grade.percent, 0.0) - @patch('lms.djangoapps.grades.new.course_grade.CourseGradeFactory.create') + @patch('lms.djangoapps.grades.new.course_grade_factory.CourseGradeFactory.create') def test_grading_exception(self, mock_course_grade): """Test that we correctly capture exception messages that bubble up from grading. Note that we only see errors at this level if the grading diff --git a/lms/djangoapps/grades/tests/test_new.py b/lms/djangoapps/grades/tests/test_new.py index dcb5422dbe..e49761a335 100644 --- a/lms/djangoapps/grades/tests/test_new.py +++ b/lms/djangoapps/grades/tests/test_new.py @@ -7,13 +7,13 @@ import datetime import ddt from django.conf import settings -from django.db.utils import DatabaseError import itertools from mock import patch import pytz from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory +from courseware.access import has_access from courseware.tests.test_submitting_problems import ProblemSubmissionTestMixin from lms.djangoapps.course_blocks.api import get_course_blocks from lms.djangoapps.grades.config.tests.utils import persistent_grades_feature_flags @@ -25,9 +25,12 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.utils import TEST_DATA_DIR from xmodule.modulestore.xml_importer import import_course_from_xml +from ..config.waffle import waffle, ASSUME_ZERO_GRADE_IF_ABSENT from ..models import PersistentSubsectionGrade -from ..new.course_grade import CourseGradeFactory -from ..new.subsection_grade import SubsectionGrade, SubsectionGradeFactory +from ..new.course_data import CourseData +from ..new.course_grade_factory import CourseGradeFactory +from ..new.course_grade import ZeroCourseGrade, CourseGrade +from ..new.subsection_grade_factory import SubsectionGrade, SubsectionGradeFactory from .utils import mock_get_score, mock_get_submissions_score @@ -100,6 +103,19 @@ class TestCourseGradeFactory(GradeTestBase): } self.course.set_grading_policy(grading_policy) + def test_course_grade_no_access(self): + """ + Test to ensure a grade can ba calculated for a student in a course, even if they themselves do not have access. + """ + invisible_course = CourseFactory.create(visible_to_staff_only=True) + access = has_access(self.request.user, 'load', invisible_course) + self.assertEqual(access.has_access, False) + self.assertEqual(access.error_code, 'not_visible_to_user') + + # with self.assertNoExceptionRaised: <- this isn't a real method, it's an implicit assumption + grade = CourseGradeFactory().create(self.request.user, invisible_course) + self.assertEqual(grade.percent, 0) + @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) @ddt.data( (True, True), @@ -118,9 +134,9 @@ class TestCourseGradeFactory(GradeTestBase): course_id=self.course.id, enabled_for_course=course_setting ): - with patch('lms.djangoapps.grades.new.course_grade.CourseGrade.load_persisted_grade') as mock_save_grades: + with patch('lms.djangoapps.grades.models.PersistentCourseGrade.read_course_grade') as mock_read_grade: grade_factory.create(self.request.user, self.course) - self.assertEqual(mock_save_grades.called, feature_flag and course_setting) + self.assertEqual(mock_read_grade.called, feature_flag and course_setting) def test_course_grade_creation(self): grade_factory = CourseGradeFactory() @@ -129,22 +145,27 @@ class TestCourseGradeFactory(GradeTestBase): self.assertEqual(course_grade.letter_grade, u'Pass') self.assertEqual(course_grade.percent, 0.5) - def test_zero_course_grade(self): - grade_factory = CourseGradeFactory() - with mock_get_score(0, 2): - course_grade = grade_factory.create(self.request.user, self.course) - self.assertIsNone(course_grade.letter_grade) - self.assertEqual(course_grade.percent, 0.0) + @ddt.data(True, False) + def test_zero_course_grade(self, assume_zero_enabled): + with waffle().override(ASSUME_ZERO_GRADE_IF_ABSENT, active=assume_zero_enabled): + grade_factory = CourseGradeFactory() + with mock_get_score(0, 2): + course_grade = grade_factory.create(self.request.user, self.course) + + self.assertIsInstance(course_grade, ZeroCourseGrade if assume_zero_enabled else CourseGrade) + self.assertIsNone(course_grade.letter_grade) + self.assertEqual(course_grade.percent, 0.0) + self.assertIsNotNone(course_grade.chapter_grades) def test_get_persisted(self): grade_factory = CourseGradeFactory() # first, create a grade in the database with mock_get_score(1, 2): - grade_factory.create(self.request.user, self.course, read_only=False) + grade_factory.update(self.request.user, self.course) # retrieve the grade, ensuring it is as expected and take just one query with self.assertNumQueries(1): - course_grade = grade_factory.get_persisted(self.request.user, self.course) + course_grade = grade_factory.read(self.request.user, self.course) self.assertEqual(course_grade.letter_grade, u'Pass') self.assertEqual(course_grade.percent, 0.5) @@ -165,10 +186,10 @@ class TestCourseGradeFactory(GradeTestBase): } self.course.set_grading_policy(new_grading_policy) - # ensure the grade can still be retrieved via get_persisted + # ensure the grade can still be retrieved via read # despite its outdated grading policy with self.assertNumQueries(1): - course_grade = grade_factory.get_persisted(self.request.user, self.course) + course_grade = grade_factory.read(self.request.user, self.course) self.assertEqual(course_grade.letter_grade, u'Pass') self.assertEqual(course_grade.percent, 0.5) @@ -211,10 +232,10 @@ class TestSubsectionGradeFactory(ProblemSubmissionTestMixin, GradeTestBase): wraps=PersistentSubsectionGrade.create_grade ) as mock_create_grade: with patch( - 'lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory._get_bulk_cached_grade', + 'lms.djangoapps.grades.new.subsection_grade_factory.SubsectionGradeFactory._get_bulk_cached_grade', wraps=self.subsection_grade_factory._get_bulk_cached_grade ) as mock_get_bulk_cached_grade: - with self.assertNumQueries(12): + with self.assertNumQueries(14): grade_a = self.subsection_grade_factory.create(self.sequence) self.assertTrue(mock_get_bulk_cached_grade.called) self.assertTrue(mock_create_grade.called) @@ -279,6 +300,26 @@ class TestSubsectionGradeFactory(ProblemSubmissionTestMixin, GradeTestBase): self.assertEqual(mock_read_saved_grade.called, feature_flag and course_setting) +class ZeroGradeTest(GradeTestBase): + """ + Tests ZeroCourseGrade (and, implicitly, ZeroSubsectionGrade) + functionality. + """ + + def test_zero(self): + """ + Creates a ZeroCourseGrade and ensures it's empty. + """ + course_data = CourseData(self.request.user, structure=self.course_structure) + chapter_grades = ZeroCourseGrade(self.request.user, course_data).chapter_grades + for chapter in chapter_grades: + for section in chapter_grades[chapter]['sections']: + for score in section.locations_to_scores.itervalues(): + self.assertEqual(score.earned, 0) + self.assertEqual(score.attempted, False) + self.assertEqual(section.all_total.earned, 0) + + class SubsectionGradeTest(GradeTestBase): """ Tests SubsectionGrade functionality. @@ -579,22 +620,18 @@ class TestCourseGradeLogging(ProblemSubmissionTestMixin, SharedModuleStoreTestCa def _create_course_grade_and_check_logging( self, - factory, + factory_method, log_mock, - log_statement + log_statement, ): """ Creates a course grade and asserts that the associated logging matches the expected totals passed in to the function. """ - factory.create(self.request.user, self.course, read_only=False) - log_mock.assert_called_with( - u"Persistent Grades: CourseGrade.{0}, course: {1}, user: {2}".format( - log_statement, - unicode(self.course.id), - unicode(self.request.user.id), - ) - ) + factory_method(self.request.user, self.course) + self.assertIn(log_statement, log_mock.info.call_args[0][0]) + self.assertIn(unicode(self.course.id), log_mock.info.call_args[0][1]) + self.assertEquals(self.request.user.id, log_mock.info.call_args[0][2]) def test_course_grade_logging(self): grade_factory = CourseGradeFactory() @@ -604,37 +641,19 @@ class TestCourseGradeLogging(ProblemSubmissionTestMixin, SharedModuleStoreTestCa course_id=self.course.id, enabled_for_course=True ): - with patch('lms.djangoapps.grades.new.course_grade.log') as log_mock: - # the course grade has not been created, so we expect each grade to be created - log_statement = u''.join(( - u"compute_and_update, read_only: {0}, subsections read/created: {1}/{2}, blocks ", - u"accessed: {3}, total graded subsections: {4}" - )).format(False, 0, 3, 3, 2) - self._create_course_grade_and_check_logging( - grade_factory, - log_mock.warning, - log_statement - ) - log_mock.reset_mock() + with patch('lms.djangoapps.grades.new.course_grade_factory.log') as log_mock: + # returns Zero when no grade, with ASSUME_ZERO_GRADE_IF_ABSENT + with waffle().override(ASSUME_ZERO_GRADE_IF_ABSENT, active=True): + self._create_course_grade_and_check_logging(grade_factory.create, log_mock, u'CreateZero') - # the course grade has been created, so we expect to read it from the db - log_statement = u"load_persisted_grade" - self._create_course_grade_and_check_logging( - grade_factory, - log_mock.info, - log_statement - ) - log_mock.reset_mock() + # read, but not persisted + self._create_course_grade_and_check_logging(grade_factory.create, log_mock, u'Update') - # only problem submission, a subsection grade update triggers - # a course grade update - self.submit_question_answer(u'test_problem_1', {u'2_1': u'choice_choice_2'}) - log_statement = u''.join(( - u"compute_and_update, read_only: {0}, subsections read/created: {1}/{2}, blocks ", - u"accessed: {3}, total graded subsections: {4}" - )).format(False, 3, 0, 3, 2) - self._create_course_grade_and_check_logging( - grade_factory, - log_mock.warning, - log_statement - ) + # update and persist + self._create_course_grade_and_check_logging(grade_factory.update, log_mock, u'Update') + + # read from persistence, using create + self._create_course_grade_and_check_logging(grade_factory.create, log_mock, u'Read') + + # read from persistence, using read + self._create_course_grade_and_check_logging(grade_factory.read, log_mock, u'Read') diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 82860c18a7..e4e55213f6 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -154,10 +154,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEquals(mock_block_structure_create.call_count, 1) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 26, True), - (ModuleStoreEnum.Type.mongo, 1, 23, False), - (ModuleStoreEnum.Type.split, 3, 25, True), - (ModuleStoreEnum.Type.split, 3, 22, False), + (ModuleStoreEnum.Type.mongo, 1, 28, True), + (ModuleStoreEnum.Type.mongo, 1, 24, False), + (ModuleStoreEnum.Type.split, 3, 27, True), + (ModuleStoreEnum.Type.split, 3, 23, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -169,8 +169,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 26), - (ModuleStoreEnum.Type.split, 3, 25), + (ModuleStoreEnum.Type.mongo, 1, 28), + (ModuleStoreEnum.Type.split, 3, 27), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -230,8 +230,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 24), - (ModuleStoreEnum.Type.split, 3, 23), + (ModuleStoreEnum.Type.mongo, 1, 25), + (ModuleStoreEnum.Type.split, 3, 24), ) @ddt.unpack def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): @@ -244,7 +244,7 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertGreater(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0) @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send') - @patch('lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update') + @patch('lms.djangoapps.grades.new.subsection_grade_factory.SubsectionGradeFactory.update') def test_retry_first_time_only(self, mock_update, mock_course_signal): """ Ensures that a task retry completes after a one-time failure. @@ -255,7 +255,7 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEquals(mock_course_signal.call_count, 1) @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v3.retry') - @patch('lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update') + @patch('lms.djangoapps.grades.new.subsection_grade_factory.SubsectionGradeFactory.update') def test_retry_on_integrity_error(self, mock_update, mock_retry): """ Ensures that tasks will be retried if IntegrityErrors are encountered. @@ -287,7 +287,7 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self._assert_retry_called(mock_retry) self.assertIn( - u"Persistent Grades: tasks._has_database_updated_with_new_score is False.", + u"Grades: tasks._has_database_updated_with_new_score is False.", mock_log.info.call_args_list[0][0][0] ) @@ -319,13 +319,13 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest else: self._assert_retry_called(mock_retry) self.assertIn( - u"Persistent Grades: tasks._has_database_updated_with_new_score is False.", + u"Grades: tasks._has_database_updated_with_new_score is False.", mock_log.info.call_args_list[0][0][0] ) @patch('lms.djangoapps.grades.tasks.log') @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v3.retry') - @patch('lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update') + @patch('lms.djangoapps.grades.new.subsection_grade_factory.SubsectionGradeFactory.update') def test_log_unknown_error(self, mock_update, mock_retry, mock_log): """ Ensures that unknown errors are logged before a retry. @@ -338,7 +338,7 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest @patch('lms.djangoapps.grades.tasks.log') @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v3.retry') - @patch('lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update') + @patch('lms.djangoapps.grades.new.subsection_grade_factory.SubsectionGradeFactory.update') def test_no_log_known_error(self, mock_update, mock_retry, mock_log): """ Ensures that known errors are not logged before a retry. @@ -395,7 +395,7 @@ class ComputeGradesForCourseTest(HasCourseWithProblemsMixin, ModuleStoreTestCase result = compute_grades_for_course.delay( course_key=six.text_type(self.course.id), batch_size=batch_size, - offset=4 + offset=4, ) self.assertTrue(result.successful) self.assertEqual( @@ -409,8 +409,8 @@ class ComputeGradesForCourseTest(HasCourseWithProblemsMixin, ModuleStoreTestCase @ddt.data(*xrange(1, 12, 3)) def test_database_calls(self, batch_size): - per_user_queries = 18 * min(batch_size, 6) # No more than 6 due to offset - with self.assertNumQueries(3 + per_user_queries): + per_user_queries = 17 * min(batch_size, 6) # No more than 6 due to offset + with self.assertNumQueries(5 + per_user_queries): with check_mongo_calls(1): compute_grades_for_course.delay( course_key=six.text_type(self.course.id), diff --git a/lms/djangoapps/grades/tests/utils.py b/lms/djangoapps/grades/tests/utils.py index 7369ac59c7..964b318245 100644 --- a/lms/djangoapps/grades/tests/utils.py +++ b/lms/djangoapps/grades/tests/utils.py @@ -14,7 +14,7 @@ def mock_passing_grade(grade_pass='Pass', percent=0.75, ): Mock the grading function to always return a passing grade. """ with patch('lms.djangoapps.grades.new.course_grade.CourseGrade._compute_letter_grade') as mock_letter_grade: - with patch('lms.djangoapps.grades.new.course_grade.CourseGrade._calc_percent') as mock_percent_grade: + with patch('lms.djangoapps.grades.new.course_grade.CourseGrade._compute_percent') as mock_percent_grade: mock_letter_grade.return_value = grade_pass mock_percent_grade.return_value = percent yield diff --git a/lms/djangoapps/grades/transformer.py b/lms/djangoapps/grades/transformer.py index 8353eac136..1b9ee6098f 100644 --- a/lms/djangoapps/grades/transformer.py +++ b/lms/djangoapps/grades/transformer.py @@ -72,6 +72,18 @@ class GradesTransformer(BlockStructureTransformer): """ pass + @classmethod + def grading_policy_hash(cls, course): + """ + Returns the grading policy hash for the given course. + """ + ordered_policy = json.dumps( + course.grading_policy, + separators=(',', ':'), # Remove spaces from separators for more compact representation + sort_keys=True, + ) + return b64encode(sha1(ordered_policy).digest()) + @classmethod def _collect_explicit_graded(cls, block_structure): """ @@ -137,27 +149,13 @@ class GradesTransformer(BlockStructureTransformer): Collect a hash of the course's grading policy, storing it as a `transformer_block_field` associated with the `GradesTransformer`. """ - def _hash_grading_policy(policy): - """ - Creates a hash from the course grading policy. - The keys are sorted in order to make the hash - agnostic to the ordering of the policy coming in. - """ - ordered_policy = json.dumps( - policy, - separators=(',', ':'), # Remove spaces from separators for more compact representation - sort_keys=True, - ) - return b64encode(sha1(ordered_policy).digest()) - course_location = block_structure.root_block_usage_key course_block = block_structure.get_xblock(course_location) - grading_policy = course_block.grading_policy block_structure.set_transformer_block_field( course_block.location, cls, "grading_policy_hash", - _hash_grading_policy(grading_policy) + cls.grading_policy_hash(course_block), ) @staticmethod diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index 9b9e7e2ae2..4a2123be43 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -17,7 +17,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory from ccx_keys.locator import CCXLocator from courseware.models import StudentModule -from grades.new.subsection_grade import SubsectionGradeFactory +from grades.new.subsection_grade_factory import SubsectionGradeFactory from grades.tests.utils import answer_problem from lms.djangoapps.ccx.tests.factories import CcxFactory from lms.djangoapps.course_blocks.api import get_course_blocks diff --git a/lms/djangoapps/instructor/views/gradebook_api.py b/lms/djangoapps/instructor/views/gradebook_api.py index 8cc8d82c7c..291601b00a 100644 --- a/lms/djangoapps/instructor/views/gradebook_api.py +++ b/lms/djangoapps/instructor/views/gradebook_api.py @@ -14,7 +14,7 @@ from opaque_keys.edx.keys import CourseKey from edxmako.shortcuts import render_to_response from courseware.courses import get_course_with_access from lms.djangoapps.instructor.views.api import require_level -from lms.djangoapps.grades.new.course_grade import CourseGradeFactory +from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from xmodule.modulestore.django import modulestore diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py index 1522ed821d..1cbf46c92d 100644 --- a/lms/djangoapps/instructor_task/tasks_helper.py +++ b/lms/djangoapps/instructor_task/tasks_helper.py @@ -42,7 +42,7 @@ from certificates.models import ( ) from courseware.courses import get_course_by_id, get_problems_in_section from lms.djangoapps.grades.context import grading_context_for_course -from lms.djangoapps.grades.new.course_grade import CourseGradeFactory +from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from courseware.model_data import DjangoKeyValueStore, FieldDataCache from courseware.models import StudentModule from courseware.module_render import get_module_for_descriptor_internal @@ -835,7 +835,7 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input, student, course_id, course_grade.percent, - course_grade.course.grade_cutoffs, + course.grade_cutoffs, student.profile.allow_certificate, student.id in whitelisted_user_ids ) @@ -855,7 +855,9 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input, else: grade_results.append([u'Not Attempted']) if assignment_info['use_subsection_headers']: - assignment_average = course_grade.grade_value['grade_breakdown'].get(assignment_type, {}).get('percent') + assignment_average = course_grade.grader_result['grade_breakdown'].get(assignment_type, {}).get( + 'percent' + ) grade_results.append([assignment_average]) grade_results = list(chain.from_iterable(grade_results)) diff --git a/lms/djangoapps/instructor_task/tests/test_integration.py b/lms/djangoapps/instructor_task/tests/test_integration.py index a329085e8c..b0d76af8af 100644 --- a/lms/djangoapps/instructor_task/tests/test_integration.py +++ b/lms/djangoapps/instructor_task/tests/test_integration.py @@ -40,7 +40,7 @@ from lms.djangoapps.instructor_task.tests.test_base import ( OPTION_2, ) from capa.responsetypes import StudentInputError -from lms.djangoapps.grades.new.course_grade import CourseGradeFactory +from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from openedx.core.lib.url_utils import quote_slashes diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 999edb8476..0390fbf568 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -117,7 +117,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase): self.assertDictContainsSubset({'attempted': num_students, 'succeeded': num_students, 'failed': 0}, result) @patch('lms.djangoapps.instructor_task.tasks_helper._get_current_task') - @patch('lms.djangoapps.grades.new.course_grade.CourseGradeFactory.iter') + @patch('lms.djangoapps.grades.new.course_grade_factory.CourseGradeFactory.iter') def test_grading_failure(self, mock_grades_iter, _mock_current_task): """ Test that any grading errors are properly reported in the @@ -294,7 +294,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase): ) @patch('lms.djangoapps.instructor_task.tasks_helper._get_current_task') - @patch('lms.djangoapps.grades.new.course_grade.CourseGradeFactory.iter') + @patch('lms.djangoapps.grades.new.course_grade_factory.CourseGradeFactory.iter') def test_unicode_in_csv_header(self, mock_grades_iter, _mock_current_task): """ Tests that CSV grade report works if unicode in headers. @@ -650,7 +650,7 @@ class TestProblemGradeReport(TestReportMixin, InstructorTaskModuleTestCase): ]) @patch('lms.djangoapps.instructor_task.tasks_helper._get_current_task') - @patch('lms.djangoapps.grades.new.course_grade.CourseGradeFactory.iter') + @patch('lms.djangoapps.grades.new.course_grade_factory.CourseGradeFactory.iter') @ddt.data(u'Cannot grade student', '') def test_grading_failure(self, error_message, mock_grades_iter, _mock_current_task): """ @@ -1775,7 +1775,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): 'failed': 3, 'skipped': 2 } - with self.assertNumQueries(184): + with self.assertNumQueries(186): self.assertCertificatesGenerated(task_input, expected_results) expected_results = { diff --git a/lms/djangoapps/lti_provider/tasks.py b/lms/djangoapps/lti_provider/tasks.py index 038951cda5..aa9fe79c25 100644 --- a/lms/djangoapps/lti_provider/tasks.py +++ b/lms/djangoapps/lti_provider/tasks.py @@ -7,7 +7,7 @@ from django.contrib.auth.models import User from django.dispatch import receiver import logging -from lms.djangoapps.grades.new.course_grade import CourseGradeFactory +from lms.djangoapps.grades.new.course_grade_factory import CourseGradeFactory from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED from lms import CELERY_APP from lti_provider.models import GradedAssignment diff --git a/openedx/core/djangolib/waffle_utils.py b/openedx/core/djangolib/waffle_utils.py index 2b83e42c10..b625624e61 100644 --- a/openedx/core/djangolib/waffle_utils.py +++ b/openedx/core/djangolib/waffle_utils.py @@ -5,8 +5,7 @@ from abc import ABCMeta from contextlib import contextmanager import logging -from waffle.models import Switch -from waffle.utils import get_setting as waffle_setting +from waffle.testutils import override_switch as waffle_override_switch from waffle import switch_is_active from request_cache import get_cache as get_request_cache @@ -81,6 +80,18 @@ class WaffleSwitchPlus(WafflePlus): self._cached_switches[namespaced_switch_name] = active log.info(u"%sSwitch '%s' set to %s for request.", self.log_prefix, namespaced_switch_name, active) + @contextmanager + def override_in_model(self, switch_name, active=True): + """ + Overrides the active value for the given switch for the duration of this + contextmanager. + Note: The value is overridden in the request cache AND in the model. + """ + with self.override(switch_name, active): + namespaced_switch_name = self._namespaced_setting_name(switch_name) + with waffle_override_switch(namespaced_switch_name, active): + yield + @property def _cached_switches(self): """