From 3ed2fcbb31758357f19b9124be7893f239b11c3a Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 10 Oct 2013 10:46:05 -0400 Subject: [PATCH 1/2] Parse floats to ints in CourseGrader model. Fixes STUD-826. --- cms/static/coffee/spec/main.coffee | 1 + .../models/settings_course_grader_spec.coffee | 20 +++++++++++++++++++ .../js/models/settings/course_grader.js | 6 +++--- .../models/settings/course_grading_policy.js | 2 +- cms/templates/edit_subsection.html | 3 +-- cms/templates/overview.html | 3 +-- 6 files changed, 27 insertions(+), 8 deletions(-) create mode 100644 cms/static/coffee/spec/models/settings_course_grader_spec.coffee diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index b23e7255fd..3fdb216e37 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -141,6 +141,7 @@ define([ "coffee/spec/models/course_spec", "coffee/spec/models/metadata_spec", "coffee/spec/models/module_spec", "coffee/spec/models/section_spec", + "coffee/spec/models/settings_course_grader_spec", "coffee/spec/models/settings_grading_spec", "coffee/spec/models/textbook_spec", "coffee/spec/models/upload_spec", diff --git a/cms/static/coffee/spec/models/settings_course_grader_spec.coffee b/cms/static/coffee/spec/models/settings_course_grader_spec.coffee new file mode 100644 index 0000000000..f6252a3590 --- /dev/null +++ b/cms/static/coffee/spec/models/settings_course_grader_spec.coffee @@ -0,0 +1,20 @@ +define ["js/models/settings/course_grader"], (CourseGrader) -> + describe "CourseGraderModel", -> + describe "parseWeight", -> + it "converts a float to an integer", -> + model = new CourseGrader({weight: 7.0001, min_count: 3.67, drop_count: 1.88}, {parse:true}) + expect(model.get('weight')).toBe(7) + expect(model.get('min_count')).toBe(3) + expect(model.get('drop_count')).toBe(1) + + it "converts a string to an integer", -> + model = new CourseGrader({weight: '7.0001', min_count: '3.67', drop_count: '1.88'}, {parse:true}) + expect(model.get('weight')).toBe(7) + expect(model.get('min_count')).toBe(3) + expect(model.get('drop_count')).toBe(1) + + it "does a no-op for integers", -> + model = new CourseGrader({weight: 7, min_count: 3, drop_count: 1}, {parse:true}) + expect(model.get('weight')).toBe(7) + expect(model.get('min_count')).toBe(3) + expect(model.get('drop_count')).toBe(1) diff --git a/cms/static/js/models/settings/course_grader.js b/cms/static/js/models/settings/course_grader.js index d04438bdff..a915c5f0ec 100644 --- a/cms/static/js/models/settings/course_grader.js +++ b/cms/static/js/models/settings/course_grader.js @@ -10,13 +10,13 @@ var CourseGrader = Backbone.Model.extend({ }, parse : function(attrs) { if (attrs['weight']) { - if (!_.isNumber(attrs.weight)) attrs.weight = parseInt(attrs.weight, 10); + attrs.weight = parseInt(attrs.weight, 10); } if (attrs['min_count']) { - if (!_.isNumber(attrs.min_count)) attrs.min_count = parseInt(attrs.min_count, 10); + attrs.min_count = parseInt(attrs.min_count, 10); } if (attrs['drop_count']) { - if (!_.isNumber(attrs.drop_count)) attrs.drop_count = parseInt(attrs.drop_count, 10); + attrs.drop_count = parseInt(attrs.drop_count, 10); } return attrs; }, diff --git a/cms/static/js/models/settings/course_grading_policy.js b/cms/static/js/models/settings/course_grading_policy.js index 7d16b61426..3ae901920d 100644 --- a/cms/static/js/models/settings/course_grading_policy.js +++ b/cms/static/js/models/settings/course_grading_policy.js @@ -20,7 +20,7 @@ var CourseGradingPolicy = Backbone.Model.extend({ graderCollection.reset(attributes.graders); } else { - graderCollection = new CourseGraderCollection(attributes.graders); + graderCollection = new CourseGraderCollection(attributes.graders, {parse:true}); graderCollection.course_location = attributes['course_location'] || this.get('course_location'); } attributes.graders = graderCollection; diff --git a/cms/templates/edit_subsection.html b/cms/templates/edit_subsection.html index 3ed4f6552e..8a4d61806c 100644 --- a/cms/templates/edit_subsection.html +++ b/cms/templates/edit_subsection.html @@ -114,9 +114,8 @@ require(["domReady!", "jquery", "js/models/location", "js/views/overview_assignm // I believe that current (New Section/New Subsection) cause full page reloads which means these aren't needed globally // but we really should change that behavior. if (!window.graderTypes) { - window.graderTypes = new CourseGraderCollection(); + window.graderTypes = new CourseGraderCollection(${course_graders|n}, {parse:true}); window.graderTypes.course_location = new Location('${parent_location}'); - window.graderTypes.reset(${course_graders|n}); } $(".gradable-status").each(function(index, ele) { diff --git a/cms/templates/overview.html b/cms/templates/overview.html index cfdb603107..5d75e3922a 100644 --- a/cms/templates/overview.html +++ b/cms/templates/overview.html @@ -25,9 +25,8 @@ require(["domReady!", "jquery", "js/models/location", "js/models/section", "js/v // I believe that current (New Section/New Subsection) cause full page reloads which means these aren't needed globally // but we really should change that behavior. if (!window.graderTypes) { - window.graderTypes = new CourseGraderCollection(); + window.graderTypes = new CourseGraderCollection(${course_graders|n}, {parse:true}); window.graderTypes.course_location = new Location('${parent_location}'); - window.graderTypes.reset(${course_graders|n}); } $(".gradable-status").each(function(index, ele) { From 8c080b6fb64442f5a056ead72a0c51e6aedc7c91 Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 10 Oct 2013 11:31:14 -0400 Subject: [PATCH 2/2] Acceptance test for fixing rounding error on weight. --- .../contentstore/features/grading.feature | 11 +++++++++++ cms/djangoapps/contentstore/features/grading.py | 16 ++++++++++++++++ .../js/models/settings/course_grading_policy.js | 2 +- 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/features/grading.feature b/cms/djangoapps/contentstore/features/grading.feature index f3ce1823e6..6c357a171e 100644 --- a/cms/djangoapps/contentstore/features/grading.feature +++ b/cms/djangoapps/contentstore/features/grading.feature @@ -59,6 +59,17 @@ Feature: CMS.Course Grading And I go back to the main course page Then I do see the assignment name "New Type" + # Note that "7" is a special weight because it revealed rounding errors (STUD-826). + Scenario: Users can set weight to Assignment types + Given I have opened a new course in Studio + And I am viewing the grading settings + When I add a new assignment type "New Type" + And I set the assignment weight to "7" + And I press the "Save" notification button + Then the assignment weight is displayed as "7" + And I reload the page + Then the assignment weight is displayed as "7" + Scenario: Settings are only persisted when saved Given I have opened a new course in Studio And I have populated the course diff --git a/cms/djangoapps/contentstore/features/grading.py b/cms/djangoapps/contentstore/features/grading.py index 24beefcd6a..b0db396081 100644 --- a/cms/djangoapps/contentstore/features/grading.py +++ b/cms/djangoapps/contentstore/features/grading.py @@ -106,6 +106,22 @@ def add_assignment_type(step, new_name): new_assignment._element.send_keys(new_name) +@step(u'I set the assignment weight to "([^"]*)"$') +def set_weight(step, weight): + weight_id = '#course-grading-assignment-gradeweight' + weight_field = world.css_find(weight_id)[-1] + old_weight = world.css_value(weight_id, -1) + for count in range(len(old_weight)): + weight_field._element.send_keys(Keys.END, Keys.BACK_SPACE) + weight_field._element.send_keys(weight) + + +@step(u'the assignment weight is displayed as "([^"]*)"$') +def verify_weight(step, weight): + weight_id = '#course-grading-assignment-gradeweight' + assert_equal(world.css_value(weight_id, -1), weight) + + @step(u'I have populated the course') def populate_course(step): step.given('I have added a new section') diff --git a/cms/static/js/models/settings/course_grading_policy.js b/cms/static/js/models/settings/course_grading_policy.js index 3ae901920d..1e23a4ecf4 100644 --- a/cms/static/js/models/settings/course_grading_policy.js +++ b/cms/static/js/models/settings/course_grading_policy.js @@ -17,7 +17,7 @@ var CourseGradingPolicy = Backbone.Model.extend({ // interesting race condition: if {parse:true} when newing, then parse called before .attributes created if (this.attributes && this.has('graders')) { graderCollection = this.get('graders'); - graderCollection.reset(attributes.graders); + graderCollection.reset(attributes.graders, {parse:true}); } else { graderCollection = new CourseGraderCollection(attributes.graders, {parse:true});