From 4fe5f3457d3dc7041df87626c30c9c5f645fa2f9 Mon Sep 17 00:00:00 2001 From: Bill Filler Date: Tue, 5 Sep 2017 10:47:56 -0400 Subject: [PATCH] Conditionally display gated content in courseware Display gated sections in course outline, navigation and in the course when user has met prerequiste condition. WL-1273, WL-1317 --- common/djangoapps/util/milestones_helpers.py | 8 +- common/lib/xmodule/xmodule/seq_module.py | 94 +++++++++-- .../xmodule/xmodule/tests/test_sequence.py | 95 +++++++++++ .../test/acceptance/pages/lms/course_home.py | 2 +- .../acceptance/tests/lms/test_lms_gating.py | 17 +- lms/djangoapps/course_api/blocks/api.py | 6 +- .../blocks/transformers/milestones.py | 12 +- .../transformers/tests/test_milestones.py | 19 ++- lms/djangoapps/courseware/access.py | 1 - lms/djangoapps/courseware/module_render.py | 2 + .../courseware/tests/test_module_render.py | 2 +- lms/djangoapps/courseware/tests/test_views.py | 3 +- lms/djangoapps/gating/api.py | 32 +--- lms/djangoapps/gating/tests/test_api.py | 14 +- .../gating/tests/test_integration.py | 18 +- .../grades/subsection_grade_factory.py | 4 +- lms/templates/_gated_content.html | 26 +++ lms/templates/seq_module.html | 15 +- manage.py | 1 - openedx/core/lib/gating/api.py | 154 +++++++++++++++++- openedx/core/lib/gating/services.py | 55 +++++++ openedx/core/lib/gating/tests/test_api.py | 76 ++++++++- .../course-outline-fragment.html | 31 +++- .../tests/views/test_course_home.py | 2 +- .../tests/views/test_course_outline.py | 138 ++++++++++++++++ .../tests/views/test_course_updates.py | 2 +- openedx/features/course_experience/utils.py | 2 +- .../course_experience/views/course_outline.py | 26 +++ 28 files changed, 753 insertions(+), 104 deletions(-) create mode 100644 lms/templates/_gated_content.html create mode 100644 openedx/core/lib/gating/services.py diff --git a/common/djangoapps/util/milestones_helpers.py b/common/djangoapps/util/milestones_helpers.py index fe2b6e2547..f73d3bf208 100644 --- a/common/djangoapps/util/milestones_helpers.py +++ b/common/djangoapps/util/milestones_helpers.py @@ -344,11 +344,14 @@ def add_course_content_milestone(course_id, content_id, relationship, milestone) return milestones_api.add_course_content_milestone(course_id, content_id, relationship, milestone) -def get_course_content_milestones(course_id, content_id, relationship, user_id=None): +def get_course_content_milestones(course_id, content_id=None, relationship='requires', user_id=None): """ Client API operation adapter/wrapper Uses the request cache to store all of a user's milestones + + Returns all content blocks in a course if content_id is None, otherwise it just returns that + specific content block. """ if not settings.FEATURES.get('MILESTONES_APP'): return [] @@ -367,6 +370,9 @@ def get_course_content_milestones(course_id, content_id, relationship, user_id=N user={"id": user_id} ) + if content_id is None: + return request_cache_dict[user_id][relationship] + return [m for m in request_cache_dict[user_id][relationship] if m['content_id'] == unicode(content_id)] diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 8f81cd9568..06002b001d 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -159,7 +159,7 @@ class ProctoringFields(object): @XBlock.wants('proctoring') @XBlock.wants('verification') -@XBlock.wants('milestones') +@XBlock.wants('gating') @XBlock.wants('credit') @XBlock.needs('user') @XBlock.needs('bookmarks') @@ -231,8 +231,6 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): banner_text, special_html = special_html_view if special_html and not masquerading_as_specific_student: return Fragment(special_html) - else: - banner_text = self._gated_content_staff_banner() return self._student_view(context, banner_text) def _special_exam_student_view(self): @@ -270,20 +268,6 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): return banner_text, hidden_content_html - def _gated_content_staff_banner(self): - """ - Checks whether the content is gated for learners. If so, - returns a banner_text depending on whether user is staff. - """ - milestones_service = self.runtime.service(self, 'milestones') - if milestones_service: - content_milestones = milestones_service.get_course_content_milestones( - self.course_id, self.location, 'requires' - ) - banner_text = _('This subsection is unlocked for learners when they meet the prerequisite requirements.') - if content_milestones and self.runtime.user_is_staff: - return banner_text - def _can_user_view_content(self, course): """ Returns whether the runtime user can view the content @@ -307,10 +291,21 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): """ display_items = self.get_display_items() self._update_position(context, len(display_items)) + prereq_met = True + prereq_meta_info = {} + + if self._required_prereq(): + if self.runtime.user_is_staff: + banner_text = _('This subsection is unlocked for learners when they meet the prerequisite requirements.') + else: + # check if prerequisite has been met + prereq_met, prereq_meta_info = self._compute_is_prereq_met(True) + if prereq_met and not self._is_gate_fulfilled(): + banner_text = _('This section is a prerequisite. You must complete this section in order to unlock additional content.') fragment = Fragment() params = { - 'items': self._render_student_view_for_items(context, display_items, fragment), + 'items': self._render_student_view_for_items(context, display_items, fragment) if prereq_met else [], 'element_id': self.location.html_id(), 'item_id': text_type(self.location), 'position': self.position, @@ -320,6 +315,7 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): 'prev_url': context.get('prev_url'), 'banner_text': banner_text, 'disable_navigation': not self.is_user_authenticated(context), + 'gated_content': self._get_gated_content_info(prereq_met, prereq_meta_info) } fragment.add_content(self.system.render_template("seq_module.html", params)) @@ -328,6 +324,68 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): return fragment + def _get_gated_content_info(self, prereq_met, prereq_meta_info): + """ + Returns a dict of information about gated_content context + """ + gated_content = {} + gated_content['gated'] = not prereq_met + gated_content['prereq_url'] = prereq_meta_info['url'] if not prereq_met else None + gated_content['prereq_section_name'] = prereq_meta_info['display_name'] if not prereq_met else None + gated_content['gated_section_name'] = self.display_name + + return gated_content + + def _is_gate_fulfilled(self): + """ + Determines if this section is a prereq and has any unfulfilled milestones. + + Returns: + True if section has no unfufilled milestones or is not a prerequisite. + False otherwise + """ + gating_service = self.runtime.service(self, 'gating') + if gating_service: + fulfilled = gating_service.is_gate_fulfilled( + self.course_id, self.location, self.runtime.user_id + ) + return fulfilled + + return True + + def _required_prereq(self): + """ + Checks whether a prerequisite is required for this Section + + Returns: + milestone if a prereq is required, None otherwise + """ + gating_service = self.runtime.service(self, 'gating') + if gating_service: + milestone = gating_service.required_prereq( + self.course_id, self.location, 'requires' + ) + return milestone + + return None + + def _compute_is_prereq_met(self, recalc_on_unmet): + """ + Evaluate if the user has completed the prerequisite + + Arguments: + recalc_on_unmet: Recalculate the subsection grade if prereq has not yet been met + + Returns: + tuple: True|False, + prereq_meta_info = { 'url': prereq_url, 'display_name': prereq_name} + """ + gating_service = self.runtime.service(self, 'gating') + if gating_service: + return gating_service.compute_is_prereq_met(self.location, self.runtime.user_id, recalc_on_unmet) + + return True, {} + def _update_position(self, context, number_of_display_items): """ Update the user's sequential position given the context and the diff --git a/common/lib/xmodule/xmodule/tests/test_sequence.py b/common/lib/xmodule/xmodule/tests/test_sequence.py index 29e13bae24..b953dc436c 100644 --- a/common/lib/xmodule/xmodule/tests/test_sequence.py +++ b/common/lib/xmodule/xmodule/tests/test_sequence.py @@ -118,6 +118,7 @@ class SequenceBlockTestCase(XModuleXmlImportTest): ) self._assert_view_at_position(html, expected_position=1) self.assertIn(unicode(self.sequence_3_1.location), html) + self.assertIn("'gated': False", html) self.assertIn("'next_url': 'NextSequential'", html) self.assertIn("'prev_url': 'PrevSequential'", html) @@ -178,3 +179,97 @@ class SequenceBlockTestCase(XModuleXmlImportTest): ) self.assertIn("hidden_content.html", html) self.assertIn(progress_url, html) + + def _assert_gated(self, html, sequence): + """ + Assert sequence content is gated + """ + self.assertIn("seq_module.html", html) + self.assertIn("'banner_text': None", html) + self.assertIn("'items': []", html) + self.assertIn("'gated': True", html) + self.assertIn("'prereq_url': 'PrereqUrl'", html) + self.assertIn("'prereq_section_name': 'PrereqSectionName'", html) + self.assertIn("'gated_section_name': u'{}'".format(unicode(sequence.display_name)), html) + self.assertIn("'next_url': 'NextSequential'", html) + self.assertIn("'prev_url': 'PrevSequential'", html) + + def _assert_prereq(self, html, sequence): + """ + Assert sequence is a prerequisite with unfulfilled gates + """ + self.assertIn("seq_module.html", html) + self.assertIn( + "'banner_text': 'This section is a prerequisite. " + "You must complete this section in order to unlock additional content.'", + html + ) + self.assertIn("'gated': False", html) + self.assertIn(unicode(sequence.location), html) + self.assertIn("'prereq_url': None", html) + self.assertIn("'prereq_section_name': None", html) + self.assertIn("'next_url': 'NextSequential'", html) + self.assertIn("'prev_url': 'PrevSequential'", html) + + def _assert_ungated(self, html, sequence): + """ + Assert sequence is not gated + """ + self.assertIn("seq_module.html", html) + self.assertIn("'banner_text': None", html) + self.assertIn("'gated': False", html) + self.assertIn(unicode(sequence.location), html) + self.assertIn("'prereq_url': None", html) + self.assertIn("'prereq_section_name': None", html) + self.assertIn("'next_url': 'NextSequential'", html) + self.assertIn("'prev_url': 'PrevSequential'", html) + + def test_gated_content(self): + """ + Test when sequence is both a prerequisite for a sequence + and gated on another prerequisite sequence + """ + # setup seq_1_2 as a gate and gated + gating_mock_1_2 = Mock() + gating_mock_1_2.return_value.is_gate_fulfilled.return_value = False + gating_mock_1_2.return_value.required_prereq.return_value = True + gating_mock_1_2.return_value.compute_is_prereq_met.return_value = [ + False, + {'url': 'PrereqUrl', 'display_name': 'PrereqSectionName'} + ] + self.sequence_1_2.xmodule_runtime._services['gating'] = gating_mock_1_2 # pylint: disable=protected-access + self.sequence_1_2.display_name = 'sequence_1_2' + + html = self._get_rendered_student_view( + self.sequence_1_2, + extra_context=dict(next_url='NextSequential', prev_url='PrevSequential'), + ) + + # expect content to be gated, with no banner + self._assert_gated(html, self.sequence_1_2) + + # change seq_1_2 to be ungated, but still a gate (prequiste) + gating_mock_1_2.return_value.is_gate_fulfilled.return_value = False + gating_mock_1_2.return_value.required_prereq.return_value = True + gating_mock_1_2.return_value.compute_is_prereq_met.return_value = [True, {}] + + html = self._get_rendered_student_view( + self.sequence_1_2, + extra_context=dict(next_url='NextSequential', prev_url='PrevSequential'), + ) + + # assert that content and preq banner is shown + self._assert_prereq(html, self.sequence_1_2) + + # change seq_1_2 to have no unfulfilled gates + gating_mock_1_2.return_value.is_gate_fulfilled.return_value = True + gating_mock_1_2.return_value.required_prereq.return_value = True + gating_mock_1_2.return_value.compute_is_prereq_met.return_value = [True, {}] + + html = self._get_rendered_student_view( + self.sequence_1_2, + extra_context=dict(next_url='NextSequential', prev_url='PrevSequential'), + ) + + # assert content shown as normal + self._assert_ungated(html, self.sequence_1_2) diff --git a/common/test/acceptance/pages/lms/course_home.py b/common/test/acceptance/pages/lms/course_home.py index 30ebde8ce4..41cb4a22c5 100644 --- a/common/test/acceptance/pages/lms/course_home.py +++ b/common/test/acceptance/pages/lms/course_home.py @@ -83,7 +83,7 @@ class CourseOutlinePage(PageObject): SECTION_SELECTOR = '.outline-item.section:nth-of-type({0})' SECTION_TITLES_SELECTOR = '.section-name h3' SUBSECTION_SELECTOR = SECTION_SELECTOR + ' .subsection:nth-of-type({1}) .outline-item' - SUBSECTION_TITLES_SELECTOR = SECTION_SELECTOR + ' .subsection .subsection-title' + SUBSECTION_TITLES_SELECTOR = SECTION_SELECTOR + ' .subsection .subsection-title .subsection-title-name' OUTLINE_RESUME_COURSE_SELECTOR = '.outline-item .resume-right' def __init__(self, browser, parent_page): diff --git a/common/test/acceptance/tests/lms/test_lms_gating.py b/common/test/acceptance/tests/lms/test_lms_gating.py index 90164ab38d..783b1d6a49 100644 --- a/common/test/acceptance/tests/lms/test_lms_gating.py +++ b/common/test/acceptance/tests/lms/test_lms_gating.py @@ -150,9 +150,13 @@ class GatingTest(UniqueCourseTest): """ Given that I am a student When I visit the LMS Courseware - Then I cannot see a gated subsection + Then I can see a gated subsection + The gated subsection should have a lock icon + and be in the format: " (Prerequisite Required)" When I fulfill the gating Prerequisite Then I can see the gated subsection + Now the gated subsection should have an unlock icon + and screen readers should read the section as: " Unlocked" """ self._setup_prereq() self._setup_gated_subsection() @@ -160,7 +164,7 @@ class GatingTest(UniqueCourseTest): self._auto_auth(self.STUDENT_USERNAME, self.STUDENT_EMAIL, False) self.course_home_page.visit() - self.assertEqual(self.course_home_page.outline.num_subsections, 1) + self.assertEqual(self.course_home_page.outline.num_subsections, 2) # Fulfill prerequisite and verify that gated subsection is shown self.courseware_page.visit() @@ -175,7 +179,9 @@ class GatingTest(UniqueCourseTest): Then I can see all gated subsections Displayed along with notification banners Then if I masquerade as a student - Then I cannot see a gated subsection + Then I can see a gated subsection + The gated subsection should have a lock icon + and be in the format: " (Prerequisite Required)" When I fufill the gating prerequisite Then I can see the gated subsection (without a banner) """ @@ -204,10 +210,11 @@ class GatingTest(UniqueCourseTest): self.course_home_page.visit() self.course_home_page.preview.set_staff_view_mode('Learner') - self.assertEqual(self.course_home_page.outline.num_subsections, 1) + self.assertEqual(self.course_home_page.outline.num_subsections, 2) self.course_home_page.outline.go_to_section('Test Section 1', 'Test Subsection 1') self.courseware_page.wait_for_page() - self.assertFalse(self.courseware_page.has_banner()) + # banner displayed informing section is a prereq + self.assertTrue(self.courseware_page.has_banner()) self.course_home_page.visit() self.course_home_page.preview.set_staff_view_mode_specific_student(self.STUDENT_USERNAME) diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index ff3a163dd6..bb2d42dd21 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -56,10 +56,14 @@ def get_blocks( requested_fields = [] include_completion = 'completion' in requested_fields include_special_exams = 'special_exam_info' in requested_fields + include_gated_sections = 'show_gated_sections' in requested_fields if user is not None: transformers += COURSE_BLOCK_ACCESS_TRANSFORMERS - transformers += [MilestonesAndSpecialExamsTransformer(include_special_exams), HiddenContentTransformer()] + transformers += [MilestonesAndSpecialExamsTransformer( + include_special_exams=include_special_exams, + include_gated_sections=include_gated_sections)] + transformers += [HiddenContentTransformer()] transformers += [ BlocksAPITransformer( block_counts, diff --git a/lms/djangoapps/course_api/blocks/transformers/milestones.py b/lms/djangoapps/course_api/blocks/transformers/milestones.py index c7309fd9dd..6f585acc46 100644 --- a/lms/djangoapps/course_api/blocks/transformers/milestones.py +++ b/lms/djangoapps/course_api/blocks/transformers/milestones.py @@ -21,8 +21,9 @@ class MilestonesAndSpecialExamsTransformer(BlockStructureTransformer): """ A transformer that handles both milestones and special (timed) exams. - It excludes all blocks with unfulfilled milestones from the student view. An entrance exam is considered a - milestone, and is not considered a "special exam". + It includes or excludes all unfulfilled milestones from the student view based on the value of `include_gated_sections`. + + An entrance exam is considered a milestone, and is not considered a "special exam". It also includes or excludes all special (timed) exams (timed, proctored, practice proctored) in/from the student view, based on the value of `include_special_exams`. @@ -35,8 +36,9 @@ class MilestonesAndSpecialExamsTransformer(BlockStructureTransformer): def name(cls): return "milestones" - def __init__(self, include_special_exams=True): + def __init__(self, include_special_exams=True, include_gated_sections=True): self.include_special_exams = include_special_exams + self.include_gated_sections = include_gated_sections @classmethod def collect(cls, block_structure): @@ -66,10 +68,10 @@ class MilestonesAndSpecialExamsTransformer(BlockStructureTransformer): if usage_info.has_staff_access: return False - elif self.has_pending_milestones_for_user(block_key, usage_info): - return True elif self.gated_by_required_content(block_key, block_structure, required_content): return True + elif not self.include_gated_sections and self.has_pending_milestones_for_user(block_key, usage_info): + return True elif (settings.FEATURES.get('ENABLE_SPECIAL_EXAMS', False) and (self.is_special_exam(block_key, block_structure) and not self.include_special_exams)): 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 c566a0ab91..7f097a1c10 100644 --- a/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py +++ b/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py @@ -137,18 +137,21 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM ( 'H', 'A', - ('course', 'A', 'B', 'C',) + ('course', 'A', 'B', 'C', 'H', 'I') ), ( 'H', 'ProctoredExam', - ('course', 'A', 'B', 'C'), + ('course', 'A', 'B', 'C', 'H', 'I'), ), ) @ddt.unpack 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 + Students should be able to see gated content blocks before and after they have completed the + prerequisite for it. + + First, checks that a student can 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. @@ -160,21 +163,21 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM self.course.enable_subsection_gating = True self.setup_gated_section(self.blocks[gated_block_ref], self.blocks[gating_block_ref]) - with self.assertNumQueries(8): + with self.assertNumQueries(6): self.get_blocks_and_check_against_expected(self.user, expected_blocks_before_completion) # clear the request cache to simulate a new request self.clear_caches() # this call triggers reevaluation of prerequisites fulfilled by the gating block. - with patch('gating.api._get_subsection_percentage', Mock(return_value=100)): + with patch('openedx.core.lib.gating.api._get_subsection_percentage', Mock(return_value=100)): lms_gating_api.evaluate_prerequisite( self.course, Mock(location=self.blocks[gating_block_ref].location), self.user, ) - with self.assertNumQueries(8): + with self.assertNumQueries(6): self.get_blocks_and_check_against_expected(self.user, self.ALL_BLOCKS_EXCEPT_SPECIAL) def test_staff_access(self): @@ -195,14 +198,14 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM self.course.enable_subsection_gating = True self.setup_gated_section(self.blocks['H'], self.blocks['A']) expected_blocks = ( - 'course', 'A', 'B', 'C', 'ProctoredExam', 'D', 'E', 'PracticeExam', 'F', 'G', 'TimedExam', 'J', 'K' + 'course', 'A', 'B', 'C', 'ProctoredExam', 'D', 'E', 'PracticeExam', 'F', 'G', 'TimedExam', 'J', 'K', 'H', 'I' ) self.get_blocks_and_check_against_expected(self.user, expected_blocks) # clear the request cache to simulate a new request self.clear_caches() # this call triggers reevaluation of prerequisites fulfilled by the gating block. - with patch('gating.api._get_subsection_percentage', Mock(return_value=100)): + with patch('openedx.core.lib.gating.api._get_subsection_percentage', Mock(return_value=100)): lms_gating_api.evaluate_prerequisite( self.course, Mock(location=self.blocks['A'].location), diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 665cfde06b..bf981d497a 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -494,7 +494,6 @@ def _has_access_descriptor(user, action, descriptor, course_key=None): return ( _visible_to_nonstaff_users(descriptor) and - _can_access_descriptor_with_milestones(user, descriptor, course_key) and ( _has_detached_class_tag(descriptor) or check_start_date(user, descriptor.days_early_for_beta, descriptor.start, course_key) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 90aa704c3a..5f698c31ed 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -52,6 +52,7 @@ from openedx.core.djangoapps.crawlers.models import CrawlersConfig from openedx.core.djangoapps.credit.services import CreditService from openedx.core.djangoapps.monitoring_utils import set_custom_metrics_for_course_key, set_monitoring_transaction_name from openedx.core.djangoapps.util.user_utils import SystemUser +from openedx.core.lib.gating.services import GatingService from openedx.core.lib.license import wrap_with_license from openedx.core.lib.url_utils import quote_slashes, unquote_slashes from openedx.core.lib.xblock_utils import request_token as xblock_request_token @@ -762,6 +763,7 @@ def get_module_system_for_user( 'milestones': milestones_helpers.get_service(), 'credit': CreditService(), 'bookmarks': BookmarksService(user=user), + 'gating': GatingService(), }, get_user_role=lambda: get_user_role(user, course_id), descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 6a1229f738..a56b260467 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -1285,7 +1285,7 @@ class TestGatedSubsectionRendering(SharedModuleStoreTestCase, MilestonesTestCase self.field_data_cache ) self.assertIsNotNone(self._find_sequential(actual['chapters'], 'Chapter', 'Open_Sequential')) - self.assertIsNone(self._find_sequential(actual['chapters'], 'Chapter', 'Gated_Sequential')) + self.assertIsNotNone(self._find_sequential(actual['chapters'], 'Chapter', 'Gated_Sequential')) self.assertIsNone(self._find_sequential(actual['chapters'], 'Non-existent_Chapter', 'Non-existent_Sequential')) self.assertIsNone(actual['previous_of_active_section']) self.assertIsNone(actual['next_of_active_section']) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 56151704e4..b56d0a865d 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -2401,7 +2401,8 @@ class TestIndexViewWithGating(ModuleStoreTestCase, MilestonesTestCaseMixin): ) ) - self.assertEquals(response.status_code, 404) + self.assertEquals(response.status_code, 200) + self.assertIn("Content Locked", response.content) class TestRenderXBlock(RenderXBlockTestMixin, ModuleStoreTestCase): diff --git a/lms/djangoapps/gating/api.py b/lms/djangoapps/gating/api.py index e31b66b1d2..84ce71eefe 100644 --- a/lms/djangoapps/gating/api.py +++ b/lms/djangoapps/gating/api.py @@ -31,37 +31,7 @@ def evaluate_prerequisite(course, subsection_grade, user): gated_content = gated_content_milestones.get(prereq_milestone['id']) if gated_content: for milestone in gated_content: - min_percentage = _get_minimum_required_percentage(milestone) - subsection_percentage = _get_subsection_percentage(subsection_grade) - if subsection_percentage >= min_percentage: - milestones_helpers.add_user_milestone({'id': user.id}, prereq_milestone) - else: - milestones_helpers.remove_user_milestone({'id': user.id}, prereq_milestone) - - -def _get_minimum_required_percentage(milestone): - """ - Returns the minimum percentage requirement for the given milestone. - """ - # 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( - u'Gating: Failed to find minimum score for gating milestone %s, defaulting to 100', - json.dumps(milestone) - ) - return min_score - - -def _get_subsection_percentage(subsection_grade): - """ - Returns the percentage value of the given subsection_grade. - """ - return subsection_grade.percent_graded * 100.0 + gating_api.update_milestone(milestone, subsection_grade, prereq_milestone, user.id) def evaluate_entrance_exam(course_grade, user): diff --git a/lms/djangoapps/gating/tests/test_api.py b/lms/djangoapps/gating/tests/test_api.py index 6a5ddec153..be601e935e 100644 --- a/lms/djangoapps/gating/tests/test_api.py +++ b/lms/djangoapps/gating/tests/test_api.py @@ -78,7 +78,7 @@ class TestEvaluatePrerequisite(GatingTestCase, MilestonesTestCaseMixin): 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_subsection_percentage') + @patch('openedx.core.lib.gating.api._get_subsection_percentage') @data((50, True), (100, True), (0, False)) @unpack def test_min_score_achieved(self, module_score, result, mock_score): @@ -88,24 +88,24 @@ class TestEvaluatePrerequisite(GatingTestCase, MilestonesTestCaseMixin): evaluate_prerequisite(self.course, self.subsection_grade, self.user) self.assertEqual(milestones_api.user_has_milestone(self.user_dict, self.prereq_milestone), result) - @patch('gating.api.log.warning') - @patch('gating.api._get_subsection_percentage') + @patch('openedx.core.lib.gating.api._get_subsection_percentage') + @patch('openedx.core.lib.gating.api._get_minimum_required_percentage') @data((50, False), (100, True)) @unpack - def test_invalid_min_score(self, module_score, result, mock_score, mock_log): + def test_invalid_min_score(self, module_score, result, mock_min_score, mock_score): self._setup_gating_milestone(None) mock_score.return_value = module_score + mock_min_score.return_value = 100 evaluate_prerequisite(self.course, self.subsection_grade, self.user) self.assertEqual(milestones_api.user_has_milestone(self.user_dict, self.prereq_milestone), result) - self.assertTrue(mock_log.called) - @patch('gating.api._get_subsection_percentage') + @patch('openedx.core.lib.gating.api._get_subsection_percentage') def test_no_prerequisites(self, mock_score): evaluate_prerequisite(self.course, self.subsection_grade, self.user) self.assertFalse(mock_score.called) - @patch('gating.api._get_subsection_percentage') + @patch('openedx.core.lib.gating.api._get_subsection_percentage') def test_no_gated_content(self, mock_score): gating_api.add_prerequisite(self.course.id, self.seq1.location) diff --git a/lms/djangoapps/gating/tests/test_integration.py b/lms/djangoapps/gating/tests/test_integration.py index e833606526..6616935239 100644 --- a/lms/djangoapps/gating/tests/test_integration.py +++ b/lms/djangoapps/gating/tests/test_integration.py @@ -120,7 +120,7 @@ class TestGatedContent(MilestonesTestCaseMixin, SharedModuleStoreTestCase): gating_api.set_required_content(self.course.id, str(self.seq2.location), str(self.seq1.location), min_score) self.prereq_milestone = gating_api.get_gating_milestone(self.course.id, self.seq1.location, 'fulfills') - def assert_access_to_gated_content(self, user, expected_access): + def assert_access_to_gated_content(self, user): """ Verifies access to gated content for the given user is as expected. """ @@ -130,8 +130,8 @@ class TestGatedContent(MilestonesTestCaseMixin, SharedModuleStoreTestCase): # 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) + # access to gated content (seq2) remains constant, access is prevented in SeqModule loading + self.assertTrue(bool(has_access(user, 'load', self.seq2, self.course.id))) def assert_user_has_prereq_milestone(self, user, expected_has_milestone): """ @@ -157,11 +157,11 @@ class TestGatedContent(MilestonesTestCaseMixin, SharedModuleStoreTestCase): def test_gated_for_nonstaff(self): self.assert_user_has_prereq_milestone(self.non_staff_user, expected_has_milestone=False) - self.assert_access_to_gated_content(self.non_staff_user, expected_access=False) + self.assert_access_to_gated_content(self.non_staff_user) def test_not_gated_for_staff(self): self.assert_user_has_prereq_milestone(self.staff_user, expected_has_milestone=False) - self.assert_access_to_gated_content(self.staff_user, expected_access=True) + self.assert_access_to_gated_content(self.staff_user) def test_gated_content_always_in_grades(self): # start with a grade from a non-gated subsection @@ -169,7 +169,7 @@ class TestGatedContent(MilestonesTestCaseMixin, SharedModuleStoreTestCase): # verify gated status and overall course grade percentage self.assert_user_has_prereq_milestone(self.non_staff_user, expected_has_milestone=False) - self.assert_access_to_gated_content(self.non_staff_user, expected_access=False) + self.assert_access_to_gated_content(self.non_staff_user) self.assert_course_grade(self.non_staff_user, .33) # fulfill the gated requirements @@ -177,16 +177,16 @@ class TestGatedContent(MilestonesTestCaseMixin, SharedModuleStoreTestCase): # verify gated status and overall course grade percentage self.assert_user_has_prereq_milestone(self.non_staff_user, expected_has_milestone=True) - self.assert_access_to_gated_content(self.non_staff_user, expected_access=True) + self.assert_access_to_gated_content(self.non_staff_user) self.assert_course_grade(self.non_staff_user, .67) @ddt.data((1, 1, True), (1, 2, True), (1, 3, False), (0, 1, False)) @ddt.unpack def test_ungating_when_fulfilled(self, earned, max_possible, result): self.assert_user_has_prereq_milestone(self.non_staff_user, expected_has_milestone=False) - self.assert_access_to_gated_content(self.non_staff_user, expected_access=False) + self.assert_access_to_gated_content(self.non_staff_user) answer_problem(self.course, self.request, self.gating_prob1, earned, max_possible) self.assert_user_has_prereq_milestone(self.non_staff_user, expected_has_milestone=result) - self.assert_access_to_gated_content(self.non_staff_user, expected_access=result) + self.assert_access_to_gated_content(self.non_staff_user) diff --git a/lms/djangoapps/grades/subsection_grade_factory.py b/lms/djangoapps/grades/subsection_grade_factory.py index fbf57fbc5c..78ad8af94d 100644 --- a/lms/djangoapps/grades/subsection_grade_factory.py +++ b/lms/djangoapps/grades/subsection_grade_factory.py @@ -63,7 +63,7 @@ class SubsectionGradeFactory(object): ) self._unsaved_subsection_grades.clear() - def update(self, subsection, only_if_higher=None, score_deleted=False, force_update_subsections=False): + def update(self, subsection, only_if_higher=None, score_deleted=False, force_update_subsections=False, persist_grade=True): """ Updates the SubsectionGrade object for the student and subsection. """ @@ -73,7 +73,7 @@ class SubsectionGradeFactory(object): subsection, self.course_data.structure, self._submissions_scores, self._csm_scores, ) - if should_persist_grades(self.course_data.course_key): + if persist_grade and should_persist_grades(self.course_data.course_key): if only_if_higher: try: grade_model = PersistentSubsectionGrade.read_grade(self.student.id, subsection.location) diff --git a/lms/templates/_gated_content.html b/lms/templates/_gated_content.html new file mode 100644 index 0000000000..ac2477f823 --- /dev/null +++ b/lms/templates/_gated_content.html @@ -0,0 +1,26 @@ +<%page args="prereq_url, prereq_section_name, gated_section_name" expression_filter="h"/> + +<%! +from django.utils.translation import ugettext as _ +from openedx.core.djangolib.markup import Text +%> + +
+

+ ${gated_section_name} + ${_("Content Locked")} +

+

+

+ ${_("Content Locked")} +

+

+ ${Text(_( + "You must complete the section '{prereq_section_name}' with the required score before you are able to view this content." + )).format(prereq_section_name=prereq_section_name)} +

+ ${_("Go to Prerequisite Section")} + +

+

+
diff --git a/lms/templates/seq_module.html b/lms/templates/seq_module.html index e806740941..bf70279ded 100644 --- a/lms/templates/seq_module.html +++ b/lms/templates/seq_module.html @@ -4,7 +4,8 @@
% if banner_text:
- + + ${_('Important!')} 

${banner_text} @@ -23,6 +24,13 @@

+ % if gated_content['gated']: + <%include file="_gated_content.html" args="prereq_url=gated_content['prereq_url'], prereq_section_name=gated_content['prereq_section_name'], gated_section_name=gated_content['gated_section_name']"/> + % else:
% for idx, item in enumerate(items): @@ -59,6 +71,7 @@
% endfor
+ % endif