handle decimal points in rubrics correctly
allow points in rubric criteria and ratings to have decimal values. in working with brian, we're operating under the following assumptions: - round to 2 decimal places - splitting always gives you a integer by default (you can change it yourself after) - if there's not room to split with an integer, repeat the low value test plan - create a new rubric - change a criterial value to something with a decimal point - try setting specific ratings with decimals - try to split between a large space (ie 10.5 and 0), should be 5 - try to split between a small space (ie 0.5 and 0), should be 0 closes #5355 Change-Id: I17e26fe18dda0847fa59dd40976e4d6f38851287 Reviewed-on: https://gerrit.instructure.com/7882 Reviewed-by: Brian Palmer <brianp@instructure.com> Tested-by: Brian Palmer <brianp@instructure.com> Reviewed-by: Ryan Florence <ryanf@instructure.com>
This commit is contained in:
Simon Williams
parent
2a86f49d64
commit
64d687ec87
|
|
@ -215,7 +215,7 @@ class Rubric < ActiveRecord::Base
|
|||
criterion = {}
|
||||
criterion[:description] = (criterion_data[:description] || t('no_description', "No Description")).strip
|
||||
criterion[:long_description] = (criterion_data[:long_description] || "").strip
|
||||
criterion[:points] = criterion_data[:points].to_i || 0
|
||||
criterion[:points] = criterion_data[:points].to_f || 0
|
||||
criterion_data[:id].strip! if criterion_data[:id]
|
||||
criterion_data[:id] = nil if criterion_data[:id] && criterion_data[:id].empty?
|
||||
criterion[:id] = unique_item_id(criterion_data[:id])
|
||||
|
|
@ -234,7 +234,7 @@ class Rubric < ActiveRecord::Base
|
|||
rating = {}
|
||||
rating[:description] = (rating_data[:description] || t('no_description', "No Description")).strip
|
||||
rating[:long_description] = (rating_data[:long_description] || "").strip
|
||||
rating[:points] = rating_data[:points].to_i || 0
|
||||
rating[:points] = rating_data[:points].to_f || 0
|
||||
rating[:criterion_id] = criterion[:id]
|
||||
rating_data[:id].strip! if rating_data[:id]
|
||||
rating[:id] = unique_item_id(rating_data[:id])
|
||||
|
|
|
|||
|
|
@ -0,0 +1,9 @@
|
|||
class ChangeRubricPointsPossibleToFloat < ActiveRecord::Migration
|
||||
def self.up
|
||||
change_column :rubrics, :points_possible, :float
|
||||
end
|
||||
|
||||
def self.down
|
||||
change_column :rubrics, :points_possible, :integer
|
||||
end
|
||||
end
|
||||
|
|
@ -76,6 +76,7 @@ var rubricEditing = {
|
|||
total += points;
|
||||
}
|
||||
});
|
||||
total = round(total, 2);
|
||||
$rubric.find(".rubric_total").text(total);
|
||||
},
|
||||
updateCriterionPoints: function($criterion, baseOnRatings) {
|
||||
|
|
@ -85,16 +86,20 @@ var rubricEditing = {
|
|||
var points = parseFloat($criterion.find(".criterion_points").val());
|
||||
if(isNaN(points)) {
|
||||
points = 5;
|
||||
} else {
|
||||
points = round(points, 2);
|
||||
}
|
||||
$criterion.find(".rating:first .points").text(points);
|
||||
// From right to left, make sure points always increase by at least one
|
||||
// From right to left, make sure points never decrease
|
||||
// and round to 2 decimal places.
|
||||
$.each(ratings, function(i, rating) {
|
||||
var $rating = $(rating);
|
||||
var data = $rating.getTemplateData({textValues: ['points']});
|
||||
if(data.points < rating_points) {
|
||||
data.points = rating_points + 1;
|
||||
$rating.fillTemplateData({data: data});
|
||||
data.points = rating_points;
|
||||
}
|
||||
data.points = round(data.points, 2);
|
||||
$rating.fillTemplateData({data: data});
|
||||
rating_points = parseFloat(data.points);
|
||||
});
|
||||
if(baseOnRatings && rating_points > points) { points = rating_points; }
|
||||
|
|
@ -107,24 +112,26 @@ var rubricEditing = {
|
|||
}
|
||||
var oldMax = parseFloat($criterion.data('criterion_points'));
|
||||
var newMax = points;
|
||||
var $ratingList = $criterion.find(".rating");
|
||||
$($ratingList[0]).find(".points").text(points);
|
||||
var lastPts = points;
|
||||
// From left to right, scale points proportionally to new range.
|
||||
// So if originally they were 3,2,1 and now we increased the
|
||||
// total possible to 9, they'd be 9,6,3
|
||||
for(var i = 1; i < $ratingList.length - 1; i++) {
|
||||
var pts = parseFloat($($ratingList[i]).find(".points").text());
|
||||
var newPts = Math.round((pts / oldMax) * newMax);
|
||||
if(isNaN(pts) || (pts == 0 && lastPts > 0)) {
|
||||
newPts = lastPts - Math.round(lastPts / ($ratingList.length - i));
|
||||
if (oldMax !== newMax) {
|
||||
var $ratingList = $criterion.find(".rating");
|
||||
$($ratingList[0]).find(".points").text(points);
|
||||
var lastPts = points;
|
||||
// From left to right, scale points proportionally to new range.
|
||||
// So if originally they were 3,2,1 and now we increased the
|
||||
// total possible to 9, they'd be 9,6,3
|
||||
for(var i = 1; i < $ratingList.length - 1; i++) {
|
||||
var pts = parseFloat($($ratingList[i]).find(".points").text());
|
||||
var newPts = Math.round((pts / oldMax) * newMax);
|
||||
if(isNaN(pts) || (pts == 0 && lastPts > 0)) {
|
||||
newPts = lastPts - Math.round(lastPts / ($ratingList.length - i));
|
||||
}
|
||||
if(newPts >= lastPts) {
|
||||
newPts = lastPts - 1;
|
||||
}
|
||||
newPts = Math.max(0, newPts);
|
||||
lastPts = newPts;
|
||||
$($ratingList[i]).find(".points").text(newPts);
|
||||
}
|
||||
if(newPts >= lastPts) {
|
||||
newPts = lastPts - 1;
|
||||
}
|
||||
newPts = Math.max(0, newPts);
|
||||
lastPts = newPts;
|
||||
$($ratingList[i]).find(".points").text(newPts);
|
||||
}
|
||||
$criterion.data('criterion_points', points);
|
||||
}
|
||||
|
|
@ -347,6 +354,11 @@ var rubricEditing = {
|
|||
};
|
||||
rubricEditing.sizeRatings = $.debounce(10, rubricEditing.originalSizeRatings);
|
||||
|
||||
var round = function(number, precision) {
|
||||
precision = Math.pow(10, precision || 0).toFixed(precision < 0 ? -precision : 0);
|
||||
return Math.round(number * precision) / precision;
|
||||
}
|
||||
|
||||
I18n.scoped('edit_rubric', function(I18n) {
|
||||
$(document).ready(function() {
|
||||
var limitToOneRubric = true;
|
||||
|
|
@ -788,15 +800,13 @@ $(document).ready(function() {
|
|||
var more_points = parseFloat($this.prev(".rating").find(".points").text());
|
||||
data.points = Math.round((pts + more_points) / 2);
|
||||
if(data.points == pts || data.points == more_points) {
|
||||
data.points = more_points;
|
||||
$criterion.find(".criterion_points").val(criterion_total + 1);
|
||||
data.points = pts;
|
||||
}
|
||||
} else {
|
||||
var less_points = parseFloat($this.next(".rating").find(".points").text());
|
||||
data.points = Math.round((pts + less_points) / 2);
|
||||
if(data.points == pts || data.points == less_points) {
|
||||
data.points = pts;
|
||||
$criterionPoints.val(criterion_total + 1);
|
||||
data.points = less_points;
|
||||
}
|
||||
}
|
||||
$td.fillTemplateData({data: data});
|
||||
|
|
@ -868,4 +878,4 @@ $(document).ready(function() {
|
|||
}
|
||||
setInterval(rubricEditing.sizeRatings, 10000);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -238,4 +238,36 @@ describe Rubric do
|
|||
@result.mastery.should eql(true)
|
||||
end
|
||||
end
|
||||
|
||||
context "fractional_points" do
|
||||
it "should allow fractional points" do
|
||||
@rubric = Rubric.new(:context => @course)
|
||||
@rubric.data = [
|
||||
{
|
||||
:points => 0.5,
|
||||
:description => "Fraction row",
|
||||
:id => 1,
|
||||
:ratings => [
|
||||
{
|
||||
:points => 0.5,
|
||||
:description => "Rockin'",
|
||||
:criterion_id => 1,
|
||||
:id => 2
|
||||
},
|
||||
{
|
||||
:points => 0,
|
||||
:description => "Lame",
|
||||
:criterion_id => 1,
|
||||
:id => 3
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
@rubric.save!
|
||||
|
||||
@rubric2 = Rubric.find(@rubric.id)
|
||||
@rubric2.data.first[:points].should eql(0.5)
|
||||
@rubric2.data.first[:ratings].first[:points].should eql(0.5)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -339,18 +339,22 @@ describe "assignment selenium tests" do
|
|||
end
|
||||
end
|
||||
|
||||
it "should add a new rubric to assignment" do
|
||||
course_with_teacher_logged_in
|
||||
def create_assignment_with_points(points)
|
||||
assignment_name = 'first test assignment'
|
||||
due_date = Time.now.utc + 2.days
|
||||
@group = @course.assignment_groups.create!(:name => "default")
|
||||
@second_group = @course.assignment_groups.create!(:name => "second default")
|
||||
@assignment = @course.assignments.create(
|
||||
:name => assignment_name,
|
||||
:due_at => due_date,
|
||||
:points_possible => points,
|
||||
:assignment_group => @group
|
||||
)
|
||||
)
|
||||
end
|
||||
|
||||
it "should add a new rubric to assignment" do
|
||||
course_with_teacher_logged_in
|
||||
create_assignment_with_points(2)
|
||||
|
||||
get "/courses/#{@course.id}/assignments/#{@assignment.id}"
|
||||
|
||||
driver.find_element(:css, '.add_rubric_link').click
|
||||
|
|
@ -363,19 +367,12 @@ describe "assignment selenium tests" do
|
|||
|
||||
it "should import rubric to assignment" do
|
||||
course_with_teacher_logged_in
|
||||
assignment_name = 'first test assignment'
|
||||
due_date = Time.now.utc + 2.days
|
||||
group = @course.assignment_groups.create!(:name => "default")
|
||||
second_group = @course.assignment_groups.create!(:name => "second default")
|
||||
assignment = @course.assignments.create!(
|
||||
:name => assignment_name,
|
||||
:due_at => due_date,
|
||||
:assignment_group => group
|
||||
)
|
||||
create_assignment_with_points(2)
|
||||
|
||||
outcome_with_rubric
|
||||
@rubric.associate_with(@course, @course, :purpose => 'grading')
|
||||
|
||||
get "/courses/#{@course.id}/assignments/#{assignment.id}"
|
||||
get "/courses/#{@course.id}/assignments/#{@assignment.id}"
|
||||
|
||||
driver.find_element(:css, '.add_rubric_link').click
|
||||
driver.find_element(:css, '#rubric_new .editing .find_rubric_link').click
|
||||
|
|
@ -389,13 +386,7 @@ describe "assignment selenium tests" do
|
|||
|
||||
it "should not adjust assignment points possible for grading rubric" do
|
||||
course_with_teacher_logged_in
|
||||
assignment_name = 'first test assignment'
|
||||
@group = @course.assignment_groups.create!(:name => "default")
|
||||
@assignment = @course.assignments.create(
|
||||
:name => assignment_name,
|
||||
:assignment_group => @group,
|
||||
:points_possible => 2
|
||||
)
|
||||
create_assignment_with_points(2)
|
||||
|
||||
get "/courses/#{@course.id}/assignments/#{@assignment.id}"
|
||||
driver.find_element(:css, "#full_assignment .points_possible").text.should == '2'
|
||||
|
|
@ -411,13 +402,7 @@ describe "assignment selenium tests" do
|
|||
|
||||
it "should adjust assignment points possible for grading rubric" do
|
||||
course_with_teacher_logged_in
|
||||
assignment_name = 'first test assignment'
|
||||
@group = @course.assignment_groups.create!(:name => "default")
|
||||
@assignment = @course.assignments.create(
|
||||
:name => assignment_name,
|
||||
:assignment_group => @group,
|
||||
:points_possible => 2
|
||||
)
|
||||
create_assignment_with_points(2)
|
||||
|
||||
get "/courses/#{@course.id}/assignments/#{@assignment.id}"
|
||||
driver.find_element(:css, "#full_assignment .points_possible").text.should == '2'
|
||||
|
|
@ -432,6 +417,31 @@ describe "assignment selenium tests" do
|
|||
driver.find_element(:css, "#full_assignment .points_possible").text.should == '5'
|
||||
end
|
||||
|
||||
it "should carry decimal values through rubric to grading" do
|
||||
course_with_teacher_logged_in
|
||||
student_in_course
|
||||
create_assignment_with_points(2.5)
|
||||
|
||||
get "/courses/#{@course.id}/assignments/#{@assignment.id}"
|
||||
|
||||
driver.find_element(:css, '.add_rubric_link').click
|
||||
driver.find_element(:css, '#criterion_1 .criterion_points').send_key :backspace
|
||||
driver.find_element(:css, '#criterion_1 .criterion_points').send_key "2.5"
|
||||
driver.find_element(:id, 'grading_rubric').click
|
||||
driver.find_element(:id, 'edit_rubric_form').submit
|
||||
wait_for_ajaximations
|
||||
|
||||
get "/courses/#{@course.id}/gradebook/speed_grader?assignment_id=#{@assignment.id}"
|
||||
|
||||
wait_for_ajaximations
|
||||
driver.find_element(:css, '.toggle_full_rubric').click
|
||||
find_with_jquery('#rubric_holder .criterion:visible .rating').click
|
||||
driver.find_element(:css, '#rubric_holder .save_rubric_button').click
|
||||
wait_for_ajaximations
|
||||
|
||||
driver.find_element(:css, '#rubric_summary_container .rubric_total').text.should == '2.5'
|
||||
end
|
||||
|
||||
it "should show a \"more errors\" errorBox if any invalid fields are hidden" do
|
||||
course_with_teacher_logged_in
|
||||
assignment_name = 'first test assignment'
|
||||
|
|
|
|||
|
|
@ -570,6 +570,12 @@ shared_examples_for "all selenium tests" do
|
|||
end
|
||||
end
|
||||
|
||||
def load_simulate_js
|
||||
# load the simulate plugin to simulate a drag event
|
||||
js = File.read('spec/selenium/jquery.simulate.js')
|
||||
driver.execute_script js
|
||||
end
|
||||
|
||||
self.use_transactional_fixtures = false
|
||||
|
||||
append_after(:each) do
|
||||
|
|
|
|||
|
|
@ -178,9 +178,7 @@ shared_examples_for "quiz selenium tests" do
|
|||
names[0].text.should == 'first question'
|
||||
names[1].text.should == 'second question'
|
||||
|
||||
# load the simulate plugin to simulate a drag event
|
||||
js = File.read('spec/selenium/jquery.simulate.js')
|
||||
driver.execute_script js
|
||||
load_simulate_js
|
||||
|
||||
# drag the first question down 100px (next slot)
|
||||
driver.execute_script <<-JS
|
||||
|
|
|
|||
|
|
@ -0,0 +1,80 @@
|
|||
require File.expand_path(File.dirname(__FILE__) + '/common')
|
||||
|
||||
describe "rubric selenium tests" do
|
||||
it_should_behave_like "in-process server selenium tests"
|
||||
|
||||
before do
|
||||
course_with_teacher_logged_in
|
||||
end
|
||||
|
||||
def create_rubric_with_criterion_points(points)
|
||||
get "/courses/#{@course.id}/rubrics"
|
||||
|
||||
driver.find_element(:css, "#right-side-wrapper .add_rubric_link").click
|
||||
driver.find_element(:css, "#criterion_1 .criterion_points").send_key :backspace
|
||||
driver.find_element(:css, "#criterion_1 .criterion_points").send_key points.to_s
|
||||
driver.find_element(:css, "#edit_rubric_form .save_button").click
|
||||
wait_for_ajaximations
|
||||
end
|
||||
|
||||
def edit_rubric_after_updating
|
||||
find_with_jquery(".rubric .edit_rubric_link:visible").click
|
||||
driver.find_element(:tag_name, "body").click
|
||||
end
|
||||
|
||||
# should be in editing mode before calling
|
||||
def split_ratings(idx, split_left)
|
||||
load_simulate_js
|
||||
|
||||
rating = find_all_with_jquery(".rubric .criterion:visible .rating")[idx]
|
||||
driver.action.move_to(rating).perform
|
||||
|
||||
if split_left
|
||||
driver.execute_script <<-JS
|
||||
$ratings = $('.rubric .criterion:visible .rating');
|
||||
$($ratings[#{idx}]).addClass('add_column');
|
||||
$($ratings[#{idx}]).addClass('add_right');
|
||||
$($ratings[#{idx - 1}]).addClass('add_left');
|
||||
$($ratings[#{idx}]).simulate("click", {});
|
||||
JS
|
||||
else
|
||||
driver.execute_script <<-JS
|
||||
$ratings = $('.rubric .criterion:visible .rating');
|
||||
$($ratings[#{idx}]).addClass('add_column');
|
||||
$($ratings[#{idx+1}]).addClass('add_right');
|
||||
$($ratings[#{idx}]).addClass('add_left');
|
||||
$($ratings[#{idx}]).simulate("click", {});
|
||||
JS
|
||||
end
|
||||
end
|
||||
|
||||
it "should allow fractional points" do
|
||||
create_rubric_with_criterion_points "5.5"
|
||||
find_with_jquery(".rubric .criterion:visible .display_criterion_points").text.should == '5.5'
|
||||
find_with_jquery(".rubric .criterion:visible .rating .points").text.should == '5.5'
|
||||
end
|
||||
|
||||
it "should round to 2 decimal places" do
|
||||
create_rubric_with_criterion_points "5.249"
|
||||
find_with_jquery(".rubric .criterion:visible .display_criterion_points").text.should == '5.25'
|
||||
end
|
||||
|
||||
it "should round to an integer when splitting" do
|
||||
create_rubric_with_criterion_points "5.5"
|
||||
edit_rubric_after_updating
|
||||
|
||||
split_ratings(1, true)
|
||||
|
||||
find_all_with_jquery(".rubric .criterion:visible .rating .points")[1].text.should == '3'
|
||||
end
|
||||
|
||||
it "should pick the lower value when splitting without room for an integer" do
|
||||
create_rubric_with_criterion_points "0.5"
|
||||
edit_rubric_after_updating
|
||||
|
||||
split_ratings(1, true)
|
||||
|
||||
find_all_with_jquery(".rubric .criterion:visible .rating .points").count.should == 3
|
||||
find_all_with_jquery(".rubric .criterion:visible .rating .points")[1].text.should == '0'
|
||||
end
|
||||
end
|
||||
|
|
@ -341,6 +341,39 @@ Spec::Runner.configure do |config|
|
|||
@quiz_submission.grade_submission
|
||||
end
|
||||
|
||||
def rubric_for_course
|
||||
@rubric = Rubric.new(:title => 'My Rubric', :context => @course)
|
||||
@rubric.data = [
|
||||
{
|
||||
:points => 3,
|
||||
:description => "First row",
|
||||
:long_description => "The first row in the rubric",
|
||||
:id => 1,
|
||||
:ratings => [
|
||||
{
|
||||
:points => 3,
|
||||
:description => "Rockin'",
|
||||
:criterion_id => 1,
|
||||
:id => 2
|
||||
},
|
||||
{
|
||||
:points => 2,
|
||||
:description => "Rockin'",
|
||||
:criterion_id => 1,
|
||||
:id => 3
|
||||
},
|
||||
{
|
||||
:points => 0,
|
||||
:description => "Lame",
|
||||
:criterion_id => 1,
|
||||
:id => 4
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
@rubric.save!
|
||||
end
|
||||
|
||||
def outcome_with_rubric(opts={})
|
||||
@outcome_group ||= LearningOutcomeGroup.default_for(@course)
|
||||
@outcome = @course.created_learning_outcomes.create!(:description => '<p>This is <b>awesome</b>.</p>', :short_description => 'new outcome')
|
||||
|
|
|
|||
Loading…
Reference in New Issue