diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py index f32cf63fb256d3813add426b39d9e4ca0fc38095..7330305b3bb9cb186c832286e8fc69ee7bd43153 100644 --- a/cms/djangoapps/contentstore/signals/handlers.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -102,7 +102,7 @@ def handle_item_deleted(**kwargs): # Remove prerequisite milestone data gating_api.remove_prerequisite(module.location) # Remove any 'requires' course content milestone relationships - gating_api.set_required_content(course_key, module.location, None, None) + gating_api.set_required_content(course_key, module.location, None, None, None) @receiver(GRADING_POLICY_CHANGED) diff --git a/cms/djangoapps/contentstore/tests/test_gating.py b/cms/djangoapps/contentstore/tests/test_gating.py index b488cd31d80791c47e3cb8248b7e759916e60b74..e61da3ace538e94c2f35042f6513ca9f12a29edb 100644 --- a/cms/djangoapps/contentstore/tests/test_gating.py +++ b/cms/djangoapps/contentstore/tests/test_gating.py @@ -41,7 +41,7 @@ class TestHandleItemDeleted(ModuleStoreTestCase, MilestonesTestCaseMixin): display_name="Gated Sequential" ) gating_api.add_prerequisite(self.course.id, self.open_seq.location) - gating_api.set_required_content(self.course.id, self.gated_seq.location, self.open_seq.location, 100) + gating_api.set_required_content(self.course.id, self.gated_seq.location, self.open_seq.location, 100, 100) @patch('contentstore.signals.handlers.gating_api.set_required_content') @patch('contentstore.signals.handlers.gating_api.remove_prerequisite') @@ -49,7 +49,9 @@ class TestHandleItemDeleted(ModuleStoreTestCase, MilestonesTestCaseMixin): """ Test gating milestone data is cleanup up when course content item is deleted """ handle_item_deleted(usage_key=self.chapter.location, user_id=0) mock_remove_prereq.assert_called_with(self.open_seq.location) - mock_set_required.assert_called_with(self.open_seq.location.course_key, self.open_seq.location, None, None) + mock_set_required.assert_called_with( + self.open_seq.location.course_key, self.open_seq.location, None, None, None + ) @patch('contentstore.signals.handlers.gating_api.set_required_content') @patch('contentstore.signals.handlers.gating_api.remove_prerequisite') @@ -57,4 +59,6 @@ class TestHandleItemDeleted(ModuleStoreTestCase, MilestonesTestCaseMixin): """ Test gating milestone data is cleanup up when course content item is deleted """ handle_item_deleted(usage_key=self.open_seq.location, user_id=0) mock_remove_prereq.assert_called_with(self.open_seq.location) - mock_set_required.assert_called_with(self.open_seq.location.course_key, self.open_seq.location, None, None) + mock_set_required.assert_called_with( + self.open_seq.location.course_key, self.open_seq.location, None, None, None + ) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 1f22a5b77b0013418e8fbb553cae08958b91fda4..5baf93c53ad9dfac201c3b667dace7cd2a0199f8 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -135,7 +135,9 @@ def xblock_handler(request, usage_key_string): :isPrereq: Set this xblock as a prerequisite which can be used to limit access to other xblocks :prereqUsageKey: Use the xblock identified by this usage key to limit access to this xblock :prereqMinScore: The minimum score that needs to be achieved on the prerequisite xblock - identifed by prereqUsageKey + identifed by prereqUsageKey. Ranging from 0 to 100. + :prereqMinCompletion: The minimum completion percentage that needs to be achieved on the + prerequisite xblock identifed by prereqUsageKey. Ranging from 0 to 100. :publish: can be: 'make_public': publish the content 'republish': publish this item *only* if it was previously published @@ -199,6 +201,7 @@ def xblock_handler(request, usage_key_string): is_prereq=request.json.get('isPrereq'), prereq_usage_key=request.json.get('prereqUsageKey'), prereq_min_score=request.json.get('prereqMinScore'), + prereq_min_completion=request.json.get('prereqMinCompletion'), publish=request.json.get('publish'), fields=request.json.get('fields'), ) @@ -480,7 +483,7 @@ def _update_with_callback(xblock, user, old_metadata=None, old_content=None): def _save_xblock(user, xblock, data=None, children_strings=None, metadata=None, nullout=None, grader_type=None, is_prereq=None, prereq_usage_key=None, prereq_min_score=None, - publish=None, fields=None): + prereq_min_completion=None, publish=None, fields=None): """ Saves xblock w/ its fields. Has special processing for grader_type, publish, and nullout and Nones in metadata. nullout means to truly set the field to None whereas nones in metadata mean to unset them (so they revert @@ -621,7 +624,11 @@ def _save_xblock(user, xblock, data=None, children_strings=None, metadata=None, if prereq_usage_key is not None: gating_api.set_required_content( - xblock.location.course_key, xblock.location, prereq_usage_key, prereq_min_score + xblock.location.course_key, + xblock.location, + prereq_usage_key, + prereq_min_score, + prereq_min_completion ) # If publish is set to 'republish' and this item is not in direct only categories and has previously been @@ -1050,12 +1057,13 @@ def _get_gating_info(course, xblock): info["prereqs"] = [ p for p in course.gating_prerequisites if unicode(xblock.location) not in p['namespace'] ] - prereq, prereq_min_score = gating_api.get_required_content( + prereq, prereq_min_score, prereq_min_completion = gating_api.get_required_content( course.id, xblock.location ) info["prereq"] = prereq info["prereq_min_score"] = prereq_min_score + info["prereq_min_completion"] = prereq_min_completion if prereq: info["visibility_state"] = VisibilityState.gated return info diff --git a/cms/djangoapps/contentstore/views/tests/test_gating.py b/cms/djangoapps/contentstore/views/tests/test_gating.py index ebbbf3444048ad08ffc6a40720d94c355eb5cff7..887e56f8eb4b303969749388574225a981296589 100644 --- a/cms/djangoapps/contentstore/views/tests/test_gating.py +++ b/cms/djangoapps/contentstore/views/tests/test_gating.py @@ -1,6 +1,7 @@ """ Unit tests for the gating feature in Studio """ +import ddt import json from mock import patch @@ -13,6 +14,7 @@ from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE from xmodule.modulestore.tests.factories import ItemFactory +@ddt.ddt class TestSubsectionGating(CourseTestCase): """ Tests for the subsection gating feature @@ -84,12 +86,13 @@ class TestSubsectionGating(CourseTestCase): self.client.ajax_post( self.seq2_url, - data={'prereqUsageKey': unicode(self.seq1.location), 'prereqMinScore': '100'} + data={'prereqUsageKey': unicode(self.seq1.location), 'prereqMinScore': '100', 'prereqMinCompletion': '100'} ) mock_set_required_content.assert_called_with( self.course.id, self.seq2.location, unicode(self.seq1.location), + '100', '100' ) @@ -101,21 +104,31 @@ class TestSubsectionGating(CourseTestCase): self.client.ajax_post( self.seq2_url, - data={'prereqUsageKey': '', 'prereqMinScore': ''} + data={'prereqUsageKey': '', 'prereqMinScore': '', 'prereqMinCompletion': ''} ) mock_set_required_content.assert_called_with( self.course.id, self.seq2.location, '', + '', '' ) @patch('contentstore.views.item.gating_api.get_prerequisites') @patch('contentstore.views.item.gating_api.get_required_content') @patch('contentstore.views.item.gating_api.is_prerequisite') - def test_get_prerequisite(self, mock_is_prereq, mock_get_required_content, mock_get_prereqs): + @ddt.data( + (90, None), + (None, 90), + (100, 100), + ) + @ddt.unpack + def test_get_prerequisite( + self, min_score, min_completion, + mock_is_prereq, mock_get_required_content, mock_get_prereqs + ): mock_is_prereq.return_value = True - mock_get_required_content.return_value = unicode(self.seq1.location), 100 + mock_get_required_content.return_value = unicode(self.seq1.location), min_score, min_completion mock_get_prereqs.return_value = [ {'namespace': '{}{}'.format(unicode(self.seq1.location), GATING_NAMESPACE_QUALIFIER)}, {'namespace': '{}{}'.format(unicode(self.seq2.location), GATING_NAMESPACE_QUALIFIER)} @@ -126,7 +139,8 @@ class TestSubsectionGating(CourseTestCase): mock_get_prereqs.assert_called_with(self.course.id) self.assertTrue(resp['is_prereq']) self.assertEqual(resp['prereq'], unicode(self.seq1.location)) - self.assertEqual(resp['prereq_min_score'], 100) + self.assertEqual(resp['prereq_min_score'], min_score) + self.assertEqual(resp['prereq_min_completion'], min_completion) self.assertEqual(resp['visibility_state'], VisibilityState.gated) @patch('contentstore.signals.handlers.gating_api.set_required_content') @@ -139,4 +153,4 @@ class TestSubsectionGating(CourseTestCase): ) self.client.delete(reverse_usage_url('xblock_handler', seq3.location)) mock_remove_prereq.assert_called_with(seq3.location) - mock_set_required.assert_called_with(seq3.location.course_key, seq3.location, None, None) + mock_set_required.assert_called_with(seq3.location.course_key, seq3.location, None, None, None) diff --git a/cms/static/js/spec/views/pages/course_outline_spec.js b/cms/static/js/spec/views/pages/course_outline_spec.js index 95dc045faa10623e8e20a7412c8600b9d8f2f172..bb9145e179417a0646536961aef70bf39578d48e 100644 --- a/cms/static/js/spec/views/pages/course_outline_spec.js +++ b/cms/static/js/spec/views/pages/course_outline_spec.js @@ -80,6 +80,7 @@ define(['jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', 'common/j prereqs: [], prereq: '', prereq_min_score: '', + prereq_min_completion: '', show_correctness: 'always', child_info: { category: 'vertical', @@ -1023,9 +1024,10 @@ define(['jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', 'common/j this.$('#is_prereq').prop('checked', true).trigger('change'); }; - selectLastPrerequisiteSubsection = function(minScore) { + selectLastPrerequisiteSubsection = function(minScore, minCompletion) { this.$('#prereq option:last').prop('selected', true).trigger('change'); this.$('#prereq_min_score').val(minScore).trigger('keyup'); + this.$('#prereq_min_completion').val(minCompletion).trigger('keyup'); }; // Helper to validate oft-checked additional option fields' visibility @@ -1045,6 +1047,7 @@ define(['jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', 'common/j delete mockSubsectionJSON.prereqs; delete mockSubsectionJSON.prereq; delete mockSubsectionJSON.prereq_min_score; + delete mockSubsectionJSON.prereq_min_completion; return createMockCourseJSON({ enable_proctored_exams: false, enable_timed_exams: false @@ -1611,10 +1614,12 @@ define(['jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', 'common/j ]); createCourseOutlinePage(this, mockCourseWithPreqsJSON, false); outlinePage.$('.outline-subsection .configure-button').click(); - selectLastPrerequisiteSubsection('80'); + selectLastPrerequisiteSubsection('80', '0'); expect($('#prereq_min_score_input').css('display')).not.toBe('none'); expect($('#prereq option:selected').val()).toBe('usage_key'); expect($('#prereq_min_score').val()).toBe('80'); + expect($('#prereq_min_completion_input').css('display')).not.toBe('none'); + expect($('#prereq_min_completion').val()).toBe('0'); $('.wrapper-modal-window .action-save').click(); }); @@ -1641,7 +1646,8 @@ define(['jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', 'common/j createMockSubsectionJSON({ prereqs: [{block_usage_key: 'usage_key', block_display_name: 'Prereq Subsection 1'}], prereq: 'usage_key', - prereq_min_score: '80' + prereq_min_score: '80', + prereq_min_completion: '50' }, []) ]) ]); @@ -1651,9 +1657,32 @@ define(['jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', 'common/j expect($('#prereq option:selected').val()).toBe('usage_key'); expect($('#prereq_min_score_input').css('display')).not.toBe('none'); expect($('#prereq_min_score').val()).toBe('80'); + expect($('#prereq_min_completion_input').css('display')).not.toBe('none'); + expect($('#prereq_min_completion').val()).toBe('50'); }); - it('can display validation error on non-integer minimum score', function() { + it('can show a saved prerequisite subsection with empty min score correctly', function() { + var mockCourseWithPreqsJSON = createMockCourseJSON({}, [ + createMockSectionJSON({}, [ + createMockSubsectionJSON({ + prereqs: [{block_usage_key: 'usage_key', block_display_name: 'Prereq Subsection 1'}], + prereq: 'usage_key', + prereq_min_score: '', + prereq_min_completion: '50' + }, []) + ]) + ]); + createCourseOutlinePage(this, mockCourseWithPreqsJSON, false); + outlinePage.$('.outline-subsection .configure-button').click(); + expect($('.gating-prereq').length).toBe(1); + expect($('#prereq option:selected').val()).toBe('usage_key'); + expect($('#prereq_min_score_input').css('display')).not.toBe('none'); + expect($('#prereq_min_score').val()).toBe('100'); + expect($('#prereq_min_completion_input').css('display')).not.toBe('none'); + expect($('#prereq_min_completion').val()).toBe('50'); + }); + + it('can display validation error on non-integer or empty minimum score', function() { var mockCourseWithPreqsJSON = createMockCourseJSON({}, [ createMockSectionJSON({}, [ createMockSubsectionJSON({ @@ -1663,12 +1692,39 @@ define(['jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', 'common/j ]); createCourseOutlinePage(this, mockCourseWithPreqsJSON, false); outlinePage.$('.outline-subsection .configure-button').click(); - selectLastPrerequisiteSubsection('abc'); + selectLastPrerequisiteSubsection('', '50'); + expect($('#prereq_min_score_error').css('display')).not.toBe('none'); + expect($('#prereq_min_completion_error').css('display')).toBe('none'); + expect($('.wrapper-modal-window .action-save').prop('disabled')).toBe(true); + expect($('.wrapper-modal-window .action-save').hasClass('is-disabled')).toBe(true); + selectLastPrerequisiteSubsection('50', ''); + expect($('#prereq_min_score_error').css('display')).toBe('none'); + expect($('#prereq_min_completion_error').css('display')).not.toBe('none'); + expect($('.wrapper-modal-window .action-save').prop('disabled')).toBe(true); + expect($('.wrapper-modal-window .action-save').hasClass('is-disabled')).toBe(true); + selectLastPrerequisiteSubsection('', ''); + expect($('#prereq_min_score_error').css('display')).not.toBe('none'); + expect($('#prereq_min_completion_error').css('display')).not.toBe('none'); + expect($('.wrapper-modal-window .action-save').prop('disabled')).toBe(true); + expect($('.wrapper-modal-window .action-save').hasClass('is-disabled')).toBe(true); + selectLastPrerequisiteSubsection('abc', '50'); expect($('#prereq_min_score_error').css('display')).not.toBe('none'); + expect($('#prereq_min_completion_error').css('display')).toBe('none'); expect($('.wrapper-modal-window .action-save').prop('disabled')).toBe(true); expect($('.wrapper-modal-window .action-save').hasClass('is-disabled')).toBe(true); - selectLastPrerequisiteSubsection('5.5'); + selectLastPrerequisiteSubsection('50', 'abc'); + expect($('#prereq_min_score_error').css('display')).toBe('none'); + expect($('#prereq_min_completion_error').css('display')).not.toBe('none'); + expect($('.wrapper-modal-window .action-save').prop('disabled')).toBe(true); + expect($('.wrapper-modal-window .action-save').hasClass('is-disabled')).toBe(true); + selectLastPrerequisiteSubsection('5.5', '50'); expect($('#prereq_min_score_error').css('display')).not.toBe('none'); + expect($('#prereq_min_completion_error').css('display')).toBe('none'); + expect($('.wrapper-modal-window .action-save').prop('disabled')).toBe(true); + expect($('.wrapper-modal-window .action-save').hasClass('is-disabled')).toBe(true); + selectLastPrerequisiteSubsection('50', '5.5'); + expect($('#prereq_min_score_error').css('display')).toBe('none'); + expect($('#prereq_min_completion_error').css('display')).not.toBe('none'); expect($('.wrapper-modal-window .action-save').prop('disabled')).toBe(true); expect($('.wrapper-modal-window .action-save').hasClass('is-disabled')).toBe(true); }); @@ -1683,12 +1739,24 @@ define(['jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', 'common/j ]); createCourseOutlinePage(this, mockCourseWithPreqsJSON, false); outlinePage.$('.outline-subsection .configure-button').click(); - selectLastPrerequisiteSubsection('-5'); + selectLastPrerequisiteSubsection('-5', '50'); expect($('#prereq_min_score_error').css('display')).not.toBe('none'); + expect($('#prereq_min_completion_error').css('display')).toBe('none'); + expect($('.wrapper-modal-window .action-save').prop('disabled')).toBe(true); + expect($('.wrapper-modal-window .action-save').hasClass('is-disabled')).toBe(true); + selectLastPrerequisiteSubsection('50', '-5'); + expect($('#prereq_min_score_error').css('display')).toBe('none'); + expect($('#prereq_min_completion_error').css('display')).not.toBe('none'); expect($('.wrapper-modal-window .action-save').prop('disabled')).toBe(true); expect($('.wrapper-modal-window .action-save').hasClass('is-disabled')).toBe(true); - selectLastPrerequisiteSubsection('105'); + selectLastPrerequisiteSubsection('105', '50'); expect($('#prereq_min_score_error').css('display')).not.toBe('none'); + expect($('#prereq_min_completion_error').css('display')).toBe('none'); + expect($('.wrapper-modal-window .action-save').prop('disabled')).toBe(true); + expect($('.wrapper-modal-window .action-save').hasClass('is-disabled')).toBe(true); + selectLastPrerequisiteSubsection('50', '105'); + expect($('#prereq_min_score_error').css('display')).toBe('none'); + expect($('#prereq_min_completion_error').css('display')).not.toBe('none'); expect($('.wrapper-modal-window .action-save').prop('disabled')).toBe(true); expect($('.wrapper-modal-window .action-save').hasClass('is-disabled')).toBe(true); }); @@ -1704,14 +1772,15 @@ define(['jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', 'common/j createCourseOutlinePage(this, mockCourseWithPreqsJSON, false); outlinePage.$('.outline-subsection .configure-button').click(); selectAdvancedSettings(); - selectLastPrerequisiteSubsection(''); - expect($('#prereq_min_score_error').css('display')).toBe('none'); - selectLastPrerequisiteSubsection('80'); + selectLastPrerequisiteSubsection('80', '50'); + expect($('#prereq_min_completion_error').css('display')).toBe('none'); expect($('#prereq_min_score_error').css('display')).toBe('none'); - selectLastPrerequisiteSubsection('0'); + selectLastPrerequisiteSubsection('0', '0'); expect($('#prereq_min_score_error').css('display')).toBe('none'); - selectLastPrerequisiteSubsection('100'); + expect($('#prereq_min_completion_error').css('display')).toBe('none'); + selectLastPrerequisiteSubsection('100', '100'); expect($('#prereq_min_score_error').css('display')).toBe('none'); + expect($('#prereq_min_completion_error').css('display')).toBe('none'); }); it('release date, due date, grading type, and staff lock can be cleared.', function() { diff --git a/cms/static/js/views/modals/course_outline_modals.js b/cms/static/js/views/modals/course_outline_modals.js index 67140eec54c218715bc70c1ee1cdd2427dfafa7a..6bff80e9772b92a88b53590de3712fdaf042de47 100644 --- a/cms/static/js/views/modals/course_outline_modals.js +++ b/cms/static/js/views/modals/course_outline_modals.js @@ -565,50 +565,68 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', className: 'edit-settings-access', events: { 'change #prereq': 'handlePrereqSelect', - 'keyup #prereq_min_score': 'validateMinScore' + 'keyup #prereq_min_completion': 'validateScoreAndCompletion', + 'keyup #prereq_min_score': 'validateScoreAndCompletion' }, afterRender: function() { AbstractEditor.prototype.afterRender.call(this); var prereq = this.model.get('prereq') || ''; - var prereq_min_score = this.model.get('prereq_min_score') || ''; + var prereqMinScore = this.model.get('prereq_min_score') || '100'; + var prereqMinCompletion = this.model.get('prereq_min_completion') || '100'; this.$('#is_prereq').prop('checked', this.model.get('is_prereq')); this.$('#prereq option[value="' + prereq + '"]').prop('selected', true); - this.$('#prereq_min_score').val(prereq_min_score); + this.$('#prereq_min_score').val(prereqMinScore); this.$('#prereq_min_score_input').toggle(prereq.length > 0); + this.$('#prereq_min_completion').val(prereqMinCompletion); + this.$('#prereq_min_completion_input').toggle(prereq.length > 0); }, handlePrereqSelect: function() { var showPrereqInput = this.$('#prereq option:selected').val().length > 0; this.$('#prereq_min_score_input').toggle(showPrereqInput); + this.$('#prereq_min_completion_input').toggle(showPrereqInput); }, - validateMinScore: function() { + isValidPercentage: function(val) { + var intVal = parseInt(val, 10); + return (typeof val !== 'undefined' && val !== '' && intVal >= 0 && intVal <= 100 && String(intVal) === val); + }, + validateScoreAndCompletion: function() { + var invalidInput = false; var minScore = this.$('#prereq_min_score').val().trim(); - var minScoreInt = parseInt(minScore); - // minScore needs to be an integer between 0 and 100 - if ( - minScore && - ( - typeof(minScoreInt) === 'undefined' || - String(minScoreInt) !== minScore || - minScoreInt < 0 || - minScoreInt > 100 - ) - ) { + var minCompletion = this.$('#prereq_min_completion').val().trim(); + + if (minScore === '' || !this.isValidPercentage(minScore)) { + invalidInput = true; this.$('#prereq_min_score_error').show(); - BaseModal.prototype.disableActionButton.call(this.parent, 'save'); } else { this.$('#prereq_min_score_error').hide(); + } + if (minCompletion === '' || !this.isValidPercentage(minCompletion)) { + invalidInput = true; + this.$('#prereq_min_completion_error').show(); + } else { + this.$('#prereq_min_completion_error').hide(); + } + if (invalidInput) { + BaseModal.prototype.disableActionButton.call(this.parent, 'save'); + } else { BaseModal.prototype.enableActionButton.call(this.parent, 'save'); } }, getRequestData: function() { var minScore = this.$('#prereq_min_score').val(); + var minCompletion = this.$('#prereq_min_completion').val(); if (minScore) { minScore = minScore.trim(); } + if (minCompletion) { + minCompletion = minCompletion.trim(); + } + return { isPrereq: this.$('#is_prereq').is(':checked'), prereqUsageKey: this.$('#prereq option:selected').val(), - prereqMinScore: minScore + prereqMinScore: minScore, + prereqMinCompletion: minCompletion }; } }); diff --git a/cms/static/sass/elements/_modal-window.scss b/cms/static/sass/elements/_modal-window.scss index 097a45a74ffcbddc80d505810c6a48d2624817cc..b683518f6abb233bdc1716b508074b963a158854 100644 --- a/cms/static/sass/elements/_modal-window.scss +++ b/cms/static/sass/elements/_modal-window.scss @@ -674,6 +674,11 @@ width: ($baseline*7); } + input.percentage { + display: inline-block; + width: ($baseline*3); + } + .tip { @extend %t-copy-sub1; @@ -810,6 +815,9 @@ .edit-settings-access { .gating-prereq { margin-bottom: 10px; + .list-fields .field { + display: block; + } } } } diff --git a/cms/templates/js/access-editor.underscore b/cms/templates/js/access-editor.underscore index 7425a0e0179505fe1edefea4c56718a2d3f4350c..dfab0c5bda92b21a29e9dd8244669cfa70e606e6 100644 --- a/cms/templates/js/access-editor.underscore +++ b/cms/templates/js/access-editor.underscore @@ -4,7 +4,7 @@ <div class="modal-section-content gating-prereq"> <ul class="list-fields list-input"> <p class="field-message"> - <%- gettext('Select a prerequisite subsection and enter a minimum score percentage to limit access to this subsection.') %> + <%- gettext('Select a prerequisite subsection and enter a minimum score percentage and minimum completion percentage to limit access to this subsection. Allowed values are 0-100') %> </p> <li class="field field-select"> <label class="label"> @@ -18,14 +18,25 @@ </label> </li> <li id="prereq_min_score_input" class="field field-input input-cosmetic"> - <label class="label"> + <label for="prereq_min_score"> <%- gettext('Minimum Score:') %> - <input type="text" id="prereq_min_score" name="prereq_min_score" class="input input-text" size="3" /> </label> + <input type="number" id="prereq_min_score" aria-describedby="msp" class="input input-text percentage" min="0" max="100" /> + <span id="msp">%</span> + </li> + <li id="prereq_min_completion_input" class="field field-input input-cosmetic"> + <label for="prereq_min_completion"> + <%- gettext('Minimum Completion:') %> + </label> + <input type="number" id="prereq_min_completion" aria-describedby="mcp" class="input input-text percentage" min="0" max="100" /> + <span id="mcp">%</span> </li> - <div id="prereq_min_score_error" class="message-status error"> + <li id="prereq_min_score_error" class="message-status error"> <%- gettext('The minimum score percentage must be a whole number between 0 and 100.') %> - </div> + </li> + <li id="prereq_min_completion_error" class="message-status error"> + <%- gettext('The minimum completion percentage must be a whole number between 0 and 100.') %> + </li> </ul> </div> <% } %> diff --git a/common/test/acceptance/pages/studio/overview.py b/common/test/acceptance/pages/studio/overview.py index c190a2692c97e1a7b0f30f6a417da57c9ffa3d40..d03609b42b7b420e9db2ea16fbb4408f1afd6174 100644 --- a/common/test/acceptance/pages/studio/overview.py +++ b/common/test/acceptance/pages/studio/overview.py @@ -720,12 +720,13 @@ class CourseOutlinePage(CoursePage, CourseOutlineContainer): self.q(css=".action-save").first.click() self.wait_for_ajax() - def add_prerequisite_to_subsection(self, min_score): + def add_prerequisite_to_subsection(self, min_score, min_completion): """ Adds a prerequisite to a subsection. """ Select(self.q(css="#prereq")[0]).select_by_index(1) self.q(css="#prereq_min_score").fill(min_score) + self.q(css="#prereq_min_completion").fill(min_completion) self.q(css=".action-save").first.click() self.wait_for_ajax() diff --git a/common/test/acceptance/tests/lms/test_lms_gating.py b/common/test/acceptance/tests/lms/test_lms_gating.py index 51a389c303bb8374e60532049bfbf692a85d9b57..b90d6e7b9605998db178064021ff8038b9764f48 100644 --- a/common/test/acceptance/tests/lms/test_lms_gating.py +++ b/common/test/acceptance/tests/lms/test_lms_gating.py @@ -106,7 +106,7 @@ class GatingTest(UniqueCourseTest): self.studio_course_outline.visit() self.studio_course_outline.open_subsection_settings_dialog(1) self.studio_course_outline.select_advanced_tab(desired_item='gated_content') - self.studio_course_outline.add_prerequisite_to_subsection("80") + self.studio_course_outline.add_prerequisite_to_subsection("80", "") def _fulfill_prerequisite(self): """ 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 0a589790b7fe45e12fecc5e18247cf88bef9ae93..7366cd360915f4e77b93503bf18ae0de481dd5d7 100644 --- a/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py +++ b/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py @@ -49,7 +49,7 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM gating_block: The block that must be completed before access is granted """ gating_api.add_prerequisite(self.course.id, unicode(gating_block.location)) - gating_api.set_required_content(self.course.id, gated_block.location, gating_block.location, 100) + gating_api.set_required_content(self.course.id, gated_block.location, gating_block.location, 100, 0) ALL_BLOCKS = ( 'course', 'A', 'B', 'C', 'ProctoredExam', 'D', 'E', 'PracticeExam', 'F', 'G', 'H', 'I', 'TimedExam', 'J', 'K' @@ -170,12 +170,11 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM self.clear_caches() # this call triggers reevaluation of prerequisites fulfilled by the gating block. - 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, - ) + lms_gating_api.evaluate_prerequisite( + self.course, + Mock(location=self.blocks[gating_block_ref].location, percent_graded=1.0), + self.user, + ) with self.assertNumQueries(6): self.get_blocks_and_check_against_expected(self.user, self.ALL_BLOCKS_EXCEPT_SPECIAL) @@ -205,12 +204,11 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM self.clear_caches() # this call triggers reevaluation of prerequisites fulfilled by the gating block. - 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), - self.user, - ) + lms_gating_api.evaluate_prerequisite( + self.course, + Mock(location=self.blocks['A'].location, percent_graded=1.0), + self.user, + ) self.get_blocks_and_check_against_expected(self.user, self.ALL_BLOCKS) def get_blocks_and_check_against_expected(self, user, expected_blocks): diff --git a/lms/djangoapps/gating/api.py b/lms/djangoapps/gating/api.py index 84ce71eefec441d96258ce75c93e779f7c834fb4..5155310a06ef3a86558cc3944cc47876c00220ab 100644 --- a/lms/djangoapps/gating/api.py +++ b/lms/djangoapps/gating/api.py @@ -18,9 +18,9 @@ log = logging.getLogger(__name__) def evaluate_prerequisite(course, subsection_grade, user): """ Evaluates any gating milestone relationships attached to the given - subsection. If the subsection_grade meets the minimum score required - by dependent subsections, the related milestone will be marked - fulfilled for the user. + subsection. If the subsection_grade and subsection_completion meets + the minimum score required by dependent subsections, the related + milestone will be marked fulfilled for the user. """ prereq_milestone = gating_api.get_gating_milestone(course.id, subsection_grade.location, 'fulfills') if prereq_milestone: @@ -30,8 +30,13 @@ def evaluate_prerequisite(course, subsection_grade, user): gated_content = gated_content_milestones.get(prereq_milestone['id']) if gated_content: + grade_percentage = subsection_grade.percent_graded * 100.0 \ + if hasattr(subsection_grade, 'percent_graded') else None + for milestone in gated_content: - gating_api.update_milestone(milestone, subsection_grade, prereq_milestone, user.id) + gating_api.update_milestone( + milestone, subsection_grade.location, prereq_milestone, user, grade_percentage + ) def evaluate_entrance_exam(course_grade, user): diff --git a/lms/djangoapps/gating/signals.py b/lms/djangoapps/gating/signals.py index f05dd33b2de69c26d3d498571a70c2c2dbeb9259..f6896cf61ac998b9e0be3b19da052c9d0e2bae17 100644 --- a/lms/djangoapps/gating/signals.py +++ b/lms/djangoapps/gating/signals.py @@ -1,9 +1,12 @@ """ Signal handlers for the gating djangoapp """ +from django.db import models from django.dispatch import receiver +from completion.models import BlockCompletion from gating import api as gating_api +from gating.tasks import task_evaluate_subsection_completion_milestones from lms.djangoapps.grades.signals.signals import SUBSECTION_SCORE_CHANGED from openedx.core.djangoapps.signals.signals import COURSE_GRADE_CHANGED @@ -24,6 +27,19 @@ def evaluate_subsection_gated_milestones(**kwargs): gating_api.evaluate_prerequisite(kwargs['course'], subsection_grade, kwargs.get('user')) +@receiver(models.signals.post_save, sender=BlockCompletion) +def evaluate_subsection_completion_milestones(**kwargs): + """ + Receives the BlockCompletion signal and triggers the + evaluation of any milestone which can be completed. + """ + instance = kwargs['instance'] + course_id = unicode(instance.course_key) + block_id = unicode(instance.block_key) + user_id = instance.user_id + task_evaluate_subsection_completion_milestones(course_id, block_id, user_id) + + @receiver(COURSE_GRADE_CHANGED) def evaluate_course_gated_milestones(**kwargs): """ diff --git a/lms/djangoapps/gating/tasks.py b/lms/djangoapps/gating/tasks.py new file mode 100644 index 0000000000000000000000000000000000000000..6bf98bee10dc0bc5941f29d527a58788f6522786 --- /dev/null +++ b/lms/djangoapps/gating/tasks.py @@ -0,0 +1,64 @@ +""" +This file contains celery tasks related to course content gating. +""" +import logging + +from celery import task +from django.contrib.auth.models import User + +from gating import api as gating_api +from lms.djangoapps.course_blocks.api import get_course_blocks +from opaque_keys.edx.keys import CourseKey, UsageKey +from xmodule.modulestore.django import modulestore + + +log = logging.getLogger(__name__) + + +@task() +def task_evaluate_subsection_completion_milestones(course_id, block_id, user_id): + """ + Updates users' milestones related to completion of a subsection. + Args: + course_id(str): Course id which triggered a completion event + block_id(str): Id of the completed block + user_id(int): Id of the user who completed a block + """ + store = modulestore() + course_key = CourseKey.from_string(course_id) + with store.bulk_operations(course_key): + course = store.get_course(course_key) + if not course or not course.enable_subsection_gating: + log.debug( + "Gating: ignoring evaluation of completion milestone because it disabled for course [%s]", course_id + ) + else: + try: + user = User.objects.get(id=user_id) + course_structure = get_course_blocks(user, store.make_course_usage_key(course_key)) + completed_block_usage_key = UsageKey.from_string(block_id).map_into_course(course.id) + subsection_block = _get_subsection_of_block(completed_block_usage_key, course_structure) + subsection = course_structure[subsection_block] + log.debug( + "Gating: Evaluating completion milestone for subsection [%s] and user [%s]", + unicode(subsection.location), user.id + ) + gating_api.evaluate_prerequisite(course, subsection, user) + except KeyError: + log.error("Gating: Given prerequisite subsection [%s] not found in course structure", block_id) + + +def _get_subsection_of_block(usage_key, block_structure): + """ + Finds subsection of a block by recursively iterating over its parents + :param usage_key: key of the block + :param block_structure: block structure + :return: sequential block + """ + parents = block_structure.get_parents(usage_key) + if parents: + for parent_block in parents: + if parent_block.block_type == 'sequential': + return parent_block + else: + return _get_subsection_of_block(parent_block, block_structure) diff --git a/lms/djangoapps/gating/tests/test_api.py b/lms/djangoapps/gating/tests/test_api.py index be601e935e8d0b349687aee5ba06c35216e87d54..0dfed52795f5363ada268241af2669eb76d6f876 100644 --- a/lms/djangoapps/gating/tests/test_api.py +++ b/lms/djangoapps/gating/tests/test_api.py @@ -68,44 +68,58 @@ class TestEvaluatePrerequisite(GatingTestCase, MilestonesTestCaseMixin): super(TestEvaluatePrerequisite, self).setUp() self.user_dict = {'id': self.user.id} self.prereq_milestone = None - self.subsection_grade = Mock(location=self.seq1.location) + self.subsection_grade = Mock(location=self.seq1.location, percent_graded=0.5) - def _setup_gating_milestone(self, min_score): + def _setup_gating_milestone(self, min_score, min_completion): """ Setup a gating milestone for testing """ 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) + gating_api.set_required_content( + self.course.id, self.seq2.location, self.seq1.location, min_score, min_completion + ) self.prereq_milestone = gating_api.get_gating_milestone(self.course.id, self.seq1.location, 'fulfills') - @patch('openedx.core.lib.gating.api._get_subsection_percentage') - @data((50, True), (100, True), (0, False)) + @patch('openedx.core.lib.gating.api.get_subsection_completion_percentage') + @data( + (50, 0, 50, 0, True), + (50, 0, 10, 0, False), + (0, 50, 0, 50, True), + (0, 50, 0, 10, False), + (50, 50, 50, 10, False), + (50, 50, 10, 50, False), + (50, 50, 50, 50, True), + ) @unpack - def test_min_score_achieved(self, module_score, result, mock_score): - self._setup_gating_milestone(50) - mock_score.return_value = module_score + def test_min_score_achieved( + self, min_score, min_completion, module_score, module_completion, result, mock_completion + ): + self._setup_gating_milestone(min_score, min_completion) + mock_completion.return_value = module_completion + self.subsection_grade.percent_graded = module_score / 100.0 evaluate_prerequisite(self.course, self.subsection_grade, self.user) self.assertEqual(milestones_api.user_has_milestone(self.user_dict, self.prereq_milestone), result) - @patch('openedx.core.lib.gating.api._get_subsection_percentage') + @patch('openedx.core.lib.gating.api.get_subsection_completion_percentage') @patch('openedx.core.lib.gating.api._get_minimum_required_percentage') - @data((50, False), (100, True)) + @data((50, 50, False), (100, 50, False), (50, 100, False), (100, 100, True)) @unpack - 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 + def test_invalid_min_score(self, module_score, module_completion, result, mock_min_score, mock_completion): + self._setup_gating_milestone(None, None) + mock_completion.return_value = module_completion + self.subsection_grade.percent_graded = module_score / 100.0 + mock_min_score.return_value = 100, 100 evaluate_prerequisite(self.course, self.subsection_grade, self.user) self.assertEqual(milestones_api.user_has_milestone(self.user_dict, self.prereq_milestone), result) - @patch('openedx.core.lib.gating.api._get_subsection_percentage') + @patch('openedx.core.lib.gating.api.get_subsection_grade_percentage') def test_no_prerequisites(self, mock_score): evaluate_prerequisite(self.course, self.subsection_grade, self.user) self.assertFalse(mock_score.called) - @patch('openedx.core.lib.gating.api._get_subsection_percentage') + @patch('openedx.core.lib.gating.api.get_subsection_grade_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 6880957b5d54f8905bea606da670d2f39530e944..8ba2ff58c95e45b60cebbcfb738e0a3ac0f734e8 100644 --- a/lms/djangoapps/gating/tests/test_integration.py +++ b/lms/djangoapps/gating/tests/test_integration.py @@ -2,6 +2,7 @@ Integration tests for gated content. """ import ddt +from completion import waffle as completion_waffle from milestones import api as milestones_api from milestones.tests.utils import MilestonesTestCaseMixin from nose.plugins.attrib import attr @@ -32,7 +33,7 @@ class TestGatedContent(MilestonesTestCaseMixin, SharedModuleStoreTestCase): def setUp(self): super(TestGatedContent, self).setUp() - self.setup_gating_milestone(50) + self.setup_gating_milestone(50, 100) self.non_staff_user = UserFactory() self.staff_user = UserFactory(is_staff=True, is_superuser=True) self.request = get_mock_request(self.non_staff_user) @@ -110,14 +111,16 @@ class TestGatedContent(MilestonesTestCaseMixin, SharedModuleStoreTestCase): display_name='problem 3', ) - def setup_gating_milestone(self, min_score): + def setup_gating_milestone(self, min_score, min_completion): """ 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, str(self.seq1.location)) - gating_api.set_required_content(self.course.id, str(self.seq2.location), str(self.seq1.location), min_score) + gating_api.set_required_content( + self.course.id, str(self.seq2.location), str(self.seq1.location), min_score, min_completion + ) self.prereq_milestone = gating_api.get_gating_milestone(self.course.id, self.seq1.location, 'fulfills') def assert_access_to_gated_content(self, user): @@ -164,29 +167,30 @@ class TestGatedContent(MilestonesTestCaseMixin, SharedModuleStoreTestCase): 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 - answer_problem(self.course, self.request, self.prob3, 10, 10) + with completion_waffle.waffle().override(completion_waffle.ENABLE_COMPLETION_TRACKING, True): + # start with a grade from a non-gated subsection + answer_problem(self.course, self.request, self.prob3, 10, 10) - # 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) - self.assert_course_grade(self.non_staff_user, .33) + # 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) + self.assert_course_grade(self.non_staff_user, .33) - # fulfill the gated requirements - answer_problem(self.course, self.request, self.gating_prob1, 10, 10) + # fulfill the gated requirements + answer_problem(self.course, self.request, self.gating_prob1, 10, 10) - # 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) - self.assert_course_grade(self.non_staff_user, .67) + # 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) + 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) + with completion_waffle.waffle().override(completion_waffle.ENABLE_COMPLETION_TRACKING, True): + answer_problem(self.course, self.request, self.gating_prob1, earned, max_possible) - 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) + self.assert_user_has_prereq_milestone(self.non_staff_user, expected_has_milestone=result) + self.assert_access_to_gated_content(self.non_staff_user) diff --git a/lms/templates/_gated_content.html b/lms/templates/_gated_content.html index 561f59e561292e1401ef4369cdd5ad1d887c5ddb..2039baec5aefb2c65b7c4ea6888d0bedfb9c6fb5 100644 --- a/lms/templates/_gated_content.html +++ b/lms/templates/_gated_content.html @@ -16,7 +16,7 @@ from openedx.core.djangolib.markup import Text </h3> <p> ${Text(_( - "You must earn a passing score for '{prereq_section_name}' to access this content." + "You must complete the prerequisites for '{prereq_section_name}' to access this content." )).format(prereq_section_name=prereq_section_name)} <p> <a href="${prereq_url}" class="btn btn-brand">${_("Go to Prerequisite Section")} diff --git a/openedx/core/lib/gating/api.py b/openedx/core/lib/gating/api.py index e93ee3d2a2072c5d34fc6a34b4e590903e5e9b9b..b9bc9800eccb0280cc9cf8c272943fb66466346e 100644 --- a/openedx/core/lib/gating/api.py +++ b/openedx/core/lib/gating/api.py @@ -7,12 +7,13 @@ import logging from django.contrib.auth.models import User from django.core.urlresolvers import reverse from django.utils.translation import ugettext as _ + +from completion.models import BlockCompletion from lms.djangoapps.courseware.access import _has_access_to_course from lms.djangoapps.course_blocks.api import get_course_blocks from lms.djangoapps.grades.subsection_grade_factory import SubsectionGradeFactory from milestones import api as milestones_api from opaque_keys.edx.keys import UsageKey -from opaque_keys.edx.locator import BlockUsageLocator from openedx.core.lib.gating.exceptions import GatingValidationError from util import milestones_helpers from xmodule.modulestore.django import modulestore @@ -230,7 +231,7 @@ def is_prerequisite(course_key, prereq_content_key): ) is not None -def set_required_content(course_key, gated_content_key, prereq_content_key, min_score): +def set_required_content(course_key, gated_content_key, prereq_content_key, min_score='', min_completion=''): """ Adds a `requires` milestone relationship for the given gated_content_key if a prerequisite prereq_content_key is provided. If prereq_content_key is None, removes the `requires` @@ -241,6 +242,7 @@ def set_required_content(course_key, gated_content_key, prereq_content_key, min_ gated_content_key (str|UsageKey): The gated content usage key prereq_content_key (str|UsageKey): The prerequisite content usage key min_score (str|int): The minimum score + min_completion (str|int): The minimum completion percentage Returns: None @@ -254,7 +256,7 @@ def set_required_content(course_key, gated_content_key, prereq_content_key, min_ if prereq_content_key: _validate_min_score(min_score) - requirements = {'min_score': min_score} + requirements = {'min_score': min_score, 'min_completion': min_completion} if not milestone: milestone = _get_prerequisite_milestone(prereq_content_key) milestones_api.add_course_content_milestone(course_key, gated_content_key, 'requires', milestone, requirements) @@ -262,7 +264,7 @@ def set_required_content(course_key, gated_content_key, prereq_content_key, min_ def get_required_content(course_key, gated_content_key): """ - Returns the prerequisite content usage key and minimum score needed for fulfillment + Returns the prerequisite content usage key, minimum score and minimum completion percentage needed for fulfillment of that prerequisite for the given gated_content_key. Args: @@ -270,16 +272,18 @@ def get_required_content(course_key, gated_content_key): gated_content_key (str|UsageKey): The gated content usage key Returns: - tuple: The prerequisite content usage key and minimum score, (None, None) if the content is not gated + tuple: The prerequisite content usage key, minimum score and minimum completion percentage, + (None, None, None) if the content is not gated """ milestone = get_gating_milestone(course_key, gated_content_key, 'requires') if milestone: return ( _get_gating_block_id(milestone), - milestone.get('requirements', {}).get('min_score') + milestone.get('requirements', {}).get('min_score', None), + milestone.get('requirements', {}).get('min_completion', None), ) else: - return None, None + return None, None, None @gating_enabled(default=[]) @@ -378,41 +382,37 @@ def compute_is_prereq_met(content_id, user_id, recalc_on_unmet=False): 'url': reverse('jump_to', kwargs={'course_id': course_key, 'location': subsection_usage_key}), 'display_name': subsection.display_name } - - try: - subsection_structure = get_course_blocks(student, subsection_usage_key) - if any(subsection_structure): - subsection_grade_factory = SubsectionGradeFactory(student, course_structure=subsection_structure) - if subsection_usage_key in subsection_structure: - # this will force a recalcuation of the subsection grade - subsection_grade = subsection_grade_factory.update(subsection_structure[subsection_usage_key], persist_grade=False) - prereq_met = update_milestone(milestone, subsection_grade, milestone, user_id) - except ItemNotFoundError as err: - log.warning("Could not find course_block for subsection=%s error=%s", subsection_usage_key, err) + prereq_met = update_milestone(milestone, subsection_usage_key, milestone, student) return prereq_met, prereq_meta_info -def update_milestone(milestone, subsection_grade, prereq_milestone, user_id): +def update_milestone(milestone, usage_key, prereq_milestone, user, grade_percentage=None, completion_percentage=None): """ Updates the milestone record based on evaluation of prerequisite met. Arguments: milestone: The gated milestone being evaluated - subsection_grade: The grade of the prerequisite subsection - prerequisite_milestone: The gating milestone - user_id: The id of the user + usage_key: Usage key of the prerequisite subsection + prereq_milestone: The gating milestone + user: The user who has fulfilled milestone + grade_percentage: Grade percentage of prerequisite subsection + completion_percentage: Completion percentage of prerequisite subsection Returns: True if prerequisite has been met, False if not """ - 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) + min_score, min_completion = _get_minimum_required_percentage(milestone) + if not grade_percentage: + grade_percentage = get_subsection_grade_percentage(usage_key, user) if min_score > 0 else 0 + if not completion_percentage: + completion_percentage = get_subsection_completion_percentage(usage_key, user) if min_completion > 0 else 0 + + if grade_percentage >= min_score and completion_percentage >= min_completion: + milestones_helpers.add_user_milestone({'id': user.id}, prereq_milestone) return True else: - milestones_helpers.remove_user_milestone({'id': user_id}, prereq_milestone) + milestones_helpers.remove_user_milestone({'id': user.id}, prereq_milestone) return False @@ -423,12 +423,74 @@ def _get_gating_block_id(milestone): return milestone.get('namespace', '').replace(GATING_NAMESPACE_QUALIFIER, '') +def get_subsection_grade_percentage(subsection_usage_key, user): + """ + Computes grade percentage for a subsection in a given course for a user + + Arguments: + subsection_usage_key: key of subsection + user: The user whose grade needs to be computed + + Returns: + User's grade percentage for given subsection + """ + subsection_grade_percentage = 0.0 + try: + subsection_structure = get_course_blocks(user, subsection_usage_key) + if any(subsection_structure): + subsection_grade_factory = SubsectionGradeFactory(user, course_structure=subsection_structure) + if subsection_usage_key in subsection_structure: + # this will force a recalculation of the subsection grade + subsection_grade = subsection_grade_factory.update( + subsection_structure[subsection_usage_key], persist_grade=False + ) + subsection_grade_percentage = subsection_grade.percent_graded * 100.0 + except ItemNotFoundError as err: + log.warning("Could not find course_block for subsection=%s error=%s", subsection_usage_key, err) + return subsection_grade_percentage + + +def get_subsection_completion_percentage(subsection_usage_key, user): + """ + Computes completion percentage for a subsection in a given course for a user + Arguments: + subsection_usage_key: key of subsection + user: The user whose completion percentage needs to be computed + Returns: + User's completion percentage for given subsection + """ + subsection_completion_percentage = 0.0 + try: + subsection_structure = get_course_blocks(user, subsection_usage_key) + if any(subsection_structure): + completable_blocks = [ + block for block in subsection_structure + if block.block_type not in ['chapter', 'sequential', 'vertical'] + ] + if not completable_blocks: + return 0 + subsection_completion_total = 0 + course_block_completions = BlockCompletion.get_course_completions(user, subsection_usage_key.course_key) + for block in completable_blocks: + if course_block_completions.get(block): + subsection_completion_total += course_block_completions.get(block) + subsection_completion_percentage = min( + 100 * (subsection_completion_total / float(len(completable_blocks))), 100 + ) + + except ItemNotFoundError as err: + log.warning("Could not find course_block for subsection=%s error=%s", subsection_usage_key, err) + + return subsection_completion_percentage + + def _get_minimum_required_percentage(milestone): """ - Returns the minimum percentage requirement for the given milestone. + Returns the minimum score and minimum completion percentage requirement for the given milestone. """ - # Default minimum score to 100 + # Default minimum score and minimum completion percentage to 100 min_score = 100 + min_completion = 100 requirements = milestone.get('requirements') if requirements: try: @@ -438,7 +500,14 @@ def _get_minimum_required_percentage(milestone): u'Gating: Failed to find minimum score for gating milestone %s, defaulting to 100', json.dumps(milestone) ) - return min_score + try: + min_completion = int(requirements.get('min_completion', 0)) + except (ValueError, TypeError): + log.warning( + u'Gating: Failed to find minimum completion percentage for gating milestone %s, defaulting to 100', + json.dumps(milestone) + ) + return min_score, min_completion def _get_subsection_percentage(subsection_grade): diff --git a/openedx/core/lib/gating/tests/test_api.py b/openedx/core/lib/gating/tests/test_api.py index c54e631ddbea17c29d22531d7b78c6985968925d..d0341e9a58f1b6d5649fec6e4b087d92dff22fe3 100644 --- a/openedx/core/lib/gating/tests/test_api.py +++ b/openedx/core/lib/gating/tests/test_api.py @@ -5,7 +5,7 @@ import unittest from mock import patch, Mock from nose.plugins.attrib import attr -from ddt import ddt, data +from ddt import ddt, data, unpack from django.conf import settings from lms.djangoapps.gating import api as lms_gating_api from milestones.tests.utils import MilestonesTestCaseMixin @@ -145,17 +145,23 @@ class TestGatingApi(ModuleStoreTestCase, MilestonesTestCaseMixin): """ Test test_required_content """ gating_api.add_prerequisite(self.course.id, self.seq1.location) - gating_api.set_required_content(self.course.id, self.seq2.location, self.seq1.location, 100) + gating_api.set_required_content(self.course.id, self.seq2.location, self.seq1.location, 100, 100) - prereq_content_key, min_score = gating_api.get_required_content(self.course.id, self.seq2.location) + prereq_content_key, min_score, min_completion = gating_api.get_required_content( + self.course.id, self.seq2.location + ) self.assertEqual(prereq_content_key, unicode(self.seq1.location)) self.assertEqual(min_score, 100) + self.assertEqual(min_completion, 100) - gating_api.set_required_content(self.course.id, self.seq2.location, None, None) + gating_api.set_required_content(self.course.id, self.seq2.location, None, None, None) - prereq_content_key, min_score = gating_api.get_required_content(self.course.id, self.seq2.location) + prereq_content_key, min_score, min_completion = gating_api.get_required_content( + self.course.id, self.seq2.location + ) self.assertIsNone(prereq_content_key) self.assertIsNone(min_score) + self.assertIsNone(min_completion) def test_get_gated_content(self): """ @@ -179,13 +185,25 @@ class TestGatingApi(ModuleStoreTestCase, MilestonesTestCaseMixin): self.assertEqual(gating_api.get_gated_content(self.course, student), []) - def test_is_gate_fulfilled(self): + @data( + (100, 0, 50, 0, False), + (100, 0, 100, 0, True), + (0, 100, 0, 50, False), + (0, 100, 0, 100, True), + (100, 100, 50, 100, False), + (100, 100, 100, 50, False), + (100, 100, 100, 100, True), + ) + @unpack + def test_is_gate_fulfilled(self, min_score, min_completion, learner_score, learner_completion, is_gate_fulfilled): """ Test if prereq section has any unfulfilled milestones """ student = UserFactory(is_staff=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, 100) + gating_api.set_required_content( + self.course.id, self.seq2.location, self.seq1.location, min_score, min_completion + ) milestone = milestones_api.add_milestone(self.generic_milestone) milestones_api.add_course_content_milestone(self.course.id, self.seq1.location, 'fulfills', milestone) @@ -193,22 +211,16 @@ class TestGatingApi(ModuleStoreTestCase, MilestonesTestCaseMixin): # complete the prerequisite to unlock the gated content # this call triggers reevaluation of prerequisites fulfilled by the gating block. - with patch.object(gating_api, '_get_subsection_percentage') as mock_grade: - mock_grade.return_value = 75 + with patch.object(gating_api, 'get_subsection_completion_percentage') as mock_grade: + mock_grade.return_value = learner_completion lms_gating_api.evaluate_prerequisite( self.course, - Mock(location=self.seq1.location), + Mock(location=self.seq1.location, percent_graded=learner_score / 100.0), student, ) - self.assertFalse(gating_api.is_gate_fulfilled(self.course.id, self.seq1.location, student.id)) - - mock_grade.return_value = 100 - lms_gating_api.evaluate_prerequisite( - self.course, - Mock(location=self.seq1.location), - student, + self.assertEqual( + gating_api.is_gate_fulfilled(self.course.id, self.seq1.location, student.id), is_gate_fulfilled ) - self.assertTrue(gating_api.is_gate_fulfilled(self.course.id, self.seq1.location, student.id)) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') def test_compute_is_prereq_met(self): @@ -217,11 +229,11 @@ class TestGatingApi(ModuleStoreTestCase, MilestonesTestCaseMixin): """ student = UserFactory(is_staff=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, 100) + gating_api.set_required_content(self.course.id, self.seq2.location, self.seq1.location, 100, 0) # complete the prerequisite to unlock the gated content # this call triggers reevaluation of prerequisites fulfilled by the gating block. - with patch.object(gating_api, '_get_subsection_percentage') as mock_grade: + with patch.object(gating_api, 'get_subsection_grade_percentage') as mock_grade: mock_grade.return_value = 75 # don't force recompute prereq_met, prereq_meta_info = gating_api.compute_is_prereq_met(self.seq2.location, student.id, False) diff --git a/openedx/features/course_experience/tests/views/test_course_outline.py b/openedx/features/course_experience/tests/views/test_course_outline.py index 8943a8f8f1e87bad7c9aa0e1c2b31c3911120251..dfbf8aca53d60105a96c4fd53e0707a72714a207 100644 --- a/openedx/features/course_experience/tests/views/test_course_outline.py +++ b/openedx/features/course_experience/tests/views/test_course_outline.py @@ -251,10 +251,10 @@ class TestCourseOutlinePageWithPrerequisites(SharedModuleStoreTestCase, Mileston # complete the prerequisite to unlock the gated content # this call triggers reevaluation of prerequisites fulfilled by the gating block. - with patch('openedx.core.lib.gating.api._get_subsection_percentage', Mock(return_value=100)): + with patch('openedx.core.lib.gating.api.get_subsection_completion_percentage', Mock(return_value=100)): lms_gating_api.evaluate_prerequisite( self.course, - Mock(location=self.course_blocks['prerequisite'].location), + Mock(location=self.course_blocks['prerequisite'].location, percent_graded=1.0), self.user, )