diff --git a/lms/djangoapps/grades/models.py b/lms/djangoapps/grades/models.py index 50cfa2819ca4..83d2eac5179f 100644 --- a/lms/djangoapps/grades/models.py +++ b/lms/djangoapps/grades/models.py @@ -477,7 +477,6 @@ def update_or_create_grade(cls, **params): grade.first_attempted = first_attempted grade.save() - cls._emit_grade_calculated_event(grade) return grade @classmethod @@ -498,8 +497,6 @@ def bulk_create_grades(cls, grade_params_iter, user_id, course_key): grades = [PersistentSubsectionGrade(**params) for params in grade_params_iter] grades = cls.objects.bulk_create(grades) - for grade in grades: - cls._emit_grade_calculated_event(grade) return grades @classmethod @@ -525,10 +522,6 @@ def _prepare_params_visible_blocks_id(cls, params): params['visible_blocks_id'] = params['visible_blocks'].hash_value del params['visible_blocks'] - @staticmethod - def _emit_grade_calculated_event(grade): - events.subsection_grade_calculated(grade) - @classmethod def _cache_key(cls, course_id): return f"subsection_grades_cache.{course_id}" diff --git a/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py b/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py index f4cf910c129c..32c937e3d3f1 100644 --- a/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py +++ b/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py @@ -7,6 +7,7 @@ from collections import OrderedDict, namedtuple from contextlib import contextmanager from datetime import datetime +from decimal import Decimal from unittest.mock import MagicMock, patch import ddt @@ -17,6 +18,7 @@ from edx_toggles.toggles.testutils import override_waffle_flag from freezegun import freeze_time from opaque_keys.edx.locator import BlockUsageLocator +from openedx_learning.api import GradedObjectScore from pytz import UTC from rest_framework import status from rest_framework.response import Response @@ -1721,6 +1723,28 @@ def test_subsection_does_not_exist(self, login_method): assert status.HTTP_422_UNPROCESSABLE_ENTITY == resp.status_code assert expected_data == resp.data + @override_settings(ENABLE_COMPETENCY_MASTERY_TRACKING=True) + def test_new_subsection_grade_records_competency_status(self): + subsection = self.subsections[self.chapter_1.location][0] + score = GradedObjectScore(object_id=str(subsection.location), fraction=Decimal('0.5')) + post_data = [{ + 'user_id': self.student.id, + 'usage_id': str(subsection.location), + 'grade': {'earned_graded_override': 1, 'possible_graded_override': 2}, + }] + + with override_waffle_flag(self.waffle_flag, active=True): + self.login_staff() + # The fixture subsections have no scorable children, so a score needs this patch. + with patch( + 'lms.djangoapps.grades.subsection_grade.CreateSubsectionGrade.graded_object_score', + return_value=score, + ), patch('lms.djangoapps.grades.subsection_grade.record_graded_object_statuses') as mock_record: + resp = self.client.post(self.get_url(), data=json.dumps(post_data), content_type='application/json') + + assert status.HTTP_202_ACCEPTED == resp.status_code + mock_record.assert_any_call(user_id=self.student.id, scores=[score]) + @ddt.data('login_staff', 'login_course_staff', 'login_course_admin') def test_override_is_created(self, login_method): """ diff --git a/lms/djangoapps/grades/subsection_grade.py b/lms/djangoapps/grades/subsection_grade.py index 5fab4fb5f116..604bc931e2e9 100644 --- a/lms/djangoapps/grades/subsection_grade.py +++ b/lms/djangoapps/grades/subsection_grade.py @@ -6,11 +6,16 @@ from abc import ABCMeta from collections import OrderedDict from datetime import datetime, timezone +from decimal import Decimal from logging import getLogger +from django.conf import settings +from django.db import transaction from lazy import lazy +from openedx_learning.api import GradedObjectScore, record_graded_object_statuses from xblock.scorable import ShowCorrectness +from lms.djangoapps.grades import events from lms.djangoapps.grades.models import BlockRecord, PersistentSubsectionGrade from lms.djangoapps.grades.scores import compute_percent, get_score, possibly_scored from xmodule import block_metadata_utils, graders # pylint: disable=wrong-import-order @@ -19,6 +24,29 @@ log = getLogger(__name__) +def _enqueue_competency_rollup(user_id, object_ids): + """ + Queues the competency rollup for the scored objects once their grade transaction has committed. + """ + # Imported here because tasks.py imports this module through the subsection grade factory. + from lms.djangoapps.grades.tasks import roll_up_competency_statuses_for_user + + roll_up_competency_statuses_for_user.apply_async(kwargs={"user_id": user_id, "object_ids": object_ids}) + + +def _record_competency_status(student, grades): + """ + Records competency status inside the caller's transaction and returns the object ids to roll up, or an empty + list when tracking is off, nothing is scored, or no rows changed. + """ + if not settings.ENABLE_COMPETENCY_MASTERY_TRACKING: + return [] + scores = [score for score in (grade.graded_object_score() for grade in grades) if score] + if not scores or not record_graded_object_statuses(user_id=student.id, scores=scores): + return [] + return [score.object_id for score in scores] + + class SubsectionGradeBase(metaclass=ABCMeta): # noqa: B024 """ Abstract base class for Subsection Grades. @@ -285,7 +313,10 @@ def update_or_create_model(self, student, score_deleted=False, force_update_subs """ Saves or updates the subsection grade in a persisted model. """ - if self._should_persist_per_attempted(score_deleted, force_update_subsections): + if not self._should_persist_per_attempted(score_deleted, force_update_subsections): + return None + + with transaction.atomic(): model = PersistentSubsectionGrade.update_or_create_grade(**self._persisted_model_params(student)) if hasattr(model, 'override'): @@ -296,20 +327,52 @@ def update_or_create_model(self, student, score_deleted=False, force_update_subs self.all_total = self._aggregated_score_from_model(model, is_graded=False) self.graded_total = self._aggregated_score_from_model(model, is_graded=True) - return model + object_ids = _record_competency_status(student, [self]) + if object_ids: + # robust: a broker failure after the grade committed must not skip the event or fail the save. + transaction.on_commit(lambda: _enqueue_competency_rollup(student.id, object_ids), robust=True) + + events.subsection_grade_calculated(model) + return model @classmethod def bulk_create_models(cls, student, subsection_grades, course_key): """ Saves the subsection grade in a persisted model. """ - params = [ - subsection_grade._persisted_model_params(student) # pylint: disable=protected-access + persisted_grades = [ + subsection_grade for subsection_grade in subsection_grades if subsection_grade if subsection_grade._should_persist_per_attempted() # pylint: disable=protected-access ] - return PersistentSubsectionGrade.bulk_create_grades(params, student.id, course_key) + params = [ + subsection_grade._persisted_model_params(student) # pylint: disable=protected-access + for subsection_grade in persisted_grades + ] + if not persisted_grades: + return None + + with transaction.atomic(): + grades = PersistentSubsectionGrade.bulk_create_grades(params, student.id, course_key) + object_ids = _record_competency_status(student, persisted_grades) + if object_ids: + # robust: a broker failure after the grades committed must not skip the events or fail the save. + transaction.on_commit(lambda: _enqueue_competency_rollup(student.id, object_ids), robust=True) + + for grade in grades or []: + events.subsection_grade_calculated(grade) + return grades + + def graded_object_score(self): + """ + Returns the graded fraction as a score, or None when nothing was attempted or nothing could be earned. + """ + if not self.attempted_graded or not self.graded_total.possible: + return None + # An override can push earned outside [0, possible]; core rejects fractions outside [0, 1]. + fraction = min(max(Decimal(str(self.percent_graded)), Decimal(0)), Decimal(1)) + return GradedObjectScore(object_id=str(self.location), fraction=fraction) def _should_persist_per_attempted(self, score_deleted=False, force_update_subsections=False): """ diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index f00bed8ab1ba..e632c3654e47 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -15,6 +15,7 @@ ) from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import CourseLocator +from openedx_learning.api import roll_up_competency_statuses from submissions import api as sub_api from common.djangoapps.student.models import CourseEnrollment @@ -185,6 +186,29 @@ def recalculate_subsection_grade_v3(self, **kwargs): _recalculate_subsection_grade(self, **kwargs) +@shared_task( + bind=True, + base=LoggedPersistOnFailureTask, + time_limit=SUBSECTION_GRADE_TIMEOUT_SECONDS, + max_retries=2, + default_retry_delay=RETRY_DELAY_SECONDS +) +def roll_up_competency_statuses_for_user(self, **kwargs): + """ + Rolls up a learner's competency statuses for the graded objects whose criteria were just updated. + + Keyword Arguments: + user_id (int): id of applicable User object + object_ids (list of string): usage keys of the graded objects that changed + """ + try: + roll_up_competency_statuses(user_id=kwargs['user_id'], object_ids=kwargs['object_ids']) + except Exception as exc: + if not isinstance(exc, KNOWN_RETRY_ERRORS): + raise + raise self.retry(kwargs=kwargs, exc=exc) from exc + + def _recalculate_subsection_grade(self, **kwargs): """ Updates a saved subsection grade. diff --git a/lms/djangoapps/grades/tests/test_api.py b/lms/djangoapps/grades/tests/test_api.py index a5f60b224206..c80221f7d532 100644 --- a/lms/djangoapps/grades/tests/test_api.py +++ b/lms/djangoapps/grades/tests/test_api.py @@ -1,10 +1,14 @@ """ Tests calling the grades api directly """ +from decimal import Decimal from unittest.mock import patch import ddt +from django.test import override_settings +from openedx_learning.api import GradedObjectScore +from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.tests.factories import UserFactory from lms.djangoapps.grades import api from lms.djangoapps.grades.models import ( @@ -12,6 +16,7 @@ PersistentSubsectionGrade, PersistentSubsectionGradeOverride, ) +from lms.djangoapps.grades.tests.utils import mock_get_score from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, # pylint: disable=wrong-import-order ) @@ -67,6 +72,23 @@ def tearDown(self): super().tearDown() PersistentSubsectionGradeOverride.objects.all().delete() + @override_settings(ENABLE_COMPETENCY_MASTERY_TRACKING=True) + def test_override_without_existing_grade_records_competency_status(self): + CourseEnrollment.enroll(self.user, self.course.id) + new_subsection = BlockFactory.create(parent=self.course, category="sequential", display_name="New Subsection") + BlockFactory.create(parent=new_subsection, category="problem", display_name="New Problem") + score = GradedObjectScore(object_id=str(new_subsection.location), fraction=Decimal('0.5')) + + with mock_get_score(1, 2), patch( + 'lms.djangoapps.grades.subsection_grade.record_graded_object_statuses' + ) as mock_record: + api.override_subsection_grade( + self.user.id, self.course.id, new_subsection.location, overrider=self.overriding_user, + earned_graded=1.0, + ) + + mock_record.assert_called_once_with(user_id=self.user.id, scores=[score]) + @ddt.data(0.0, None, 3.0) def test_override_subsection_grade(self, earned_graded): api.override_subsection_grade( diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index ed7e886d6f58..201e931cf943 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -73,7 +73,7 @@ def _assert_section_order(course_grade): with self.assertNumQueries(3), mock_get_score(1, 2): _assert_read(expected_pass=False, expected_percent=0) # start off with grade of 0 - num_queries = 44 + num_queries = 48 with self.assertNumQueries(num_queries), mock_get_score(1, 2): grade_factory.update(self.request.user, self.course, force_update_subsections=True) @@ -87,14 +87,14 @@ def _assert_section_order(course_grade): with self.assertNumQueries(3): _assert_read(expected_pass=True, expected_percent=0.5) # NOT updated to grade of .25 - num_queries = 20 + num_queries = 24 with self.assertNumQueries(num_queries), mock_get_score(2, 2): grade_factory.update(self.request.user, self.course, force_update_subsections=True) with self.assertNumQueries(3): _assert_read(expected_pass=True, expected_percent=1.0) # updated to grade of 1.0 - num_queries = 30 + num_queries = 34 with self.assertNumQueries(num_queries), mock_get_score(0, 0): # the subsection now is worth zero grade_factory.update(self.request.user, self.course, force_update_subsections=True) diff --git a/lms/djangoapps/grades/tests/test_models.py b/lms/djangoapps/grades/tests/test_models.py index 063724f2e9a1..1e773b5fcc12 100644 --- a/lms/djangoapps/grades/tests/test_models.py +++ b/lms/djangoapps/grades/tests/test_models.py @@ -286,16 +286,6 @@ def test_unattempted_save_does_not_remove_attempt(self): assert isinstance(grade.first_attempted, datetime) assert grade.earned_all == 6.0 - def test_update_or_create_event(self): - with patch('lms.djangoapps.grades.events.tracker') as tracker_mock: - grade = PersistentSubsectionGrade.update_or_create_grade(**self.params) - self._assert_tracker_emitted_event(tracker_mock, grade) - - def test_create_event(self): - with patch('lms.djangoapps.grades.events.tracker') as tracker_mock: - grade = PersistentSubsectionGrade.update_or_create_grade(**self.params) - self._assert_tracker_emitted_event(tracker_mock, grade) - def test_grade_override(self): """ Creating a subsection grade override should NOT change the score values @@ -322,30 +312,6 @@ def test_grade_override(self): assert grade.possible_all == override.possible_all_override assert grade.possible_graded == override.possible_graded_override - def _assert_tracker_emitted_event(self, tracker_mock, grade): - """ - Helper function to ensure that the mocked event tracker - was called with the expected info based on the passed grade. - """ - tracker_mock.emit.assert_called_with( - 'edx.grades.subsection.grade_calculated', - { - 'user_id': str(grade.user_id), - 'course_id': str(grade.course_id), - 'block_id': str(grade.usage_key), - 'course_version': str(grade.course_version), - 'weighted_total_earned': grade.earned_all, - 'weighted_total_possible': grade.possible_all, - 'weighted_graded_earned': grade.earned_graded, - 'weighted_graded_possible': grade.possible_graded, - 'first_attempted': str(grade.first_attempted), - 'subtree_edited_timestamp': str(grade.subtree_edited_timestamp), - 'event_transaction_id': str(get_event_transaction_id()), - 'event_transaction_type': str(get_event_transaction_type()), - 'visible_blocks_hash': str(grade.visible_blocks_id), - } - ) - def test_clear_subsection_grade(self): PersistentSubsectionGrade.update_or_create_grade(**self.params) deleted = PersistentSubsectionGrade.delete_subsection_grades_for_learner( diff --git a/lms/djangoapps/grades/tests/test_subsection_grade.py b/lms/djangoapps/grades/tests/test_subsection_grade.py index 406aa920dd47..f235f7c46fe5 100644 --- a/lms/djangoapps/grades/tests/test_subsection_grade.py +++ b/lms/djangoapps/grades/tests/test_subsection_grade.py @@ -3,10 +3,34 @@ """ +from datetime import UTC, datetime +from decimal import Decimal +from unittest.mock import patch + +import pytest from ddt import data, ddt, unpack +from django.db import DatabaseError, connection +from django.test import override_settings +from openedx_learning.models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyMasteryStatus, + CompetencyRuleProfile, + CompetencyTaxonomy, + MasteryStatus, + RuleType, + StudentCompetencyCriteriaStatus, +) +from openedx_tagging.models import ObjectTag, Tag + +from common.djangoapps.student.tests.factories import UserFactory +from common.djangoapps.track.event_transaction_utils import get_event_transaction_id, get_event_transaction_type +from lms.djangoapps.grades.tasks import roll_up_competency_statuses_for_user +from xmodule.graders import AggregatedScore -from ..models import PersistentSubsectionGrade +from ..models import PersistentSubsectionGrade, PersistentSubsectionGradeOverride from ..subsection_grade import CreateSubsectionGrade, ReadSubsectionGrade +from ..subsection_grade_factory import SubsectionGradeFactory from .base import GradeTestBase from .utils import mock_get_score @@ -57,3 +81,462 @@ def test_zero(self): self.subsection_grade_factory._csm_scores, # pylint: disable=protected-access ) assert grade.percent_graded == 0.0 + + +RECORD_PATH = 'lms.djangoapps.grades.subsection_grade.record_graded_object_statuses' +EMIT_PATH = 'lms.djangoapps.grades.subsection_grade.events.subsection_grade_calculated' +ENQUEUE_PATH = 'lms.djangoapps.grades.tasks.roll_up_competency_statuses_for_user.apply_async' + + +class SubsectionGradeFixtureMixin: + """ + Helpers shared by the competency test classes; expects to be mixed into a GradeTestBase. + """ + + def _create_grade(self, subsection=None): + """ + Returns an unsaved CreateSubsectionGrade for the given subsection (default: the first sequence). + """ + factory = SubsectionGradeFactory(self.request.user, self.course, self.course_structure) + return factory.create(subsection or self.sequence, read_only=True, force_calculate=True) + + +@override_settings(ENABLE_COMPETENCY_MASTERY_TRACKING=True) +@ddt +class SubsectionGradeCompetencyStatusTest(SubsectionGradeFixtureMixin, GradeTestBase): + """ + Tests that persisting a subsection grade passes the right scores to the competency API in the grade's transaction. + """ + + def test_single_path_records_fraction(self): + with mock_get_score(1, 2), patch(RECORD_PATH) as mock_record: + self._create_grade().update_or_create_model(self.request.user) + + mock_record.assert_called_once() + assert mock_record.call_args.kwargs['user_id'] == self.request.user.id + scores = mock_record.call_args.kwargs['scores'] + assert [(score.object_id, score.fraction) for score in scores] == [ + (str(self.sequence.location), Decimal('0.5')), + ] + + def test_staff_override_uses_override_adjusted_fraction(self): + with mock_get_score(1, 4), patch(RECORD_PATH): + self.subsection_grade_factory.update(self.sequence) + model = PersistentSubsectionGrade.read_grade(self.request.user.id, self.sequence.location) + PersistentSubsectionGradeOverride.update_or_create_override( + UserFactory(), model, earned_graded_override=4.0, possible_graded_override=4.0, + ) + + with mock_get_score(1, 4), patch(RECORD_PATH) as mock_record: + self.subsection_grade_factory.update(self.sequence) + + assert mock_record.call_args.kwargs['scores'][0].fraction == Decimal('1') + + def test_record_failure_rolls_back_grade_and_skips_event(self): + with mock_get_score(1, 2), patch(RECORD_PATH, side_effect=DatabaseError), patch(EMIT_PATH) as mock_emit: + with pytest.raises(DatabaseError): + self._create_grade().update_or_create_model(self.request.user) + + assert PersistentSubsectionGrade.objects.count() == 0 + mock_emit.assert_not_called() + + def test_event_is_emitted_after_the_atomic_block(self): + # An open atomic block adds a savepoint id, so the depth tells whether a call ran inside the block. + depths = {} + with mock_get_score(1, 2): + grade = self._create_grade() + with patch(RECORD_PATH, side_effect=lambda **_: depths.update(record=len(connection.savepoint_ids))): + with patch(EMIT_PATH, side_effect=lambda _: depths.update(emit=len(connection.savepoint_ids))): + grade.update_or_create_model(self.request.user) + + assert depths['record'] == depths['emit'] + 1 + + def test_zero_possible_passes_no_score(self): + with mock_get_score(0, 0), patch(RECORD_PATH) as mock_record: + self._create_grade().update_or_create_model(self.request.user, force_update_subsections=True) + + mock_record.assert_not_called() + + def test_unattempted_passes_no_score(self): + with mock_get_score(1, 2, first_attempted=None), patch(RECORD_PATH) as mock_record: + self._create_grade().update_or_create_model(self.request.user, force_update_subsections=True) + + mock_record.assert_not_called() + + @override_settings(ENABLE_COMPETENCY_MASTERY_TRACKING=False) + def test_setting_off_saves_grades_without_calling_core(self): + with mock_get_score(1, 2), patch(RECORD_PATH) as mock_record: + self._create_grade().update_or_create_model(self.request.user) + CreateSubsectionGrade.bulk_create_models( + UserFactory(), [self._create_grade(self.sequence2)], self.course.id, + ) + + mock_record.assert_not_called() + assert PersistentSubsectionGrade.objects.count() == 2 + + def test_bulk_path_records_each_qualifying_subsection(self): + with mock_get_score(1, 2), patch(RECORD_PATH) as mock_record, patch(EMIT_PATH) as mock_emit: + grades = CreateSubsectionGrade.bulk_create_models( + self.request.user, + [self._create_grade(self.sequence), self._create_grade(self.sequence2)], + self.course.id, + ) + + mock_record.assert_called_once() + scores = mock_record.call_args.kwargs['scores'] + assert {(score.object_id, score.fraction) for score in scores} == { + (str(self.sequence.location), Decimal('0.5')), + (str(self.sequence2.location), Decimal('0.5')), + } + assert mock_emit.call_count == len(grades) == 2 + + def test_bulk_path_omits_zero_possible_subsections(self): + with mock_get_score(0, 0), patch(RECORD_PATH) as mock_record: + CreateSubsectionGrade.bulk_create_models( + self.request.user, [self._create_grade(self.sequence)], self.course.id, + ) + + mock_record.assert_not_called() + + def test_bulk_path_failure_rolls_back_grades_and_skips_events(self): + with mock_get_score(1, 2), patch(RECORD_PATH, side_effect=DatabaseError), patch(EMIT_PATH) as mock_emit: + with pytest.raises(DatabaseError): + CreateSubsectionGrade.bulk_create_models( + self.request.user, [self._create_grade(self.sequence)], self.course.id, + ) + + assert PersistentSubsectionGrade.objects.count() == 0 + mock_emit.assert_not_called() + + def test_single_path_enqueues_rollup_once_after_commit(self): + with mock_get_score(1, 2), patch(RECORD_PATH, return_value=1), patch(ENQUEUE_PATH) as mock_enqueue: + with self.captureOnCommitCallbacks(execute=True) as callbacks: + self._create_grade().update_or_create_model(self.request.user) + assert len(callbacks) == 1 + + mock_enqueue.assert_called_once_with( + kwargs={'user_id': self.request.user.id, 'object_ids': [str(self.sequence.location)]}, + ) + + def test_single_path_does_not_enqueue_before_commit(self): + with mock_get_score(1, 2), patch(RECORD_PATH, return_value=1), patch(ENQUEUE_PATH) as mock_enqueue: + with self.captureOnCommitCallbacks(execute=False): + self._create_grade().update_or_create_model(self.request.user) + + mock_enqueue.assert_not_called() + + def test_single_path_does_not_enqueue_when_no_rows_changed(self): + with mock_get_score(1, 2), patch(RECORD_PATH, return_value=0), patch(ENQUEUE_PATH) as mock_enqueue: + with self.captureOnCommitCallbacks(execute=True): + self._create_grade().update_or_create_model(self.request.user) + + mock_enqueue.assert_not_called() + + def test_single_path_does_not_enqueue_when_record_fails(self): + with mock_get_score(1, 2), patch(RECORD_PATH, side_effect=DatabaseError), patch(ENQUEUE_PATH) as mock_enqueue: + with self.captureOnCommitCallbacks(execute=True): + with pytest.raises(DatabaseError): + self._create_grade().update_or_create_model(self.request.user) + + mock_enqueue.assert_not_called() + + def test_single_path_enqueue_failure_does_not_skip_event_or_raise(self): + with mock_get_score(1, 2), patch(RECORD_PATH, return_value=1), patch(EMIT_PATH) as mock_emit: + with patch(ENQUEUE_PATH, side_effect=OSError('broker down')): + with self.captureOnCommitCallbacks(execute=True): + self._create_grade().update_or_create_model(self.request.user) + + mock_emit.assert_called_once() + + def test_single_path_enqueue_kwargs_are_what_the_task_reads(self): + with mock_get_score(1, 2), patch(RECORD_PATH, return_value=1), patch(ENQUEUE_PATH) as mock_enqueue: + with self.captureOnCommitCallbacks(execute=True): + self._create_grade().update_or_create_model(self.request.user) + captured = mock_enqueue.call_args.kwargs['kwargs'] + + with patch('lms.djangoapps.grades.tasks.roll_up_competency_statuses') as mock_roll_up: + roll_up_competency_statuses_for_user.apply(kwargs=captured) + + mock_roll_up.assert_called_once_with( + user_id=self.request.user.id, object_ids=[str(self.sequence.location)], + ) + + @override_settings(ENABLE_COMPETENCY_MASTERY_TRACKING=False) + def test_setting_off_does_not_enqueue(self): + with mock_get_score(1, 2), patch(RECORD_PATH, return_value=1), patch(ENQUEUE_PATH) as mock_enqueue: + with self.captureOnCommitCallbacks(execute=True): + self._create_grade().update_or_create_model(self.request.user) + CreateSubsectionGrade.bulk_create_models( + UserFactory(), [self._create_grade(self.sequence2)], self.course.id, + ) + + mock_enqueue.assert_not_called() + + def test_bulk_path_enqueues_one_rollup_with_every_scored_object(self): + with mock_get_score(1, 2), patch(RECORD_PATH, return_value=2), patch(ENQUEUE_PATH) as mock_enqueue: + with self.captureOnCommitCallbacks(execute=True): + CreateSubsectionGrade.bulk_create_models( + self.request.user, + [self._create_grade(self.sequence), self._create_grade(self.sequence2)], + self.course.id, + ) + + mock_enqueue.assert_called_once() + kwargs = mock_enqueue.call_args.kwargs['kwargs'] + assert kwargs['user_id'] == self.request.user.id + assert sorted(kwargs['object_ids']) == sorted([str(self.sequence.location), str(self.sequence2.location)]) + + def test_bulk_path_does_not_enqueue_when_no_rows_changed(self): + with mock_get_score(1, 2), patch(RECORD_PATH, return_value=0), patch(ENQUEUE_PATH) as mock_enqueue: + with self.captureOnCommitCallbacks(execute=True): + CreateSubsectionGrade.bulk_create_models( + self.request.user, [self._create_grade(self.sequence)], self.course.id, + ) + + mock_enqueue.assert_not_called() + + def test_bulk_path_does_not_enqueue_when_record_fails(self): + with mock_get_score(1, 2), patch(RECORD_PATH, side_effect=DatabaseError), patch(ENQUEUE_PATH) as mock_enqueue: + with self.captureOnCommitCallbacks(execute=True): + with pytest.raises(DatabaseError): + CreateSubsectionGrade.bulk_create_models( + self.request.user, [self._create_grade(self.sequence)], self.course.id, + ) + + mock_enqueue.assert_not_called() + + def test_bulk_path_enqueue_failure_does_not_skip_events_or_raise(self): + with mock_get_score(1, 2), patch(RECORD_PATH, return_value=1), patch(EMIT_PATH) as mock_emit: + with patch(ENQUEUE_PATH, side_effect=OSError('broker down')): + with self.captureOnCommitCallbacks(execute=True): + CreateSubsectionGrade.bulk_create_models( + self.request.user, [self._create_grade(self.sequence)], self.course.id, + ) + + mock_emit.assert_called_once() + + def test_bulk_path_enqueue_omits_zero_possible_subsections(self): + with mock_get_score(1, 2): + scored = self._create_grade(self.sequence) + with mock_get_score(0, 0): + unscorable = self._create_grade(self.sequence2) + + with patch(RECORD_PATH, return_value=1), patch(ENQUEUE_PATH) as mock_enqueue: + with self.captureOnCommitCallbacks(execute=True): + CreateSubsectionGrade.bulk_create_models( + self.request.user, [scored, unscorable], self.course.id, + ) + + assert mock_enqueue.call_args.kwargs['kwargs']['object_ids'] == [str(self.sequence.location)] + + def test_bulk_path_empty_input_returns_none_without_transaction_or_record(self): + with patch(RECORD_PATH) as mock_record, patch( + 'lms.djangoapps.grades.subsection_grade.transaction.atomic' + ) as mock_atomic: + assert CreateSubsectionGrade.bulk_create_models(self.request.user, [], self.course.id) is None + + mock_record.assert_not_called() + mock_atomic.assert_not_called() + + @data((3, 2, '1'), (-1, 2, '0'), (0.7, 1.0, '0.7'), (1, 4, '0.25')) + @unpack + def test_fraction_is_clamped_and_exact(self, earned, possible, expected): + with mock_get_score(earned, possible): + score = self._create_grade().graded_object_score() + + assert score.fraction == Decimal(expected) + + def test_fraction_absorbs_summation_noise(self): + with mock_get_score(1, 2): + grade = self._create_grade() + noisy_sum = 0.0 + for _ in range(8): + noisy_sum += 0.1 + assert noisy_sum != 0.8 + grade.graded_total = AggregatedScore( + noisy_sum, 1.0, True, first_attempted=datetime(2000, 1, 1, tzinfo=UTC), + ) + + fraction = grade.graded_object_score().fraction + + assert fraction == Decimal('0.8') + assert fraction >= Decimal('0.8') + + +class SubsectionGradeEventTest(SubsectionGradeFixtureMixin, GradeTestBase): + """ + Tests that persisting subsection grades through CreateSubsectionGrade emits the grade calculated event. + """ + + def test_update_or_create_model_emits_event(self): + with mock_get_score(1, 2), patch('lms.djangoapps.grades.events.tracker') as tracker_mock: + model = self._create_grade().update_or_create_model(self.request.user) + + self._assert_tracker_emitted_event(tracker_mock, model) + + def test_bulk_create_models_emits_event_for_each_grade(self): + with mock_get_score(1, 2), patch('lms.djangoapps.grades.events.tracker') as tracker_mock: + models = CreateSubsectionGrade.bulk_create_models( + self.request.user, + [self._create_grade(self.sequence), self._create_grade(self.sequence2)], + self.course.id, + ) + + assert len(models) == 2 + for model in models: + self._assert_tracker_emitted_event(tracker_mock, model) + + def _assert_tracker_emitted_event(self, tracker_mock, grade): + """ + Ensures the mocked event tracker was called with the expected info based on the passed grade. + """ + tracker_mock.emit.assert_any_call( + 'edx.grades.subsection.grade_calculated', + { + 'user_id': str(grade.user_id), + 'course_id': str(grade.course_id), + 'block_id': str(grade.usage_key), + 'course_version': str(grade.course_version), + 'weighted_total_earned': grade.earned_all, + 'weighted_total_possible': grade.possible_all, + 'weighted_graded_earned': grade.earned_graded, + 'weighted_graded_possible': grade.possible_graded, + 'first_attempted': str(grade.first_attempted), + 'subtree_edited_timestamp': str(grade.subtree_edited_timestamp), + 'event_transaction_id': str(get_event_transaction_id()), + 'event_transaction_type': str(get_event_transaction_type()), + 'visible_blocks_hash': str(grade.visible_blocks_id), + } + ) + + +@override_settings(ENABLE_COMPETENCY_MASTERY_TRACKING=True) +class SubsectionGradeCompetencyIntegrationTest(SubsectionGradeFixtureMixin, GradeTestBase): + """ + Grades subsections against real competency criteria and checks the learner statuses stored with the grade. + """ + + def setUp(self): + super().setUp() + # The test database is built without migrations, so the seeded status rows are created here. + for status in MasteryStatus: + CompetencyMasteryStatus.objects.get_or_create(id=status.value, defaults={'status': status.label}) + + def _add_criterion(self, subsection, threshold=0.75): + """ + Tags the subsection the way Studio does (object_id is the usage key string) and attaches a criterion to it. + """ + block_id = subsection.location.block_id + taxonomy = CompetencyTaxonomy.objects.create(name=f'Taxonomy {block_id}', export_id=f'taxonomy-{block_id}') + tag = Tag.objects.create(taxonomy=taxonomy, value=f'Competency {block_id}') + object_tag = ObjectTag.objects.create(object_id=str(subsection.location), taxonomy=taxonomy, tag=tag) + profile = CompetencyRuleProfile.objects.create( + competency_taxonomy=taxonomy, + rule_type=RuleType.GRADE, + rule_payload={'op': 'gte', 'value': threshold, 'scale': 'percent'}, + ) + return CompetencyCriterion.objects.create( + group=CompetencyCriteriaGroup.objects.create(tag=tag), object_tag=object_tag, rule_profile=profile, + ) + + def _status(self, criterion): + """ + Returns the learner's stored status id for the criterion. + """ + return StudentCompetencyCriteriaStatus.objects.get(user=self.request.user, criterion=criterion).status_id + + def _grade(self, earned, possible, subsection=None): + """ + Persists the subsection grade through the factory with the given graded score. + """ + with mock_get_score(earned, possible): + self.subsection_grade_factory.update(subsection or self.sequence) + + def _override_to_pass(self): + """ + Creates a staff override that sets the first sequence's graded score to 0.8. + """ + model = PersistentSubsectionGrade.read_grade(self.request.user.id, self.sequence.location) + PersistentSubsectionGradeOverride.update_or_create_override( + UserFactory(), model, earned_graded_override=4.0, possible_graded_override=5.0, + ) + + def test_grade_meeting_threshold_demonstrates_criterion(self): + criterion = self._add_criterion(self.sequence) + + self._grade(4, 5) + + assert self._status(criterion) == MasteryStatus.DEMONSTRATED + + def test_grade_below_threshold_is_attempted_not_demonstrated(self): + criterion = self._add_criterion(self.sequence) + + self._grade(1, 4) + + assert self._status(criterion) == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + + def test_staff_override_raising_failing_score_demonstrates_criterion(self): + criterion = self._add_criterion(self.sequence) + self._grade(1, 4) + assert self._status(criterion) == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + + self._override_to_pass() + self._grade(1, 4) + + assert self._status(criterion) == MasteryStatus.DEMONSTRATED + + def test_withdrawn_override_keeps_criterion_demonstrated(self): + criterion = self._add_criterion(self.sequence) + self._grade(1, 4) + self._override_to_pass() + self._grade(1, 4) + + PersistentSubsectionGradeOverride.objects.all().delete() + self._grade(1, 4) + + assert self._status(criterion) == MasteryStatus.DEMONSTRATED + + def test_subsection_without_criteria_writes_no_status(self): + self._grade(4, 5) + + assert PersistentSubsectionGrade.objects.count() == 1 + assert not StudentCompetencyCriteriaStatus.objects.exists() + + def test_status_write_failure_leaves_no_grade(self): + self._add_criterion(self.sequence) + + with mock_get_score(4, 5), patch.object(StudentCompetencyCriteriaStatus, 'save', side_effect=DatabaseError): + with pytest.raises(DatabaseError): + self.subsection_grade_factory.update(self.sequence) + + assert not PersistentSubsectionGrade.objects.exists() + + def test_each_updated_subsection_records_its_own_status(self): + criterion = self._add_criterion(self.sequence) + criterion2 = self._add_criterion(self.sequence2) + + with mock_get_score(4, 5): + for subsection in (self.sequence, self.sequence2): + self.subsection_grade_factory.update(subsection) + + assert self._status(criterion) == MasteryStatus.DEMONSTRATED + assert self._status(criterion2) == MasteryStatus.DEMONSTRATED + + def test_factory_create_records_status(self): + criterion = self._add_criterion(self.sequence) + + with mock_get_score(4, 5): + SubsectionGradeFactory(self.request.user, self.course, self.course_structure).create( + self.sequence, force_calculate=True, + ) + + assert self._status(criterion) == MasteryStatus.DEMONSTRATED + + def test_bulk_create_unsaved_records_status(self): + criterion = self._add_criterion(self.sequence) + + with mock_get_score(4, 5): + self.subsection_grade_factory.create(self.sequence, read_only=True, force_calculate=True) + self.subsection_grade_factory.bulk_create_unsaved() + + assert self._status(criterion) == MasteryStatus.DEMONSTRATED diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 18e26d02a294..7de65624b5df 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -12,6 +12,7 @@ import ddt import pytz from django.db.utils import IntegrityError +from django.test import TestCase from django.utils import timezone from edx_toggles.toggles.testutils import override_waffle_flag from stevedore.extension import Extension, ExtensionManager @@ -33,6 +34,7 @@ compute_grades_for_course, compute_grades_for_course_v2, recalculate_subsection_grade_v3, + roll_up_competency_statuses_for_user, ) from openedx.core.djangoapps.content.block_structure.exceptions import BlockStructureNotFound from xmodule.modulestore import ModuleStoreEnum @@ -156,8 +158,8 @@ def test_block_structure_created_only_once(self): assert mock_block_structure_create.call_count == 1 @ddt.data( - (ModuleStoreEnum.Type.split, 1, 47, True), - (ModuleStoreEnum.Type.split, 1, 47, False), + (ModuleStoreEnum.Type.split, 1, 49, True), + (ModuleStoreEnum.Type.split, 1, 49, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -168,7 +170,7 @@ def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, creat self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.split, 1, 47), + (ModuleStoreEnum.Type.split, 1, 49), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -256,7 +258,7 @@ def test_problem_block_with_restricted_access(self, mock_subsection_signal): UserPartition.scheme_extensions = None @ddt.data( - (ModuleStoreEnum.Type.split, 1, 47), + (ModuleStoreEnum.Type.split, 1, 49), ) @ddt.unpack def test_persistent_grades_on_course(self, default_store, num_mongo_queries, num_sql_queries): @@ -412,6 +414,40 @@ def _assert_retry_not_called(self, mock_retry): assert not mock_retry.called +class RollUpCompetencyStatusesForUserTest(TestCase): + """ + Tests for the task that rolls up competency statuses after a graded write. + """ + + def setUp(self): + super().setUp() + self.task_kwargs = {'user_id': 7, 'object_ids': ['block-v1:org+course+run+type@sequential+block@abc']} + + @patch('lms.djangoapps.grades.tasks.roll_up_competency_statuses') + def test_delegates_to_core_with_exact_kwargs(self, mock_roll_up): + result = roll_up_competency_statuses_for_user.apply(kwargs=self.task_kwargs) + + assert result.successful() + mock_roll_up.assert_called_once_with(**self.task_kwargs) + + @patch('lms.djangoapps.grades.tasks.roll_up_competency_statuses_for_user.retry') + @patch('lms.djangoapps.grades.tasks.roll_up_competency_statuses', new=MagicMock(side_effect=IntegrityError("WHAMMY"))) + def test_retries_with_same_kwargs_on_known_error(self, mock_retry): + roll_up_competency_statuses_for_user.apply(kwargs=self.task_kwargs) + + mock_retry.assert_called_once() + assert mock_retry.call_args.kwargs['kwargs'] == self.task_kwargs + assert isinstance(mock_retry.call_args.kwargs['exc'], IntegrityError) + + @patch('lms.djangoapps.grades.tasks.roll_up_competency_statuses_for_user.retry') + @patch('lms.djangoapps.grades.tasks.roll_up_competency_statuses', new=MagicMock(side_effect=ValueError("not retryable"))) + def test_reraises_unknown_error_without_retry(self, mock_retry): + result = roll_up_competency_statuses_for_user.apply(kwargs=self.task_kwargs) + + assert isinstance(result.result, ValueError) + mock_retry.assert_not_called() + + @ddt.ddt class ComputeGradesForCourseTest(HasCourseWithProblemsMixin, ModuleStoreTestCase): """ diff --git a/lms/envs/common.py b/lms/envs/common.py index ecfa2f2fc512..8ed2504f3e3e 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -673,6 +673,17 @@ # .. toggle_tickets: https://openedx.atlassian.net/browse/ENT-3818 ENABLE_COURSE_ASSESSMENT_GRADE_CHANGE_SIGNAL = False +# .. toggle_name: settings.ENABLE_COMPETENCY_MASTERY_TRACKING +# .. toggle_implementation: DjangoSetting +# .. toggle_default: False +# .. toggle_description: Set to True to update a learner's competency criterion status in the same transaction that +# persists a subsection grade. Also a safety valve: turning it off stops competency code from running on grade writes. +# .. toggle_warning: When enabled, a competency storage failure makes the grade write fail, and each grade write that +# changes a status queues a roll-up task. +# .. toggle_use_cases: opt_in +# .. toggle_creation_date: 2026-10-01 +ENABLE_COMPETENCY_MASTERY_TRACKING = False + # .. toggle_name: settings.ALLOW_ADMIN_ENTERPRISE_COURSE_ENROLLMENT_DELETION # .. toggle_implementation: DjangoSetting # .. toggle_default: False diff --git a/lms/envs/production.py b/lms/envs/production.py index 24350824c391..b9b033fc1192 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -478,6 +478,8 @@ def get_env_setting(setting): 'queue': POLICY_CHANGE_GRADES_ROUTING_KEY}, # noqa: F405 'lms.djangoapps.grades.tasks.recalculate_subsection_grade_v3': { 'queue': SINGLE_LEARNER_COURSE_REGRADE_ROUTING_KEY}, # noqa: F405 + 'lms.djangoapps.grades.tasks.roll_up_competency_statuses_for_user': { + 'queue': SINGLE_LEARNER_COURSE_REGRADE_ROUTING_KEY}, # noqa: F405 'openedx.core.djangoapps.programs.tasks.award_program_certificates': { 'queue': PROGRAM_CERTIFICATES_ROUTING_KEY}, # noqa: F405 'openedx.core.djangoapps.programs.tasks.revoke_program_certificates': {