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

EDUCATOR-5039 (#24546)

Browse Source 
* EDUCATOR-5039

* EDUCATOR-5039 - pr updates
This commit is contained in:
Andytr1
2020-07-21 13:36:18 -04:00
committed by GitHub
parent 9d62bf16b2
commit af072a5158
3 changed files with 194 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

87
cms/djangoapps/contentstore/tests/test_course_settings.py
View File

@@ -26,6 +26,7 @@ from course_modes.models import CourseMode
from models.settings.course_grading import GRADING_POLICY_CHANGED_EVENT_TYPE, CourseGradingModel, hash_grading_policy
from models.settings.course_metadata import CourseMetadata
from models.settings.encoder import CourseSettingsEncoder
from models.settings.waffle import MATERIAL_RECOMPUTE_ONLY_FLAG
from openedx.core.djangoapps.models.course_details import CourseDetails
from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag
from student.roles import CourseInstructorRole, CourseStaffRole
@@ -518,9 +519,9 @@ class CourseGradingTest(CourseTestCase):
uuid.return_value = "mockUUID"
self.course = CourseFactory.create(default_store=store)
test_grader = CourseGradingModel.fetch(self.course.id)
# there should be no event raised after this call, since nothing got modified
altered_grader = CourseGradingModel.update_from_json(self.course.id, test_grader.__dict__, self.user)
self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "Noop update")
grading_policy_1 = self._grading_policy_hash_for_course()
test_grader.graders[0]['weight'] = test_grader.graders[0].get('weight') * 2
altered_grader = CourseGradingModel.update_from_json(self.course.id, test_grader.__dict__, self.user)
self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "Weight[0] * 2")
@@ -548,11 +549,9 @@ class CourseGradingTest(CourseTestCase):
# one for each of the calls to update_from_json()
send_signal.assert_has_calls([
# pylint: disable=line-too-long
mock.call(sender=CourseGradingModel, user_id=self.user.id, course_key=self.course.id, grading_policy_hash=grading_policy_1),
mock.call(sender=CourseGradingModel, user_id=self.user.id, course_key=self.course.id, grading_policy_hash=grading_policy_2),
mock.call(sender=CourseGradingModel, user_id=self.user.id, course_key=self.course.id, grading_policy_hash=grading_policy_3),
mock.call(sender=CourseGradingModel, user_id=self.user.id, course_key=self.course.id, grading_policy_hash=grading_policy_4),
mock.call(sender=CourseGradingModel, user_id=self.user.id, course_key=self.course.id, grading_policy_hash=grading_policy_4),
# pylint: enable=line-too-long
])
@@ -569,10 +568,86 @@ class CourseGradingTest(CourseTestCase):
'event_transaction_id': 'mockUUID',
}
) for policy_hash in (
grading_policy_1, grading_policy_2, grading_policy_3, grading_policy_4, grading_policy_4
grading_policy_2, grading_policy_3, grading_policy_4
)
])
@ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
def test_must_fire_grading_event_and_signal_multiple_type(self, store):
"""
Verifies that 'must_fire_grading_event_and_signal' ignores (returns False) if we modify
short_label and or name
use test_must_fire_grading_event_and_signal_multiple_type_2_split to run this test only
"""
self.course = CourseFactory.create(default_store=store)
# .raw_grader approximates what our UI sends down. It uses decimal representation of percent
# without it, the weights would be percentages
raw_grader_list = modulestore().get_course(self.course.id).raw_grader
course_grading_model = CourseGradingModel.fetch(self.course.id)
raw_grader_list[0]['type'] += '_foo'
raw_grader_list[0]['short_label'] += '_foo'
raw_grader_list[2]['type'] += '_foo'
raw_grader_list[3]['type'] += '_foo'
result = CourseGradingModel.must_fire_grading_event_and_signal(
self.course.id,
raw_grader_list,
modulestore().get_course(self.course.id),
course_grading_model.__dict__
)
self.assertTrue(result)
@override_waffle_flag(MATERIAL_RECOMPUTE_ONLY_FLAG, True)
@ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
def test_must_fire_grading_event_and_signal_multiple_type_waffle_on(self, store):
"""
Verifies that 'must_fire_grading_event_and_signal' ignores (returns False) if we modify
short_label and or name
use test_must_fire_grading_event_and_signal_multiple_type_2_split to run this test only
"""
self.course = CourseFactory.create(default_store=store)
# .raw_grader approximates what our UI sends down. It uses decimal representation of percent
# without it, the weights would be percentages
raw_grader_list = modulestore().get_course(self.course.id).raw_grader
course_grading_model = CourseGradingModel.fetch(self.course.id)
raw_grader_list[0]['type'] += '_foo'
raw_grader_list[0]['short_label'] += '_foo'
raw_grader_list[2]['type'] += '_foo'
raw_grader_list[3]['type'] += '_foo'
result = CourseGradingModel.must_fire_grading_event_and_signal(
self.course.id,
raw_grader_list,
modulestore().get_course(self.course.id),
course_grading_model.__dict__
)
self.assertFalse(result)
@ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
def test_must_fire_grading_event_and_signal_return_true(self, store):
"""
Verifies that 'must_fire_grading_event_and_signal' ignores (returns False) if we modify
short_label and or name
use _2_split suffix to run this test only
"""
self.course = CourseFactory.create(default_store=store)
# .raw_grader approximates what our UI sends down. It uses decimal representation of percent
# without it, the weights would be percentages
raw_grader_list = modulestore().get_course(self.course.id).raw_grader
course_grading_model = CourseGradingModel.fetch(self.course.id)
raw_grader_list[0]['weight'] *= 2
raw_grader_list[0]['short_label'] += '_foo'
raw_grader_list[2]['type'] += '_foo'
raw_grader_list[3]['type'] += '_foo'
result = CourseGradingModel.must_fire_grading_event_and_signal(
self.course.id,
raw_grader_list,
modulestore().get_course(self.course.id),
course_grading_model.__dict__
)
self.assertTrue(result)
@mock.patch('track.event_transaction_utils.uuid4')
@mock.patch('models.settings.course_grading.tracker')
@mock.patch('contentstore.signals.signals.GRADING_POLICY_CHANGED.send')
@@ -583,7 +658,6 @@ class CourseGradingTest(CourseTestCase):
self.course.id, test_grader.graders[1], self.user
)
self.assertDictEqual(test_grader.graders[1], altered_grader, "Noop update")
grading_policy_1 = self._grading_policy_hash_for_course()
test_grader.graders[1]['min_count'] = test_grader.graders[1].get('min_count') + 2
altered_grader = CourseGradingModel.update_grader_from_json(
@@ -600,7 +674,6 @@ class CourseGradingTest(CourseTestCase):
# one for each of the calls to update_grader_from_json()
send_signal.assert_has_calls([
# pylint: disable=line-too-long
mock.call(sender=CourseGradingModel, user_id=self.user.id, course_key=self.course.id, grading_policy_hash=grading_policy_1),
mock.call(sender=CourseGradingModel, user_id=self.user.id, course_key=self.course.id, grading_policy_hash=grading_policy_2),
mock.call(sender=CourseGradingModel, user_id=self.user.id, course_key=self.course.id, grading_policy_hash=grading_policy_3),
# pylint: enable=line-too-long
@@ -617,7 +690,7 @@ class CourseGradingTest(CourseTestCase):
'event_transaction_id': 'mockUUID',
'event_transaction_type': 'edx.grades.grading_policy_changed',
}
) for policy_hash in {grading_policy_1, grading_policy_2, grading_policy_3}
) for policy_hash in {grading_policy_2, grading_policy_3}
], any_order=True)
@mock.patch('track.event_transaction_utils.uuid4')

93
cms/djangoapps/models/settings/course_grading.py
View File

@@ -12,6 +12,7 @@ from eventtracking import tracker
from contentstore.signals.signals import GRADING_POLICY_CHANGED
from track.event_transaction_utils import create_new_event_transaction_id
from xmodule.modulestore.django import modulestore
from models.settings.waffle import material_recompute_only
GRADING_POLICY_CHANGED_EVENT_TYPE = 'edx.grades.grading_policy_changed'
@@ -69,7 +70,12 @@ class CourseGradingModel(object):
descriptor = modulestore().get_course(course_key)
graders_parsed = [CourseGradingModel.parse_grader(jsonele) for jsonele in jsondict['graders']]
fire_signal = CourseGradingModel.must_fire_grading_event_and_signal(
course_key,
graders_parsed,
descriptor,
jsondict
)
descriptor.raw_grader = graders_parsed
descriptor.grade_cutoffs = jsondict['grade_cutoffs']
@@ -78,10 +84,84 @@ class CourseGradingModel(object):
CourseGradingModel.update_grace_period_from_json(course_key, jsondict['grace_period'], user)
CourseGradingModel.update_minimum_grade_credit_from_json(course_key, jsondict['minimum_grade_credit'], user)
_grading_event_and_signal(course_key, user.id)
if fire_signal:
_grading_event_and_signal(course_key, user.id)
return CourseGradingModel.fetch(course_key)
@staticmethod
def update_from_json_selective(course_key, jsondict, user):
"""
New version that doesn't fire change events when only name or short name are changed.
Decode the json into CourseGradingModel and save any changes. Returns the modified model.
Probably not the usual path for updates as it's too coarse grained.
"""
descriptor = modulestore().get_course(course_key)
graders_parsed = [CourseGradingModel.parse_grader(jsonele) for jsonele in jsondict['graders']]
fire_signal = CourseGradingModel.must_fire_grading_event_and_signal(
course_key,
graders_parsed,
descriptor,
jsondict
)
descriptor.raw_grader = graders_parsed
descriptor.grade_cutoffs = jsondict['grade_cutoffs']
modulestore().update_item(descriptor, user.id)
CourseGradingModel.update_grace_period_from_json(course_key, jsondict['grace_period'], user)
CourseGradingModel.update_minimum_grade_credit_from_json(course_key, jsondict['minimum_grade_credit'], user)
if fire_signal:
_grading_event_and_signal(course_key, user.id)
return CourseGradingModel.fetch(course_key)
@staticmethod
def must_fire_grading_event_and_signal(course_key, proposed_grader_settings, course_from_modulestore, jsondict):
"""
Detects if substantive enough changes were made to the proposed grader settings to warrant the firing of
_grading_event_and_sngal
Substantive changes mean the following values were changed:
drop_count, weight, min_count
An assignment type was added or removed
"""
if course_from_modulestore.grade_cutoffs != jsondict['grade_cutoffs'] or \
len(proposed_grader_settings) != len(course_from_modulestore.raw_grader):
return True
# because grading policy lists remain in the same order, we can do a single loop
for i in range(len(course_from_modulestore.raw_grader)):
if CourseGradingModel.must_fire_grading_event_and_signal_single_grader(
course_key,
proposed_grader_settings[i],
course_from_modulestore.raw_grader[i]
):
return True
return False
@staticmethod
def must_fire_grading_event_and_signal_single_grader(
course_key,
proposed_grader_settings,
existing_grader_settings
):
"""
Detects changes in an individual grader vs an entire grading policy
Detects if substantive enough changes were made to the proposed grader settings to warrant the firing of
_grading_event_and_sngal
Substantive changes mean the following values were changed:
drop_count, weight, min_count
"""
if not material_recompute_only(course_key):
return True
if existing_grader_settings['drop_count'] != proposed_grader_settings['drop_count'] or \
existing_grader_settings['weight'] != proposed_grader_settings['weight'] or \
existing_grader_settings['min_count'] != proposed_grader_settings['min_count']:
return True
return False
@staticmethod
def update_grader_from_json(course_key, grader, user):
"""
@@ -94,13 +174,20 @@ class CourseGradingModel(object):
index = int(grader.get('id', len(descriptor.raw_grader)))
grader = CourseGradingModel.parse_grader(grader)
fire_signal = True
if index < len(descriptor.raw_grader):
fire_signal = CourseGradingModel.must_fire_grading_event_and_signal_single_grader(
course_key,
grader,
descriptor.raw_grader[index]
)
descriptor.raw_grader[index] = grader
else:
descriptor.raw_grader.append(grader)
modulestore().update_item(descriptor, user.id)
_grading_event_and_signal(course_key, user.id)
if fire_signal:
_grading_event_and_signal(course_key, user.id)
return CourseGradingModel.jsonize_grader(index, descriptor.raw_grader[index])

24
cms/djangoapps/models/settings/waffle.py Normal file
View File

@@ -0,0 +1,24 @@
"""
Togglable settings for Course Grading behavior
"""
from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag
WAFFLE_NAMESPACE = 'grades'
# edx/edx-platform feature
MATERIAL_RECOMPUTE_ONLY = 'MATERIAL_RECOMPUTE_ONLY'
MATERIAL_RECOMPUTE_ONLY_FLAG = CourseWaffleFlag(
waffle_namespace=WAFFLE_NAMESPACE,
flag_name=MATERIAL_RECOMPUTE_ONLY,
)
def material_recompute_only(course_key):
"""
Checks to see if the CourseWaffleFlag or Django setting for material recomputer only is enabled
"""
if MATERIAL_RECOMPUTE_ONLY_FLAG.is_enabled(course_key):
return True
return False