diff --git a/common/djangoapps/third_party_auth/management/commands/saml.py b/common/djangoapps/third_party_auth/management/commands/saml.py index dd2c6c1049..8a1590ae66 100644 --- a/common/djangoapps/third_party_auth/management/commands/saml.py +++ b/common/djangoapps/third_party_auth/management/commands/saml.py @@ -4,7 +4,6 @@ Management commands for third_party_auth """ from django.core.management.base import BaseCommand, CommandError import logging -from third_party_auth.models import SAMLConfiguration from third_party_auth.tasks import fetch_saml_metadata @@ -16,9 +15,6 @@ class Command(BaseCommand): parser.add_argument('--pull', action='store_true', help="Pull updated metadata from external IDPs") def handle(self, *args, **options): - if not SAMLConfiguration.is_enabled(): - raise CommandError("SAML support is disabled via SAMLConfiguration.") - if options['pull']: log_handler = logging.StreamHandler(self.stdout) log_handler.setLevel(logging.DEBUG) diff --git a/common/test/acceptance/pages/lms/staff_view.py b/common/test/acceptance/pages/lms/staff_view.py index db23f89fd1..c87fef1d7d 100644 --- a/common/test/acceptance/pages/lms/staff_view.py +++ b/common/test/acceptance/pages/lms/staff_view.py @@ -95,8 +95,8 @@ class StaffDebugPage(PageObject): This delete's a student's state for the problem """ if user: - self.q(css='input[id^=sd_fu_]').fill(user) - self.q(css='.staff-modal .staff-debug-sdelete').click() + self.q(css='input[id^=sd_fu_]').first.fill(user) + self.q(css='.staff-modal .staff-debug-sdelete').first.click() def rescore(self, user=None): """ diff --git a/common/test/acceptance/tests/lms/test_progress_page.py b/common/test/acceptance/tests/lms/test_progress_page.py index cad30b789c..3553cc9e44 100644 --- a/common/test/acceptance/tests/lms/test_progress_page.py +++ b/common/test/acceptance/tests/lms/test_progress_page.py @@ -15,6 +15,7 @@ from ...pages.lms.courseware import CoursewarePage from ...pages.lms.instructor_dashboard import InstructorDashboardPage from ...pages.lms.problem import ProblemPage from ...pages.lms.progress import ProgressPage +from ...pages.lms.staff_view import StaffPage, StaffDebugPage from ...pages.studio.component_editor import ComponentEditorView from ...pages.studio.utils import type_in_codemirror from ...pages.studio.overview import CourseOutlinePage @@ -192,6 +193,22 @@ class PersistentGradesTest(ProgressPageBaseTest): type_in_codemirror(self, 0, modified_content) modal.q(css='.action-save').click() + def _delete_student_state_for_problem(self): + """ + As staff, clicks the "delete student state" button, + deleting the student user's state for the problem. + """ + with self._logged_in_session(staff=True): + self.courseware_page.visit() + staff_page = StaffPage(self.browser, self.course_id) + self.assertEqual(staff_page.staff_view_mode, "Staff") + staff_page.q(css='a.instructor-info-action').nth(1).click() + staff_debug_page = StaffDebugPage(self.browser) + staff_debug_page.wait_for_page() + staff_debug_page.delete_state(self.USERNAME) + msg = staff_debug_page.idash_msg[0] + self.assertEqual(u'Successfully deleted student state for user {0}'.format(self.USERNAME), msg) + @ddt.data( _edit_problem_content, _change_subsection_structure, @@ -223,6 +240,13 @@ class PersistentGradesTest(ProgressPageBaseTest): self.assertEqual(self._get_problem_scores(), [(1, 1), (0, 1)]) self.assertEqual(self._get_section_score(), (1, 2)) + def test_progress_page_updates_when_student_state_deleted(self): + self._check_progress_page_with_scored_problem() + self._delete_student_state_for_problem() + with self._logged_in_session(): + self.assertEqual(self._get_problem_scores(), [(0, 1), (0, 1)]) + self.assertEqual(self._get_section_score(), (0, 2)) + class SubsectionGradingPolicyTest(ProgressPageBaseTest): """ diff --git a/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py b/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py index a626d096dc..5f88c8004e 100644 --- a/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py +++ b/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py @@ -1,7 +1,7 @@ """ Tests for ProctoredExamTransformer. """ -from mock import patch +from mock import patch, Mock from nose.plugins.attrib import attr import ddt @@ -53,8 +53,7 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM 'course', 'A', 'B', 'C', 'ProctoredExam', 'D', 'E', 'PracticeExam', 'F', 'G', 'H', 'I', 'TimedExam', 'J', 'K' ) - # The special exams (proctored, practice, timed) are not visible to - # students via the Courses API. + # The special exams (proctored, practice, timed) should never be visible to students ALL_BLOCKS_EXCEPT_SPECIAL = ('course', 'A', 'B', 'C', 'H', 'I') def get_course_hierarchy(self): @@ -135,27 +134,27 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM ( 'H', 'A', - ('course', 'A', 'B', 'C'), + 'B', + ('course', 'A', 'B', 'C',) ), ( 'H', 'ProctoredExam', + 'D', ('course', 'A', 'B', 'C'), ), ) @ddt.unpack - def test_gated(self, gated_block_ref, gating_block_ref, expected_blocks_before_completion): + def test_gated(self, gated_block_ref, gating_block_ref, gating_block_child, 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. Then, checks that the student can see the gated block after the - gating block has been completed. + 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. + Then, checks that the student can see the gated block after the gating block has been completed. - expected_blocks_before_completion is the set of blocks we expect to be - visible to the student before the student has completed the gating block. + expected_blocks_before_completion is the set of blocks we expect to be visible to the student + before the student has completed the gating block. - The test data includes one special exam and one non-special block as the - gating blocks. + The test data includes one special exam and one non-special block as the gating blocks. """ self.course.enable_subsection_gating = True self.setup_gated_section(self.blocks[gated_block_ref], self.blocks[gating_block_ref]) @@ -166,16 +165,16 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM # clear the request cache to simulate a new request self.clear_caches() - # 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, - ) + # 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)): - with self.assertNumQueries(3): + # 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, + UsageKey.from_string(unicode(self.blocks[gating_block_child].location)), + self.user.id) + with self.assertNumQueries(2): self.get_blocks_and_check_against_expected(self.user, self.ALL_BLOCKS_EXCEPT_SPECIAL) def test_staff_access(self): diff --git a/lms/djangoapps/gating/api.py b/lms/djangoapps/gating/api.py index 64e35602cb..416d70d583 100644 --- a/lms/djangoapps/gating/api.py +++ b/lms/djangoapps/gating/api.py @@ -6,14 +6,35 @@ import json from collections import defaultdict from django.contrib.auth.models import User +from xmodule.modulestore.django import modulestore from openedx.core.lib.gating import api as gating_api +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, user, subsection_usage_key, new_score): +def evaluate_prerequisite(course, prereq_content_key, user_id): """ Finds the parent subsection of the content in the course and evaluates any milestone relationships attached to that subsection. If the calculated @@ -21,40 +42,44 @@ def evaluate_prerequisite(course, user, subsection_usage_key, new_score): dependent subsections, the related milestone will be fulfilled for the user. Arguments: - user (User): User for which evaluation should occur + user_id (int): ID of User for which evaluation should occur course (CourseModule): The course - subsection_usage_key (UsageKey): Usage key of the updated subsection - new_score (float): New score of the given subsection, in percentage. + prereq_content_key (UsageKey): The prerequisite content usage key Returns: None """ - 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) + xblock = modulestore().get_item(prereq_content_key) + sequential = _get_xblock_parent(xblock, '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) - 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.0 - requirements = milestone.get('requirements') - if requirements: - try: - min_score = float(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: + 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) + ) - 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) + 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) diff --git a/lms/djangoapps/gating/signals.py b/lms/djangoapps/gating/signals.py index 06c79ad90c..bfa5b974a7 100644 --- a/lms/djangoapps/gating/signals.py +++ b/lms/djangoapps/gating/signals.py @@ -2,12 +2,14 @@ Signal handlers for the gating djangoapp """ from django.dispatch import receiver -from lms.djangoapps.grades.signals.signals import SUBSECTION_SCORE_UPDATED +from opaque_keys.edx.keys import CourseKey, UsageKey +from xmodule.modulestore.django import modulestore +from lms.djangoapps.grades.signals.signals import SCORE_CHANGED from gating import api as gating_api -@receiver(SUBSECTION_SCORE_UPDATED) -def handle_subsection_score_updated(**kwargs): +@receiver(SCORE_CHANGED) +def handle_score_changed(**kwargs): """ Receives the SCORE_CHANGED signal sent by LMS when a student's score has changed for a given component and triggers the evaluation of any milestone relationships @@ -19,13 +21,10 @@ def handle_subsection_score_updated(**kwargs): Returns: None """ - course = kwargs['course'] + course = modulestore().get_course(CourseKey.from_string(kwargs.get('course_id'))) 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, + UsageKey.from_string(kwargs.get('usage_id')), + kwargs.get('user').id, ) diff --git a/lms/djangoapps/gating/tests/test_api.py b/lms/djangoapps/gating/tests/test_api.py index 5e63cf9ef3..65d677a740 100644 --- a/lms/djangoapps/gating/tests/test_api.py +++ b/lms/djangoapps/gating/tests/test_api.py @@ -6,18 +6,15 @@ from nose.plugins.attrib import attr from ddt import ddt, data, unpack from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from courseware.tests.helpers import LoginEnrollmentTestCase -from lms.djangoapps.courseware.access import has_access -from lms.djangoapps.courseware.tests.helpers import get_request_for_user -from lms.djangoapps.grades.tests.utils import answer_problem -from lms.djangoapps.grades.new.course_grade import CourseGradeFactory from milestones import api as milestones_api from milestones.tests.utils import MilestonesTestCaseMixin from openedx.core.lib.gating import api as gating_api -from request_cache.middleware import RequestCache +from gating.api import _get_xblock_parent, evaluate_prerequisite -class GatingTestCase(ModuleStoreTestCase): +class GatingTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): """ Base TestCase class for setting up a basic course structure and testing the gating feature @@ -37,16 +34,6 @@ class GatingTestCase(ModuleStoreTestCase): display_name='edX 101' ) self.course.enable_subsection_gating = True - grading_policy = { - "GRADER": [{ - "type": "Homework", - "min_count": 3, - "drop_count": 0, - "short_label": "HW", - "weight": 1.0 - }] - } - self.course.grading_policy = grading_policy self.course.save() self.store.update_item(self.course, 0) @@ -54,197 +41,136 @@ class GatingTestCase(ModuleStoreTestCase): self.chapter1 = ItemFactory.create( parent_location=self.course.location, category='chapter', - display_name='chapter 1' + display_name='untitled chapter 1' ) # create sequentials self.seq1 = ItemFactory.create( parent_location=self.chapter1.location, category='sequential', - display_name='gating sequential 1', - graded=True, - format='Homework', + display_name='untitled sequential 1' ) self.seq2 = ItemFactory.create( parent_location=self.chapter1.location, category='sequential', - display_name='gated sequential 2', - graded=True, - format='Homework', + display_name='untitled sequential 2' ) - self.seq3 = ItemFactory.create( - parent_location=self.chapter1.location, - category='sequential', - display_name='sequential 3', - graded=True, - format='Homework', + + # create vertical + self.vert1 = ItemFactory.create( + parent_location=self.seq1.location, + category='vertical', + display_name='untitled vertical 1' ) # create problem - self.gating_prob1 = ItemFactory.create( - parent_location=self.seq1.location, + self.prob1 = ItemFactory.create( + parent_location=self.vert1.location, category='problem', - display_name='gating problem 1', - ) - self.gated_prob2 = ItemFactory.create( - parent_location=self.seq2.location, - category='problem', - display_name='gated problem 2', - ) - self.prob3 = ItemFactory.create( - parent_location=self.seq3.location, - category='problem', - display_name='problem 3', + display_name='untitled problem 1' ) # create orphan - self.orphan = ItemFactory.create( + self.prob2 = ItemFactory.create( parent_location=self.course.location, category='problem', - display_name='orphan' - ) - - self.prereq_milestone = None - - def setup_gating_milestone(self, min_score): - """ - Setup a gating milestone for testing. - Gating content: seq1 (must be fulfilled before access to seq2) - Gated content: seq2 (requires completion of seq1 before access) - """ - gating_api.add_prerequisite(self.course.id, self.seq1.location) - gating_api.set_required_content(self.course.id, self.seq2.location, self.seq1.location, min_score) - self.prereq_milestone = gating_api.get_gating_milestone(self.course.id, self.seq1.location, 'fulfills') - - def verify_access_to_gated_content(self, user, expected_access): - """ - Verifies access to gated content for the given user is as expected. - """ - # clear the request cache to flush any cached access results - RequestCache.clear_request_cache() - - # access to gating content (seq1) remains constant - self.assertTrue(bool(has_access(user, 'load', self.seq1, self.course.id))) - - # access to gated content (seq2) is as expected - self.assertEquals(bool(has_access(user, 'load', self.seq2, self.course.id)), expected_access) - - def verify_user_has_prereq_milestone(self, user, expected_has_milestone): - """ - Verifies whether or not the user has the prereq milestone - """ - self.assertEquals( - milestones_api.user_has_milestone({'id': user.id}, self.prereq_milestone), - expected_has_milestone, + display_name='untitled problem 2' ) -@attr(shard=3) -class TestGatedContent(GatingTestCase, MilestonesTestCaseMixin): +class TestGetXBlockParent(GatingTestCase): """ - Tests for gated content. + Tests for the get_xblock_parent function """ - def setUp(self): - super(TestGatedContent, self).setUp() - self.setup_gating_milestone(100) - self.non_staff_user, _ = self.create_non_staff_user() - def test_gated_for_nonstaff(self): - self.verify_user_has_prereq_milestone(self.non_staff_user, expected_has_milestone=False) - self.verify_access_to_gated_content(self.non_staff_user, expected_access=False) + def test_get_direct_parent(self): + """ Test test_get_direct_parent """ - def test_not_gated_for_staff(self): - self.verify_user_has_prereq_milestone(self.user, expected_has_milestone=False) - self.verify_access_to_gated_content(self.user, expected_access=True) + result = _get_xblock_parent(self.vert1) + self.assertEqual(result.location, self.seq1.location) - def _verify_course_grade(self, user, expected_percent): - """ - Verifies the given user's course grade is the expected percentage. - Also verifies the user's grade information contains values for - all problems in the course, whether or not they are currently - gated. - """ - course_grade = CourseGradeFactory(user).create(self.course) - for prob in [self.gating_prob1, self.gated_prob2, self.prob3]: - self.assertIn(prob.location, course_grade.locations_to_scores) - self.assertNotIn(self.orphan.location, course_grade.locations_to_scores) + def test_get_parent_with_category(self): + """ Test test_get_parent_of_category """ - self.assertEquals(course_grade.percent, expected_percent) + result = _get_xblock_parent(self.vert1, 'sequential') + self.assertEqual(result.location, self.seq1.location) + result = _get_xblock_parent(self.vert1, 'chapter') + self.assertEqual(result.location, self.chapter1.location) - def test_gated_content_always_in_grades(self): - request = get_request_for_user(self.non_staff_user) + def test_get_parent_none(self): + """ Test test_get_parent_none """ - # start with a grade from a non-gated subsection - answer_problem(self.course, request, self.prob3, 10, 10) - - # verify gated status and overall course grade percentage - self.verify_user_has_prereq_milestone(self.non_staff_user, expected_has_milestone=False) - self.verify_access_to_gated_content(self.non_staff_user, expected_access=False) - self._verify_course_grade(self.non_staff_user, .33) - - # fulfill the gated requirements - answer_problem(self.course, request, self.gating_prob1, 10, 10) - - # verify gated status and overall course grade percentage - self.verify_user_has_prereq_milestone(self.non_staff_user, expected_has_milestone=True) - self.verify_access_to_gated_content(self.non_staff_user, expected_access=True) - self._verify_course_grade(self.non_staff_user, .67) + result = _get_xblock_parent(self.vert1, 'unit') + self.assertIsNone(result) @attr(shard=3) @ddt -class TestHandleSubsectionGradeUpdates(GatingTestCase, MilestonesTestCaseMixin): +class TestEvaluatePrerequisite(GatingTestCase, MilestonesTestCaseMixin): """ - Tests for gated content when subsection grade is updated. + Tests for the evaluate_prerequisite function """ def setUp(self): - super(TestHandleSubsectionGradeUpdates, self).setUp() - self.user, _ = self.create_non_staff_user() # run tests for a non-staff user - self.request = get_request_for_user(self.user) + super(TestEvaluatePrerequisite, self).setUp() + self.user_dict = {'id': self.user.id} + self.prereq_milestone = None - def test_signal_handler_called(self): - with patch('lms.djangoapps.gating.signals.gating_api.evaluate_prerequisite') as mock_handler: - self.assertFalse(mock_handler.called) - answer_problem(self.course, self.request, self.gating_prob1, 1, 1) - self.assertTrue(mock_handler.called) + def _setup_gating_milestone(self, min_score): + """ + Setup a gating milestone for testing + """ - @data((1, 2, True), (1, 1, True), (0, 1, False)) + gating_api.add_prerequisite(self.course.id, self.seq1.location) + gating_api.set_required_content(self.course.id, self.seq2.location, self.seq1.location, min_score) + self.prereq_milestone = gating_api.get_gating_milestone(self.course.id, self.seq1.location, 'fulfills') + + @patch('gating.api.get_module_score') + @data((.5, True), (1, True), (0, False)) @unpack - def test_min_score_achieved(self, earned, max_possible, result): - self.setup_gating_milestone(50) + def test_min_score_achieved(self, module_score, result, mock_module_score): + """ Test test_min_score_achieved """ - self.verify_user_has_prereq_milestone(self.user, expected_has_milestone=False) - self.verify_access_to_gated_content(self.user, expected_access=False) + self._setup_gating_milestone(50) - answer_problem(self.course, self.request, self.gating_prob1, earned, max_possible) - - self.verify_user_has_prereq_milestone(self.user, expected_has_milestone=result) - self.verify_access_to_gated_content(self.user, expected_access=result) + mock_module_score.return_value = module_score + evaluate_prerequisite(self.course, self.prob1.location, self.user.id) + self.assertEqual(milestones_api.user_has_milestone(self.user_dict, self.prereq_milestone), result) @patch('gating.api.log.warning') - @data((1, 2, False), (1, 1, True)) + @patch('gating.api.get_module_score') + @data((.5, False), (1, True)) @unpack - def test_invalid_min_score(self, earned, max_possible, result, mock_log): - self.setup_gating_milestone(None) + def test_invalid_min_score(self, module_score, result, mock_module_score, mock_log): + """ Test test_invalid_min_score """ - answer_problem(self.course, self.request, self.gating_prob1, earned, max_possible) - self.verify_user_has_prereq_milestone(self.user, expected_has_milestone=result) + self._setup_gating_milestone(None) + + mock_module_score.return_value = module_score + evaluate_prerequisite(self.course, self.prob1.location, self.user.id) + self.assertEqual(milestones_api.user_has_milestone(self.user_dict, self.prereq_milestone), result) self.assertTrue(mock_log.called) - def test_orphaned_xblock(self): - with patch('lms.djangoapps.gating.signals.gating_api.evaluate_prerequisite') as mock_handler: - self.assertFalse(mock_handler.called) - answer_problem(self.course, self.request, self.orphan, 1, 1) - self.assertFalse(mock_handler.called) + @patch('gating.api.get_module_score') + def test_orphaned_xblock(self, mock_module_score): + """ Test test_orphaned_xblock """ - @patch('gating.api.milestones_helpers') - def test_no_prerequisites(self, mock_milestones): - answer_problem(self.course, self.request, self.gating_prob1, 1, 1) - self.assertFalse(mock_milestones.called) + evaluate_prerequisite(self.course, self.prob2.location, self.user.id) + self.assertFalse(mock_module_score.called) - @patch('gating.api.milestones_helpers') - def test_no_gated_content(self, mock_milestones): + @patch('gating.api.get_module_score') + def test_no_prerequisites(self, mock_module_score): + """ Test test_no_prerequisites """ + + evaluate_prerequisite(self.course, self.prob1.location, self.user.id) + self.assertFalse(mock_module_score.called) + + @patch('gating.api.get_module_score') + def test_no_gated_content(self, mock_module_score): + """ Test test_no_gated_content """ + + # Setup gating milestones data gating_api.add_prerequisite(self.course.id, self.seq1.location) - answer_problem(self.course, self.request, self.gating_prob1, 1, 1) - self.assertFalse(mock_milestones.called) + + evaluate_prerequisite(self.course, self.prob1.location, self.user.id) + self.assertFalse(mock_module_score.called) diff --git a/lms/djangoapps/gating/tests/test_signals.py b/lms/djangoapps/gating/tests/test_signals.py index f910529e77..16116da608 100644 --- a/lms/djangoapps/gating/tests/test_signals.py +++ b/lms/djangoapps/gating/tests/test_signals.py @@ -1,7 +1,7 @@ """ Unit tests for gating.signals module """ -from mock import patch, MagicMock +from mock import patch from opaque_keys.edx.keys import UsageKey from student.tests.factories import UserFactory @@ -9,7 +9,7 @@ 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_subsection_score_updated +from gating.signals import handle_score_changed class TestHandleScoreChanged(ModuleStoreTestCase): @@ -24,22 +24,28 @@ class TestHandleScoreChanged(ModuleStoreTestCase): @patch('gating.signals.gating_api.evaluate_prerequisite') def test_gating_enabled(self, mock_evaluate): + """ 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_subsection_score_updated( + handle_score_changed( sender=None, - course=self.course, + points_possible=1, + points_earned=1, user=self.user, - subsection_grade=MagicMock(), + course_id=unicode(self.course.id), + usage_id=unicode(self.test_usage_key) ) - mock_evaluate.assert_called() + mock_evaluate.assert_called_with(self.course, self.test_usage_key, self.user.id) # pylint: disable=no-member @patch('gating.signals.gating_api.evaluate_prerequisite') def test_gating_disabled(self, mock_evaluate): - handle_subsection_score_updated( + """ Test evaluate_prerequisite is not called when course.enable_subsection_gating is False """ + handle_score_changed( sender=None, - course=self.course, + points_possible=1, + points_earned=1, user=self.user, - subsection_grade=MagicMock(), + course_id=unicode(self.course.id), + usage_id=unicode(self.test_usage_key) ) mock_evaluate.assert_not_called() diff --git a/lms/djangoapps/grades/module_grades.py b/lms/djangoapps/grades/module_grades.py new file mode 100644 index 0000000000..a0e95a57e9 --- /dev/null +++ b/lms/djangoapps/grades/module_grades.py @@ -0,0 +1,96 @@ +""" +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) diff --git a/lms/djangoapps/grades/new/subsection_grade.py b/lms/djangoapps/grades/new/subsection_grade.py index 8f722523ec..74a3f90f99 100644 --- a/lms/djangoapps/grades/new/subsection_grade.py +++ b/lms/djangoapps/grades/new/subsection_grade.py @@ -112,14 +112,14 @@ class SubsectionGrade(object): """ Saves the subsection grade in a persisted model. """ - self._log_event(log.info, u"create_model", student) + self._log_event(log.debug, u"create_model", student) return PersistentSubsectionGrade.create_grade(**self._persisted_model_params(student)) def update_or_create_model(self, student): """ Saves or updates the subsection grade in a persisted model. """ - self._log_event(log.info, u"update_or_create_model", student) + self._log_event(log.debug, u"update_or_create_model", student) return PersistentSubsectionGrade.update_or_create_grade(**self._persisted_model_params(student)) def _compute_block_score( @@ -251,6 +251,11 @@ 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) @@ -259,10 +264,8 @@ class SubsectionGradeFactory(object): self.student, block_structure, self._submissions_scores, self._csm_scores ) - if PersistentGradesEnabledFlag.feature_enabled(self.course.id): - grade_model = subsection_grade.update_or_create_model(self.student) - self._update_saved_subsection_grade(subsection.location, grade_model) - + grade_model = subsection_grade.update_or_create_model(self.student) + self._update_saved_subsection_grade(subsection.location, grade_model) return subsection_grade @lazy diff --git a/lms/djangoapps/grades/scores.py b/lms/djangoapps/grades/scores.py index 86832ef156..22e378bda6 100644 --- a/lms/djangoapps/grades/scores.py +++ b/lms/djangoapps/grades/scores.py @@ -108,20 +108,23 @@ def get_score(submissions_scores, csm_scores, persisted_block, block): _get_score_from_persisted_or_latest_block(persisted_block, block, weight) ) - assert weighted_possible is not None - has_valid_denominator = weighted_possible > 0.0 - graded = _get_graded_from_block(persisted_block, block) if has_valid_denominator else False + if weighted_possible is None or weighted_earned is None: + return None - return ProblemScore( - raw_earned, - raw_possible, - weighted_earned, - weighted_possible, - weight, - graded, - display_name=display_name_with_default_escaped(block), - module_id=block.location, - ) + else: + has_valid_denominator = weighted_possible > 0.0 + graded = _get_graded_from_block(persisted_block, block) if has_valid_denominator else False + + return ProblemScore( + raw_earned, + raw_possible, + weighted_earned, + weighted_possible, + weight, + graded, + display_name=display_name_with_default_escaped(block), + module_id=block.location, + ) def weighted_score(raw_earned, raw_possible, weight): @@ -191,7 +194,10 @@ def _get_score_from_persisted_or_latest_block(persisted_block, block, weight): else: raw_possible = block.transformer_data[GradesTransformer].max_score - return (raw_earned, raw_possible) + weighted_score(raw_earned, raw_possible, weight) + if raw_possible is None: + return (raw_earned, raw_possible) + (None, None) + else: + return (raw_earned, raw_possible) + weighted_score(raw_earned, raw_possible, weight) def _get_weight_from_block(persisted_block, block): diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index 56dd9017ad..b13fefd7c9 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -13,7 +13,7 @@ from openedx.core.djangoapps.content.block_structure.api import get_course_in_ca from student.models import user_by_anonymous_id from submissions.models import score_set, score_reset -from .signals import SCORE_CHANGED, SUBSECTION_SCORE_UPDATED +from .signals import SCORE_CHANGED from ..config.models import PersistentGradesEnabledFlag from ..transformer import GradesTransformer from ..new.subsection_grade import SubsectionGradeFactory @@ -95,6 +95,8 @@ def recalculate_subsection_grade_handler(sender, **kwargs): # pylint: disable=u """ student = kwargs['user'] course_key = CourseLocator.from_string(kwargs['course_id']) + if not PersistentGradesEnabledFlag.feature_enabled(course_key): + return scored_block_usage_key = UsageKey.from_string(kwargs['usage_id']).replace(course_key=course_key) collected_block_structure = get_course_in_cache(course_key) @@ -113,12 +115,6 @@ def recalculate_subsection_grade_handler(sender, **kwargs): # pylint: disable=u subsection_usage_key, collected_block_structure=collected_block_structure, ) - subsection_grade = subsection_grade_factory.update( + subsection_grade_factory.update( transformed_subsection_structure[subsection_usage_key], transformed_subsection_structure ) - SUBSECTION_SCORE_UPDATED.send( - sender=None, - course=course, - user=student, - subsection_grade=subsection_grade, - ) diff --git a/lms/djangoapps/grades/signals/signals.py b/lms/djangoapps/grades/signals/signals.py index b3a3b67f8e..250b27e9ab 100644 --- a/lms/djangoapps/grades/signals/signals.py +++ b/lms/djangoapps/grades/signals/signals.py @@ -14,20 +14,8 @@ SCORE_CHANGED = Signal( providing_args=[ 'points_possible', # Maximum score available for the exercise 'points_earned', # Score obtained by the user - 'user', # User object + 'user_id', # Integer User ID 'course_id', # Unicode string representing the course 'usage_id' # Unicode string indicating the courseware instance ] ) - - -# 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_UPDATED = Signal( - providing_args=[ - 'course', # Course object - 'user', # User object - 'subsection_grade', # SubsectionGrade object - ] -) diff --git a/lms/djangoapps/grades/tests/test_grades.py b/lms/djangoapps/grades/tests/test_grades.py index 038fff9147..655bd6f7ca 100644 --- a/lms/djangoapps/grades/tests/test_grades.py +++ b/lms/djangoapps/grades/tests/test_grades.py @@ -10,7 +10,11 @@ from nose.plugins.attrib import attr from opaque_keys.edx.locations import SlashSeparatedCourseKey from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory -from courseware.tests.helpers import get_request_for_user +from courseware.model_data import set_score +from courseware.tests.helpers import ( + LoginEnrollmentTestCase, + get_request_for_user +) from lms.djangoapps.course_blocks.api import get_course_blocks from student.tests.factories import UserFactory from student.models import CourseEnrollment @@ -23,6 +27,7 @@ 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 @@ -337,3 +342,194 @@ 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) diff --git a/lms/djangoapps/grades/tests/test_signals.py b/lms/djangoapps/grades/tests/test_signals.py index 51705a0460..2054f967f5 100644 --- a/lms/djangoapps/grades/tests/test_signals.py +++ b/lms/djangoapps/grades/tests/test_signals.py @@ -244,7 +244,7 @@ class ScoreChangedUpdatesSubsectionGradeTest(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(3): + with check_mongo_calls(2) and self.assertNumQueries(0): recalculate_subsection_grade_handler(None, **self.score_changed_kwargs) @skip("Pending completion of TNL-5089") diff --git a/lms/djangoapps/grades/tests/utils.py b/lms/djangoapps/grades/tests/utils.py index 951e800329..0d640100bd 100644 --- a/lms/djangoapps/grades/tests/utils.py +++ b/lms/djangoapps/grades/tests/utils.py @@ -30,14 +30,12 @@ def mock_get_score(earned=0, possible=1): def answer_problem(course, request, problem, score=1, max_value=1): """ - Records an answer for the given problem. + Records a correct answer for the given problem. Arguments: course (Course): Course object, the course the required problem is in request (Request): request Object problem (xblock): xblock object, the problem to be answered - score (float): The new score for the problem - max_value (float): The new maximum score for the problem """ user = request.user @@ -48,10 +46,11 @@ def answer_problem(course, request, problem, score=1, max_value=1): course, depth=2 ) + # pylint: disable=protected-access module = get_module( user, request, - problem.location, + problem.scope_ids.usage_id, field_data_cache, - ) - module.runtime.publish(problem, 'grade', grade_dict) + )._xmodule + module.system.publish(problem, 'grade', grade_dict) diff --git a/lms/djangoapps/grades/transformer.py b/lms/djangoapps/grades/transformer.py index df2e0a4b37..481c2e700c 100644 --- a/lms/djangoapps/grades/transformer.py +++ b/lms/djangoapps/grades/transformer.py @@ -3,6 +3,7 @@ Grades Transformer """ from django.test.client import RequestFactory from functools import reduce as functools_reduce +from logging import getLogger from courseware.model_data import FieldDataCache from courseware.module_render import get_module_for_descriptor @@ -11,6 +12,9 @@ from openedx.core.lib.block_structure.transformer import BlockStructureTransform from openedx.core.djangoapps.util.user_utils import SystemUser +log = getLogger(__name__) + + class GradesTransformer(BlockStructureTransformer): """ The GradesTransformer collects grading information and stores it on @@ -119,8 +123,10 @@ class GradesTransformer(BlockStructureTransformer): Collect the `max_score` from the given module, storing it as a `transformer_block_field` associated with the `GradesTransformer`. """ - score = module.max_score() - block_structure.set_transformer_block_field(module.location, cls, 'max_score', score) + max_score = module.max_score() + block_structure.set_transformer_block_field(module.location, cls, 'max_score', max_score) + if max_score is None: + log.warning("GradesTransformer: max_score is None for {}".format(module.location)) @staticmethod def _iter_scorable_xmodules(block_structure): diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index becae8f2ba..69acebff36 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -18,8 +18,8 @@ from courseware.model_data import FieldDataCache from courseware.module_render import get_module_for_descriptor from courseware.models import StudentModule from edxmako.shortcuts import render_to_string -from grades.scores import weighted_score -from grades.signals.signals import SCORE_CHANGED +from lms.djangoapps.grades.scores import weighted_score +from lms.djangoapps.grades.signals.signals import SCORE_CHANGED from lang_pref import LANGUAGE_KEY from student.models import CourseEnrollment, CourseEnrollmentAllowed from submissions import api as sub_api # installed from the edx-submissions repository @@ -317,7 +317,11 @@ def _fire_score_changed_for_block(course_id, student, block, module_state_key): field_data_cache=cache, course_key=course_id ) - points_earned, points_possible = weighted_score(0, module.max_score(), getattr(module, 'weight', None)) + max_score = module.max_score() + if max_score is None: + return + else: + points_earned, points_possible = weighted_score(0, max_score, getattr(module, 'weight', None)) else: points_earned, points_possible = 0, 0 SCORE_CHANGED.send( @@ -325,8 +329,8 @@ def _fire_score_changed_for_block(course_id, student, block, module_state_key): points_possible=points_possible, points_earned=points_earned, user=student, - course_id=course_id, - usage_id=module_state_key + course_id=unicode(course_id), + usage_id=unicode(module_state_key) ) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 312a40adab..4a1c292eca 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -3190,7 +3190,8 @@ class TestInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginEnrollmentTes }) self.assertEqual(response.status_code, 400) - def test_reset_student_attempts_delete(self): + @patch('courseware.module_render.SCORE_CHANGED.send') + def test_reset_student_attempts_delete(self, _mock_signal): """ Test delete single student state. """ url = reverse('reset_student_attempts', kwargs={'course_id': self.course.id.to_deprecated_string()}) response = self.client.post(url, { diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index 0b65f1c8a4..ca7b0f3aea 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -378,7 +378,8 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase): reset_student_attempts(self.course_key, self.user, msk, requesting_user=self.user) self.assertEqual(json.loads(module().state)['attempts'], 0) - def test_delete_student_attempts(self): + @mock.patch('courseware.module_render.SCORE_CHANGED.send') + def test_delete_student_attempts(self, _mock_signal): msk = self.course_key.make_usage_key('dummy', 'module') original_state = json.dumps({'attempts': 32, 'otherstuff': 'alsorobots'}) StudentModule.objects.create( @@ -404,7 +405,7 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase): # Disable the score change signal to prevent other components from being # pulled into tests. @mock.patch('courseware.module_render.SCORE_CHANGED.send') - def test_delete_submission_scores(self, _lti_mock): + def test_delete_submission_scores(self, _mock_signal): user = UserFactory() problem_location = self.course_key.make_usage_key('dummy', 'module') @@ -548,7 +549,7 @@ class TestStudentModuleGrading(SharedModuleStoreTestCase): self.course, get_course_blocks(self.user, self.course.location) ) - grade = subsection_grade_factory.update(self.sequence) + grade = subsection_grade_factory.create(self.sequence) self.assertEqual(grade.all_total.earned, all_earned) self.assertEqual(grade.graded_total.earned, graded_earned) self.assertEqual(grade.all_total.possible, all_possible) diff --git a/lms/djangoapps/instructor/tests/test_services.py b/lms/djangoapps/instructor/tests/test_services.py index 2cf59a663e..648f6350ee 100644 --- a/lms/djangoapps/instructor/tests/test_services.py +++ b/lms/djangoapps/instructor/tests/test_services.py @@ -49,7 +49,8 @@ class InstructorServiceTests(SharedModuleStoreTestCase): state=json.dumps({'attempts': 2}), ) - def test_reset_student_attempts_delete(self): + @mock.patch('courseware.module_render.SCORE_CHANGED.send') + def test_reset_student_attempts_delete(self, _mock_signal): """ Test delete student state. """