Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "feat: FC-0001 Remove Persistent Course Grades Flags (#30540)" (#30744)

Browse Source 
This reverts commit 4a9243ef9f.
This commit is contained in:
connorhaugh
2022-07-19 13:45:31 -04:00
committed by GitHub
parent 8de910b970
commit 45c2858d61
25 changed files with 607 additions and 153 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
lms/djangoapps/certificates/generation_handler.py
View File

@@ -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)
course_grade = _get_course_grade(user, course_key, send_course_grade_signals=False)
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)
course_grade = _get_course_grade(user, course_key, send_course_grade_signals=False)
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,11 +411,14 @@ def _get_grade_value(course_grade):
return ''
def _get_course_grade(user, course_key):
def _get_course_grade(user, course_key, send_course_grade_signals=True):
"""
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)
return CourseGradeFactory().read(user, course_key=course_key, send_course_grade_signals=send_course_grade_signals)
def _get_enrollment_mode(user, course_key):

79
lms/djangoapps/course_home_api/progress/tests/test_views.py
View File

@@ -7,6 +7,7 @@ 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
@@ -20,9 +21,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
)
@@ -192,54 +193,48 @@ 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):
chapter = ItemFactory(parent=self.course, category='chapter')
subsection = ItemFactory.create(parent=chapter, category="sequential", display_name="Subsection")
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")
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)
CourseEnrollment.enroll(self.user, self.course.id)
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

27
lms/djangoapps/courseware/tests/test_views.py
View File

@@ -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
from edx_toggles.toggles.testutils import override_waffle_flag, override_waffle_switch
from freezegun import freeze_time
from opaque_keys.edx.keys import CourseKey, UsageKey
from pytz import UTC
@@ -75,6 +75,7 @@ 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
@@ -1489,20 +1490,28 @@ class ProgressPageTests(ProgressPageBaseTests):
with self.assertNumQueries(query_count, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST), check_mongo_calls(2):
self._get_progress_page()
def test_progress_queries(self):
@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):
ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1))
self.setup_course()
with self.assertNumQueries(
52, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST
), check_mongo_calls(2):
self._get_progress_page()
for _ in range(2):
with override_waffle_switch(ASSUME_ZERO_GRADE_IF_ABSENT, active=enable_waffle):
with self.assertNumQueries(
36, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST
initial, 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(

23
lms/djangoapps/grades/admin.py
View File

@@ -3,12 +3,31 @@ Django admin page for grades models
"""
from config_models.admin import ConfigurationModelAdmin
from config_models.admin import ConfigurationModelAdmin, KeyedConfigurationModelAdmin
from django.contrib import admin
from lms.djangoapps.grades.config.forms import CoursePersistentGradesAdminForm
from lms.djangoapps.grades.config.models import (
ComputeGradesSetting
ComputeGradesSetting,
CoursePersistentGradesFlag,
PersistentGradesEnabledFlag
)
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)

28
lms/djangoapps/grades/config/__init__.py
View File

@@ -0,0 +1,28 @@
"""
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)

27
lms/djangoapps/grades/config/forms.py Normal file
View File

@@ -0,0 +1,27 @@
"""
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)

87
lms/djangoapps/grades/config/models.py
View File

@@ -5,7 +5,92 @@ controlling persistent grades.
from config_models.models import ConfigurationModel
from django.db.models import IntegerField, TextField
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"
class ComputeGradesSetting(ConfigurationModel):

93
lms/djangoapps/grades/config/tests/test_models.py Normal file
View File

@@ -0,0 +1,93 @@
"""
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)

30
lms/djangoapps/grades/config/tests/utils.py Normal file
View File

@@ -0,0 +1,30 @@
"""
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

11
lms/djangoapps/grades/config/waffle.py
View File

@@ -14,6 +14,17 @@ 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

10
lms/djangoapps/grades/course_grade.py
View File

@@ -13,6 +13,7 @@ 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
@@ -293,7 +294,14 @@ class CourseGrade(CourseGradeBase):
Returns whether any of the subsections in this course
have been attempted by the student.
"""
return True
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
def _get_subsection_grade(self, subsection, force_update_subsections=False):
if self.force_update_subsections:

74
lms/djangoapps/grades/course_grade_factory.py
View File

@@ -9,6 +9,7 @@ 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
@@ -31,10 +32,14 @@ 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 or creates an initial value of zero.
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.
At least one of course, collected_block_structure, course_structure,
or course_key should be provided.
@@ -43,7 +48,12 @@ class CourseGradeFactory:
try:
return self._read(user, course_data)
except PersistentCourseGrade.DoesNotExist:
return self._create_zero(user, course_data)
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
def update(
self,
@@ -135,6 +145,9 @@ 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)
@@ -147,15 +160,19 @@ class CourseGradeFactory:
)
@staticmethod
def _update(user, course_data, force_update_subsections=False):
def _update(user, course_data, force_update_subsections=False, send_course_grade_signals=True):
"""
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
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
"""
if force_update_subsections:
should_persist = should_persist_grades(course_data.course_key)
if should_persist and force_update_subsections:
prefetch_grade_overrides_and_visible_blocks(user, course_data.course_key)
course_grade = CourseGrade(
@@ -165,7 +182,7 @@ class CourseGradeFactory:
)
course_grade = course_grade.update()
should_persist = course_grade.attempted
should_persist = should_persist and course_grade.attempted
if should_persist:
course_grade._subsection_grade_factory.bulk_create_unsaved() # lint-amnesty, pylint: disable=protected-access
PersistentCourseGrade.update_or_create(
@@ -179,26 +196,27 @@ class CourseGradeFactory:
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,
)
if course_grade.passed:
COURSE_GRADE_NOW_PASSED.send(
sender=CourseGradeFactory,
if send_course_grade_signals:
COURSE_GRADE_CHANGED.send_robust(
sender=None,
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,
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_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',

2
lms/djangoapps/grades/management/commands/tests/test_recalculate_subsection_grades.py
View File

@@ -7,6 +7,7 @@ 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
@@ -20,6 +21,7 @@ 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):
"""

23
lms/djangoapps/grades/migrations/0019_auto_20220525_1200.py
View File

@@ -1,23 +0,0 @@
# 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',
),
]

3
lms/djangoapps/grades/settings/test.py
View File

@@ -1,3 +1,4 @@
# lint-amnesty, pylint: disable=missing-module-docstring
def plugin_settings(settings):
pass # amnesty, pylint: disable=unnecessary-pass
settings.FEATURES['PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS'] = True
settings.FEATURES['ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS'] = True

27
lms/djangoapps/grades/subsection_grade_factory.py
View File

@@ -12,6 +12,7 @@ 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
@@ -40,7 +41,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.
grade currently exists, even if the assume_zero_if_absent flag is enabled for the course.
"""
self._log_event(
log.debug, f"create, read_only: {read_only}, subsection: {subsection.location}", subsection,
@@ -48,17 +49,18 @@ class SubsectionGradeFactory:
subsection_grade = self._get_bulk_cached_grade(subsection)
if not subsection_grade:
if not force_calculate:
if assume_zero_if_absent(self.course_data.course_key) and 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 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 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)
return subsection_grade
def bulk_create_unsaved(self):
@@ -80,7 +82,7 @@ class SubsectionGradeFactory:
subsection, self.course_data.structure, self._submissions_scores, self._csm_scores,
)
if persist_grade:
if persist_grade and should_persist_grades(self.course_data.course_key):
if only_if_higher:
try:
grade_model = PersistentSubsectionGrade.read_grade(self.student.id, subsection.location)
@@ -140,10 +142,11 @@ class SubsectionGradeFactory:
course, for future access of other subsections.
Returns None if not found.
"""
saved_subsection_grades = self._get_bulk_cached_subsection_grades()
grade = saved_subsection_grades.get(subsection.location)
if grade:
return ReadSubsectionGrade(subsection, grade, self)
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)
def _get_bulk_cached_subsection_grades(self):
"""

36
lms/djangoapps/grades/tests/test_course_grade.py
View File

@@ -3,6 +3,8 @@ 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
@@ -10,6 +12,7 @@ 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
@@ -17,6 +20,7 @@ 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):
"""
@@ -29,27 +33,29 @@ class ZeroGradeTest(GradeTestBase):
"""
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.problem_scores.values():
assert score.earned == 0
assert score.first_attempted is None
assert section.all_total.earned == 0
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
@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 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 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
class TestScoreForModule(SharedModuleStoreTestCase):

80
lms/djangoapps/grades/tests/test_course_grade_factory.py
View File

@@ -1,16 +1,24 @@
"""
Tests for the CourseGradeFactory class.
"""
from unittest.mock import patch
import itertools
from unittest.mock import patch, Mock
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
@@ -46,6 +54,57 @@ 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()
@@ -99,14 +158,17 @@ class TestCourseGradeFactory(GradeTestBase):
with self.assertNumQueries(3):
_assert_read(expected_pass=False, expected_percent=0.0) # updated to grade of 0.0
@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
@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
def test_read_optimization(self):
grade_factory = CourseGradeFactory()

24
lms/djangoapps/grades/tests/test_subsection_grade_factory.py
View File

@@ -10,6 +10,7 @@ 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
@@ -104,6 +105,29 @@ 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),

35
lms/djangoapps/grades/tests/test_tasks.py
View File

@@ -10,7 +10,9 @@ 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
@@ -23,6 +25,7 @@ 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
@@ -44,7 +47,7 @@ class HasCourseWithProblemsMixin:
"""
Mixin to provide tests with a sample course with graded subsections
"""
def set_up_course(self, create_multiple_subsections=False, course_end=None):
def set_up_course(self, enable_persistent_grades=True, create_multiple_subsections=False, course_end=None):
"""
Configures the course for this test.
"""
@@ -54,6 +57,8 @@ 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")
@@ -98,6 +103,7 @@ 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):
"""
@@ -108,6 +114,7 @@ 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)):
@@ -145,6 +152,7 @@ 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),
@@ -162,6 +170,7 @@ 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()
@@ -174,6 +183,7 @@ 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):
@@ -213,13 +223,28 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
)
@ddt.data(
(ModuleStoreEnum.Type.mongo, 1, 41),
(ModuleStoreEnum.Type.split, 2, 41),
(ModuleStoreEnum.Type.mongo, 1, 24),
(ModuleStoreEnum.Type.split, 2, 24),
)
@ddt.unpack
def test_persistent_grades_on_course(self, default_store, num_mongo_queries, num_sql_queries):
def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries):
with self.store.default_store(default_store):
self.set_up_course()
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)
with check_mongo_calls(num_mongo_queries):
with self.assertNumQueries(num_sql_queries):
self._apply_recalculate_subsection_grade()

3
lms/djangoapps/instructor_task/tests/test_tasks_helper.py
View File

@@ -1783,7 +1783,8 @@ 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'):
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}):
result = CourseGradeReport.generate(None, None, self.course.id, {}, 'graded')
self.assertDictContainsSubset(
{'action_name': 'graded', 'attempted': 1, 'succeeded': 1, 'failed': 0},

2
lms/envs/bok_choy.yml
View File

@@ -159,6 +159,8 @@ 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 **'