From 02fd084b888a42ace3aa46a213148d7194d87339 Mon Sep 17 00:00:00 2001 From: Brian Palmer Date: Tue, 21 Feb 2012 09:56:49 -0700 Subject: [PATCH] allow setting a letter grade on no-points assignments Previously this worked for 0.0 point assignments, but not assignments where points_possible was nil. Fixes #7009 test plan: * create a letter grade assignment and leave the points possible blank * try giving a student a letter grade on both gradebook 1 and 2 * verify that the letter grade is preserved when you refresh, and displays correctly to the student on their grades page Change-Id: I8fecf57ef4fa889cb6692243b2df027eb4e79ae4 Reviewed-on: https://gerrit.instructure.com/8859 Tested-by: Hudson Reviewed-by: Simon Williams --- app/models/assignment.rb | 20 +++++++++++++++---- app/models/submission.rb | 2 +- spec/models/assignment_spec.rb | 34 ++++++++++++++++++++++++++++++++ spec/selenium/gradebook2_spec.rb | 17 ++++++++++++++++ 4 files changed, 68 insertions(+), 5 deletions(-) diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 6ce2bcb91c1..73da2f8da39 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -523,7 +523,7 @@ class Assignment < ActiveRecord::Base result = "#{result}%" elsif self.grading_type == "pass_fail" if self.points_possible.to_f > 0.0 - passed = score.to_f == self.points_possible + passed = score.to_f == self.points_possible.to_f elsif given_grade # the score for a zero-point pass/fail assignment could be considered # either pass *or* fail, so look at what the current given grade is @@ -534,8 +534,20 @@ class Assignment < ActiveRecord::Base end result = passed ? "complete" : "incomplete" elsif self.grading_type == "letter_grade" - score = score.to_f / self.points_possible - result = GradingStandard.score_to_grade(self.grading_scheme, score * 100) + if self.points_possible.to_f > 0.0 + score = score.to_f / self.points_possible.to_f + result = GradingStandard.score_to_grade(self.grading_scheme, score * 100) + elsif given_grade + # the score for a zero-point letter_grade assignment could be considered + # to be *any* grade, so look at what the current given grade is + # instead of trying to calculate it + result = given_grade + else + # there's not really any reasonable value we can set here -- if the + # assignment is worth no points, and the grader didn't enter an + # explicit letter grade, any letter grade is as valid as any other. + result = GradingStandard.score_to_grade(self.grading_scheme, score.to_f) + end end result.to_s end @@ -556,7 +568,7 @@ class Assignment < ActiveRecord::Base else # try to treat it as a letter grade if grading_scheme && standard_based_score = GradingStandard.grade_to_score(grading_scheme, grade) - (points_possible * standard_based_score).round / 100.0 + ((points_possible || 0.0) * standard_based_score).round / 100.0 else nil end diff --git a/app/models/submission.rb b/app/models/submission.rb index 98a46fe0096..ddbb7ccc4d1 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -369,7 +369,7 @@ class Submission < ActiveRecord::Base if self.score_changed? @score_changed = true if self.assignment - self.grade = self.assignment.score_to_grade(self.score) if self.assignment.points_possible.to_f > 0.0 || self.assignment.grading_type != 'pass_fail' + self.grade = self.assignment.score_to_grade(self.score, self.grade) else self.grade = self.score.to_s end diff --git a/spec/models/assignment_spec.rb b/spec/models/assignment_spec.rb index fb4e91ff09f..61276e4c1b2 100644 --- a/spec/models/assignment_spec.rb +++ b/spec/models/assignment_spec.rb @@ -192,6 +192,40 @@ describe Assignment do @submission.user_id.should eql(@user.id) end + it "should preserve letter grades with zero points possible" do + setup_assignment_without_submission + @assignment.grading_type = 'letter_grade' + @assignment.points_possible = 0.0 + @assignment.save! + + s = @assignment.grade_student(@user, :grade => 'C') + s.should be_is_a(Array) + @assignment.reload + @assignment.submissions.size.should eql(1) + @submission = @assignment.submissions.first + @submission.state.should eql(:graded) + @submission.score.should eql(0.0) + @submission.grade.should eql('C') + @submission.user_id.should eql(@user.id) + end + + it "should preserve letter grades with no points possible" do + setup_assignment_without_submission + @assignment.grading_type = 'letter_grade' + @assignment.points_possible = nil + @assignment.save! + + s = @assignment.grade_student(@user, :grade => 'C') + s.should be_is_a(Array) + @assignment.reload + @assignment.submissions.size.should eql(1) + @submission = @assignment.submissions.first + @submission.state.should eql(:graded) + @submission.score.should eql(0.0) + @submission.grade.should eql('C') + @submission.user_id.should eql(@user.id) + end + it "should give a grade to extra credit assignments" do setup_assignment_without_submission @assignment.grading_type = 'points' diff --git a/spec/selenium/gradebook2_spec.rb b/spec/selenium/gradebook2_spec.rb index c8aef1e4678..bf115b0c9dd 100644 --- a/spec/selenium/gradebook2_spec.rb +++ b/spec/selenium/gradebook2_spec.rb @@ -341,6 +341,23 @@ describe "gradebook2" do driver.execute_script '$.store.clear();' end + it "should allow setting a letter grade on a no-points assignment" do + assignment_model(:course => @course, :grading_type => 'letter_grade', :points_possible => nil, :title => 'no-points') + get "/courses/#{@course.id}/gradebook2" + wait_for_ajaximations + + edit_grade(driver.find_element(:css, '#gradebook_grid [row="0"] .l3'), 'A-') + + get "/courses/#{@course.id}/gradebook2" + wait_for_ajaximations + + driver.find_element(:css, '#gradebook_grid [row="0"] .l3').text.should == 'A-' + @assignment.submissions.size.should == 1 + sub = @assignment.submissions.first + sub.grade.should == 'A-' + sub.score.should == 0.0 + end + it "should change grades and validate course total is correct" do expected_edited_total = "33.3%"