diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index c2d7c402d5..767f0625f4 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -389,12 +389,14 @@ class CapaMixin(CapaFields): Return some html with data about the module """ progress = self.get_progress() + curr_score, total_possible = (progress.frac() if progress else (0, 0)) return self.runtime.render_template('problem_ajax.html', { 'element_id': self.location.html_id(), 'id': self.location.to_deprecated_string(), 'ajax_url': self.runtime.ajax_url, - 'progress_status': Progress.to_js_status_str(progress), - 'progress_detail': Progress.to_js_detail_str(progress), + 'current_score': curr_score, + 'total_possible': total_possible, + 'attempts_used': self.attempts, 'content': self.get_problem_html(encapsulate=False), 'graded': self.graded, }) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 07aca64aa3..f90604e511 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -92,6 +92,7 @@ class CapaModule(CapaMixin, XModule): return 'Error: {} is not a known capa action'.format(dispatch) before = self.get_progress() + before_attempts = self.attempts try: result = handlers[dispatch](data) @@ -117,11 +118,14 @@ class CapaModule(CapaMixin, XModule): raise ProcessingError(generic_error_message), None, traceback_obj after = self.get_progress() - + after_attempts = self.attempts + progress_changed = (after != before) or (after_attempts != before_attempts) + curr_score, total_possible = (after.frac() if after else (0, 0)) result.update({ - 'progress_changed': after != before, - 'progress_status': Progress.to_js_status_str(after), - 'progress_detail': Progress.to_js_detail_str(after), + 'progress_changed': progress_changed, + 'current_score': curr_score, + 'total_possible': total_possible, + 'attempts_used': after_attempts, }) return json.dumps(result, cls=ComplexEncoder) diff --git a/common/lib/xmodule/xmodule/js/spec/capa/display_spec.coffee b/common/lib/xmodule/xmodule/js/spec/capa/display_spec.coffee index 6ba5a613e5..545d7c8c29 100644 --- a/common/lib/xmodule/xmodule/js/spec/capa/display_spec.coffee +++ b/common/lib/xmodule/xmodule/js/spec/capa/display_spec.coffee @@ -91,9 +91,10 @@ describe 'Problem', -> beforeEach -> @problem = new Problem($('.xblock-student_view')) - testProgessData = (problem, status, detail, graded, expected_progress_after_render) -> - problem.el.data('progress_status', status) - problem.el.data('progress_detail', detail) + testProgessData = (problem, score, total_possible, attempts, graded, expected_progress_after_render) -> + problem.el.data('problem-score', score); + problem.el.data('problem-total-possible', total_possible); + problem.el.data('attempts-used', attempts); problem.el.data('graded', graded) expect(problem.$('.problem-progress').html()).toEqual "" problem.renderProgressState() @@ -101,35 +102,41 @@ describe 'Problem', -> describe 'with a status of "none"', -> it 'reports the number of points possible and graded', -> - testProgessData(@problem, 'none', '0/1', "True", "1 point possible (graded)") + testProgessData(@problem, 0, 1, 0, "True", "1 point possible (graded)") it 'displays the number of points possible when rendering happens with the content', -> - testProgessData(@problem, 'none', '0/2', "True", "2 points possible (graded)") + testProgessData(@problem, 0, 2, 0, "True", "2 points possible (graded)") it 'reports the number of points possible and ungraded', -> - testProgessData(@problem, 'none', '0/1', "False", "1 point possible (ungraded)") + testProgessData(@problem, 0, 1, 0, "False", "1 point possible (ungraded)") it 'displays ungraded if number of points possible is 0', -> - testProgessData(@problem, 'none', '0', "False", "0 points possible (ungraded)") + testProgessData(@problem, 0, 0, 0, "False", "0 points possible (ungraded)") it 'displays ungraded if number of points possible is 0, even if graded value is True', -> - testProgessData(@problem, 'none', '0', "True", "0 points possible (ungraded)") + testProgessData(@problem, 0, 0, 0, "True", "0 points possible (ungraded)") + + it 'reports the correct score with status none and >0 attempts', -> + testProgessData(@problem, 0, 1, 1, "True", "0/1 point (graded)") + + it 'reports the correct score with >1 weight, status none, and >0 attempts', -> + testProgessData(@problem, 0, 2, 2, "True", "0/2 points (graded)") describe 'with any other valid status', -> it 'reports the current score', -> - testProgessData(@problem, 'foo', '1/1', "True", "1/1 point (graded)") + testProgessData(@problem, 1, 1, 1, "True", "1/1 point (graded)") it 'shows current score when rendering happens with the content', -> - testProgessData(@problem, 'test status', '2/2', "True", "2/2 points (graded)") + testProgessData(@problem, 2, 2, 1, "True", "2/2 points (graded)") it 'reports the current score even if problem is ungraded', -> - testProgessData(@problem, 'test status', '1/1', "False", "1/1 point (ungraded)") + testProgessData(@problem, 1, 1, 1, "False", "1/1 point (ungraded)") describe 'with valid status and string containing an integer like "0" for detail', -> # These tests are to address a failure specific to Chrome 51 and 52 + it 'shows 0 points possible for the detail', -> - testProgessData(@problem, 'foo', '0', "False", "") + testProgessData(@problem, 0, 0, 1, "False", "0 points possible (ungraded)") describe 'render', -> beforeEach -> diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.js b/common/lib/xmodule/xmodule/js/src/capa/display.js index d24e4f98e3..2de0fb26af 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.js +++ b/common/lib/xmodule/xmodule/js/src/capa/display.js @@ -210,83 +210,71 @@ }; Problem.prototype.renderProgressState = function() { - var a, detail, earned, graded, possible, progress, progressTemplate, status; - detail = this.el.data('progress_detail'); - status = this.el.data('progress_status'); + var graded, progress, progressTemplate, curScore, totalScore, attemptsUsed; + curScore = this.el.data('problem-score'); + totalScore = this.el.data('problem-total-possible'); + attemptsUsed = this.el.data('attempts-used'); graded = this.el.data('graded'); - // Render 'x/y point(s)' if student has attempted question - if (status !== 'none' && (detail !== null && detail !== undefined) && (jQuery.type(detail) === 'string') && - detail.indexOf('/') > 0) { - a = detail.split('/'); - earned = parseFloat(a[0]); - possible = parseFloat(a[1]); - if (graded === 'True' && possible !== 0) { + if (curScore === undefined || totalScore === undefined) { + progress = ''; + } else if (attemptsUsed === 0 || totalScore === 0) { + // Render 'x point(s) possible' if student has not yet attempted question + if (graded === 'True' && totalScore !== 0) { + progressTemplate = ngettext( + // Translators: %(num_points)s is the number of points possible (examples: 1, 3, 10).; + '%(num_points)s point possible (graded)', '%(num_points)s points possible (graded)', + totalScore + ); + } else { + progressTemplate = ngettext( + // Translators: %(num_points)s is the number of points possible (examples: 1, 3, 10).; + '%(num_points)s point possible (ungraded)', '%(num_points)s points possible (ungraded)', + totalScore + ); + } + progress = interpolate(progressTemplate, {num_points: totalScore}, true); + } else { + // Render 'x/y point(s)' if student has attempted question + if (graded === 'True' && totalScore !== 0) { progressTemplate = ngettext( // This comment needs to be on one line to be properly scraped for the translators. // Translators: %(earned)s is the number of points earned. %(possible)s is the total number of points (examples: 0/1, 1/1, 2/3, 5/10). The total number of points will always be at least 1. We pluralize based on the total number of points (example: 0/1 point; 1/2 points); '%(earned)s/%(possible)s point (graded)', '%(earned)s/%(possible)s points (graded)', - possible + totalScore ); } else { progressTemplate = ngettext( // This comment needs to be on one line to be properly scraped for the translators. // Translators: %(earned)s is the number of points earned. %(possible)s is the total number of points (examples: 0/1, 1/1, 2/3, 5/10). The total number of points will always be at least 1. We pluralize based on the total number of points (example: 0/1 point; 1/2 points); '%(earned)s/%(possible)s point (ungraded)', '%(earned)s/%(possible)s points (ungraded)', - possible + totalScore ); } progress = interpolate( progressTemplate, { - earned: earned, - possible: possible + earned: curScore, + possible: totalScore }, true ); } - - // Render 'x point(s) possible' if student has not yet attempted question - // Status is set to none when a user has a score of 0, and 0 when the problem has a weight of 0. - if (status === 'none' || status === 0) { - if ((detail !== null && detail !== undefined) && (jQuery.type(detail) === 'string') && - detail.indexOf('/') > 0) { - a = detail.split('/'); - possible = parseFloat(a[1]); - } else { - possible = 0; - } - if (graded === 'True' && possible !== 0) { - progressTemplate = ngettext( - // Translators: %(num_points)s is the number of points possible (examples: 1, 3, 10).; - '%(num_points)s point possible (graded)', '%(num_points)s points possible (graded)', - possible - ); - } else { - progressTemplate = ngettext( - // Translators: %(num_points)s is the number of points possible (examples: 1, 3, 10).; - '%(num_points)s point possible (ungraded)', '%(num_points)s points possible (ungraded)', - possible - ); - } - progress = interpolate( - progressTemplate, - {num_points: possible}, true - ); - } return this.$('.problem-progress').text(progress); }; Problem.prototype.updateProgress = function(response) { if (response.progress_changed) { - this.el.data('progress_status', response.progress_status); - this.el.data('progress_detail', response.progress_detail); + this.el.data('problem-score', response.current_score); + this.el.data('problem-total-possible', response.total_possible); + this.el.data('attempts-used', response.attempts_used); this.el.trigger('progressChanged'); } return this.renderProgressState(); }; Problem.prototype.forceUpdate = function(response) { - this.el.data('progress_status', response.progress_status); - this.el.data('progress_detail', response.progress_detail); + this.el.data('problem-score', response.current_score); + this.el.data('problem-total-possible', response.total_possible); + this.el.data('attempts-used', response.attempts_used); this.el.trigger('progressChanged'); return this.renderProgressState(); }; diff --git a/common/lib/xmodule/xmodule/js/src/sequence/display.coffee b/common/lib/xmodule/xmodule/js/src/sequence/display.coffee index 7b416e8160..e1b647bdd3 100644 --- a/common/lib/xmodule/xmodule/js/src/sequence/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/sequence/display.coffee @@ -99,18 +99,6 @@ class @Sequence new_progress = _this.mergeProgress progress, new_progress @progressTable[@position] = new_progress - @setProgress(new_progress, @link_for(@position)) - - setProgress: (progress, element) -> - # If progress is "NA", don't add any css class - element.removeClass('progress-none') - .removeClass('progress-some') - .removeClass('progress-done') - - switch progress - when 'none' then element.addClass('progress-none') - when 'in_progress' then element.addClass('progress-some') - when 'done' then element.addClass('progress-done') enableButton: (button_class, button_action) -> @$(button_class).removeClass('disabled').removeAttr('disabled').click(button_action) diff --git a/common/lib/xmodule/xmodule/progress.py b/common/lib/xmodule/xmodule/progress.py index 470aa22a0e..afb05a5c2e 100644 --- a/common/lib/xmodule/xmodule/progress.py +++ b/common/lib/xmodule/xmodule/progress.py @@ -1,14 +1,6 @@ ''' Progress class for modules. Represents where a student is in a module. -Useful things to know: - - Use Progress.to_js_status_str() to convert a progress into a simple - status string to pass to js. - - Use Progress.to_js_detail_str() to convert a progress into a more detailed - string to pass to js. - -In particular, these functions have a canonical handing of None. - For most subclassing needs, you should only need to reimplement frac() and __str__(). ''' @@ -140,25 +132,3 @@ class Progress(object): (n, d) = a.frac() (n2, d2) = b.frac() return Progress(n + n2, d + d2) - - @staticmethod - def to_js_status_str(progress): - ''' - Return the "status string" version of the passed Progress - object that should be passed to js. Use this function when - sending Progress objects to js to limit dependencies. - ''' - if progress is None: - return "0" - return progress.ternary_str() - - @staticmethod - def to_js_detail_str(progress): - ''' - Return the "detail string" version of the passed Progress - object that should be passed to js. Use this function when - passing Progress objects to js to limit dependencies. - ''' - if progress is None: - return "0" - return str(progress) diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index f343fe4c10..0313bf67ad 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -337,15 +337,12 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): is_bookmarked = bookmarks_service.is_bookmarked(usage_key=item.scope_ids.usage_id) context["bookmarked"] = is_bookmarked - progress = item.get_progress() rendered_item = item.render(STUDENT_VIEW, context) fragment.add_frag_resources(rendered_item) iteminfo = { 'content': rendered_item.content, 'page_title': getattr(item, 'tooltip_title', ''), - 'progress_status': Progress.to_js_status_str(progress), - 'progress_detail': Progress.to_js_detail_str(progress), 'type': item.get_icon_class(), 'id': item.scope_ids.usage_id.to_deprecated_string(), 'bookmarked': is_bookmarked, diff --git a/common/lib/xmodule/xmodule/tests/test_progress.py b/common/lib/xmodule/xmodule/tests/test_progress.py index 4f39c4debf..1981ef4bf3 100644 --- a/common/lib/xmodule/xmodule/tests/test_progress.py +++ b/common/lib/xmodule/xmodule/tests/test_progress.py @@ -85,27 +85,6 @@ class ProgressTest(unittest.TestCase): self.assertEqual(str(Progress(2.0034, 7)), '2/7') self.assertEqual(str(Progress(0.999, 7)), '1/7') - def test_ternary_str(self): - self.assertEqual(self.not_started.ternary_str(), "none") - self.assertEqual(self.half_done.ternary_str(), "in_progress") - self.assertEqual(self.done.ternary_str(), "done") - - def test_to_js_status(self): - '''Test the Progress.to_js_status_str() method''' - - self.assertEqual(Progress.to_js_status_str(self.not_started), "none") - self.assertEqual(Progress.to_js_status_str(self.half_done), "in_progress") - self.assertEqual(Progress.to_js_status_str(self.done), "done") - self.assertEqual(Progress.to_js_status_str(None), "0") - - def test_to_js_detail_str(self): - '''Test the Progress.to_js_detail_str() method''' - f = Progress.to_js_detail_str - for prg in (self.not_started, self.half_done, self.done): - self.assertEqual(f(prg), str(prg)) - # But None should be encoded as 0 - self.assertEqual(f(None), "0") - def test_add(self): '''Test the Progress.add_counts() method''' prg1 = Progress(0, 2) diff --git a/lms/djangoapps/courseware/features/problems.feature b/lms/djangoapps/courseware/features/problems.feature index b23ce18bbb..a35951b9e0 100644 --- a/lms/djangoapps/courseware/features/problems.feature +++ b/lms/djangoapps/courseware/features/problems.feature @@ -121,23 +121,23 @@ Feature: LMS.Answer problems Then I should see a score of "" Examples: - | ProblemType | Correctness | Score | Points Possible | - | drop down | correct | 1/1 point (ungraded) | 1 point possible (ungraded) | - | drop down | incorrect | 1 point possible (ungraded) | 1 point possible (ungraded) | - | multiple choice | correct | 1/1 point (ungraded) | 1 point possible (ungraded) | - | multiple choice | incorrect | 1 point possible (ungraded) | 1 point possible (ungraded) | - | checkbox | correct | 1/1 point (ungraded) | 1 point possible (ungraded) | - | checkbox | incorrect | 1 point possible (ungraded) | 1 point possible (ungraded) | - | radio | correct | 1/1 point (ungraded) | 1 point possible (ungraded) | - | radio | incorrect | 1 point possible (ungraded) | 1 point possible (ungraded) | - | numerical | correct | 1/1 point (ungraded) | 1 point possible (ungraded) | - | numerical | incorrect | 1 point possible (ungraded) | 1 point possible (ungraded) | - | formula | correct | 1/1 point (ungraded) | 1 point possible (ungraded) | - | formula | incorrect | 1 point possible (ungraded) | 1 point possible (ungraded) | - | script | correct | 2/2 points (ungraded) | 2 points possible (ungraded) | - | script | incorrect | 2 points possible (ungraded) | 2 points possible (ungraded) | - | image | correct | 1/1 point (ungraded) | 1 point possible (ungraded) | - | image | incorrect | 1 point possible (ungraded) | 1 point possible (ungraded) | + | ProblemType | Correctness | Score | Points Possible | + | drop down | correct | 1/1 point (ungraded) | 0/1 point (ungraded) | + | drop down | incorrect | 0/1 point (ungraded) | 0/1 point (ungraded) | + | multiple choice | correct | 1/1 point (ungraded) | 0/1 point (ungraded) | + | multiple choice | incorrect | 0/1 point (ungraded) | 0/1 point (ungraded) | + | checkbox | correct | 1/1 point (ungraded) | 0/1 point (ungraded) | + | checkbox | incorrect | 0/1 point (ungraded) | 0/1 point (ungraded) | + | radio | correct | 1/1 point (ungraded) | 0/1 point (ungraded) | + | radio | incorrect | 0/1 point (ungraded) | 0/1 point (ungraded) | + | numerical | correct | 1/1 point (ungraded) | 0/1 point (ungraded) | + | numerical | incorrect | 0/1 point (ungraded) | 0/1 point (ungraded) | + | formula | correct | 1/1 point (ungraded) | 0/1 point (ungraded) | + | formula | incorrect | 0/1 point (ungraded) | 0/1 point (ungraded) | + | script | correct | 2/2 points (ungraded) | 0/2 points (ungraded) | + | script | incorrect | 0/2 points (ungraded) | 0/2 points (ungraded) | + | image | correct | 1/1 point (ungraded) | 0/1 point (ungraded) | + | image | incorrect | 0/1 point (ungraded) | 0/1 point (ungraded) | Scenario: I can see my score on a problem when I answer it and after I reset it Given I am viewing a "" problem with randomization "" with reset button on @@ -147,23 +147,23 @@ Feature: LMS.Answer problems Then I should see a score of "" Examples: - | ProblemType | Correctness | Score | Points Possible | Randomization | - | drop down | correct | 1/1 point (ungraded) | 1 point possible (ungraded) | never | - | drop down | incorrect | 1 point possible (ungraded) | 1 point possible (ungraded) | never | - | multiple choice | correct | 1/1 point (ungraded) | 1 point possible (ungraded) | never | - | multiple choice | incorrect | 1 point possible (ungraded) | 1 point possible (ungraded) | never | - | checkbox | correct | 1/1 point (ungraded) | 1 point possible (ungraded) | never | - | checkbox | incorrect | 1 point possible (ungraded) | 1 point possible (ungraded) | never | - | radio | correct | 1/1 point (ungraded) | 1 point possible (ungraded) | never | - | radio | incorrect | 1 point possible (ungraded) | 1 point possible (ungraded) | never | - | numerical | correct | 1/1 point (ungraded) | 1 point possible (ungraded) | never | - | numerical | incorrect | 1 point possible (ungraded) | 1 point possible (ungraded) | never | - | formula | correct | 1/1 point (ungraded) | 1 point possible (ungraded) | never | - | formula | incorrect | 1 point possible (ungraded) | 1 point possible (ungraded) | never | - | script | correct | 2/2 points (ungraded) | 2 points possible (ungraded) | never | - | script | incorrect | 2 points possible (ungraded) | 2 points possible (ungraded) | never | - | image | correct | 1/1 point (ungraded) | 1 point possible (ungraded) | never | - | image | incorrect | 1 point possible (ungraded) | 1 point possible (ungraded) | never | + | ProblemType | Correctness | Score | Points Possible | Randomization | + | drop down | correct | 1/1 point (ungraded) | 0/1 point (ungraded) | never | + | drop down | incorrect | 0/1 point (ungraded) | 0/1 point (ungraded) | never | + | multiple choice | correct | 1/1 point (ungraded) | 0/1 point (ungraded) | never | + | multiple choice | incorrect | 0/1 point (ungraded) | 0/1 point (ungraded) | never | + | checkbox | correct | 1/1 point (ungraded) | 0/1 point (ungraded) | never | + | checkbox | incorrect | 0/1 point (ungraded) | 0/1 point (ungraded) | never | + | radio | correct | 1/1 point (ungraded) | 0/1 point (ungraded) | never | + | radio | incorrect | 0/1 point (ungraded) | 0/1 point (ungraded) | never | + | numerical | correct | 1/1 point (ungraded) | 0/1 point (ungraded) | never | + | numerical | incorrect | 0/1 point (ungraded) | 0/1 point (ungraded) | never | + | formula | correct | 1/1 point (ungraded) | 0/1 point (ungraded) | never | + | formula | incorrect | 0/1 point (ungraded) | 0/1 point (ungraded) | never | + | script | correct | 2/2 points (ungraded) | 0/2 points (ungraded) | never | + | script | incorrect | 0/2 points (ungraded) | 0/2 points (ungraded) | never | + | image | correct | 1/1 point (ungraded) | 0/1 point (ungraded) | never | + | image | incorrect | 0/1 point (ungraded) | 0/1 point (ungraded) | never | Scenario: I can see my score on a problem to which I submit a blank answer Given I am viewing a "" problem @@ -172,7 +172,7 @@ Feature: LMS.Answer problems Examples: | ProblemType | Points Possible | - | image | 1 point possible (ungraded) | + | image | 0/1 point (ungraded) | Scenario: I can reset the correctness of a problem after changing my answer Given I am viewing a "" problem diff --git a/lms/djangoapps/courseware/tests/test_masquerade.py b/lms/djangoapps/courseware/tests/test_masquerade.py index a441d5667f..d142e1bd62 100644 --- a/lms/djangoapps/courseware/tests/test_masquerade.py +++ b/lms/djangoapps/courseware/tests/test_masquerade.py @@ -293,7 +293,9 @@ class TestStaffMasqueradeAsSpecificStudent(StaffMasqueradeTestCase, ProblemSubmi The return value is a string like u'1/2'. """ - return json.loads(self.look_at_question(self.problem_display_name).content)['progress_detail'] + json_data = json.loads(self.look_at_question(self.problem_display_name).content) + progress = '%s/%s' % (str(json_data['current_score']), str(json_data['total_possible'])) + return progress @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) def test_masquerade_as_specific_user_on_self_paced(self): diff --git a/lms/djangoapps/courseware/tests/test_split_module.py b/lms/djangoapps/courseware/tests/test_split_module.py index a8a68f12dd..6161179c02 100644 --- a/lms/djangoapps/courseware/tests/test_split_module.py +++ b/lms/djangoapps/courseware/tests/test_split_module.py @@ -125,7 +125,7 @@ class SplitTestBase(SharedModuleStoreTestCase): content = resp.content # Assert we see the proper icon in the top display - self.assertIn('
% if attempts_allowed: - ${_("You have used {num_used} of {num_total} attempts").format(num_used=attempts_used, num_total=attempts_allowed)} + ${ungettext("You have used {num_used} of {num_total} attempt", "You have used {num_used} of {num_total} attempts", attempts_allowed).format(num_used=attempts_used, num_total=attempts_allowed)} % endif
diff --git a/lms/templates/problem_ajax.html b/lms/templates/problem_ajax.html index ae55c3e3a6..941e0855df 100644 --- a/lms/templates/problem_ajax.html +++ b/lms/templates/problem_ajax.html @@ -1 +1,9 @@ -
+
+
diff --git a/lms/templates/seq_module.html b/lms/templates/seq_module.html index edaea4fe80..47c6aac7d6 100644 --- a/lms/templates/seq_module.html +++ b/lms/templates/seq_module.html @@ -20,12 +20,8 @@