Merge pull request #13786 from edx/aj/TNL-5549_problemMetaRefactor

Aj/tnl 5549 problem meta refactor
This commit is contained in:
Albert (AJ) St. Aubin
2016-10-21 13:03:32 -04:00
committed by GitHub
14 changed files with 117 additions and 176 deletions

View File

@@ -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,
})

View File

@@ -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)

View File

@@ -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 ->

View File

@@ -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();
};

View File

@@ -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)

View File

@@ -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)

View File

@@ -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,

View File

@@ -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)