diff --git a/app/models/assignment.rb b/app/models/assignment.rb
index eb1935103ae..a53378d1023 100644
--- a/app/models/assignment.rb
+++ b/app/models/assignment.rb
@@ -897,7 +897,7 @@ class Assignment < ActiveRecord::Base
students.each do |student|
submission_updated = false
- submission = self.find_or_create_submission(student) #submissions.find_or_create_by_user_id(student.id) #(:first, :conditions => {:assignment_id => self.id, :user_id => student.id})
+ submission = self.find_or_create_submission(student)
if student == original_student || !grade_group_students_individually
previously_graded = submission.grade.present?
submission.attributes = opts
@@ -923,12 +923,6 @@ class Assignment < ActiveRecord::Base
submission.group = group
submission.graded_at = Time.zone.now if did_grade
previously_graded ? submission.with_versioning(:explicit => true) { submission.save! } : submission.save!
-
- unless self.rubric_association
- self.learning_outcome_alignments.each do |alignment|
- submission.create_outcome_result(alignment)
- end
- end
end
submission.add_comment(comment) if comment && (group_comment == "1" || student == original_student)
submissions << submission if group_comment == "1" || student == original_student || submission_updated
diff --git a/app/models/learning_outcome.rb b/app/models/learning_outcome.rb
index 037e931b5aa..63772abc9ca 100644
--- a/app/models/learning_outcome.rb
+++ b/app/models/learning_outcome.rb
@@ -91,12 +91,16 @@ class LearningOutcome < ActiveRecord::Base
end
def self.update_alignments(asset, context, new_outcome_ids)
- old_alignments = asset.learning_outcome_alignments
- old_outcome_ids = old_alignments.map(&:learning_outcome_id).compact.uniq
+ old_outcome_ids = asset.learning_outcome_alignments.
+ where("learning_outcome_id IS NOT NULL").
+ pluck(:learning_outcome_id).
+ uniq
defunct_outcome_ids = old_outcome_ids - new_outcome_ids
unless defunct_outcome_ids.empty?
- asset.learning_outcome_alignments.where(:learning_outcome_id => defunct_outcome_ids).update_all(:workflow_state => 'deleted')
+ asset.learning_outcome_alignments.
+ where(:learning_outcome_id => defunct_outcome_ids).
+ update_all(:workflow_state => 'deleted')
end
missing_outcome_ids = new_outcome_ids - old_outcome_ids
diff --git a/app/models/rubric.rb b/app/models/rubric.rb
index 462fbf42b0f..b199e6da0b5 100644
--- a/app/models/rubric.rb
+++ b/app/models/rubric.rb
@@ -113,11 +113,14 @@ class Rubric < ActiveRecord::Base
end
end
- attr_accessor :alignments_changed
def update_alignments
- return unless @alignments_changed
- outcome_ids = (self.data || []).map{|c| c[:learning_outcome_id] }.compact.map(&:to_i).uniq
- LearningOutcome.update_alignments(self, context, outcome_ids)
+ if data_changed? || workflow_state_changed?
+ outcome_ids = []
+ unless deleted?
+ outcome_ids = (data || []).map{|c| c[:learning_outcome_id] }.compact.map(&:to_i).uniq
+ end
+ LearningOutcome.update_alignments(self, context, outcome_ids)
+ end
true
end
@@ -205,7 +208,6 @@ class Rubric < ActiveRecord::Base
if criterion_data[:learning_outcome_id].present?
outcome = LearningOutcome.find_by_id(criterion_data[:learning_outcome_id])
if outcome
- @alignments_changed = true
criterion[:learning_outcome_id] = outcome.id
criterion[:mastery_points] = ((criterion_data[:mastery_points] || outcome.data[:rubric_criterion][:mastery_points]).to_f rescue nil)
criterion[:ignore_for_scoring] = criterion_data[:ignore_for_scoring] == '1'
@@ -282,7 +284,6 @@ class Rubric < ActiveRecord::Base
item.data = hash[:data]
item.data.each do |crit|
if crit[:learning_outcome_migration_id]
- item.alignments_changed = true
if migration.respond_to?(:outcome_to_id_map) && id = migration.outcome_to_id_map[crit[:learning_outcome_migration_id]]
crit[:learning_outcome_id] = id
elsif lo = context.created_learning_outcomes.find_by_migration_id(crit[:learning_outcome_migration_id])
@@ -302,15 +303,4 @@ class Rubric < ActiveRecord::Base
item
end
-
- def self.generate(opts={})
- context = opts[:context]
- raise "Context required for rubrics" unless context
- rubric = context.rubrics.build(:user => opts[:user])
- user = opts[:user]
- params = opts[:data]
- rubric.update_criteria(params)
- rubric
- end
-
end
diff --git a/app/models/rubric_association.rb b/app/models/rubric_association.rb
index 058fe23bdde..5bc3571d231 100644
--- a/app/models/rubric_association.rb
+++ b/app/models/rubric_association.rb
@@ -27,7 +27,7 @@ class RubricAssociation < ActiveRecord::Base
belongs_to :association, :polymorphic => true
belongs_to :context, :polymorphic => true
- has_many :rubric_assessments, :dependent => :destroy
+ has_many :rubric_assessments
has_many :assessment_requests, :dependent => :destroy
has_a_broadcast_policy
@@ -41,8 +41,9 @@ class RubricAssociation < ActiveRecord::Base
after_create :link_to_assessments
before_save :update_old_rubric
after_destroy :update_rubric
+ after_destroy :update_alignments
after_save :assert_uniqueness
- after_save :update_outcome_relations
+ after_save :update_alignments
serialize :summary_data
ValidAssociationModels = {
@@ -96,9 +97,12 @@ class RubricAssociation < ActiveRecord::Base
end
end
- def update_outcome_relations
+ def update_alignments
return unless assignment
- outcome_ids = rubric.learning_outcome_alignments.map(&:learning_outcome_id)
+ outcome_ids = []
+ unless self.destroyed?
+ outcome_ids = rubric.learning_outcome_alignments.map(&:learning_outcome_id)
+ end
LearningOutcome.update_alignments(assignment, context, outcome_ids)
true
end
diff --git a/app/models/submission.rb b/app/models/submission.rb
index 86611be1944..53a9bc3939c 100644
--- a/app/models/submission.rb
+++ b/app/models/submission.rb
@@ -1090,39 +1090,6 @@ class Submission < ActiveRecord::Base
hash
end
- def create_outcome_result(alignment, explicit_mastery=false)
- # find or create the user's unique LearningOutcomeResult for this alignment
- # of the submission's assignment.
- result = alignment.learning_outcome_results.
- for_association(assignment).
- find_or_initialize_by_user_id(user.id)
-
- # force the context and artifact
- result.artifact = self
- result.context = alignment.context
-
- # mastery
- result.possible = assignment.points_possible
- result.score = score
- if alignment.tag == "points_mastery"
- result.mastery = result.score && assignment.mastery_score && result.score >= assignment.mastery_score
- elsif alignment.tag == "explicit_mastery"
- result.mastery = explicit_mastery
- else
- result.mastery = nil
- end
-
- # attempt
- result.attempt = attempt
-
- # title
- result.title = "#{user.name}, #{assignment.title}"
-
- result.assessed_at = Time.now
- result.save_to_version(result.attempt)
- result
- end
-
def update_participation
# TODO: can we do this in bulk?
return if assignment.deleted? || assignment.muted?
diff --git a/app/views/outcomes/show.html.erb b/app/views/outcomes/show.html.erb
index a0eac09044f..d9eed49bcf8 100644
--- a/app/views/outcomes/show.html.erb
+++ b/app/views/outcomes/show.html.erb
@@ -27,7 +27,24 @@
<% @results.each do |result| %>
- <%= render :partial => "outcome_result", :object => result %>
+ <%#
+ There are a few old LearningOutcomeResults that have a nil artifact, and
+ many that have a Submission as an artifact. We want to get rid of these
+ because they come from bad data, and step 1 toward that goal is to stop
+ creating them and hide them in the UI.
+
+ The LORs with a nil artifact are very old and I believe came from a very
+ early incarnation of outcomes. The LORs with a Submission as an artifact
+ came from a combination of two code problems. The first was old code
+ that allowed an assignment that was aligned with an outcome but did not
+ have a rubric to create LORs directly based on it's submission. The
+ second was a bug that prevented the assignment <-> outcome link from
+ being destroyed when a rubric with an outcome was removed from an
+ assignment.
+ %>
+ <% if result.artifact_type.present? && result.artifact_type != 'Submission' %>
+ <%= render :partial => "outcome_result", :object => result %>
+ <% end %>
<% end %>
<%= will_paginate(@results) %>
diff --git a/spec/apis/v1/assignments_api_spec.rb b/spec/apis/v1/assignments_api_spec.rb
index 94e5cfb7821..83b0ef4c65d 100644
--- a/spec/apis/v1/assignments_api_spec.rb
+++ b/spec/apis/v1/assignments_api_spec.rb
@@ -208,6 +208,7 @@ describe AssignmentsApiController, :type => :integration do
:free_form_criterion_comments => true)
@assignment.create_rubric_association(:rubric => @rubric,
+ :context => @course,
:purpose => 'grading',
:use_for_grading => true)
json = api_get_assignments_index_from_course(@course)
diff --git a/spec/controllers/rubric_associations_controller_spec.rb b/spec/controllers/rubric_associations_controller_spec.rb
index 0748900623e..681cd970637 100644
--- a/spec/controllers/rubric_associations_controller_spec.rb
+++ b/spec/controllers/rubric_associations_controller_spec.rb
@@ -125,5 +125,20 @@ describe RubricAssociationsController do
assigns[:association].should_not be_nil
assigns[:association].should be_frozen
end
+
+ it "should remove aligments links" do
+ course_with_teacher_logged_in(:active_all => true)
+ outcome_with_rubric
+ rubric_association_model(:user => @user, :context => @course, :rubric => @rubric)
+
+ @rubric_association_object.reload.learning_outcome_alignments.count.should == 1
+ @rubric.reload.learning_outcome_alignments.count.should == 1
+
+ delete 'destroy', :course_id => @course.id, :id => @rubric_association.id
+
+ @rubric.reload.deleted?.should be_true
+ @rubric_association_object.reload.learning_outcome_alignments.count.should == 0
+ @rubric.reload.learning_outcome_alignments.count.should == 0
+ end
end
end
diff --git a/spec/controllers/rubrics_controller_spec.rb b/spec/controllers/rubrics_controller_spec.rb
index e8806f7e94f..3dec498bf44 100644
--- a/spec/controllers/rubrics_controller_spec.rb
+++ b/spec/controllers/rubrics_controller_spec.rb
@@ -47,6 +47,7 @@ describe RubricsController do
post 'create', :course_id => @course.id
assert_unauthorized
end
+
it "should assign variables" do
course_with_teacher_logged_in(:active_all => true)
post 'create', :course_id => @course.id, :rubric => {}
@@ -55,6 +56,7 @@ describe RubricsController do
response.should be_success
end
+
it "should create an association if specified" do
course_with_teacher_logged_in(:active_all => true)
association = @course.assignments.create!(assignment_valid_attributes)
@@ -64,6 +66,72 @@ describe RubricsController do
assigns[:rubric].rubric_associations.length.should eql(1)
response.should be_success
end
+
+ it "should associate outcomes correctly" do
+ course_with_teacher_logged_in(:active_all => true)
+ assignment = @course.assignments.create!(assignment_valid_attributes)
+ outcome_group = @course.root_outcome_group
+ outcome = @course.created_learning_outcomes.create!(
+ :description => 'hi',
+ :short_description => 'hi'
+ )
+ outcome_group.add_outcome(outcome)
+ outcome_group.save!
+
+ create_params = {
+ "course_id" => @course.id,
+ "points_possible" => "5",
+ "rubric" => {
+ "criteria" => {
+ "0" => {
+ "description" => "hi",
+ "id" => "",
+ "learning_outcome_id" => outcome.id,
+ "long_description" => "",
+ "mastery_points" => "3",
+ "points" => "5",
+ "ratings" => {
+ "0" => {
+ "description" => "Exceeds Expectations",
+ "id" => "blank",
+ "points" => "5"
+ },
+ "1" => {
+ "description" => "Meets Expectations",
+ "id" => "blank",
+ "points" => "3"
+ },
+ "2" => {
+ "description" => "Does Not Meet Expectations",
+ "id" => "blank_2",
+ "points" => "0"
+ }
+ }
+ }
+ },
+ "free_form_criterion_comments" => "0",
+ "points_possible" => "5",
+ "title" => "Some Rubric"
+ },
+ "rubric_association" => {
+ "association_id" => assignment.id,
+ "association_type" => "Assignment",
+ "hide_score_total" => "0",
+ "id" => "",
+ "purpose" => "grading",
+ "use_for_grading" => "1"
+ },
+ "rubric_association_id" => "",
+ "rubric_id" => "new",
+ "skip_updating_points_possible" => "false",
+ "title" => "Some Rubric"
+ }
+
+ post 'create', create_params
+
+ assignment.reload.learning_outcome_alignments.count.should == 1
+ Rubric.last.learning_outcome_alignments.count.should == 1
+ end
end
describe "PUT 'update'" do
@@ -203,6 +271,135 @@ describe RubricsController do
assigns[:rubric].rubric_associations.find_by_id(@rubric_association.id).title.should eql("some title")
response.should be_success
end
+
+ it "should add an outcome association if one is linked" do
+ course_with_teacher_logged_in(:active_all => true)
+ assignment = @course.assignments.create!(assignment_valid_attributes)
+ rubric_association_model(:user => @user, :context => @course)
+ outcome_group = @course.root_outcome_group
+ outcome = @course.created_learning_outcomes.create!(
+ :description => 'hi',
+ :short_description => 'hi'
+ )
+ outcome_group.add_outcome(outcome)
+ outcome_group.save!
+
+ update_params = {
+ "course_id" => @course.id,
+ "id" => @rubric.id,
+ "points_possible" => "5",
+ "rubric" => {
+ "criteria" => {
+ "0" => {
+ "description" => "hi",
+ "id" => "",
+ "learning_outcome_id" => outcome.id,
+ "long_description" => "",
+ "points" => "5",
+ "ratings" => {
+ "0" => {
+ "description" => "Exceeds Expectations",
+ "id" => "blank",
+ "points" => "5"
+ },
+ "1" => {
+ "description" => "Meets Expectations",
+ "id" => "blank",
+ "points" => "3"
+ },
+ "2" => {
+ "description" => "Does Not Meet Expectations",
+ "id" => "blank_2",
+ "points" => "0"
+ }
+ }
+ }
+ },
+ "free_form_criterion_comments" => "0",
+ "points_possible" => "5",
+ "title" => "Some Rubric"
+ },
+ "rubric_association" => {
+ "association_id" => assignment.id,
+ "association_type" => "Assignment",
+ "hide_score_total" => "0",
+ "id" => @rubric_association.id,
+ "purpose" => "grading",
+ "use_for_grading" => "1"
+ },
+ "rubric_association_id" => @rubric_association.id,
+ "rubric_id" => @rubric.id,
+ "skip_updating_points_possible" => "false",
+ "title" => "Some Rubric"
+ }
+
+ assignment.reload.learning_outcome_alignments.count.should == 0
+ @rubric.reload.learning_outcome_alignments.count.should == 0
+
+ put 'update', update_params
+
+ assignment.reload.learning_outcome_alignments.count.should == 1
+ @rubric.reload.learning_outcome_alignments.count.should == 1
+ end
+
+ it "should remove an outcome association if one is removed" do
+ course_with_teacher_logged_in(:active_all => true)
+ outcome_with_rubric
+ assignment = @course.assignments.create!(assignment_valid_attributes)
+ association = @rubric.associate_with(assignment, @course, :purpose => 'grading')
+
+ update_params = {
+ "course_id" => @course.id,
+ "id" => @rubric.id,
+ "points_possible" => "5",
+ "rubric" => {
+ "criteria" => {
+ "0" => {
+ "description" => "Description of criterion",
+ "id" => "",
+ "learning_outcome_id" => "",
+ "long_description" => "",
+ "points" => "5",
+ "ratings" => {
+ "0" => {
+ "description" => "Full Marks",
+ "id" => "blank",
+ "points" => "5"
+ },
+ "1" => {
+ "description" => "No Marks",
+ "id" => "blank_2",
+ "points" => "0"
+ }
+ }
+ }
+ },
+ "free_form_criterion_comments" => "0",
+ "points_possible" => "5",
+ "title" => "Some Rubric"
+ },
+ "rubric_association" => {
+ "association_id" => assignment.id,
+ "association_type" => "Assignment",
+ "hide_score_total" => "0",
+ "id" => association.id,
+ "purpose" => "grading",
+ "use_for_grading" => "1"
+ },
+ "rubric_association_id" => association.id,
+ "rubric_id" => @rubric.id,
+ "skip_updating_points_possible" => "false",
+ "title" => "Some Rubric"
+ }
+
+ assignment.reload.learning_outcome_alignments.count.should == 1
+ @rubric.reload.learning_outcome_alignments.count.should == 1
+
+ put 'update', update_params
+
+ assignment.reload.learning_outcome_alignments.count.should == 0
+ @rubric.reload.learning_outcome_alignments.count.should == 0
+ end
end
describe "DELETE 'destroy'" do
diff --git a/spec/models/content_migration_spec.rb b/spec/models/content_migration_spec.rb
index 002d0daaed5..3b010741421 100644
--- a/spec/models/content_migration_spec.rb
+++ b/spec/models/content_migration_spec.rb
@@ -691,7 +691,6 @@ describe ContentMigration do
:learning_outcome_id => lo2.id
}
]
- rub.alignments_changed = true
rub.save!
rub.associate_with(@copy_from, @copy_from)
@@ -724,7 +723,6 @@ describe ContentMigration do
:learning_outcome_id => lo.id
}
]
- rub.alignments_changed = true
rub.save!
from_assign = @copy_from.assignments.create!(:title => "some assignment")
diff --git a/spec/models/learning_outcome_spec.rb b/spec/models/learning_outcome_spec.rb
index 1856e82227f..632d2761a08 100644
--- a/spec/models/learning_outcome_spec.rb
+++ b/spec/models/learning_outcome_spec.rb
@@ -46,7 +46,6 @@ describe LearningOutcome do
:learning_outcome_id => @outcome.id
}
]
- @rubric.instance_variable_set('@alignments_changed', true)
@rubric.save!
@rubric.should_not be_new_record
@rubric.reload
@@ -80,7 +79,6 @@ describe LearningOutcome do
:learning_outcome_id => @outcome.id
}
]
- @rubric.instance_variable_set('@alignments_changed', true)
@rubric.save!
@rubric.should_not be_new_record
@rubric.reload
@@ -157,7 +155,6 @@ describe LearningOutcome do
:learning_outcome_id => @outcome2.id
}
]
- @rubric.instance_variable_set('@alignments_changed', true)
@rubric.save!
@rubric.reload
@rubric.should_not be_new_record
@@ -191,7 +188,6 @@ describe LearningOutcome do
:learning_outcome_id => @outcome.id
}
]
- @rubric.instance_variable_set('@alignments_changed', true)
@rubric.save!
@rubric.reload
@rubric.should_not be_new_record
@@ -277,7 +273,6 @@ describe LearningOutcome do
:learning_outcome_id => @outcome.id
}
]
- @rubric.instance_variable_set('@alignments_changed', true)
@rubric.save!
@rubric.reload
@rubric.should_not be_new_record
@@ -321,6 +316,7 @@ describe LearningOutcome do
@result.mastery.should eql(false)
n = @result.version_number
end
+
it "should not override rubric-based alignments with non-rubric-based alignments for the same assignment" do
assignment_model
@outcome = @course.created_learning_outcomes.create!(:title => 'outcome')
@@ -347,7 +343,6 @@ describe LearningOutcome do
:learning_outcome_id => @outcome.id
}
]
- @rubric.instance_variable_set('@alignments_changed', true)
@rubric.save!
@rubric.reload
@rubric.should_not be_new_record
@@ -388,37 +383,6 @@ describe LearningOutcome do
@result.mastery.should eql(false)
n = @result.version_number
end
- it "should build an outcome result for a non-rubric alignment" do
- assignment_model
- @assignment.mastery_score = 2
- @assignment.save!
- @outcome = @course.created_learning_outcomes.create!(:title => 'outcome')
- @alignment = @outcome.align(@assignment, @course, :mastery_type => "points")
- @alignment.should_not be_nil
- @alignment.content.should eql(@assignment)
- @alignment.context.should eql(@course)
-
- @user = user(:active_all => true)
- @e = @course.enroll_student(@user)
- @alignment.rubric_association_id.should eql(nil)
- @assignment.reload
- @assignment.learning_outcome_alignments.should_not be_empty
- @assignment.learning_outcome_alignments.length.should eql(1)
- @assignment.learning_outcome_alignments.map(&:learning_outcome_id).uniq.should eql([@outcome.id])
-
- @submission = @assignment.grade_student(@user, :grade => "10").first
- @outcome.learning_outcome_results.should_not be_empty
- @outcome.learning_outcome_results.length.should eql(1)
-
- @result = @outcome.learning_outcome_results.select{|r| r.artifact_type == 'Submission'}.first
- @result.should_not be_nil
- @result.user_id.should eql(@user.id)
- @result.possible.should eql(1.5)
- @result.score.should eql(10.0)
- @result.original_score.should eql(10.0)
- @result.original_possible.should eql(1.5)
- @result.mastery.should eql(true)
- end
end
describe "permissions" do
diff --git a/spec/models/rubric_association_spec.rb b/spec/models/rubric_association_spec.rb
index 919a59529ac..e3ebaef57ed 100644
--- a/spec/models/rubric_association_spec.rb
+++ b/spec/models/rubric_association_spec.rb
@@ -20,34 +20,40 @@
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
describe RubricAssociation do
- context "when course has multiple students enrolled" do
+
+ def rubric_association_params_for_assignment(assign)
+ HashWithIndifferentAccess.new({
+ hide_score_total: "0",
+ purpose: "grading",
+ skip_updating_points_possible: false,
+ update_if_existing: true,
+ use_for_grading: "1",
+ association: assign
+ })
+ end
+
+ context "course rubrics" do
before :each do
# Create a course, 2 students and enroll them
- @test_course = course(:active_course => true)
- @teacher = @test_course.teachers.first
- @student_1 = user(:active_user => true)
- @student_2 = user(:active_user => true)
- @test_course.enroll_student(@student_1)
- @test_course.enroll_student(@student_2)
+ course_with_teacher(:active_course => true, :active_user => true)
+ @student_1 = student_in_course(:active_user => true).user
+ @student_2 = student_in_course(:active_user => true).user
end
context "when a peer-review assignment has been completed AFTER rubric created" do
before :each do
# Create the assignment
- @assignment = course.assignments.create!(:title => 'Test Assignment', :peer_reviews => true)
- @assignment.workflow_state = 'published'
- @assignment.submission_types = 'online_text_entry'
- @assignment.context = @test_course
+ @assignment = @course.assignments.create!(
+ :title => 'Test Assignment',
+ :peer_reviews => true,
+ :submission_types => 'online_text_entry'
+ )
# Create the rubric
- @rubric = @test_course.rubrics.build
- @rubric.user = @teacher
- @rubric.save!
- rubric_association = HashWithIndifferentAccess.new({"hide_score_total"=>"0", "purpose"=>"grading",
- "skip_updating_points_possible"=>false, "update_if_existing"=>true,
- "use_for_grading"=>"1", "association"=>@assignment}) #, "id"=>3})
+ @rubric = @course.rubrics.create! { |r| r.user = @teacher }
- @rubric_assoc = RubricAssociation.generate(@teacher, @rubric, @test_course, rubric_association)
+ ra_params = rubric_association_params_for_assignment(@assignment)
+ @rubric_assoc = RubricAssociation.generate(@teacher, @rubric, @course, ra_params)
# students complete it
@assignment.submit_homework(@student_1, :submission_type => 'online_text_entry', :body => 'Finished first')
@@ -70,10 +76,11 @@ describe RubricAssociation do
context "when a peer-review assignment has been completed BEFORE rubric created" do
before :each do
# Create the assignment
- @assignment = course.assignments.create!(:title => 'Test Assignment', :peer_reviews => true)
- @assignment.workflow_state = 'published'
- @assignment.submission_types = 'online_text_entry'
- @assignment.context = @test_course
+ @assignment = @course.assignments.create!(
+ :title => 'Test Assignment',
+ :peer_reviews => true,
+ :submission_types => 'online_text_entry'
+ )
# students complete it
@assignment.submit_homework(@student_1, :submission_type => 'online_text_entry', :body => 'Finished first')
@@ -89,14 +96,9 @@ describe RubricAssociation do
context "and a rubric is created" do
before :each do
- @rubric = @test_course.rubrics.build
- @rubric.user = @teacher
- @rubric.save!
- rubric_association = HashWithIndifferentAccess.new({"hide_score_total"=>"0", "purpose"=>"grading",
- "skip_updating_points_possible"=>false, "update_if_existing"=>true,
- "use_for_grading"=>"1", "association"=>@assignment}) #, "id"=>3})
-
- @rubric_assoc = RubricAssociation.generate(@teacher, @rubric, @test_course, rubric_association)
+ @rubric = @course.rubrics.create! { |r| r.user = @teacher }
+ ra_params = rubric_association_params_for_assignment(@assignment)
+ @rubric_assoc = RubricAssociation.generate(@teacher, @rubric, @course, ra_params)
end
it "should have 2 assessment_requests" do
@@ -105,6 +107,67 @@ describe RubricAssociation do
end
end
end
+
+ context "#update_alignments" do
+ it "should do nothing if it is not associated to an assignment" do
+ rubric = @course.rubrics.create!
+ ra = RubricAssociation.create!(
+ :rubric => @rubric,
+ :association => @course,
+ :context => @course,
+ :purpose => 'bookmark'
+ )
+ LearningOutcome.expects(:update_alignments).never
+ ra.update_alignments
+ end
+
+ it "should align the outcome to the assignment when created and remove when destroyed" do
+ assignment = @course.assignments.create!(
+ :title => 'Test Assignment',
+ :peer_reviews => true,
+ :submission_types => 'online_text_entry'
+ )
+ outcome_with_rubric
+ ra = @rubric.rubric_associations.create!(
+ :association => assignment,
+ :context => @course,
+ :purpose => 'grading'
+ )
+ assignment.reload.learning_outcome_alignments.count.should == 1
+
+ ra.destroy
+ assignment.reload.learning_outcome_alignments.count.should == 0
+ end
+ end
+
+ it "should not delete assessments when an association is destroyed" do
+ assignment = @course.assignments.create!(
+ :title => 'Test Assignment',
+ :peer_reviews => true,
+ :submission_types => 'online_text_entry'
+ )
+ outcome_with_rubric
+ ra = @rubric.rubric_associations.create!(
+ :association => assignment,
+ :context => @course,
+ :purpose => 'grading'
+ )
+ assess = ra.assess({
+ :user => @student_1,
+ :assessor => @teacher,
+ :artifact => assignment.find_or_create_submission(@student_1),
+ :assessment => {
+ :assessment_type => 'grading',
+ :criterion_crit1 => {
+ :points => 5
+ }
+ }
+ })
+
+ assess.should_not be_nil
+ ra.destroy
+ assess.reload.should_not be_nil
+ end
end
context "when a rubric is associated with an account" do
diff --git a/spec/models/rubric_spec.rb b/spec/models/rubric_spec.rb
index b979c55268c..d32fb0ce389 100644
--- a/spec/models/rubric_spec.rb
+++ b/spec/models/rubric_spec.rb
@@ -47,7 +47,6 @@ describe Rubric do
:learning_outcome_id => @outcome.id
}
]
- @rubric.instance_variable_set('@alignments_changed', true)
@rubric.save!
@rubric.should_not be_new_record
@rubric.learning_outcome_alignments(true).should_not be_empty
@@ -80,7 +79,6 @@ describe Rubric do
:learning_outcome_id => @outcome.id
}
]
- @rubric.instance_variable_set('@alignments_changed', true)
@rubric.save!
@rubric.should_not be_new_record
@rubric.learning_outcome_alignments(true).should_not be_empty
@@ -107,6 +105,7 @@ describe Rubric do
@rubric.save!
@rubric.learning_outcome_alignments.active.should be_empty
end
+
it "should create learning outcome associations for multiple outcome rows" do
assignment_model
@outcome = @course.created_learning_outcomes.create!(:title => 'outcome')
@@ -154,12 +153,12 @@ describe Rubric do
:learning_outcome_id => @outcome2.id
}
]
- @rubric.instance_variable_set('@alignments_changed', true)
@rubric.save!
@rubric.should_not be_new_record
@rubric.learning_outcome_alignments(true).should_not be_empty
@rubric.learning_outcome_alignments.map(&:learning_outcome_id).sort.should eql([@outcome.id, @outcome2.id].sort)
end
+
it "should create outcome results when outcome-aligned rubrics are assessed" do
assignment_model
@outcome = @course.created_learning_outcomes.create!(:title => 'outcome')
@@ -186,7 +185,6 @@ describe Rubric do
:learning_outcome_id => @outcome.id
}
]
- @rubric.instance_variable_set('@alignments_changed', true)
@rubric.save!
@rubric.should_not be_new_record
@rubric.learning_outcome_alignments(true).should_not be_empty
diff --git a/spec/selenium/assignments_rubrics_spec.rb b/spec/selenium/assignments_rubrics_spec.rb
index cd45ed42344..aafb0e3a963 100644
--- a/spec/selenium/assignments_rubrics_spec.rb
+++ b/spec/selenium/assignments_rubrics_spec.rb
@@ -182,7 +182,6 @@ describe "assignment rubrics" do
@submission = @assignment.submit_homework(@student, {:url => "http://www.instructure.com/"})
@rubric.data[0][:ignore_for_scoring] = '1'
@rubric.points_possible = 5
- @rubric.alignments_changed = true
@rubric.save!
@assignment.points_possible = 5
@assignment.save!
diff --git a/spec/selenium/teacher_rubrics_spec.rb b/spec/selenium/teacher_rubrics_spec.rb
index 87cea25dd92..6f28339ba2c 100644
--- a/spec/selenium/teacher_rubrics_spec.rb
+++ b/spec/selenium/teacher_rubrics_spec.rb
@@ -50,7 +50,6 @@ describe "course rubrics" do
@association = @rubric.associate_with(@assignment, @course, :use_for_grading => true, :purpose => 'grading')
@rubric.data[0][:ignore_for_scoring] = '1'
@rubric.points_possible = 5
- @rubric.alignments_changed = true
@rubric.save!
get "/courses/#{@course.id}/rubrics/#{@rubric.id}"
diff --git a/spec/selenium/teacher_speed_grader_spec.rb b/spec/selenium/teacher_speed_grader_spec.rb
index e874a3bba5e..dfa4b4096aa 100644
--- a/spec/selenium/teacher_speed_grader_spec.rb
+++ b/spec/selenium/teacher_speed_grader_spec.rb
@@ -432,7 +432,6 @@ describe "speed grader" do
:learning_outcome_id => @ignored.id,
:ignore_for_scoring => '1',
}]
- @rubric.alignments_changed = true
@rubric.save!
get "/courses/#{@course.id}/gradebook/speed_grader?assignment_id=#{@assignment.id}"
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 8d3a75b37ee..a03dc3383c6 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -613,52 +613,52 @@ Spec::Runner.configure do |config|
@outcome_group.add_outcome(@outcome)
@outcome_group.save!
- @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",
- }
- },
- :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",
- }
- }
- }
- }
- })
- @rubric.instance_variable_set('@alignments_changed', true)
- @rubric.save!
- @rubric.update_alignments
+ rubric_params = {
+ :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",
+ }
+ },
+ :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",
+ }
+ }
+ }
+ }
+ }
+
+ @rubric = @course.rubrics.build
+ @rubric.update_criteria(rubric_params)
+ @rubric.reload
end
def grading_standard_for(context, opts={})
diff --git a/vendor/plugins/account_reports/lib/canvas/account_reports/outcome_reports.rb b/vendor/plugins/account_reports/lib/canvas/account_reports/outcome_reports.rb
index 3282bc18aaa..d68015661a6 100644
--- a/vendor/plugins/account_reports/lib/canvas/account_reports/outcome_reports.rb
+++ b/vendor/plugins/account_reports/lib/canvas/account_reports/outcome_reports.rb
@@ -104,7 +104,8 @@ module Canvas::AccountReports
AND sub.user_id = pseudonyms.user_id", parameters])).
where("
ct.tag_type = 'learning_outcome'
- AND ct.workflow_state <> 'deleted'"
+ AND ct.workflow_state <> 'deleted'
+ AND (r.id IS NULL OR (r.artifact_type IS NOT NULL AND r.artifact_type <> 'Submission'))"
)
unless @include_deleted
@@ -205,7 +206,8 @@ module Canvas::AccountReports
AND r.artifact_type = 'QuizSubmission'
LEFT OUTER JOIN assessment_questions aq ON aq.id = r.associated_asset_id
AND r.associated_asset_type = 'AssessmentQuestion'").
- where("ct.workflow_state <> 'deleted'")
+ where("ct.workflow_state <> 'deleted'
+ AND (r.id IS NULL OR (r.artifact_type IS NOT NULL AND r.artifact_type <> 'Submission'))")
unless @include_deleted
students = students.where("p.workflow_state<>'deleted' AND c.workflow_state='available'")
diff --git a/vendor/plugins/account_reports/spec_canvas/outcome_reports_spec.rb b/vendor/plugins/account_reports/spec_canvas/outcome_reports_spec.rb
index 0aa3b8c0414..733f630cbc2 100644
--- a/vendor/plugins/account_reports/spec_canvas/outcome_reports_spec.rb
+++ b/vendor/plugins/account_reports/spec_canvas/outcome_reports_spec.rb
@@ -215,6 +215,20 @@ describe "Outcom Reports" do
"https://#{HostUrl.context_host(@course1)}/courses/#{@course1.id}/assignments/#{@assignment.id}"]
end
+
+ it "should not incluce invalid learning outcome results" do
+ ct = @assignment.learning_outcome_alignments.last
+ lor = ct.learning_outcome_results.for_association(@assignment).build
+ lor.user = @user1
+ lor.artifact = @submission
+ lor.context = ct.context
+ lor.possible = @assignment.points_possible
+ lor.score = @submission.score
+ lor.save!
+
+ parsed = ReportSpecHelper.run_report(@account, @type, {}, 1)
+ parsed.length.should == 2
+ end
end
describe "outcome results report" do
@@ -324,4 +338,4 @@ describe "Outcom Reports" do
end
end
-end
\ No newline at end of file
+end