Skip to content

E
edx-platform
Commit 9fb9222a by Nimisha Asthagiri Committed by GitHub
Options

Merge pull request #13806 from edx/beryl/gating-again 

Unrevert "Fix Gating to use grades API, instead of its own calculations"
parents 5835468d 7aacae9a
Showing with 101 additions and 388 deletions
lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py
"""
Tests for ProctoredExamTransformer.
"""
from mock import patch, Mock
from mock import patch
from nose.plugins.attrib import attr
import ddt
from gating import api as lms_gating_api
from lms.djangoapps.course_blocks.transformers.tests.helpers import CourseStructureTestCase
from milestones.tests.utils import MilestonesTestCaseMixin
from opaque_keys.edx.keys import UsageKey
from openedx.core.lib.gating import api as gating_api
from student.tests.factories import CourseEnrollmentFactory
......@@ -134,18 +133,16 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM
(
'H',
'A',
'B',
('course', 'A', 'B', 'C',)
),
(
'H',
'ProctoredExam',
'D',
('course', 'A', 'B', 'C'),
),
)
@ddt.unpack
def test_gated(self, gated_block_ref, gating_block_ref, gating_block_child, expected_blocks_before_completion):
def test_gated(self, gated_block_ref, gating_block_ref, expected_blocks_before_completion):
"""
First, checks that a student cannot see the gated block when it is gated by the gating block and no
attempt has been made to complete the gating block.
......@@ -165,17 +162,14 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM
# clear the request cache to simulate a new request
self.clear_caches()
# mock the api that the lms gating api calls to get the score for each block to always return 1 (ie 100%)
with patch('gating.api.get_module_score', Mock(return_value=1)):
# this call triggers reevaluation of prerequisites fulfilled by the parent of the
# block passed in, so we pass in a child of the gating block
lms_gating_api.evaluate_prerequisite(
self.course,
self.blocks[gating_block_child],
self.user.id,
)
with self.assertNumQueries(2):
# this call triggers reevaluation of prerequisites fulfilled by the gating block
lms_gating_api.evaluate_prerequisite(
self.course,
self.user,
self.blocks[gating_block_ref].location,
100.0,
)
with self.assertNumQueries(3):
self.get_blocks_and_check_against_expected(self.user, self.ALL_BLOCKS_EXCEPT_SPECIAL)
def test_staff_access(self):
......
lms/djangoapps/gating/api.py
......@@ -10,34 +10,14 @@ import logging
from openedx.core.lib.gating import api as gating_api
from opaque_keys.edx.keys import UsageKey
from lms.djangoapps.courseware.entrance_exams import get_entrance_exam_score
from lms.djangoapps.grades.module_grades import get_module_score
from util import milestones_helpers
log = logging.getLogger(__name__)
def _get_xblock_parent(xblock, category=None):
"""
Returns the parent of the given XBlock. If an optional category is supplied,
traverses the ancestors of the XBlock and returns the first with the
given category.
Arguments:
xblock (XBlock): Get the parent of this XBlock
category (str): Find an ancestor with this category (e.g. sequential)
"""
parent = xblock.get_parent()
if parent and category:
if parent.category == category:
return parent
else:
return _get_xblock_parent(parent, category)
return parent
@gating_api.gating_enabled(default=False)
def evaluate_prerequisite(course, block, user_id):
def evaluate_prerequisite(course, user, subsection_usage_key, new_score):
"""
Finds the parent subsection of the content in the course and evaluates
any milestone relationships attached to that subsection. If the calculated
......@@ -52,39 +32,33 @@ def evaluate_prerequisite(course, block, user_id):
Returns:
None
"""
sequential = _get_xblock_parent(block, 'sequential')
if sequential:
prereq_milestone = gating_api.get_gating_milestone(
course.id,
sequential.location.for_branch(None),
'fulfills'
)
if prereq_milestone:
gated_content_milestones = defaultdict(list)
for milestone in gating_api.find_gating_milestones(course.id, None, 'requires'):
gated_content_milestones[milestone['id']].append(milestone)
prereq_milestone = gating_api.get_gating_milestone(
course.id,
subsection_usage_key,
'fulfills'
)
if prereq_milestone:
gated_content_milestones = defaultdict(list)
for milestone in gating_api.find_gating_milestones(course.id, None, 'requires'):
gated_content_milestones[milestone['id']].append(milestone)
gated_content = gated_content_milestones.get(prereq_milestone['id'])
if gated_content:
user = User.objects.get(id=user_id)
score = get_module_score(user, course, sequential) * 100
for milestone in gated_content:
# Default minimum score to 100
min_score = 100
requirements = milestone.get('requirements')
if requirements:
try:
min_score = int(requirements.get('min_score'))
except (ValueError, TypeError):
log.warning(
'Failed to find minimum score for gating milestone %s, defaulting to 100',
json.dumps(milestone)
)
gated_content = gated_content_milestones.get(prereq_milestone['id'])
if gated_content:
for milestone in gated_content:
# Default minimum score to 100
min_score = 100
requirements = milestone.get('requirements')
if requirements:
try:
min_score = int(requirements.get('min_score'))
except (ValueError, TypeError):
# Use default value of 100
pass
if score >= min_score:
milestones_helpers.add_user_milestone({'id': user_id}, prereq_milestone)
else:
milestones_helpers.remove_user_milestone({'id': user_id}, prereq_milestone)
if new_score >= min_score:
milestones_helpers.add_user_milestone({'id': user.id}, prereq_milestone)
else:
milestones_helpers.remove_user_milestone({'id': user.id}, prereq_milestone)
def evaluate_entrance_exam(course, block, user_id):
......
lms/djangoapps/gating/signals.py
......@@ -4,7 +4,7 @@ Signal handlers for the gating djangoapp
from django.dispatch import receiver
from gating import api as gating_api
from lms.djangoapps.grades.signals.signals import SCORE_CHANGED
from lms.djangoapps.grades.signals.signals import SCORE_CHANGED, SUBSECTION_SCORE_CHANGED
from opaque_keys.edx.keys import CourseKey, UsageKey
from xmodule.modulestore.django import modulestore
......@@ -24,5 +24,26 @@ def handle_score_changed(**kwargs):
"""
course = modulestore().get_course(CourseKey.from_string(kwargs.get('course_id')))
block = modulestore().get_item(UsageKey.from_string(kwargs.get('usage_id')))
gating_api.evaluate_prerequisite(course, block, kwargs.get('user_id'))
gating_api.evaluate_entrance_exam(course, block, kwargs.get('user_id'))
@receiver(SUBSECTION_SCORE_CHANGED)
def handle_subsection_score_changed(**kwargs):
"""
Receives the SUBSECTION_SCORE_CHANGED signal and triggers the evaluation of
any milestone relationships which are attached to the updated content.
Arguments:
kwargs (dict): Contains user ID, course key, and content usage key
Returns:
None
"""
course = kwargs['course']
if course.enable_subsection_gating:
subsection_grade = kwargs['subsection_grade']
new_score = subsection_grade.graded_total.earned / subsection_grade.graded_total.possible * 100.0
gating_api.evaluate_prerequisite(
course,
kwargs['user'],
subsection_grade.location,
new_score,
)
lms/djangoapps/gating/tests/test_signals.py
"""
Unit tests for gating.signals module
"""
from mock import patch
from mock import patch, MagicMock
from opaque_keys.edx.keys import UsageKey
from student.tests.factories import UserFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from xmodule.modulestore.django import modulestore
from gating.signals import handle_score_changed
from gating.signals import handle_subsection_score_changed
class TestHandleScoreChanged(ModuleStoreTestCase):
......@@ -27,25 +26,21 @@ class TestHandleScoreChanged(ModuleStoreTestCase):
""" Test evaluate_prerequisite is called when course.enable_subsection_gating is True """
self.course.enable_subsection_gating = True
modulestore().update_item(self.course, 0)
handle_score_changed(
handle_subsection_score_changed(
sender=None,
points_possible=1,
points_earned=1,
user_id=self.user.id,
course_id=unicode(self.course.id),
usage_id=unicode(self.test_usage_key)
course=self.course,
user=self.user,
subsection_grade=MagicMock(),
)
mock_evaluate.assert_called_with(self.course, self.course, self.user.id) # pylint: disable=no-member
mock_evaluate.assert_called()
@patch('gating.signals.gating_api.evaluate_prerequisite')
def test_gating_disabled(self, mock_evaluate):
""" Test evaluate_prerequisite is not called when course.enable_subsection_gating is False """
handle_score_changed(
handle_subsection_score_changed(
sender=None,
points_possible=1,
points_earned=1,
user_id=self.user.id,
course_id=unicode(self.course.id),
usage_id=unicode(self.test_usage_key)
course=self.course,
user=self.user,
subsection_grade=MagicMock(),
)
mock_evaluate.assert_not_called()
lms/djangoapps/grades/module_grades.py deleted 100644 → 0
"""
Functionality for module-level grades.
"""
# TODO The score computation in this file is not accurate
# since it is summing percentages instead of computing a
# final percentage of the individual sums.
# Regardless, this file and its code should be removed soon
# as part of TNL-5062.
from django.test.client import RequestFactory
from courseware.model_data import FieldDataCache, ScoresClient
from courseware.module_render import get_module_for_descriptor
from opaque_keys.edx.locator import BlockUsageLocator
from util.module_utils import yield_dynamic_descriptor_descendants
def _get_mock_request(student):
"""
Make a fake request because grading code expects to be able to look at
the request. We have to attach the correct user to the request before
grading that student.
"""
request = RequestFactory().get('/')
request.user = student
return request
def _calculate_score_for_modules(user_id, course, modules):
"""
Calculates the cumulative score (percent) of the given modules
"""
# removing branch and version from exam modules locator
# otherwise student module would not return scores since module usage keys would not match
modules = [m for m in modules]
locations = [
BlockUsageLocator(
course_key=course.id,
block_type=module.location.block_type,
block_id=module.location.block_id
)
if isinstance(module.location, BlockUsageLocator) and module.location.version
else module.location
for module in modules
]
scores_client = ScoresClient(course.id, user_id)
scores_client.fetch_scores(locations)
# Iterate over all of the exam modules to get score percentage of user for each of them
module_percentages = []
ignore_categories = ['course', 'chapter', 'sequential', 'vertical', 'randomize', 'library_content']
for index, module in enumerate(modules):
if module.category not in ignore_categories and (module.graded or module.has_score):
module_score = scores_client.get(locations[index])
if module_score:
correct = module_score.correct or 0
total = module_score.total or 1
module_percentages.append(correct / total)
return sum(module_percentages) / float(len(module_percentages)) if module_percentages else 0
def get_module_score(user, course, module):
"""
Collects all children of the given module and calculates the cumulative
score for this set of modules for the given user.
Arguments:
user (User): The user
course (CourseModule): The course
module (XBlock): The module
Returns:
float: The cumulative score
"""
def inner_get_module(descriptor):
"""
Delegate to get_module_for_descriptor
"""
field_data_cache = FieldDataCache([descriptor], course.id, user)
return get_module_for_descriptor(
user,
_get_mock_request(user),
descriptor,
field_data_cache,
course.id,
course=course
)
modules = yield_dynamic_descriptor_descendants(
module,
user.id,
inner_get_module
)
return _calculate_score_for_modules(user.id, course, modules)
lms/djangoapps/grades/new/subsection_grade.py
......@@ -253,11 +253,6 @@ class SubsectionGradeFactory(object):
"""
Updates the SubsectionGrade object for the student and subsection.
"""
# Save ourselves the extra queries if the course does not persist
# subsection grades.
if not PersistentGradesEnabledFlag.feature_enabled(self.course.id):
return
self._log_event(log.warning, u"update, subsection: {}".format(subsection.location))
block_structure = self._get_block_structure(block_structure)
......@@ -282,8 +277,10 @@ class SubsectionGradeFactory(object):
):
return orig_subsection_grade
grade_model = calculated_grade.update_or_create_model(self.student)
self._update_saved_subsection_grade(subsection.location, grade_model)
if PersistentGradesEnabledFlag.feature_enabled(self.course.id):
grade_model = calculated_grade.update_or_create_model(self.student)
self._update_saved_subsection_grade(subsection.location, grade_model)
return calculated_grade
@lazy
......
lms/djangoapps/grades/signals/signals.py
......@@ -23,6 +23,10 @@ SCORE_CHANGED = Signal(
)
# Signal that indicates that a user's score for a problem has been published
# for possible persistence and update. Typically, most clients should listen
# to the SCORE_CHANGED signal instead, since that is signalled only after the
# problem's score is changed.
SCORE_PUBLISHED = Signal(
providing_args=[
'block', # Course block object
......@@ -33,3 +37,15 @@ SCORE_PUBLISHED = Signal(
# made only if the new score is higher than previous.
]
)
# Signal that indicates that a user's score for a subsection has been updated.
# This is a downstream signal of SCORE_CHANGED sent for each affected containing
# subsection.
SUBSECTION_SCORE_CHANGED = Signal(
providing_args=[
'course', # Course object
'user', # User object
'subsection_grade', # SubsectionGrade object
]
)
lms/djangoapps/grades/tasks.py
......@@ -13,9 +13,9 @@ from opaque_keys.edx.locator import CourseLocator
from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache
from xmodule.modulestore.django import modulestore
from .config.models import PersistentGradesEnabledFlag
from .transformer import GradesTransformer
from .new.subsection_grade import SubsectionGradeFactory
from .signals.signals import SUBSECTION_SCORE_CHANGED
from .transformer import GradesTransformer
@task(default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY)
......@@ -31,9 +31,6 @@ def recalculate_subsection_grade(user_id, course_id, usage_id, only_if_higher):
value.
"""
course_key = CourseLocator.from_string(course_id)
if not PersistentGradesEnabledFlag.feature_enabled(course_key):
return
student = User.objects.get(id=user_id)
scored_block_usage_key = UsageKey.from_string(usage_id).replace(course_key=course_key)
......@@ -54,10 +51,17 @@ def recalculate_subsection_grade(user_id, course_id, usage_id, only_if_higher):
subsection_usage_key,
collected_block_structure=collected_block_structure,
)
subsection_grade_factory.update(
subsection_grade = subsection_grade_factory.update(
transformed_subsection_structure[subsection_usage_key],
transformed_subsection_structure,
only_if_higher,
)
SUBSECTION_SCORE_CHANGED.send(
sender=None,
course=course,
user=student,
subsection_grade=subsection_grade,
)
except IntegrityError as exc:
raise recalculate_subsection_grade.retry(args=[user_id, course_id, usage_id], exc=exc)
lms/djangoapps/grades/tests/test_grades.py
......@@ -27,7 +27,6 @@ from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from .utils import answer_problem
from .. import course_grades
from ..course_grades import summary as grades_summary
from ..module_grades import get_module_score
from ..new.course_grade import CourseGradeFactory
from ..new.subsection_grade import SubsectionGradeFactory
......@@ -342,194 +341,3 @@ class TestScoreForModule(SharedModuleStoreTestCase):
earned, possible = self.course_grade.score_for_module(self.m.location)
self.assertEqual(earned, 0)
self.assertEqual(possible, 0)
class TestGetModuleScore(LoginEnrollmentTestCase, SharedModuleStoreTestCase):
"""
Test get_module_score
"""
@classmethod
def setUpClass(cls):
super(TestGetModuleScore, cls).setUpClass()
cls.course = CourseFactory.create()
cls.chapter = ItemFactory.create(
parent=cls.course,
category="chapter",
display_name="Test Chapter"
)
cls.seq1 = ItemFactory.create(
parent=cls.chapter,
category='sequential',
display_name="Test Sequential 1",
graded=True
)
cls.seq2 = ItemFactory.create(
parent=cls.chapter,
category='sequential',
display_name="Test Sequential 2",
graded=True
)
cls.seq3 = ItemFactory.create(
parent=cls.chapter,
category='sequential',
display_name="Test Sequential 3",
graded=True
)
cls.vert1 = ItemFactory.create(
parent=cls.seq1,
category='vertical',
display_name='Test Vertical 1'
)
cls.vert2 = ItemFactory.create(
parent=cls.seq2,
category='vertical',
display_name='Test Vertical 2'
)
cls.vert3 = ItemFactory.create(
parent=cls.seq3,
category='vertical',
display_name='Test Vertical 3'
)
cls.randomize = ItemFactory.create(
parent=cls.vert2,
category='randomize',
display_name='Test Randomize'
)
cls.library_content = ItemFactory.create(
parent=cls.vert3,
category='library_content',
display_name='Test Library Content'
)
problem_xml = MultipleChoiceResponseXMLFactory().build_xml(
question_text='The correct answer is Choice 3',
choices=[False, False, True, False],
choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3']
)
cls.problem1 = ItemFactory.create(
parent=cls.vert1,
category="problem",
display_name="Test Problem 1",
data=problem_xml
)
cls.problem2 = ItemFactory.create(
parent=cls.vert1,
category="problem",
display_name="Test Problem 2",
data=problem_xml
)
cls.problem3 = ItemFactory.create(
parent=cls.randomize,
category="problem",
display_name="Test Problem 3",
data=problem_xml
)
cls.problem4 = ItemFactory.create(
parent=cls.randomize,
category="problem",
display_name="Test Problem 4",
data=problem_xml
)
cls.problem5 = ItemFactory.create(
parent=cls.library_content,
category="problem",
display_name="Test Problem 5",
data=problem_xml
)
cls.problem6 = ItemFactory.create(
parent=cls.library_content,
category="problem",
display_name="Test Problem 6",
data=problem_xml
)
def setUp(self):
"""
Set up test course
"""
super(TestGetModuleScore, self).setUp()
self.request = get_request_for_user(UserFactory())
self.client.login(username=self.request.user.username, password="test")
CourseEnrollment.enroll(self.request.user, self.course.id)
self.course_structure = get_course_blocks(self.request.user, self.course.location)
# warm up the score cache to allow accurate query counts, even if tests are run in random order
get_module_score(self.request.user, self.course, self.seq1)
def test_subsection_scores(self):
"""
Test test_get_module_score
"""
# One query is for getting the list of disabled XBlocks (which is
# then stored in the request).
with self.assertNumQueries(1):
score = get_module_score(self.request.user, self.course, self.seq1)
new_score = SubsectionGradeFactory(self.request.user, self.course, self.course_structure).create(self.seq1)
self.assertEqual(score, 0)
self.assertEqual(new_score.all_total.earned, 0)
answer_problem(self.course, self.request, self.problem1)
answer_problem(self.course, self.request, self.problem2)
with self.assertNumQueries(1):
score = get_module_score(self.request.user, self.course, self.seq1)
new_score = SubsectionGradeFactory(self.request.user, self.course, self.course_structure).create(self.seq1)
self.assertEqual(score, 1.0)
self.assertEqual(new_score.all_total.earned, 2.0)
# These differ because get_module_score normalizes the subsection score
# to 1, which can cause incorrect aggregation behavior that will be
# fixed by TNL-5062.
answer_problem(self.course, self.request, self.problem1)
answer_problem(self.course, self.request, self.problem2, 0)
with self.assertNumQueries(1):
score = get_module_score(self.request.user, self.course, self.seq1)
new_score = SubsectionGradeFactory(self.request.user, self.course, self.course_structure).create(self.seq1)
self.assertEqual(score, .5)
self.assertEqual(new_score.all_total.earned, 1.0)
def test_get_module_score_with_empty_score(self):
"""
Test test_get_module_score_with_empty_score
"""
set_score(self.request.user.id, self.problem1.location, None, None) # pylint: disable=no-member
set_score(self.request.user.id, self.problem2.location, None, None) # pylint: disable=no-member
with self.assertNumQueries(1):
score = get_module_score(self.request.user, self.course, self.seq1)
self.assertEqual(score, 0)
answer_problem(self.course, self.request, self.problem1)
with self.assertNumQueries(1):
score = get_module_score(self.request.user, self.course, self.seq1)
self.assertEqual(score, 0.5)
answer_problem(self.course, self.request, self.problem2)
with self.assertNumQueries(1):
score = get_module_score(self.request.user, self.course, self.seq1)
self.assertEqual(score, 1.0)
def test_get_module_score_with_randomize(self):
"""
Test test_get_module_score_with_randomize
"""
answer_problem(self.course, self.request, self.problem3)
answer_problem(self.course, self.request, self.problem4)
score = get_module_score(self.request.user, self.course, self.seq2)
self.assertEqual(score, 1.0)
def test_get_module_score_with_library_content(self):
"""
Test test_get_module_score_with_library_content
"""
answer_problem(self.course, self.request, self.problem5)
answer_problem(self.course, self.request, self.problem6)
score = get_module_score(self.request.user, self.course, self.seq3)
self.assertEqual(score, 1.0)
lms/djangoapps/grades/tests/test_tasks.py
......@@ -104,7 +104,7 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
with self.store.default_store(default_store):
self.set_up_course(enable_subsection_grades=False)
self.assertFalse(PersistentGradesEnabledFlag.feature_enabled(self.course.id))
with check_mongo_calls(2) and self.assertNumQueries(0):
with check_mongo_calls(2) and self.assertNumQueries(5):
recalculate_subsection_grade.apply(args=tuple(self.score_changed_kwargs.values()))
@skip("Pending completion of TNL-5089")
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment