allow multiple rubric associations for grading; fixes #7091
tweak our javascript so that the rubrics controller update method knows that we want to save the rubric association even if we can't modify the rubric. also clean up some unused code in the rubric spec that was confusingly named read_only, but not related to the rubric read_only property. test-plan: - create a rubric - create two assignments that have the same points - associate the rubric with the first assignment and select it be used for grading. make sure it sticks across refresh (and the rubric is not duplicated) - same for the second assignment Change-Id: I2c32d4da35eff508680f0a19c97cb1111fe4cef1 Reviewed-on: https://gerrit.instructure.com/8981 Tested-by: Hudson <hudson@instructure.com> Reviewed-by: Brian Palmer <brianp@instructure.com>
This commit is contained in:
parent
db48cddd3e
commit
22f9590e15
|
@ -64,20 +64,18 @@ class RubricsController < ApplicationController
|
|||
@association_object = RubricAssociation.get_association_object(params[:rubric_association])
|
||||
params[:rubric][:user] = @current_user if params[:rubric]
|
||||
if (!@association_object || authorized_action(@association_object, @current_user, :read)) && authorized_action(@context, @current_user, :manage_grades)
|
||||
if params[:rubric_association_id].present?
|
||||
@association = @context.rubric_associations.find_by_id(params[:rubric_association_id])
|
||||
end
|
||||
@association = @context.rubric_associations.find_by_id(params[:rubric_association_id]) if params[:rubric_association_id].present?
|
||||
@association_object ||= @association.association if @association
|
||||
params[:rubric_association][:association] = @association_object
|
||||
params[:rubric_association][:update_if_existing] = params[:action] == 'update'
|
||||
skip_points_update = !!(params[:skip_updating_points_possible] =~ /true/i)
|
||||
params[:rubric_association][:skip_updating_points_possible] = skip_points_update
|
||||
@rubric = @association.rubric if params[:id] && @association && (@association.rubric_id == params[:id].to_i || (@association.rubric && @association.rubric.migration_id == "cloned_from_#{params[:id]}"))
|
||||
@rubric ||= @context.rubrics.find_by_id(params[:id]) if params[:id]
|
||||
@rubric ||= @context.rubrics.find_by_id(params[:id]) if params[:id].present?
|
||||
@association = nil unless @association && @rubric && @association.rubric_id == @rubric.id
|
||||
params[:rubric_association][:id] = @association.id if @association
|
||||
# Update the rubric if you can
|
||||
# Better specify params[:rubric_association][:id] if you want it to update an existing association
|
||||
# Better specify params[:rubric_association_id] if you want it to update an existing association
|
||||
|
||||
# If this is a brand new rubric OR if the rubric isn't editable,
|
||||
# then create a new rubric
|
||||
|
|
|
@ -55,6 +55,7 @@ class Rubric < ActiveRecord::Base
|
|||
given {|user, session| self.cached_context_grants_right?(user, session, :manage)}
|
||||
can :read and can :create and can :delete_associations
|
||||
|
||||
# read_only means "associated with > 1 object for grading purposes"
|
||||
given {|user, session| !self.read_only && self.rubric_associations.for_grading.length < 2 && self.cached_context_grants_right?(user, session, :manage_assignments)}
|
||||
can :update and can :delete
|
||||
|
||||
|
@ -207,7 +208,7 @@ class Rubric < ActiveRecord::Base
|
|||
end
|
||||
|
||||
def will_change_with_update?(params)
|
||||
return true if params[:free_form_criterion_comments] && self.free_form_criterion_comments != (params[:free_form_criterion_comments] == '1')
|
||||
return true if params[:free_form_criterion_comments] && !!self.free_form_criterion_comments != (params[:free_form_criterion_comments] == '1')
|
||||
data = generate_criteria(params)
|
||||
return true if data.title != self.title || data.points_possible != self.points_possible
|
||||
return true if data.criteria != self.criteria
|
||||
|
@ -238,7 +239,7 @@ class Rubric < ActiveRecord::Base
|
|||
criterion[:ignore_for_scoring] = criterion_data[:ignore_for_scoring] == '1'
|
||||
end
|
||||
end
|
||||
(criterion_data[:ratings] || []).each do |jdx, rating_data|
|
||||
(criterion_data[:ratings] || {}).each do |jdx, rating_data|
|
||||
rating = {}
|
||||
rating[:description] = (rating_data[:description] || t('no_description', "No Description")).strip
|
||||
rating[:long_description] = (rating_data[:long_description] || "").strip
|
||||
|
|
|
@ -99,7 +99,7 @@ $(document).ready(function() {
|
|||
<%= render :partial => "shared/rubric_criterion_dialog" %>
|
||||
</div>
|
||||
<%= render :partial => "shared/find_outcome" %>
|
||||
<%= render :partial => "shared/rubric", :object => nil, :locals => {:association => @assignment, :editable => can_do(@assignment, @current_user, :update), :has_assessments => false, :edit_view => true, :read_only => !can_do(@assignment, @current_user, :update)} %>
|
||||
<%= render :partial => "shared/rubric", :object => nil, :locals => {:association => @assignment, :editable => can_do(@assignment, @current_user, :update), :has_assessments => false, :edit_view => true} %>
|
||||
<% if can_do(@assignment, @current_user, :update) %>
|
||||
<div style="text-align: center; font-size: 1.2em; margin-top: 10px; display: none;">
|
||||
<a href="<%= context_url(@context, :context_rubrics_url) %>" class="add_rubric_link rubric" style="<%= hidden if @assignment.rubric_association %>"><%= t 'links.assign_rubric', 'Assign Rubric' %></a>
|
||||
|
|
|
@ -13,7 +13,7 @@
|
|||
<% end %>
|
||||
<%= render :partial => "shared/rubric_criterion_dialog" %>
|
||||
<%= render :partial => "shared/find_outcome" %>
|
||||
<%= render :partial => "shared/rubric", :object => nil, :locals => {:association => @assignment, :editable => can_do(@assignment, @current_user, :update), :has_assessments => false, :edit_view => true, :read_only => !can_do(@assignment, @current_user, :update)} %>
|
||||
<%= render :partial => "shared/rubric", :object => nil, :locals => {:association => @assignment, :editable => can_do(@assignment, @current_user, :update), :has_assessments => false, :edit_view => true} %>
|
||||
<% if can_do(@assignment, @current_user, :update) %>
|
||||
<div style="text-align: center; font-size: 1.2em; margin-top: 10px; display: none;">
|
||||
<a href="<%= context_url(@context, :context_rubrics_url) %>" class="add_rubric_link rubric" style="<%= hidden if @assignment.rubric_association %>"><%= t 'links.assign_rubric', "Assign Rubric" %></a>
|
||||
|
|
|
@ -1,5 +1,4 @@
|
|||
<% rubric_association ||= nil; editable ||= false; association_object = nil; edit_view ||= false
|
||||
read_only ||= false
|
||||
rubric ||= nil; assessing ||= false; assessment ||= nil; has_assessments ||= false
|
||||
for_context ||= false
|
||||
context = @context
|
||||
|
@ -36,7 +35,7 @@
|
|||
<% end %>
|
||||
</div>
|
||||
</div>
|
||||
<div style="float: right; font-size: 0.8em; <%= hidden if editable || !edit_view || read_only %>" class="links displaying locked">
|
||||
<div style="float: right; font-size: 0.8em; display: none;" class="links displaying locked">
|
||||
<span style="<%= hidden if editable %>"><%= t 'messages.locked', "Can't change a rubric once you've started using it." %></span>
|
||||
<% if for_context %>
|
||||
<a href="<%= context_url(context, :context_rubric_url, rubric ? rubric.id : "{{ id }}") %>" class="delete_rubric_url" style="display: none;"> </a>
|
||||
|
|
|
@ -241,6 +241,9 @@ require([
|
|||
}
|
||||
data['rubric[free_form_criterion_comments]'] = $rubric.find(".rubric_custom_rating").attr('checked') ? "1" : "0";
|
||||
data['rubric_association[id]'] = vals.rubric_association_id;
|
||||
// make sure the association is always updated, see the comment on
|
||||
// RubricsController#update
|
||||
data['rubric_association_id'] = vals.rubric_association_id;
|
||||
var criterion_idx = 0;
|
||||
$rubric.find(".criterion:not(.blank)").each(function() {
|
||||
var $criterion = $(this);
|
||||
|
|
|
@ -199,6 +199,34 @@ describe "assignment rubrics" do
|
|||
wait_for_ajaximations
|
||||
driver.find_element(:css, '.grading_value').attribute(:value).should == "5"
|
||||
end
|
||||
|
||||
def mark_rubric_for_grading(rubric, expect_confirmation)
|
||||
driver.find_element(:css, "#rubric_#{rubric.id} .edit_rubric_link").click
|
||||
driver.switch_to.alert.accept if expect_confirmation
|
||||
find_with_jquery(".grading_rubric_checkbox:visible").click
|
||||
find_with_jquery(".save_button:visible").click
|
||||
wait_for_ajaximations
|
||||
end
|
||||
|
||||
it "should allow multiple rubric associations for grading" do
|
||||
outcome_with_rubric
|
||||
@assignment1 = @course.assignments.create!(:name => "assign 1", :points_possible => @rubric.points_possible)
|
||||
@assignment2 = @course.assignments.create!(:name => "assign 2", :points_possible => @rubric.points_possible)
|
||||
|
||||
@association1 = @rubric.associate_with(@assignment1, @course, :purpose => 'grading')
|
||||
@association2 = @rubric.associate_with(@assignment2, @course, :purpose => 'grading')
|
||||
|
||||
get "/courses/#{@course.id}/assignments/#{@assignment1.id}"
|
||||
mark_rubric_for_grading(@rubric, true)
|
||||
|
||||
get "/courses/#{@course.id}/assignments/#{@assignment2.id}"
|
||||
mark_rubric_for_grading(@rubric, true)
|
||||
|
||||
@association1.reload.use_for_grading.should be_true
|
||||
@association1.rubric.id.should == @rubric.id
|
||||
@association2.reload.use_for_grading.should be_true
|
||||
@association2.rubric.id.should == @rubric.id
|
||||
end
|
||||
end
|
||||
|
||||
context "assignment rubrics as a student" do
|
||||
|
|
|
@ -238,7 +238,7 @@ describe "speedgrader" do
|
|||
rubric.find_element(:css, '.criterion_comments img').click
|
||||
driver.find_element(:css, 'textarea.criterion_comments').send_keys('special rubric comment')
|
||||
driver.find_element(:css, '#rubric_criterion_comments_dialog .save_button').click
|
||||
second_criterion = rubric.find_element(:id, 'criterion_2')
|
||||
second_criterion = rubric.find_element(:id, "criterion_#{@rubric.criteria[1][:id]}")
|
||||
second_criterion.find_element(:css, '.ratings .edge_rating').click
|
||||
rubric.find_element(:css, '.rubric_total').should include_text('8')
|
||||
driver.find_element(:css, '#rubric_full .save_rubric_button').click
|
||||
|
@ -363,9 +363,9 @@ describe "speedgrader" do
|
|||
wait_for_animations
|
||||
rubric = driver.find_element(:id, 'rubric_full')
|
||||
rubric.should be_displayed
|
||||
first_criterion = rubric.find_element(:id, 'criterion_1')
|
||||
first_criterion = rubric.find_element(:id, "criterion_#{@rubric.criteria[0][:id]}")
|
||||
first_criterion.find_element(:css, '.ratings .edge_rating').click
|
||||
second_criterion = rubric.find_element(:id, 'criterion_2')
|
||||
second_criterion = rubric.find_element(:id, "criterion_#{@rubric.criteria[1][:id]}")
|
||||
second_criterion.find_element(:css, '.ratings .edge_rating').click
|
||||
rubric.find_element(:css, '.rubric_total').should include_text('8')
|
||||
driver.find_element(:css, '#rubric_full .save_rubric_button').click
|
||||
|
@ -373,22 +373,22 @@ describe "speedgrader" do
|
|||
driver.find_element(:css, '.toggle_full_rubric').click
|
||||
wait_for_animations
|
||||
|
||||
driver.execute_script("return $('#criterion_1 input.criterion_points').val();").should == "3"
|
||||
driver.execute_script("return $('#criterion_2 input.criterion_points').val();").should == "5"
|
||||
driver.execute_script("return $('#criterion_#{@rubric.criteria[0][:id]} input.criterion_points').val();").should == "3"
|
||||
driver.execute_script("return $('#criterion_#{@rubric.criteria[1][:id]} input.criterion_points').val();").should == "5"
|
||||
|
||||
driver.find_element(:css, '#gradebook_header .next').click
|
||||
wait_for_ajaximations
|
||||
|
||||
driver.find_element(:id, 'rubric_full').should be_displayed
|
||||
driver.execute_script("return $('#criterion_1 input.criterion_points').val();").should == ""
|
||||
driver.execute_script("return $('#criterion_2 input.criterion_points').val();").should == ""
|
||||
driver.execute_script("return $('#criterion_#{@rubric.criteria[0][:id]} input.criterion_points').val();").should == ""
|
||||
driver.execute_script("return $('#criterion_#{@rubric.criteria[1][:id]} input.criterion_points').val();").should == ""
|
||||
|
||||
driver.find_element(:css, '#gradebook_header .prev').click
|
||||
wait_for_ajaximations
|
||||
|
||||
driver.find_element(:id, 'rubric_full').should be_displayed
|
||||
driver.execute_script("return $('#criterion_1 input.criterion_points').val();").should == "3"
|
||||
driver.execute_script("return $('#criterion_2 input.criterion_points').val();").should == "5"
|
||||
driver.execute_script("return $('#criterion_#{@rubric.criteria[0][:id]} input.criterion_points').val();").should == "3"
|
||||
driver.execute_script("return $('#criterion_#{@rubric.criteria[1][:id]} input.criterion_points').val();").should == "5"
|
||||
end
|
||||
|
||||
it "should handle versions correctly" do
|
||||
|
|
|
@ -432,56 +432,49 @@ Spec::Runner.configure do |config|
|
|||
@outcome_group.add_item(@outcome)
|
||||
@outcome_group.save!
|
||||
|
||||
@rubric = Rubric.new(:title => 'My Rubric', :context => @course, :points_possible => 8, :hide_score_total => false)
|
||||
@rubric.data = [
|
||||
{
|
||||
:points => 3,
|
||||
:description => "Outcome row",
|
||||
:long_description => @outcome.description,
|
||||
:id => 1,
|
||||
:ratings => [
|
||||
{
|
||||
:points => 3,
|
||||
:description => "Rockin'",
|
||||
:criterion_id => 1,
|
||||
:id => 2
|
||||
@rubric = Rubric.generate(:context => @course,
|
||||
:data => {
|
||||
:title => 'My Rubric',
|
||||
:hide_score_total => false,
|
||||
:criteria => {
|
||||
"0" => {
|
||||
:points => 3,
|
||||
:mastery_points => 0,
|
||||
:description => "Outcome row",
|
||||
:long_description => @outcome.description,
|
||||
:ratings => {
|
||||
"0" => {
|
||||
:points => 3,
|
||||
:description => "Rockin'",
|
||||
},
|
||||
"1" => {
|
||||
:points => 0,
|
||||
:description => "Lame",
|
||||
}
|
||||
},
|
||||
{
|
||||
:points => 0,
|
||||
:description => "Lame",
|
||||
:criterion_id => 1,
|
||||
:id => 3
|
||||
:learning_outcome_id => @outcome.id
|
||||
},
|
||||
"1" => {
|
||||
:points => 5,
|
||||
:description => "no outcome row",
|
||||
:long_description => 'non outcome criterion',
|
||||
:ratings => {
|
||||
"0" => {
|
||||
:points => 5,
|
||||
:description => "Amazing",
|
||||
},
|
||||
"1" => {
|
||||
:points => 3,
|
||||
:description => "not too bad",
|
||||
},
|
||||
"2" => {
|
||||
:points => 0,
|
||||
:description => "no bueno",
|
||||
}
|
||||
}
|
||||
],
|
||||
:learning_outcome_id => @outcome.id
|
||||
},
|
||||
{
|
||||
:points => 5,
|
||||
:description => "no outcome row",
|
||||
:long_description => 'non outcome criterion',
|
||||
:id => 2,
|
||||
:ratings => [
|
||||
{
|
||||
:points => 5,
|
||||
:description => "Amazing",
|
||||
:criterion_id => 2,
|
||||
:id => 4
|
||||
},
|
||||
{
|
||||
:points => 3,
|
||||
:description => "not too bad",
|
||||
:criterion_id => 2,
|
||||
:id => 5
|
||||
},
|
||||
{
|
||||
:points => 0,
|
||||
:description => "no bueno",
|
||||
:criterion_id => 2,
|
||||
:id => 6
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
]
|
||||
})
|
||||
@rubric.instance_variable_set('@outcomes_changed', true)
|
||||
@rubric.save!
|
||||
@rubric.update_outcome_tags
|
||||
|
|
Loading…
Reference in New Issue