Refactor how grade override values are persisted.
This commit is contained in:
committed by
Alex Dusenbery
parent
f7d10b603a
commit
577315f2a7
@@ -2,7 +2,6 @@
|
||||
Grades API URLs.
|
||||
"""
|
||||
|
||||
from django.conf import settings
|
||||
from django.conf.urls import include, url
|
||||
|
||||
|
||||
|
||||
@@ -32,7 +32,7 @@ from lms.djangoapps.grades.models import (
|
||||
PersistentCourseGrade,
|
||||
PersistentSubsectionGrade,
|
||||
PersistentSubsectionGradeOverride,
|
||||
PersistentSubsectionGradeOverrideHistory
|
||||
PersistentSubsectionGradeOverrideHistory,
|
||||
)
|
||||
from lms.djangoapps.grades.subsection_grade import CreateSubsectionGrade
|
||||
from lms.djangoapps.grades.tasks import are_grades_frozen, recalculate_subsection_grade_v3
|
||||
@@ -731,16 +731,11 @@ class GradebookBulkUpdateView(GradeViewMixin, PaginatedAPIView):
|
||||
Helper method to create a `PersistentSubsectionGradeOverride` object
|
||||
and send a `SUBSECTION_OVERRIDE_CHANGED` signal.
|
||||
"""
|
||||
override, _ = PersistentSubsectionGradeOverride.objects.update_or_create(
|
||||
grade=subsection_grade_model,
|
||||
defaults=self._clean_override_data(override_data),
|
||||
)
|
||||
|
||||
_ = PersistentSubsectionGradeOverrideHistory.objects.create(
|
||||
override_id=override.id,
|
||||
user=request_user,
|
||||
override = PersistentSubsectionGradeOverride.update_or_create_override(
|
||||
requesting_user=request_user,
|
||||
subsection_grade_model=subsection_grade_model,
|
||||
feature=PersistentSubsectionGradeOverrideHistory.GRADEBOOK,
|
||||
action=PersistentSubsectionGradeOverrideHistory.CREATE_OR_UPDATE,
|
||||
**override_data
|
||||
)
|
||||
|
||||
set_event_transaction_type(SUBSECTION_GRADE_CALCULATED)
|
||||
@@ -765,23 +760,6 @@ class GradebookBulkUpdateView(GradeViewMixin, PaginatedAPIView):
|
||||
subsection_grade_calculated(subsection_grade_model)
|
||||
return override
|
||||
|
||||
def _clean_override_data(self, override_data):
|
||||
"""
|
||||
Helper method to strip any grade override field names that won't work
|
||||
as defaults when calling PersistentSubsectionGradeOverride.update_or_create().
|
||||
"""
|
||||
allowed_fields = {
|
||||
'earned_all_override',
|
||||
'possible_all_override',
|
||||
'earned_graded_override',
|
||||
'possible_graded_override',
|
||||
}
|
||||
stripped_data = {}
|
||||
for field in override_data.keys():
|
||||
if field in allowed_fields:
|
||||
stripped_data[field] = override_data[field]
|
||||
return stripped_data
|
||||
|
||||
@staticmethod
|
||||
def _log_update_result(
|
||||
request_user,
|
||||
|
||||
@@ -385,6 +385,8 @@ class GradebookViewTest(GradebookViewTestBase):
|
||||
Helper function to mock a subsection grade.
|
||||
"""
|
||||
model = MagicMock(**kwargs)
|
||||
if 'override' not in kwargs:
|
||||
del model.override
|
||||
factory = MagicMock()
|
||||
return ReadSubsectionGrade(subsection, model, factory)
|
||||
|
||||
@@ -623,42 +625,48 @@ class GradebookViewTest(GradebookViewTestBase):
|
||||
with patch('lms.djangoapps.grades.course_grade_factory.CourseGradeFactory.read') as mock_grade:
|
||||
course_grade = self.mock_course_grade(self.student, passed=True, letter_grade='A', percent=0.85)
|
||||
|
||||
mock_override = MagicMock(
|
||||
earned_all_override=1.0,
|
||||
possible_all_override=2.0,
|
||||
earned_graded_override=1.0,
|
||||
possible_graded_override=2.0,
|
||||
)
|
||||
mock_subsection_grades = {
|
||||
self.subsections[self.chapter_1.location][0].location: self.mock_subsection_grade(
|
||||
self.subsections[self.chapter_1.location][0],
|
||||
earned_all=1.0,
|
||||
possible_all=2.0,
|
||||
earned_graded=1.0,
|
||||
earned_graded=2.0,
|
||||
possible_graded=2.0,
|
||||
first_attempted=None,
|
||||
override=MagicMock(),
|
||||
override=mock_override,
|
||||
),
|
||||
self.subsections[self.chapter_1.location][1].location: self.mock_subsection_grade(
|
||||
self.subsections[self.chapter_1.location][1],
|
||||
earned_all=1.0,
|
||||
possible_all=2.0,
|
||||
earned_graded=1.0,
|
||||
earned_graded=2.0,
|
||||
possible_graded=2.0,
|
||||
first_attempted=None,
|
||||
override=MagicMock(),
|
||||
override=mock_override,
|
||||
),
|
||||
self.subsections[self.chapter_2.location][0].location: self.mock_subsection_grade(
|
||||
self.subsections[self.chapter_2.location][0],
|
||||
earned_all=1.0,
|
||||
possible_all=2.0,
|
||||
earned_graded=1.0,
|
||||
earned_graded=2.0,
|
||||
possible_graded=2.0,
|
||||
first_attempted=None,
|
||||
override=MagicMock(),
|
||||
override=mock_override,
|
||||
),
|
||||
self.subsections[self.chapter_2.location][1].location: self.mock_subsection_grade(
|
||||
self.subsections[self.chapter_2.location][1],
|
||||
earned_all=1.0,
|
||||
possible_all=2.0,
|
||||
earned_graded=1.0,
|
||||
earned_graded=2.0,
|
||||
possible_graded=2.0,
|
||||
first_attempted=None,
|
||||
override=MagicMock(),
|
||||
override=mock_override,
|
||||
),
|
||||
}
|
||||
course_grade.subsection_grade = lambda key: mock_subsection_grades[key]
|
||||
@@ -1163,20 +1171,31 @@ class GradebookBulkUpdateViewTest(GradebookViewTestBase):
|
||||
|
||||
# We should now have PersistentSubsectionGradeOverride records corresponding to
|
||||
# our bulk-update request, and PersistentSubsectionGrade records with grade values
|
||||
# equal to those of the override.
|
||||
for usage_key, expected_grades in (
|
||||
(self.subsections[self.chapter_1.location][0].location, GradeFields(3, 3, 2, 2)),
|
||||
(self.subsections[self.chapter_1.location][1].location, GradeFields(3, 4, 3, 4)),
|
||||
# equal to the aggregate of their problem scores (in this case, zeros, since we
|
||||
# didn't mock out CourseGradeFactory.read() to return a non-zero score for anything).
|
||||
for usage_key, expected_grades, expected_grade_overrides in (
|
||||
(
|
||||
self.subsections[self.chapter_1.location][0].location,
|
||||
GradeFields(0, 0, 0, 0),
|
||||
GradeFields(3, 3, 2, 2)
|
||||
),
|
||||
(
|
||||
self.subsections[self.chapter_1.location][1].location,
|
||||
GradeFields(0, 0, 0, 0),
|
||||
GradeFields(3, 4, 3, 4)
|
||||
),
|
||||
):
|
||||
# this selects related PersistentSubsectionGradeOverride objects.
|
||||
grade = PersistentSubsectionGrade.read_grade(
|
||||
user_id=self.student.id,
|
||||
usage_key=usage_key,
|
||||
)
|
||||
for field_name in expected_grade_overrides._fields:
|
||||
expected_value = getattr(expected_grade_overrides, field_name)
|
||||
self.assertEqual(expected_value, getattr(grade.override, field_name + '_override'))
|
||||
for field_name in expected_grades._fields:
|
||||
expected_value = getattr(expected_grades, field_name)
|
||||
self.assertEqual(expected_value, getattr(grade, field_name))
|
||||
self.assertEqual(expected_value, getattr(grade.override, field_name + '_override'))
|
||||
|
||||
update_records = PersistentSubsectionGradeOverrideHistory.objects.filter(user=request_user)
|
||||
self.assertEqual(update_records.count(), 3)
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
1. Gradebook read and write APIs
|
||||
--------------------------------
|
||||
Gradebook read and write APIs
|
||||
-----------------------------
|
||||
|
||||
Status
|
||||
======
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
2. Gradebook User Interface and Experience
|
||||
------------------------------------------
|
||||
Gradebook User Interface and Experience
|
||||
---------------------------------------
|
||||
|
||||
Status
|
||||
======
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
Persistent Subsection and Course Grades
|
||||
---------------------------------------
|
||||
|
||||
Status
|
||||
======
|
||||
|
||||
Accepted (circa June 2017)
|
||||
|
||||
Context
|
||||
=======
|
||||
|
||||
There are several reasons that we need to persist a learner's computed grade in the database,
|
||||
both at the subsection-level, course-level and problem-level. Prior to the introduction
|
||||
of persistent subsection and course grades, only problem-level grades were persisted, with
|
||||
higher-level grades computed dynamically from these problem grades.
|
||||
|
||||
Decisions
|
||||
=========
|
||||
|
||||
More background and the decisions around persistent subsection and course grades
|
||||
are captured in the `Robust Grades Document
|
||||
<https://openedx.atlassian.net/wiki/spaces/EDUCATOR/pages/95912121/Robust+Grades+Design>`_.
|
||||
@@ -0,0 +1,33 @@
|
||||
Persistent Subsection Grade Overrides
|
||||
-------------------------------------
|
||||
|
||||
New decisions have been made about subsection grade overrides, see 0006-overrides-applied-separately_.
|
||||
|
||||
.. _0006-overrides-applied-separately: 0006-overrides-applied-separately.rst
|
||||
|
||||
Status
|
||||
======
|
||||
|
||||
Accepted (circa Summer 2017)
|
||||
|
||||
Superseded (January 2019)
|
||||
|
||||
Context
|
||||
=======
|
||||
|
||||
It is sometimes necessary to override a learner's score for an entire subsection/assignment.
|
||||
For example, for a graded and proctored exam, we may wish to automatically fail a learner's
|
||||
attempt on the exam with a score of zero if the proctor indicates suspicious behavior
|
||||
on the part of the learner.
|
||||
|
||||
Decisions
|
||||
=========
|
||||
|
||||
* We want to allow for the automatic or manual creation of overrides at the subsection-level.
|
||||
* We'll create a class called ``PersistentSubsectionGradeOverride`` to store the overridden scores, and
|
||||
an instance of this class will always be associated with an instance of ``PersistentSubsectionGrade``.
|
||||
* When updating a ``PersistentSubsectionGrade``, we'll persist the overridden scores from an associated
|
||||
``PersistentSubsectionGradeOverride`` (if one exists) instead of the computed value for the subsection grade.
|
||||
* No more than one ``PersistentSubsectionGradeOverride`` will ever exist for a given ``PersistentSubsectionGrade``.
|
||||
That is, write actions against ``PersistentSubsectionGradeOverride`` are always an update-or-create action,
|
||||
using the primary key of the ``PersistentSubsectionGrade`` to perform the ``SELECT ... FOR UPDATE`` query.
|
||||
37
lms/djangoapps/grades/docs/decisions/0005-grade-freeze.rst
Normal file
37
lms/djangoapps/grades/docs/decisions/0005-grade-freeze.rst
Normal file
@@ -0,0 +1,37 @@
|
||||
Freezing Changes to Subsection and Course Grades
|
||||
------------------------------------------------
|
||||
|
||||
Status
|
||||
======
|
||||
|
||||
Accepted (circa Summer 2018)
|
||||
|
||||
Context
|
||||
=======
|
||||
|
||||
We would like to prevent subsection-level and course-level grades from being
|
||||
updated in a course that has ended, after some fixed period of time. After that fixed
|
||||
period of time, we consider grades for that course "frozen" - they can no longer be changed
|
||||
by the system. This is important for maintaining the integrity of grades and a learner's completion of a course,
|
||||
particularly in credit-bearing courses.
|
||||
|
||||
Decisions
|
||||
=========
|
||||
|
||||
* In ``lms/djangoapps/grades/tasks.py``, we'll introduce an ``are_grades_frozen`` function
|
||||
to determine, for a given course key, whether subsection and course grades should now be
|
||||
frozen for that course.
|
||||
* The fixed period of time after course end at which grades will be frozen is 30 days.
|
||||
* We'll freeze grades after 30 days for all courses, unless course waffle flag override is
|
||||
enabled. An enabled override causes grades to not be frozen (after any amount of time)
|
||||
for that particular course.
|
||||
* Any grades celery task that can update grades will now check if grades are frozen
|
||||
before taking any action. If grades for the course are frozen, the task will simply
|
||||
return without taking any further action.
|
||||
|
||||
Consequences
|
||||
============
|
||||
|
||||
As a consequence of this decision, persistent grades will now be inconsistent with the
|
||||
values computed from input scores, course content, grading policy, etc. if changes
|
||||
to any of these occur after the frozen date.
|
||||
@@ -0,0 +1,39 @@
|
||||
Subsection Grade Override History and Refactoring
|
||||
-------------------------------------------------
|
||||
|
||||
For background on persistent subsection grade overrides, see 0004-persistent-grade-overrides_.
|
||||
|
||||
.. _0004-persistent-grade-overrides: 0004-persistent-grade-overrides.rst
|
||||
|
||||
Status
|
||||
======
|
||||
|
||||
Accepted (January 2019)
|
||||
|
||||
Context
|
||||
=======
|
||||
|
||||
We want to maintain the score values in the ``PersistentSubsectionGrade`` table as a
|
||||
source of truth as it relates to the learner's attempt at an assignment. This is necessary
|
||||
when, for instance, we want to see an "audit trail" of manually changed grades via the
|
||||
Gradebook feature. We'd like an instructor to see both the original grade as obtained
|
||||
by the student, as well as all changes made via ``PersistentSubsectionGradeOverrides``.
|
||||
Therefore, we need to make two changes:
|
||||
|
||||
* The existence of a ``PersistentSubsectionGradeOverride`` should no longer change the score values
|
||||
of the associated ``PersistentSubsectionGrade``.
|
||||
* We'll introduce a ``PersistentSubsectionGradeOverrideHistory`` table to track changes to grades
|
||||
made via grade overrides.
|
||||
|
||||
Decisions
|
||||
=========
|
||||
|
||||
* We'll introduce a ``PersistentSubsectionGradeOverrideHistory`` table. This table will track
|
||||
who created the override, the reason for the change (i.e. due to proctoring failure or a change
|
||||
via Gradebook), and any comments made by the overriding user.
|
||||
* As detailed in this `Jira issue <https://openedx.atlassian.net/browse/EDUCATOR-3835>`_, we'll
|
||||
refactor the grades override logic to no longer override the parameters of a ``PersistentSubsectionGrade``
|
||||
during updates where an associated ``PersistentSubsectionGradeOverride`` exists.
|
||||
* The ``SubsectionGrade`` classes should now account for any associated overrides and factor those
|
||||
into the subsection- and course-level grades when presenting grade data via a client
|
||||
(e.g. Gradebook, instructor grade reports, the Progress page).
|
||||
@@ -415,7 +415,6 @@ class PersistentSubsectionGrade(TimeStampedModel):
|
||||
cls._prepare_params(params)
|
||||
VisibleBlocks.cached_get_or_create(params['user_id'], params['visible_blocks'])
|
||||
cls._prepare_params_visible_blocks_id(params)
|
||||
cls._prepare_params_override(params)
|
||||
|
||||
# TODO: do we NEED to pop these?
|
||||
first_attempted = params.pop('first_attempted')
|
||||
@@ -428,6 +427,7 @@ class PersistentSubsectionGrade(TimeStampedModel):
|
||||
usage_key=usage_key,
|
||||
defaults=params,
|
||||
)
|
||||
grade.override = PersistentSubsectionGradeOverride.get_override(user_id, usage_key)
|
||||
if first_attempted is not None and grade.first_attempted is None:
|
||||
grade.first_attempted = first_attempted
|
||||
grade.save()
|
||||
@@ -450,7 +450,6 @@ class PersistentSubsectionGrade(TimeStampedModel):
|
||||
user_id, course_key, [params['visible_blocks'] for params in grade_params_iter]
|
||||
)
|
||||
map(cls._prepare_params_visible_blocks_id, grade_params_iter)
|
||||
map(cls._prepare_params_override, grade_params_iter)
|
||||
|
||||
grades = [PersistentSubsectionGrade(**params) for params in grade_params_iter]
|
||||
grades = cls.objects.bulk_create(grades)
|
||||
@@ -481,19 +480,6 @@ class PersistentSubsectionGrade(TimeStampedModel):
|
||||
params['visible_blocks_id'] = params['visible_blocks'].hash_value
|
||||
del params['visible_blocks']
|
||||
|
||||
@classmethod
|
||||
def _prepare_params_override(cls, params):
|
||||
override = PersistentSubsectionGradeOverride.get_override(params['user_id'], params['usage_key'])
|
||||
if override:
|
||||
if override.earned_all_override is not None:
|
||||
params['earned_all'] = override.earned_all_override
|
||||
if override.possible_all_override is not None:
|
||||
params['possible_all'] = override.possible_all_override
|
||||
if override.earned_graded_override is not None:
|
||||
params['earned_graded'] = override.earned_graded_override
|
||||
if override.possible_graded_override is not None:
|
||||
params['possible_graded'] = override.possible_graded_override
|
||||
|
||||
@staticmethod
|
||||
def _emit_grade_calculated_event(grade):
|
||||
events.subsection_grade_calculated(grade)
|
||||
@@ -690,6 +676,55 @@ class PersistentSubsectionGradeOverride(models.Model):
|
||||
except PersistentSubsectionGradeOverride.DoesNotExist:
|
||||
pass
|
||||
|
||||
@classmethod
|
||||
def update_or_create_override(
|
||||
cls, requesting_user, subsection_grade_model, feature=None, action=None, **override_data
|
||||
):
|
||||
"""
|
||||
Creates or updates an override object for the given PersistentSubsectionGrade.
|
||||
Args:
|
||||
requesting_user: The user that is creating the override (so we can record this action in
|
||||
a PersistentSubsectionGradeOverrideHistory record).
|
||||
subsection_grade_model: The PersistentSubsectionGrade object associated with this override.
|
||||
override_data: The parameters of score values used to create the override record.
|
||||
"""
|
||||
override, _ = PersistentSubsectionGradeOverride.objects.update_or_create(
|
||||
grade=subsection_grade_model,
|
||||
defaults=cls._prepare_override_params(subsection_grade_model, override_data),
|
||||
)
|
||||
|
||||
action = action or PersistentSubsectionGradeOverrideHistory.CREATE_OR_UPDATE
|
||||
|
||||
PersistentSubsectionGradeOverrideHistory.objects.create(
|
||||
override_id=override.id,
|
||||
user=requesting_user,
|
||||
feature=feature,
|
||||
action=action,
|
||||
)
|
||||
return override
|
||||
|
||||
@staticmethod
|
||||
def _prepare_override_params(subsection_grade_model, override_data):
|
||||
"""
|
||||
Helper method to strip any grade override field names that won't work
|
||||
as defaults when calling PersistentSubsectionGradeOverride.update_or_create(),
|
||||
and to use default values from the associated PersistentSubsectionGrade
|
||||
for any override fields that are not specified.
|
||||
"""
|
||||
allowed_fields_and_defaults = {
|
||||
'earned_all_override': 'earned_all',
|
||||
'possible_all_override': 'possible_all',
|
||||
'earned_graded_override': 'earned_graded',
|
||||
'possible_graded_override': 'possible_graded',
|
||||
}
|
||||
cleaned_data = {}
|
||||
for override_field_name, field_name in allowed_fields_and_defaults.items():
|
||||
cleaned_data[override_field_name] = override_data.get(
|
||||
override_field_name,
|
||||
getattr(subsection_grade_model, field_name)
|
||||
)
|
||||
return cleaned_data
|
||||
|
||||
|
||||
class PersistentSubsectionGradeOverrideHistory(models.Model):
|
||||
"""
|
||||
|
||||
@@ -4,6 +4,7 @@ Grade service
|
||||
from datetime import datetime
|
||||
|
||||
import pytz
|
||||
from six import text_type
|
||||
|
||||
from lms.djangoapps.utils import _get_key
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey
|
||||
@@ -46,17 +47,13 @@ class GradesService(object):
|
||||
|
||||
If override does not exist, returns None. If subsection grade does not exist, will raise an exception.
|
||||
"""
|
||||
course_key = _get_key(course_key_or_id, CourseKey)
|
||||
usage_key = _get_key(usage_key_or_id, UsageKey)
|
||||
|
||||
grade = self.get_subsection_grade(user_id, course_key, usage_key)
|
||||
# Verify that a corresponding subsection grade exists for the given user and usage_key
|
||||
# Raises PersistentSubsectionGrade.DoesNotExist if it does not exist.
|
||||
_ = self.get_subsection_grade(user_id, course_key_or_id, usage_key_or_id)
|
||||
|
||||
try:
|
||||
return PersistentSubsectionGradeOverride.objects.get(
|
||||
grade=grade
|
||||
)
|
||||
except PersistentSubsectionGradeOverride.DoesNotExist:
|
||||
return None
|
||||
return PersistentSubsectionGradeOverride.get_override(user_id, usage_key)
|
||||
|
||||
def override_subsection_grade(self, user_id, course_key_or_id, usage_key_or_id, earned_all=None,
|
||||
earned_graded=None):
|
||||
@@ -69,23 +66,17 @@ class GradesService(object):
|
||||
course_key = _get_key(course_key_or_id, CourseKey)
|
||||
usage_key = _get_key(usage_key_or_id, UsageKey)
|
||||
|
||||
grade = PersistentSubsectionGrade.objects.get(
|
||||
grade = PersistentSubsectionGrade.read_grade(
|
||||
user_id=user_id,
|
||||
course_id=course_key,
|
||||
usage_key=usage_key
|
||||
)
|
||||
|
||||
# Create override that will prevent any future updates to grade
|
||||
override, _ = PersistentSubsectionGradeOverride.objects.update_or_create(
|
||||
grade=grade,
|
||||
earned_all_override=earned_all,
|
||||
earned_graded_override=earned_graded
|
||||
)
|
||||
|
||||
_ = PersistentSubsectionGradeOverrideHistory.objects.create(
|
||||
override_id=override.id,
|
||||
override = PersistentSubsectionGradeOverride.update_or_create_override(
|
||||
requesting_user=None,
|
||||
subsection_grade_model=grade,
|
||||
feature=PersistentSubsectionGradeOverrideHistory.PROCTORING,
|
||||
action=PersistentSubsectionGradeOverrideHistory.CREATE_OR_UPDATE
|
||||
earned_all_override=earned_all,
|
||||
earned_graded_override=earned_graded,
|
||||
)
|
||||
|
||||
# Cache a new event id and event type which the signal handler will use to emit a tracking log event.
|
||||
@@ -97,8 +88,8 @@ class GradesService(object):
|
||||
SUBSECTION_OVERRIDE_CHANGED.send(
|
||||
sender=None,
|
||||
user_id=user_id,
|
||||
course_id=unicode(course_key),
|
||||
usage_id=unicode(usage_key),
|
||||
course_id=text_type(course_key),
|
||||
usage_id=text_type(usage_key),
|
||||
only_if_higher=False,
|
||||
modified=override.modified,
|
||||
score_deleted=False,
|
||||
@@ -139,8 +130,8 @@ class GradesService(object):
|
||||
SUBSECTION_OVERRIDE_CHANGED.send(
|
||||
sender=None,
|
||||
user_id=user_id,
|
||||
course_id=unicode(course_key),
|
||||
usage_id=unicode(usage_key),
|
||||
course_id=text_type(course_key),
|
||||
usage_id=text_type(usage_key),
|
||||
only_if_higher=False,
|
||||
modified=datetime.now().replace(tzinfo=pytz.UTC), # Not used when score_deleted=True
|
||||
score_deleted=True,
|
||||
|
||||
@@ -43,7 +43,7 @@ class SubsectionGradeBase(object):
|
||||
Returns whether any problem in this subsection
|
||||
was attempted by the student.
|
||||
"""
|
||||
|
||||
# pylint: disable=no-member
|
||||
assert self.all_total is not None, (
|
||||
"SubsectionGrade not fully populated yet. Call init_from_structure or init_from_model "
|
||||
"before use."
|
||||
@@ -182,24 +182,34 @@ class NonZeroSubsectionGrade(SubsectionGradeBase):
|
||||
block,
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _aggregated_score_from_model(grade_model, is_graded):
|
||||
"""
|
||||
Helper method that returns `AggregatedScore` objects based on
|
||||
the values in the given `grade_model`. If the given model
|
||||
has an associated override, the values from the override are
|
||||
used instead.
|
||||
"""
|
||||
score_type = 'graded' if is_graded else 'all'
|
||||
grade_object = grade_model
|
||||
if hasattr(grade_model, 'override'):
|
||||
score_type = 'graded_override' if is_graded else 'all_override'
|
||||
grade_object = grade_model.override
|
||||
return AggregatedScore(
|
||||
tw_earned=getattr(grade_object, 'earned_{}'.format(score_type)),
|
||||
tw_possible=getattr(grade_object, 'possible_{}'.format(score_type)),
|
||||
graded=is_graded,
|
||||
first_attempted=grade_model.first_attempted,
|
||||
)
|
||||
|
||||
|
||||
class ReadSubsectionGrade(NonZeroSubsectionGrade):
|
||||
"""
|
||||
Class for Subsection grades that are read from the database.
|
||||
"""
|
||||
def __init__(self, subsection, model, factory):
|
||||
all_total = AggregatedScore(
|
||||
tw_earned=model.earned_all,
|
||||
tw_possible=model.possible_all,
|
||||
graded=False,
|
||||
first_attempted=model.first_attempted,
|
||||
)
|
||||
graded_total = AggregatedScore(
|
||||
tw_earned=model.earned_graded,
|
||||
tw_possible=model.possible_graded,
|
||||
graded=True,
|
||||
first_attempted=model.first_attempted,
|
||||
)
|
||||
all_total = self._aggregated_score_from_model(model, is_graded=False)
|
||||
graded_total = self._aggregated_score_from_model(model, is_graded=True)
|
||||
override = model.override if hasattr(model, 'override') else None
|
||||
|
||||
# save these for later since we compute problem_scores lazily
|
||||
@@ -216,6 +226,7 @@ class ReadSubsectionGrade(NonZeroSubsectionGrade):
|
||||
It means we look through the user-specific subtree of this subsection,
|
||||
taking into account which problems are visible to the user.
|
||||
"""
|
||||
# pylint: disable=protected-access
|
||||
problem_scores = OrderedDict()
|
||||
for block in self.model.visible_blocks.blocks:
|
||||
problem_score = self._compute_block_score(
|
||||
@@ -254,29 +265,17 @@ class CreateSubsectionGrade(NonZeroSubsectionGrade):
|
||||
"""
|
||||
if self._should_persist_per_attempted(score_deleted, force_update_subsections):
|
||||
model = PersistentSubsectionGrade.update_or_create_grade(**self._persisted_model_params(student))
|
||||
self._update_aggregated_scores_from_model(model)
|
||||
|
||||
if hasattr(model, 'override'):
|
||||
# When we're doing an update operation, the PersistentSubsectionGrade model
|
||||
# will be updated based on the problem_scores, but if a grade override
|
||||
# exists that's related to the updated persistent grade, we need to update
|
||||
# the aggregated scores for this object to reflect the override.
|
||||
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
|
||||
|
||||
def _update_aggregated_scores_from_model(self, model):
|
||||
"""
|
||||
Updates this grade's `all_total` and `graded_total` attributes
|
||||
to reflect the values from the related persisted model.
|
||||
This is important, because PersistentSubsectionGradeOverrides
|
||||
are only taken into account when reading/writing at the persistence layer,
|
||||
so after we update a PersistentSubsectionGrade model (which could involve
|
||||
writing values that are overridden by the PersistentSubsectionGradeOverride model)
|
||||
we also need to update this grade object's attributes to reflect the
|
||||
now (possibly) overriden values.
|
||||
TODO: https://openedx.atlassian.net/browse/EDUCATOR-3835
|
||||
"""
|
||||
self.all_total.earned = model.earned_all
|
||||
self.all_total.possible = model.possible_all
|
||||
self.all_total.first_attempted = model.first_attempted
|
||||
|
||||
self.graded_total.earned = model.earned_graded
|
||||
self.graded_total.possible = model.possible_graded
|
||||
self.graded_total.first_attempted = model.first_attempted
|
||||
|
||||
@classmethod
|
||||
def bulk_create_models(cls, student, subsection_grades, course_key):
|
||||
"""
|
||||
|
||||
@@ -1,3 +1,6 @@
|
||||
"""
|
||||
Tests for the CourseGradeFactory class.
|
||||
"""
|
||||
import itertools
|
||||
|
||||
import ddt
|
||||
@@ -95,7 +98,7 @@ class TestCourseGradeFactory(GradeTestBase):
|
||||
with self.assertNumQueries(5), mock_get_score(1, 2):
|
||||
_assert_read(expected_pass=False, expected_percent=0) # start off with grade of 0
|
||||
|
||||
num_queries = 44
|
||||
num_queries = 47
|
||||
with self.assertNumQueries(num_queries), mock_get_score(1, 2):
|
||||
grade_factory.update(self.request.user, self.course, force_update_subsections=True)
|
||||
|
||||
@@ -109,14 +112,14 @@ class TestCourseGradeFactory(GradeTestBase):
|
||||
with self.assertNumQueries(5):
|
||||
_assert_read(expected_pass=True, expected_percent=0.5) # NOT updated to grade of .25
|
||||
|
||||
num_queries = 23
|
||||
num_queries = 26
|
||||
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(5):
|
||||
_assert_read(expected_pass=True, expected_percent=1.0) # updated to grade of 1.0
|
||||
|
||||
num_queries = 26
|
||||
num_queries = 29
|
||||
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)
|
||||
|
||||
|
||||
@@ -23,8 +23,10 @@ from lms.djangoapps.grades.models import (
|
||||
PersistentCourseGrade,
|
||||
PersistentSubsectionGrade,
|
||||
PersistentSubsectionGradeOverride,
|
||||
PersistentSubsectionGradeOverrideHistory,
|
||||
VisibleBlocks
|
||||
)
|
||||
from student.tests.factories import UserFactory
|
||||
from track.event_transaction_utils import get_event_transaction_id, get_event_transaction_type
|
||||
|
||||
|
||||
@@ -226,6 +228,7 @@ class PersistentSubsectionGradeTest(GradesModelTestCase):
|
||||
"visible_blocks": self.block_records,
|
||||
"first_attempted": datetime(2000, 1, 1, 12, 30, 45, tzinfo=pytz.UTC),
|
||||
}
|
||||
self.user = UserFactory(id=self.params['user_id'])
|
||||
|
||||
@ddt.data('course_version', 'subtree_edited_timestamp')
|
||||
def test_optional_fields(self, field):
|
||||
@@ -300,12 +303,31 @@ class PersistentSubsectionGradeTest(GradesModelTestCase):
|
||||
self._assert_tracker_emitted_event(tracker_mock, grade)
|
||||
|
||||
def test_grade_override(self):
|
||||
"""
|
||||
Creating a subsection grade override should NOT change the score values
|
||||
of the related PersistentSubsectionGrade.
|
||||
"""
|
||||
grade = PersistentSubsectionGrade.update_or_create_grade(**self.params)
|
||||
override = PersistentSubsectionGradeOverride(grade=grade, earned_all_override=0.0, earned_graded_override=0.0)
|
||||
override.save()
|
||||
override = PersistentSubsectionGradeOverride.update_or_create_override(
|
||||
requesting_user=self.user,
|
||||
subsection_grade_model=grade,
|
||||
earned_all_override=0.0,
|
||||
earned_graded_override=0.0,
|
||||
feature=PersistentSubsectionGradeOverrideHistory.GRADEBOOK,
|
||||
)
|
||||
|
||||
grade = PersistentSubsectionGrade.update_or_create_grade(**self.params)
|
||||
self.assertEqual(grade.earned_all, 0.0)
|
||||
self.assertEqual(grade.earned_graded, 0.0)
|
||||
self.assertEqual(self.params['earned_all'], grade.earned_all)
|
||||
self.assertEqual(self.params['earned_graded'], grade.earned_graded)
|
||||
|
||||
# Any score values that aren't specified should use the values from grade as defaults
|
||||
self.assertEqual(0, override.earned_all_override)
|
||||
self.assertEqual(0, override.earned_graded_override)
|
||||
self.assertEqual(grade.possible_all, override.possible_all_override)
|
||||
self.assertEqual(grade.possible_graded, override.possible_graded_override)
|
||||
|
||||
# An override history record should be created
|
||||
self.assertEqual(1, PersistentSubsectionGradeOverrideHistory.objects.filter(override_id=override.id).count())
|
||||
|
||||
def _assert_tracker_emitted_event(self, tracker_mock, grade):
|
||||
"""
|
||||
|
||||
@@ -24,6 +24,7 @@ class MockWaffleFlag(object):
|
||||
def __init__(self, state):
|
||||
self.state = state
|
||||
|
||||
# pylint: disable=unused-argument
|
||||
def is_enabled(self, course_key):
|
||||
return self.state
|
||||
|
||||
@@ -38,7 +39,7 @@ class GradesServiceTests(ModuleStoreTestCase):
|
||||
def setUp(self):
|
||||
super(GradesServiceTests, self).setUp()
|
||||
self.service = GradesService()
|
||||
self.course = CourseFactory.create(org='edX', number='DemoX', display_name='Demo_Course')
|
||||
self.course = CourseFactory.create(org='edX', number='DemoX', display_name='Demo_Course', run='Spring2019')
|
||||
self.subsection = ItemFactory.create(parent=self.course, category="subsection", display_name="Subsection")
|
||||
self.user = UserFactory()
|
||||
self.grade = PersistentSubsectionGrade.update_or_create_grade(
|
||||
@@ -123,11 +124,11 @@ class GradesServiceTests(ModuleStoreTestCase):
|
||||
}
|
||||
)
|
||||
|
||||
# test with id strings as parameters instead
|
||||
# test with course key parameter as string instead
|
||||
self.assertDictEqual(self.subsection_grade_override_to_dict(self.service.get_subsection_grade_override(
|
||||
user_id=self.user.id,
|
||||
course_key_or_id=unicode(self.course.id),
|
||||
usage_key_or_id=unicode(self.subsection.location)
|
||||
usage_key_or_id=self.subsection.location
|
||||
)), {
|
||||
'earned_all_override': override.earned_all_override,
|
||||
'earned_graded_override': override.earned_graded_override
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
"""
|
||||
Tests of the SubsectionGrade classes.
|
||||
"""
|
||||
from ddt import data, ddt, unpack
|
||||
|
||||
from ..models import PersistentSubsectionGrade
|
||||
from ..subsection_grade import CreateSubsectionGrade, ReadSubsectionGrade
|
||||
from .utils import mock_get_score
|
||||
from .base import GradeTestBase
|
||||
from ddt import data, ddt, unpack
|
||||
|
||||
|
||||
@ddt
|
||||
|
||||
@@ -2,12 +2,18 @@
|
||||
Tests for the SubsectionGradeFactory class.
|
||||
"""
|
||||
import ddt
|
||||
from courseware.tests.test_submitting_problems import ProblemSubmissionTestMixin
|
||||
from django.conf import settings
|
||||
from lms.djangoapps.grades.config.tests.utils import persistent_grades_feature_flags
|
||||
from mock import patch
|
||||
from django.conf import settings
|
||||
|
||||
from ..models import PersistentSubsectionGrade, PersistentSubsectionGradeOverride
|
||||
from courseware.tests.test_submitting_problems import ProblemSubmissionTestMixin
|
||||
from lms.djangoapps.grades.config.tests.utils import persistent_grades_feature_flags
|
||||
from student.tests.factories import UserFactory
|
||||
|
||||
from ..models import (
|
||||
PersistentSubsectionGrade,
|
||||
PersistentSubsectionGradeOverride,
|
||||
PersistentSubsectionGradeOverrideHistory,
|
||||
)
|
||||
from ..subsection_grade_factory import ZeroSubsectionGrade
|
||||
from .base import GradeTestBase
|
||||
from .utils import mock_get_score
|
||||
@@ -125,10 +131,12 @@ class TestSubsectionGradeFactory(ProblemSubmissionTestMixin, GradeTestBase):
|
||||
self.assertEqual(3, persistent_grade.possible_graded)
|
||||
|
||||
# Now create the override
|
||||
PersistentSubsectionGradeOverride.objects.create(
|
||||
grade=persistent_grade,
|
||||
PersistentSubsectionGradeOverride.update_or_create_override(
|
||||
UserFactory(),
|
||||
persistent_grade,
|
||||
earned_graded_override=0,
|
||||
earned_all_override=0,
|
||||
feature=PersistentSubsectionGradeOverrideHistory.GRADEBOOK,
|
||||
)
|
||||
|
||||
# Now, even if the problem scores interface gives us a 2/3,
|
||||
|
||||
@@ -176,10 +176,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
|
||||
self.assertEquals(mock_block_structure_create.call_count, 1)
|
||||
|
||||
@ddt.data(
|
||||
(ModuleStoreEnum.Type.mongo, 1, 34, True),
|
||||
(ModuleStoreEnum.Type.mongo, 1, 34, False),
|
||||
(ModuleStoreEnum.Type.split, 3, 34, True),
|
||||
(ModuleStoreEnum.Type.split, 3, 34, False),
|
||||
(ModuleStoreEnum.Type.mongo, 1, 35, True),
|
||||
(ModuleStoreEnum.Type.mongo, 1, 35, False),
|
||||
(ModuleStoreEnum.Type.split, 3, 35, True),
|
||||
(ModuleStoreEnum.Type.split, 3, 35, False),
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections):
|
||||
@@ -191,8 +191,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
|
||||
self._apply_recalculate_subsection_grade()
|
||||
|
||||
@ddt.data(
|
||||
(ModuleStoreEnum.Type.mongo, 1, 34),
|
||||
(ModuleStoreEnum.Type.split, 3, 34),
|
||||
(ModuleStoreEnum.Type.mongo, 1, 35),
|
||||
(ModuleStoreEnum.Type.split, 3, 35),
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls):
|
||||
@@ -252,8 +252,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
|
||||
self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0)
|
||||
|
||||
@ddt.data(
|
||||
(ModuleStoreEnum.Type.mongo, 1, 35),
|
||||
(ModuleStoreEnum.Type.split, 3, 35),
|
||||
(ModuleStoreEnum.Type.mongo, 1, 36),
|
||||
(ModuleStoreEnum.Type.split, 3, 36),
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries):
|
||||
|
||||
Reference in New Issue
Block a user