From 4a9243ef9fc0edbe34e7f4550f057cbc161406a0 Mon Sep 17 00:00:00 2001 From: Sagirov Evgeniy <34642612+UvgenGen@users.noreply.github.com> Date: Tue, 19 Jul 2022 18:30:37 +0300 Subject: [PATCH] feat: FC-0001 Remove Persistent Course Grades Flags (#30540) * feat: revert Fix certificate generation without persistent grades * feat: Remove Persistent Course Grades Flags * test: update tests --- common/test/db_fixtures/grades.json | 11 --- .../0012-emit-course-completion-events.rst | 7 -- .../certificates/generation_handler.py | 11 +-- .../progress/tests/test_views.py | 79 ++++++++-------- lms/djangoapps/courseware/tests/test_views.py | 27 ++---- lms/djangoapps/grades/admin.py | 23 +---- lms/djangoapps/grades/config/__init__.py | 28 ------ lms/djangoapps/grades/config/forms.py | 27 ------ lms/djangoapps/grades/config/models.py | 87 +---------------- .../grades/config/tests/test_models.py | 93 ------------------- lms/djangoapps/grades/config/tests/utils.py | 30 ------ lms/djangoapps/grades/config/waffle.py | 11 --- lms/djangoapps/grades/course_grade.py | 10 +- lms/djangoapps/grades/course_grade_factory.py | 74 ++++++--------- .../test_recalculate_subsection_grades.py | 2 - .../migrations/0019_auto_20220525_1200.py | 23 +++++ lms/djangoapps/grades/settings/test.py | 3 +- .../grades/subsection_grade_factory.py | 27 +++--- .../grades/tests/test_course_grade.py | 36 +++---- .../grades/tests/test_course_grade_factory.py | 80 ++-------------- .../tests/test_subsection_grade_factory.py | 24 ----- lms/djangoapps/grades/tests/test_tasks.py | 35 +------ .../tests/test_tasks_helper.py | 3 +- lms/envs/bok_choy.yml | 2 - .../credentials/tests/test_tasks.py | 7 -- 25 files changed, 153 insertions(+), 607 deletions(-) delete mode 100644 common/test/db_fixtures/grades.json delete mode 100644 lms/djangoapps/grades/config/forms.py delete mode 100644 lms/djangoapps/grades/config/tests/test_models.py delete mode 100644 lms/djangoapps/grades/config/tests/utils.py create mode 100644 lms/djangoapps/grades/migrations/0019_auto_20220525_1200.py diff --git a/common/test/db_fixtures/grades.json b/common/test/db_fixtures/grades.json deleted file mode 100644 index 7e977d482c..0000000000 --- a/common/test/db_fixtures/grades.json +++ /dev/null @@ -1,11 +0,0 @@ -[ - { - "pk": 1, - "model": "grades.persistentgradesenabledflag", - "fields": { - "enabled": true, - "enabled_for_all_courses": true, - "change_date": "2016-09-01T00:00:00Z" - } - } -] diff --git a/docs/decisions/0012-emit-course-completion-events.rst b/docs/decisions/0012-emit-course-completion-events.rst index 72a5b959f8..80b4bc6e2f 100644 --- a/docs/decisions/0012-emit-course-completion-events.rst +++ b/docs/decisions/0012-emit-course-completion-events.rst @@ -21,10 +21,3 @@ Decision #. edX platform will be configured to emit an event named ``edx.course.grade.passed.first_time`` when a learner passes a course for the first time. This event will therefore be emitted when the learner has achieved passing grade AND passing timestamp of grade for this learner and course does not exist as seen `here`_. #. For LRS consumers who may be interested in knowing whenever a learner passes or fails a course, edX platform will be configured to emit events named ``edx.course.grade.now_passed`` and ``edx.course.grade.now_failed``. - -Consequences ------------- - -#. `edx.course.grade.passed.first_time` event will only be emitted for a course if `PersistentGradesEnabledFlag` is `True` for that course. - -.. _here: https://github.com/edx/edx-platform/blob/8aedebcdb29bb16b94786503c12a52b07c73dff5/lms/djangoapps/grades/models.py#L647 diff --git a/lms/djangoapps/certificates/generation_handler.py b/lms/djangoapps/certificates/generation_handler.py index a1a3992eb3..8c76416357 100644 --- a/lms/djangoapps/certificates/generation_handler.py +++ b/lms/djangoapps/certificates/generation_handler.py @@ -62,7 +62,7 @@ def generate_allowlist_certificate_task(user, course_key, generation_mode=None, Create a task to generate an allowlist certificate for this user in this course run. """ enrollment_mode = _get_enrollment_mode(user, course_key) - course_grade = _get_course_grade(user, course_key, send_course_grade_signals=False) + course_grade = _get_course_grade(user, course_key) if _can_generate_allowlist_certificate(user, course_key, enrollment_mode): try: return _generate_certificate_task( @@ -91,7 +91,7 @@ def _generate_regular_certificate_task(user, course_key, generation_mode=None, d eligible and a certificate can be generated. """ enrollment_mode = _get_enrollment_mode(user, course_key) - course_grade = _get_course_grade(user, course_key, send_course_grade_signals=False) + course_grade = _get_course_grade(user, course_key) if _can_generate_regular_certificate(user, course_key, enrollment_mode, course_grade): return _generate_certificate_task(user=user, course_key=course_key, enrollment_mode=enrollment_mode, course_grade=course_grade, generation_mode=generation_mode, @@ -411,14 +411,11 @@ def _get_grade_value(course_grade): return '' -def _get_course_grade(user, course_key, send_course_grade_signals=True): +def _get_course_grade(user, course_key): """ Get the user's course grade in this course run. Note that this may be None. - - Use send_course_grade_signals=False to avoid firing the course grade signals recursively. - See details in lms/djangoapps/grades/course_grade_factory.py _update method. """ - return CourseGradeFactory().read(user, course_key=course_key, send_course_grade_signals=send_course_grade_signals) + return CourseGradeFactory().read(user, course_key=course_key) def _get_enrollment_mode(user, course_key): diff --git a/lms/djangoapps/course_home_api/progress/tests/test_views.py b/lms/djangoapps/course_home_api/progress/tests/test_views.py index 4c330c7cc3..60f2741b49 100644 --- a/lms/djangoapps/course_home_api/progress/tests/test_views.py +++ b/lms/djangoapps/course_home_api/progress/tests/test_views.py @@ -7,7 +7,6 @@ from unittest.mock import patch import dateutil import ddt -from django.conf import settings from django.urls import reverse from django.utils.timezone import now from edx_toggles.toggles.testutils import override_waffle_flag @@ -21,9 +20,9 @@ from lms.djangoapps.course_home_api.tests.utils import BaseCourseHomeTests from lms.djangoapps.course_home_api.models import DisableProgressPageStackedConfig from lms.djangoapps.course_home_api.toggles import COURSE_HOME_MICROFRONTEND_PROGRESS_TAB from lms.djangoapps.grades.api import CourseGradeFactory -from lms.djangoapps.grades.config.tests.utils import persistent_grades_feature_flags from lms.djangoapps.grades.constants import GradeOverrideFeatureEnum from lms.djangoapps.grades.models import ( + PersistentCourseGrade, PersistentSubsectionGrade, PersistentSubsectionGradeOverride ) @@ -193,48 +192,54 @@ class ProgressTabTestViews(BaseCourseHomeTests): assert not gated_score['learner_has_access'] assert ungated_score['learner_has_access'] - @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) def test_override_is_visible(self): - with persistent_grades_feature_flags(global_flag=True): - chapter = ItemFactory(parent=self.course, category='chapter') - subsection = ItemFactory.create(parent=chapter, category="sequential", display_name="Subsection") + chapter = ItemFactory(parent=self.course, category='chapter') + subsection = ItemFactory.create(parent=chapter, category="sequential", display_name="Subsection") - CourseEnrollment.enroll(self.user, self.course.id) + CourseEnrollment.enroll(self.user, self.course.id) + course_grade_params = { + "user_id": self.user.id, + "course_id": self.course.id, + "percent_grade": 77.7, + "letter_grade": "pass", + "passed": True, + } + PersistentCourseGrade.update_or_create(**course_grade_params) - params = { - "user_id": self.user.id, - "usage_key": subsection.location, - "course_version": self.course.course_version, - "subtree_edited_timestamp": "2016-08-01 18:53:24.354741Z", - "earned_all": 6.0, - "possible_all": 12.0, - "earned_graded": 6.0, - "possible_graded": 8.0, - "visible_blocks": [], - "first_attempted": datetime.now(), - } + params = { + "user_id": self.user.id, + "usage_key": subsection.location, + "course_version": self.course.course_version, + "subtree_edited_timestamp": "2016-08-01 18:53:24.354741Z", + "earned_all": 6.0, + "possible_all": 12.0, + "earned_graded": 6.0, + "possible_graded": 8.0, + "visible_blocks": [], + "first_attempted": datetime.now(), + } - created_grade = PersistentSubsectionGrade.update_or_create_grade(**params) - proctoring_failure_comment = "Failed Test Proctoring" - PersistentSubsectionGradeOverride.update_or_create_override( - requesting_user=self.staff_user, - subsection_grade_model=created_grade, - earned_all_override=0.0, - earned_graded_override=0.0, - system=GradeOverrideFeatureEnum.proctoring, - feature=GradeOverrideFeatureEnum.proctoring, - comment=proctoring_failure_comment - ) + created_grade = PersistentSubsectionGrade.update_or_create_grade(**params) + proctoring_failure_comment = "Failed Test Proctoring" + PersistentSubsectionGradeOverride.update_or_create_override( + requesting_user=self.staff_user, + subsection_grade_model=created_grade, + earned_all_override=0.0, + earned_graded_override=0.0, + system=GradeOverrideFeatureEnum.proctoring, + feature=GradeOverrideFeatureEnum.proctoring, + comment=proctoring_failure_comment + ) - response = self.client.get(self.url) - assert response.status_code == 200 + response = self.client.get(self.url) + assert response.status_code == 200 - sections = response.data['section_scores'] - overridden_subsection = sections[1]['subsections'][0] - override_entry = overridden_subsection["override"] + sections = response.data['section_scores'] + overridden_subsection = sections[1]['subsections'][0] + override_entry = overridden_subsection["override"] - assert override_entry['system'] == GradeOverrideFeatureEnum.proctoring - assert override_entry['reason'] == proctoring_failure_comment + assert override_entry['system'] == GradeOverrideFeatureEnum.proctoring + assert override_entry['reason'] == proctoring_failure_comment def test_view_other_students_progress_page(self): # Test the ability to view progress pages of other students by changing the url diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 8eea7537e9..bf1fb21028 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -23,7 +23,7 @@ from django.test import RequestFactory, TestCase from django.test.client import Client from django.test.utils import override_settings from django.urls import reverse, reverse_lazy -from edx_toggles.toggles.testutils import override_waffle_flag, override_waffle_switch +from edx_toggles.toggles.testutils import override_waffle_flag from freezegun import freeze_time from opaque_keys.edx.keys import CourseKey, UsageKey from pytz import UTC @@ -75,7 +75,6 @@ from lms.djangoapps.courseware.tests.helpers import MasqueradeMixin, get_expirat from lms.djangoapps.courseware.testutils import RenderXBlockTestMixin from lms.djangoapps.courseware.toggles import COURSEWARE_OPTIMIZED_RENDER_XBLOCK from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient -from lms.djangoapps.grades.config.waffle import ASSUME_ZERO_GRADE_IF_ABSENT from lms.djangoapps.instructor.access import allow_access from lms.djangoapps.verify_student.services import IDVerificationService from openedx.core.djangoapps.catalog.tests.factories import CourseFactory as CatalogCourseFactory @@ -1490,28 +1489,20 @@ class ProgressPageTests(ProgressPageBaseTests): with self.assertNumQueries(query_count, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST), check_mongo_calls(2): self._get_progress_page() - @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) - @ddt.data( - (False, 60, 42), - (True, 52, 36) - ) - @ddt.unpack - def test_progress_queries(self, enable_waffle, initial, subsequent): + def test_progress_queries(self): ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1)) self.setup_course() - with override_waffle_switch(ASSUME_ZERO_GRADE_IF_ABSENT, active=enable_waffle): + with self.assertNumQueries( + 52, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST + ), check_mongo_calls(2): + self._get_progress_page() + + for _ in range(2): with self.assertNumQueries( - initial, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST + 36, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST ), check_mongo_calls(2): self._get_progress_page() - # subsequent accesses to the progress page require fewer queries. - for _ in range(2): - with self.assertNumQueries( - subsequent, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST - ), check_mongo_calls(2): - self._get_progress_page() - @patch.dict(settings.FEATURES, {'ENABLE_CERTIFICATES_IDV_REQUIREMENT': True}) @ddt.data( *itertools.product( diff --git a/lms/djangoapps/grades/admin.py b/lms/djangoapps/grades/admin.py index 8405fee792..7e34daef17 100644 --- a/lms/djangoapps/grades/admin.py +++ b/lms/djangoapps/grades/admin.py @@ -3,31 +3,12 @@ Django admin page for grades models """ -from config_models.admin import ConfigurationModelAdmin, KeyedConfigurationModelAdmin +from config_models.admin import ConfigurationModelAdmin from django.contrib import admin -from lms.djangoapps.grades.config.forms import CoursePersistentGradesAdminForm from lms.djangoapps.grades.config.models import ( - ComputeGradesSetting, - CoursePersistentGradesFlag, - PersistentGradesEnabledFlag + ComputeGradesSetting ) -class CoursePersistentGradesAdmin(KeyedConfigurationModelAdmin): - """ - Admin for enabling subsection grades on a course-by-course basis. - Allows searching by course id. - """ - form = CoursePersistentGradesAdminForm - search_fields = ['course_id'] - fieldsets = ( - (None, { - 'fields': ('course_id', 'enabled'), - 'description': 'Enter a valid course id. If it is invalid, an error message will display.' - }), - ) - -admin.site.register(CoursePersistentGradesFlag, CoursePersistentGradesAdmin) -admin.site.register(PersistentGradesEnabledFlag, ConfigurationModelAdmin) admin.site.register(ComputeGradesSetting, ConfigurationModelAdmin) diff --git a/lms/djangoapps/grades/config/__init__.py b/lms/djangoapps/grades/config/__init__.py index 7f976601dc..e69de29bb2 100644 --- a/lms/djangoapps/grades/config/__init__.py +++ b/lms/djangoapps/grades/config/__init__.py @@ -1,28 +0,0 @@ -""" -Defines grading configuration. -""" - - -from django.conf import settings - -from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag -from lms.djangoapps.grades.config.waffle import 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 ( - settings.FEATURES.get('ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS') or - ASSUME_ZERO_GRADE_IF_ABSENT.is_enabled() - ) - ) - - -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/forms.py b/lms/djangoapps/grades/config/forms.py deleted file mode 100644 index fe2f35dd7e..0000000000 --- a/lms/djangoapps/grades/config/forms.py +++ /dev/null @@ -1,27 +0,0 @@ -""" -Defines a form for providing validation of subsection grade templates. -""" - - -import logging - -from django import forms - -from lms.djangoapps.grades.config.models import CoursePersistentGradesFlag -from openedx.core.lib.courses import clean_course_id - -log = logging.getLogger(__name__) - - -class CoursePersistentGradesAdminForm(forms.ModelForm): - """Input form for subsection grade enabling, allowing us to verify data.""" - - class Meta: - model = CoursePersistentGradesFlag - fields = '__all__' - - def clean_course_id(self): - """ - Validate the course id - """ - return clean_course_id(self) diff --git a/lms/djangoapps/grades/config/models.py b/lms/djangoapps/grades/config/models.py index 938654bfd4..653a7e66f6 100644 --- a/lms/djangoapps/grades/config/models.py +++ b/lms/djangoapps/grades/config/models.py @@ -5,92 +5,7 @@ controlling persistent grades. from config_models.models import ConfigurationModel -from django.conf import settings -from django.db.models import BooleanField, IntegerField, TextField - -from opaque_keys.edx.django.models import CourseKeyField - -from openedx.core.lib.cache_utils import request_cached - - -class PersistentGradesEnabledFlag(ConfigurationModel): - """ - Enables persistent grades across the platform. - When this feature flag is set to true, individual courses - must also have persistent grades enabled for the - feature to take effect. - - .. no_pii: - - .. toggle_name: PersistentGradesEnabledFlag.enabled - .. toggle_implementation: ConfigurationModel - .. toggle_default: False - .. toggle_description: When enabled, grades are persisted. This means that PersistentCourseGrade objects are - created for student grades. In order for this to take effect, CoursePersistentGradesFlag objects must also be - created individually for each course. Alternatively, the PersistentGradesEnabledFlag.enabled_for_all_courses - waffle flag or the PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS feature flag can be set to True to enable this - feature for all courses. - .. toggle_use_cases: temporary - .. toggle_creation_date: 2016-08-26 - .. toggle_target_removal_date: None - .. toggle_tickets: https://github.com/edx/edx-platform/pull/13329 - """ - # this field overrides course-specific settings to enable the feature for all courses - enabled_for_all_courses = BooleanField(default=False) - - @classmethod - @request_cached() - def feature_enabled(cls, course_id=None): - """ - Looks at the currently active configuration model to determine whether - the persistent grades feature is available. - - If the flag is not enabled, the feature is not available. - If the flag is enabled and the provided course_id is for an course - with persistent grades enabled, the feature is available. - If the flag is enabled and no course ID is given, - we return True since the global setting is enabled. - """ - if settings.FEATURES.get('PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS'): - return True - if not PersistentGradesEnabledFlag.is_enabled(): - return False - elif not PersistentGradesEnabledFlag.current().enabled_for_all_courses and course_id: - effective = CoursePersistentGradesFlag.objects.filter(course_id=course_id).order_by('-change_date').first() - return effective.enabled if effective is not None else False - return True - - class Meta: - app_label = "grades" - - def __str__(self): - current_model = PersistentGradesEnabledFlag.current() - return "PersistentGradesEnabledFlag: enabled {}".format( - current_model.is_enabled() - ) - - -class CoursePersistentGradesFlag(ConfigurationModel): - """ - Enables persistent grades for a specific - course. Only has an effect if the general - flag above is set to True. - - .. no_pii: - """ - KEY_FIELDS = ('course_id',) - - class Meta: - app_label = "grades" - - # The course that these features are attached to. - course_id = CourseKeyField(max_length=255, db_index=True) - - def __str__(self): - not_en = "Not " - if self.enabled: - not_en = "" - return f"Course '{str(self.course_id)}': Persistent Grades {not_en}Enabled" +from django.db.models import IntegerField, TextField class ComputeGradesSetting(ConfigurationModel): diff --git a/lms/djangoapps/grades/config/tests/test_models.py b/lms/djangoapps/grades/config/tests/test_models.py deleted file mode 100644 index b4f1105c2b..0000000000 --- a/lms/djangoapps/grades/config/tests/test_models.py +++ /dev/null @@ -1,93 +0,0 @@ -""" -Tests for the models that control the -persistent grading feature. -""" - - -import itertools -from unittest.mock import patch - -import ddt -from django.conf import settings -from django.test import TestCase -from opaque_keys.edx.locator import CourseLocator - -from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag -from lms.djangoapps.grades.config.tests.utils import persistent_grades_feature_flags - - -@patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) -@ddt.ddt -class PersistentGradesFeatureFlagTests(TestCase): - """ - Tests the behavior of the feature flags for persistent grading. - These are set via Django admin settings. - """ - - def setUp(self): - super().setUp() - self.course_id_1 = CourseLocator(org="edx", course="course", run="run") - self.course_id_2 = CourseLocator(org="edx", course="course2", run="run") - - @ddt.data(*itertools.product( - (True, False), - (True, False), - (True, False), - )) - @ddt.unpack - def test_persistent_grades_feature_flags(self, global_flag, enabled_for_all_courses, enabled_for_course_1): - with persistent_grades_feature_flags( - global_flag=global_flag, - enabled_for_all_courses=enabled_for_all_courses, - course_id=self.course_id_1, - enabled_for_course=enabled_for_course_1 - ): - assert PersistentGradesEnabledFlag.feature_enabled() == global_flag - assert PersistentGradesEnabledFlag.feature_enabled( - self.course_id_1 - ) == (global_flag and (enabled_for_all_courses or enabled_for_course_1)) - assert PersistentGradesEnabledFlag.feature_enabled( - self.course_id_2 - ) == (global_flag and enabled_for_all_courses) - - def test_enable_disable_course_flag(self): - """ - Ensures that the flag, once enabled for a course, can also be disabled. - """ - with persistent_grades_feature_flags( - global_flag=True, - enabled_for_all_courses=False, - course_id=self.course_id_1, - enabled_for_course=True - ): - assert PersistentGradesEnabledFlag.feature_enabled(self.course_id_1) - # Prior to TNL-5698, creating a second object would fail due to db constraints - with persistent_grades_feature_flags( - global_flag=True, - enabled_for_all_courses=False, - course_id=self.course_id_1, - enabled_for_course=False - ): - assert not PersistentGradesEnabledFlag.feature_enabled(self.course_id_1) - - def test_enable_disable_globally(self): - """ - Ensures that the flag, once enabled globally, can also be disabled. - """ - with persistent_grades_feature_flags( - global_flag=True, - enabled_for_all_courses=True, - ): - assert PersistentGradesEnabledFlag.feature_enabled() - assert PersistentGradesEnabledFlag.feature_enabled(self.course_id_1) - with persistent_grades_feature_flags( - global_flag=True, - enabled_for_all_courses=False, - ): - assert PersistentGradesEnabledFlag.feature_enabled() - assert not PersistentGradesEnabledFlag.feature_enabled(self.course_id_1) - with persistent_grades_feature_flags( - global_flag=False, - ): - assert not PersistentGradesEnabledFlag.feature_enabled() - assert not PersistentGradesEnabledFlag.feature_enabled(self.course_id_1) diff --git a/lms/djangoapps/grades/config/tests/utils.py b/lms/djangoapps/grades/config/tests/utils.py deleted file mode 100644 index b019b73a28..0000000000 --- a/lms/djangoapps/grades/config/tests/utils.py +++ /dev/null @@ -1,30 +0,0 @@ -""" -Provides helper functions for tests that want -to configure flags related to persistent grading. -""" - - -from contextlib import contextmanager - -from edx_django_utils.cache import RequestCache - -from lms.djangoapps.grades.config.models import CoursePersistentGradesFlag, PersistentGradesEnabledFlag - - -@contextmanager -def persistent_grades_feature_flags( - global_flag, - enabled_for_all_courses=False, - course_id=None, - enabled_for_course=False -): - """ - Most test cases will use a single call to this manager, - as they need to set the global setting and the course-specific - setting for a single course. - """ - RequestCache.clear_all_namespaces() - PersistentGradesEnabledFlag.objects.create(enabled=global_flag, enabled_for_all_courses=enabled_for_all_courses) - if course_id: - CoursePersistentGradesFlag.objects.create(course_id=course_id, enabled=enabled_for_course) - yield diff --git a/lms/djangoapps/grades/config/waffle.py b/lms/djangoapps/grades/config/waffle.py index ced1f4a228..5873766d11 100644 --- a/lms/djangoapps/grades/config/waffle.py +++ b/lms/djangoapps/grades/config/waffle.py @@ -14,17 +14,6 @@ LOG_PREFIX = 'Grades: ' # Switches -# .. toggle_name: grades.assume_zero_grade_if_absent -# .. toggle_implementation: WaffleSwitch -# .. toggle_default: False -# .. toggle_description: When enabled, an absent grade is assumed to be zero. Alternatively, defining the -# `settings.FEATURES["ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS"]` feature flag in the LMS will enable this feature -# for all courses. -# .. toggle_use_cases: open_edx -# .. toggle_creation_date: 2017-04-11 -# .. toggle_tickets: https://github.com/edx/edx-platform/pull/14771 -# .. toggle_warning: This requires the PersistentGradesEnabledFlag to be enabled. -ASSUME_ZERO_GRADE_IF_ABSENT = WaffleSwitch(f'{WAFFLE_NAMESPACE}.assume_zero_grade_if_absent', __name__) # .. toggle_name: grades.disable_regrade_on_policy_change # .. toggle_implementation: WaffleSwitch # .. toggle_default: False diff --git a/lms/djangoapps/grades/course_grade.py b/lms/djangoapps/grades/course_grade.py index 551e5bafbe..a4e3c49e8a 100644 --- a/lms/djangoapps/grades/course_grade.py +++ b/lms/djangoapps/grades/course_grade.py @@ -13,7 +13,6 @@ from lazy import lazy from openedx.core.lib.grade_utils import round_away_from_zero from xmodule import block_metadata_utils # lint-amnesty, pylint: disable=wrong-import-order -from .config import assume_zero_if_absent from .scores import compute_percent from .subsection_grade import ZeroSubsectionGrade from .subsection_grade_factory import SubsectionGradeFactory @@ -294,14 +293,7 @@ class CourseGrade(CourseGradeBase): Returns whether any of the subsections in this course have been attempted by the student. """ - if assume_zero_if_absent(self.course_data.course_key): - return True - - for chapter in self.chapter_grades.values(): - for subsection_grade in chapter['sections']: - if subsection_grade.all_total.first_attempted: - return True - return False + return True def _get_subsection_grade(self, subsection, force_update_subsections=False): if self.force_update_subsections: diff --git a/lms/djangoapps/grades/course_grade_factory.py b/lms/djangoapps/grades/course_grade_factory.py index 4b310e4de0..513cb5d88e 100644 --- a/lms/djangoapps/grades/course_grade_factory.py +++ b/lms/djangoapps/grades/course_grade_factory.py @@ -9,7 +9,6 @@ from openedx.core.djangoapps.signals.signals import ( COURSE_GRADE_NOW_FAILED, COURSE_GRADE_NOW_PASSED ) -from .config import assume_zero_if_absent, should_persist_grades from .course_data import CourseData from .course_grade import CourseGrade, ZeroCourseGrade from .models import PersistentCourseGrade @@ -32,14 +31,10 @@ class CourseGradeFactory: course_structure=None, course_key=None, create_if_needed=True, - send_course_grade_signals=True, ): """ Returns the CourseGrade for the given user in the course. - Reads the value from storage. - If not in storage, returns a ZeroGrade if ASSUME_ZERO_GRADE_IF_ABSENT. - Else if create_if_needed, computes and returns a new value. - Else, returns None. + Reads the value from storage or creates an initial value of zero. At least one of course, collected_block_structure, course_structure, or course_key should be provided. @@ -48,12 +43,7 @@ class CourseGradeFactory: try: return self._read(user, course_data) except PersistentCourseGrade.DoesNotExist: - if assume_zero_if_absent(course_data.course_key): - return self._create_zero(user, course_data) - elif create_if_needed: - return self._update(user, course_data, send_course_grade_signals=send_course_grade_signals) - else: - return None + return self._create_zero(user, course_data) def update( self, @@ -145,9 +135,6 @@ class CourseGradeFactory: 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(user.id, course_data.course_key) log.debug('Grades: Read, %s, User: %s, %s', str(course_data), user.id, persistent_grade) @@ -160,19 +147,15 @@ class CourseGradeFactory: ) @staticmethod - def _update(user, course_data, force_update_subsections=False, send_course_grade_signals=True): + def _update(user, course_data, force_update_subsections=False): """ - Computes, saves, and returns a CourseGrade object for the given user and course. - - send_course_grade_signals defines if signals should be sent. Use it to avoid recursion issues in - cases when the signal listener trying to get grades but Persistent Grades are disabled. - If True - sends: - COURSE_GRADE_CHANGED signal to listeners and - COURSE_GRADE_NOW_PASSED if learner has passed course or - COURSE_GRADE_NOW_FAILED if learner is now failing course + Computes, saves, and returns a CourseGrade object for the + given user and course. + Sends a COURSE_GRADE_CHANGED signal to listeners and + COURSE_GRADE_NOW_PASSED if learner has passed course or + COURSE_GRADE_NOW_FAILED if learner is now failing course """ - should_persist = should_persist_grades(course_data.course_key) - if should_persist and force_update_subsections: + if force_update_subsections: prefetch_grade_overrides_and_visible_blocks(user, course_data.course_key) course_grade = CourseGrade( @@ -182,7 +165,7 @@ class CourseGradeFactory: ) course_grade = course_grade.update() - should_persist = should_persist and course_grade.attempted + should_persist = course_grade.attempted if should_persist: course_grade._subsection_grade_factory.bulk_create_unsaved() # lint-amnesty, pylint: disable=protected-access PersistentCourseGrade.update_or_create( @@ -196,27 +179,26 @@ class CourseGradeFactory: passed=course_grade.passed, ) - if send_course_grade_signals: - COURSE_GRADE_CHANGED.send_robust( - sender=None, + COURSE_GRADE_CHANGED.send_robust( + sender=None, + user=user, + course_grade=course_grade, + course_key=course_data.course_key, + deadline=course_data.course.end, + ) + if course_grade.passed: + COURSE_GRADE_NOW_PASSED.send( + sender=CourseGradeFactory, user=user, - course_grade=course_grade, - course_key=course_data.course_key, - deadline=course_data.course.end, + course_id=course_data.course_key, + ) + else: + COURSE_GRADE_NOW_FAILED.send( + sender=CourseGradeFactory, + user=user, + course_id=course_data.course_key, + grade=course_grade, ) - if course_grade.passed: - COURSE_GRADE_NOW_PASSED.send( - sender=CourseGradeFactory, - user=user, - course_id=course_data.course_key, - ) - else: - COURSE_GRADE_NOW_FAILED.send( - sender=CourseGradeFactory, - user=user, - course_id=course_data.course_key, - grade=course_grade, - ) log.info( 'Grades: Update, %s, User: %s, %s, persisted: %s', diff --git a/lms/djangoapps/grades/management/commands/tests/test_recalculate_subsection_grades.py b/lms/djangoapps/grades/management/commands/tests/test_recalculate_subsection_grades.py index 40c53a1637..8f679cd1c6 100644 --- a/lms/djangoapps/grades/management/commands/tests/test_recalculate_subsection_grades.py +++ b/lms/djangoapps/grades/management/commands/tests/test_recalculate_subsection_grades.py @@ -7,7 +7,6 @@ from datetime import datetime from unittest.mock import MagicMock, patch import ddt -from django.conf import settings from opaque_keys.edx.keys import CourseKey from pytz import utc @@ -21,7 +20,6 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-a DATE_FORMAT = "%Y-%m-%d %H:%M" -@patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) @ddt.ddt class TestRecalculateSubsectionGrades(HasCourseWithProblemsMixin, ModuleStoreTestCase): """ diff --git a/lms/djangoapps/grades/migrations/0019_auto_20220525_1200.py b/lms/djangoapps/grades/migrations/0019_auto_20220525_1200.py new file mode 100644 index 0000000000..09b7985330 --- /dev/null +++ b/lms/djangoapps/grades/migrations/0019_auto_20220525_1200.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2.13 on 2022-05-25 12:00 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('grades', '0018_add_waffle_flag_defaults'), + ] + + operations = [ + migrations.RemoveField( + model_name='persistentgradesenabledflag', + name='changed_by', + ), + migrations.DeleteModel( + name='CoursePersistentGradesFlag', + ), + migrations.DeleteModel( + name='PersistentGradesEnabledFlag', + ), + ] diff --git a/lms/djangoapps/grades/settings/test.py b/lms/djangoapps/grades/settings/test.py index 45cc11cf9e..11f60ed2ec 100644 --- a/lms/djangoapps/grades/settings/test.py +++ b/lms/djangoapps/grades/settings/test.py @@ -1,4 +1,3 @@ # lint-amnesty, pylint: disable=missing-module-docstring def plugin_settings(settings): - settings.FEATURES['PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS'] = True - settings.FEATURES['ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS'] = True + pass # amnesty, pylint: disable=unnecessary-pass diff --git a/lms/djangoapps/grades/subsection_grade_factory.py b/lms/djangoapps/grades/subsection_grade_factory.py index a2ebe2e934..7e6fea6c09 100644 --- a/lms/djangoapps/grades/subsection_grade_factory.py +++ b/lms/djangoapps/grades/subsection_grade_factory.py @@ -12,7 +12,6 @@ from submissions import api as submissions_api from common.djangoapps.student.models import anonymous_id_for_user from lms.djangoapps.courseware.model_data import ScoresClient -from lms.djangoapps.grades.config import assume_zero_if_absent, should_persist_grades from lms.djangoapps.grades.models import PersistentSubsectionGrade from lms.djangoapps.grades.scores import possibly_scored from openedx.core.djangoapps.signals.signals import COURSE_ASSESSMENT_GRADE_CHANGED @@ -41,7 +40,7 @@ class SubsectionGradeFactory: If read_only is True, doesn't save any updates to the grades. force_calculate - If true, will cause this function to return a `CreateSubsectionGrade` object if no cached - grade currently exists, even if the assume_zero_if_absent flag is enabled for the course. + grade currently exists. """ self._log_event( log.debug, f"create, read_only: {read_only}, subsection: {subsection.location}", subsection, @@ -49,18 +48,17 @@ class SubsectionGradeFactory: subsection_grade = self._get_bulk_cached_grade(subsection) if not subsection_grade: - if assume_zero_if_absent(self.course_data.course_key) and not force_calculate: + if not force_calculate: subsection_grade = ZeroSubsectionGrade(subsection, self.course_data) else: subsection_grade = CreateSubsectionGrade( subsection, 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[subsection_grade.location] = subsection_grade - else: - grade_model = subsection_grade.update_or_create_model(self.student) - self._update_saved_subsection_grade(subsection.location, grade_model) + if read_only: + self._unsaved_subsection_grades[subsection_grade.location] = subsection_grade + else: + grade_model = subsection_grade.update_or_create_model(self.student) + self._update_saved_subsection_grade(subsection.location, grade_model) return subsection_grade def bulk_create_unsaved(self): @@ -82,7 +80,7 @@ class SubsectionGradeFactory: subsection, self.course_data.structure, self._submissions_scores, self._csm_scores, ) - if persist_grade and should_persist_grades(self.course_data.course_key): + if persist_grade: if only_if_higher: try: grade_model = PersistentSubsectionGrade.read_grade(self.student.id, subsection.location) @@ -142,11 +140,10 @@ class SubsectionGradeFactory: 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() - grade = saved_subsection_grades.get(subsection.location) - if grade: - return ReadSubsectionGrade(subsection, grade, self) + saved_subsection_grades = self._get_bulk_cached_subsection_grades() + grade = saved_subsection_grades.get(subsection.location) + if grade: + return ReadSubsectionGrade(subsection, grade, self) def _get_bulk_cached_subsection_grades(self): """ diff --git a/lms/djangoapps/grades/tests/test_course_grade.py b/lms/djangoapps/grades/tests/test_course_grade.py index 77ea291957..c0a6d68b8f 100644 --- a/lms/djangoapps/grades/tests/test_course_grade.py +++ b/lms/djangoapps/grades/tests/test_course_grade.py @@ -3,8 +3,6 @@ from unittest.mock import patch import ddt from crum import set_current_request -from django.conf import settings -from edx_toggles.toggles.testutils import override_waffle_switch from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_AMNESTY_MODULESTORE, SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -12,7 +10,6 @@ from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangolib.testing.utils import get_mock_request -from ..config.waffle import ASSUME_ZERO_GRADE_IF_ABSENT from ..course_data import CourseData from ..course_grade import ZeroCourseGrade from ..course_grade_factory import CourseGradeFactory @@ -20,7 +17,6 @@ from .base import GradeTestBase from .utils import answer_problem -@patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) @ddt.ddt class ZeroGradeTest(GradeTestBase): """ @@ -33,29 +29,27 @@ class ZeroGradeTest(GradeTestBase): """ Creates a ZeroCourseGrade and ensures it's empty. """ - with override_waffle_switch(ASSUME_ZERO_GRADE_IF_ABSENT, active=assume_zero_enabled): - 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.problem_scores.values(): - assert score.earned == 0 - assert score.first_attempted is None - assert section.all_total.earned == 0 + 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.problem_scores.values(): + assert score.earned == 0 + assert score.first_attempted is None + assert section.all_total.earned == 0 @ddt.data(True, False) def test_zero_null_scores(self, assume_zero_enabled): """ Creates a zero course grade and ensures that null scores aren't included in the section problem scores. """ - with override_waffle_switch(ASSUME_ZERO_GRADE_IF_ABSENT, active=assume_zero_enabled): - with patch('lms.djangoapps.grades.subsection_grade.get_score', return_value=None): - 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: - assert {} != chapter_grades[chapter]['sections'] - for section in chapter_grades[chapter]['sections']: - assert {} == section.problem_scores + with patch('lms.djangoapps.grades.subsection_grade.get_score', return_value=None): + 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: + assert {} != chapter_grades[chapter]['sections'] + for section in chapter_grades[chapter]['sections']: + assert {} == section.problem_scores class TestScoreForModule(SharedModuleStoreTestCase): diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index 01eabf1531..508bbc2f1c 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -1,24 +1,16 @@ """ Tests for the CourseGradeFactory class. """ -import itertools -from unittest.mock import patch, Mock +from unittest.mock import patch import ddt -from django.conf import settings -from edx_toggles.toggles.testutils import override_waffle_switch -import pytest from common.djangoapps.student.tests.factories import UserFactory -from lms.djangoapps.certificates.config import AUTO_CERTIFICATE_GENERATION from lms.djangoapps.courseware.access import has_access -from lms.djangoapps.grades.config.tests.utils import persistent_grades_feature_flags from openedx.core.djangoapps.content.block_structure.factory import BlockStructureFactory -from openedx.core.djangoapps.signals.signals import COURSE_GRADE_NOW_PASSED from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order -from ..config.waffle import ASSUME_ZERO_GRADE_IF_ABSENT from ..course_grade import CourseGrade, ZeroCourseGrade from ..course_grade_factory import CourseGradeFactory from ..subsection_grade import ReadSubsectionGrade, ZeroSubsectionGrade @@ -54,57 +46,6 @@ class TestCourseGradeFactory(GradeTestBase): grade = CourseGradeFactory().read(self.request.user, invisible_course) assert grade.percent == 0 - @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) - @ddt.data( - (True, True), - (True, False), - (False, True), - (False, False), - ) - @ddt.unpack - def test_course_grade_feature_gating(self, feature_flag, course_setting): - # Grades are only saved if the feature flag and the advanced setting are - # both set to True. - grade_factory = CourseGradeFactory() - with persistent_grades_feature_flags( - global_flag=feature_flag, - enabled_for_all_courses=False, - course_id=self.course.id, - enabled_for_course=course_setting - ): - with patch('lms.djangoapps.grades.models.PersistentCourseGrade.read') as mock_read_grade: - grade_factory.read(self.request.user, self.course) - assert mock_read_grade.called == (feature_flag and course_setting) - - @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) - def test_no_recursion_without_persistent_grades(self): - """ - Course grade signals should not be fired recursively when persistent grades are disabled. - """ - self.mock_process_signal = Mock() # pylint: disable=attribute-defined-outside-init - - def handler(**kwargs): - """ - Mock signal receiver. - """ - self.mock_process_signal() - - with persistent_grades_feature_flags( - global_flag=False, - enabled_for_all_courses=False, - course_id=self.course.id, - enabled_for_course=False - ): - with override_waffle_switch(AUTO_CERTIFICATE_GENERATION, active=True), mock_get_score(2, 2): - COURSE_GRADE_NOW_PASSED.connect(handler) - try: - CourseGradeFactory().update(self.request.user, self.course) - except RecursionError: - pytest.fail("The COURSE_GRADE_NOW_PASSED signal fired recursively.") - - self.mock_process_signal.assert_called_once() - COURSE_GRADE_NOW_PASSED.disconnect(handler) - def test_read_and_update(self): grade_factory = CourseGradeFactory() @@ -158,17 +99,14 @@ class TestCourseGradeFactory(GradeTestBase): with self.assertNumQueries(3): _assert_read(expected_pass=False, expected_percent=0.0) # updated to grade of 0.0 - @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) - @ddt.data(*itertools.product((True, False), (True, False))) - @ddt.unpack - def test_read_zero(self, assume_zero_enabled, create_if_needed): - with override_waffle_switch(ASSUME_ZERO_GRADE_IF_ABSENT, active=assume_zero_enabled): - grade_factory = CourseGradeFactory() - course_grade = grade_factory.read(self.request.user, self.course, create_if_needed=create_if_needed) - if create_if_needed or assume_zero_enabled: - self._assert_zero_grade(course_grade, ZeroCourseGrade if assume_zero_enabled else CourseGrade) - else: - assert course_grade is None + @ddt.data((True, False)) + def test_read_zero(self, create_if_needed): + grade_factory = CourseGradeFactory() + course_grade = grade_factory.read(self.request.user, self.course, create_if_needed=create_if_needed) + if create_if_needed: + self._assert_zero_grade(course_grade, ZeroCourseGrade) + else: + assert course_grade is None def test_read_optimization(self): grade_factory = CourseGradeFactory() diff --git a/lms/djangoapps/grades/tests/test_subsection_grade_factory.py b/lms/djangoapps/grades/tests/test_subsection_grade_factory.py index 2609a2ac72..95e0e62361 100644 --- a/lms/djangoapps/grades/tests/test_subsection_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_subsection_grade_factory.py @@ -10,7 +10,6 @@ from django.conf import settings from common.djangoapps.student.tests.factories import UserFactory from lms.djangoapps.courseware.tests.test_submitting_problems import ProblemSubmissionTestMixin -from lms.djangoapps.grades.config.tests.utils import persistent_grades_feature_flags from ..constants import GradeOverrideFeatureEnum from ..models import PersistentSubsectionGrade, PersistentSubsectionGradeOverride @@ -105,29 +104,6 @@ class TestSubsectionGradeFactory(ProblemSubmissionTestMixin, GradeTestBase): verify_update_if_higher((1, 4), (2, 4)) # previous value was greater verify_update_if_higher((3, 4), (3, 4)) # previous value was less - @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) - @ddt.data( - (True, True), - (True, False), - (False, True), - (False, False), - ) - @ddt.unpack - def test_subsection_grade_feature_gating(self, feature_flag, course_setting): - # Grades are only saved if the feature flag and the advanced setting are - # both set to True. - with patch( - 'lms.djangoapps.grades.models.PersistentSubsectionGrade.bulk_read_grades' - ) as mock_read_saved_grade: - with persistent_grades_feature_flags( - global_flag=feature_flag, - enabled_for_all_courses=False, - course_id=self.course.id, - enabled_for_course=course_setting - ): - self.subsection_grade_factory.create(self.sequence) - assert mock_read_saved_grade.called == (feature_flag and course_setting) - @ddt.data( (0, None), (None, 3), diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 1c121fcfe0..f9c1beb930 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -10,9 +10,7 @@ from datetime import datetime, timedelta from unittest.mock import MagicMock, patch import ddt -import pytest import pytz -from django.conf import settings from django.db.utils import IntegrityError from django.utils import timezone from edx_toggles.toggles.testutils import override_waffle_flag @@ -25,7 +23,6 @@ from common.djangoapps.student.tests.factories import UserFactory from common.djangoapps.track.event_transaction_utils import create_new_event_transaction_id, get_event_transaction_id from common.djangoapps.util.date_utils import to_timestamp from lms.djangoapps.grades import tasks -from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag from lms.djangoapps.grades.config.waffle import ENFORCE_FREEZE_GRADE_AFTER_COURSE_END from lms.djangoapps.grades.constants import ScoreDatabaseTableEnum from lms.djangoapps.grades.models import PersistentCourseGrade, PersistentSubsectionGrade @@ -47,7 +44,7 @@ class HasCourseWithProblemsMixin: """ Mixin to provide tests with a sample course with graded subsections """ - def set_up_course(self, enable_persistent_grades=True, create_multiple_subsections=False, course_end=None): + def set_up_course(self, create_multiple_subsections=False, course_end=None): """ Configures the course for this test. """ @@ -57,8 +54,6 @@ class HasCourseWithProblemsMixin: run='run', end=course_end ) - if not enable_persistent_grades: - PersistentGradesEnabledFlag.objects.create(enabled=False) self.chapter = ItemFactory.create(parent=self.course, category="chapter", display_name="Chapter") self.sequential = ItemFactory.create(parent=self.chapter, category='sequential', display_name="Sequential1") @@ -103,7 +98,6 @@ class HasCourseWithProblemsMixin: # pylint: enable=attribute-defined-outside-init,no-member -@patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) @ddt.ddt class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTestCase): """ @@ -114,7 +108,6 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest def setUp(self): super().setUp() self.user = UserFactory() - PersistentGradesEnabledFlag.objects.create(enabled_for_all_courses=True, enabled=True) @contextmanager def mock_csm_get_score(self, score=MagicMock(grade=1.0, max_grade=2.0)): @@ -152,7 +145,6 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest def test_block_structure_created_only_once(self): self.set_up_course() - assert PersistentGradesEnabledFlag.feature_enabled(self.course.id) with patch( 'openedx.core.djangoapps.content.block_structure.factory.BlockStructureFactory.create_from_store', side_effect=BlockStructureNotFound(self.course.location), @@ -170,7 +162,6 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): with self.store.default_store(default_store): self.set_up_course(create_multiple_subsections=create_multiple_subsections) - assert PersistentGradesEnabledFlag.feature_enabled(self.course.id) with check_mongo_calls(num_mongo_calls): with self.assertNumQueries(num_sql_calls): self._apply_recalculate_subsection_grade() @@ -183,7 +174,6 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): with self.store.default_store(default_store): self.set_up_course(create_multiple_subsections=True) - assert PersistentGradesEnabledFlag.feature_enabled(self.course.id) num_problems = 10 for _ in range(num_problems): @@ -223,28 +213,13 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest ) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 24), - (ModuleStoreEnum.Type.split, 2, 24), + (ModuleStoreEnum.Type.mongo, 1, 41), + (ModuleStoreEnum.Type.split, 2, 41), ) @ddt.unpack - def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): + def test_persistent_grades_on_course(self, default_store, num_mongo_queries, num_sql_queries): with self.store.default_store(default_store): - self.set_up_course(enable_persistent_grades=False) - with check_mongo_calls(num_mongo_queries): - with self.assertNumQueries(num_sql_queries): - self._apply_recalculate_subsection_grade() - with pytest.raises(PersistentCourseGrade.DoesNotExist): - PersistentCourseGrade.read(self.user.id, self.course.id) - assert len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)) == 0 - - @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 42), - (ModuleStoreEnum.Type.split, 2, 42), - ) - @ddt.unpack - def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): - with self.store.default_store(default_store): - self.set_up_course(enable_persistent_grades=True) + self.set_up_course() with check_mongo_calls(num_mongo_queries): with self.assertNumQueries(num_sql_queries): self._apply_recalculate_subsection_grade() diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 1807a8af96..f935bd6956 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -1783,8 +1783,7 @@ class TestGradeReport(TestReportMixin, InstructorTaskModuleTestCase): def test_grade_report(self, persistent_grades_enabled): self.submit_student_answer(self.student.username, 'Problem1', ['Option 1']) - with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'), \ - patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': persistent_grades_enabled}): + with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'): result = CourseGradeReport.generate(None, None, self.course.id, {}, 'graded') self.assertDictContainsSubset( {'action_name': 'graded', 'attempted': 1, 'succeeded': 1, 'failed': 0}, diff --git a/lms/envs/bok_choy.yml b/lms/envs/bok_choy.yml index ba2ca578d5..c49c34b437 100644 --- a/lms/envs/bok_choy.yml +++ b/lms/envs/bok_choy.yml @@ -159,8 +159,6 @@ FEATURES: ENABLE_ENROLLMENT_TRACK_USER_PARTITION: True ENTRANCE_EXAMS: True ENABLE_SPECIAL_EXAMS: True - PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS: True - ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS: True GITHUB_REPO_ROOT: '** OVERRIDDEN **' diff --git a/openedx/core/djangoapps/credentials/tests/test_tasks.py b/openedx/core/djangoapps/credentials/tests/test_tasks.py index c48f4c5044..02edd17f07 100644 --- a/openedx/core/djangoapps/credentials/tests/test_tasks.py +++ b/openedx/core/djangoapps/credentials/tests/test_tasks.py @@ -488,13 +488,6 @@ class TestSendGradeIfInteresting(TestCase): assert mock_send_grade_to_credentials.delay.call_args[0] == (self.user.username, str(self.key), True, 'B', 0.81) mock_send_grade_to_credentials.delay.reset_mock() - @mock.patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) - def test_send_grade_without_grade(self, mock_is_course_run_in_a_program, mock_send_grade_to_credentials, - _mock_is_learner_issuance_enabled): - mock_is_course_run_in_a_program.return_value = True - tasks.send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None) - assert not mock_send_grade_to_credentials.delay.called - def test_send_grade_without_issuance_enabled(self, _mock_is_course_run_in_a_program, mock_send_grade_to_credentials, mock_is_learner_issuance_enabled): mock_is_learner_issuance_enabled.return_value = False