From e2f64145a9e62aebe1b7837ab0698c8948d7a10c Mon Sep 17 00:00:00 2001 From: James Williams Date: Fri, 28 Aug 2015 05:58:07 -0600 Subject: [PATCH] drop comments from rubric_assessments Change-Id: I653ebae164ea588756b16091af9033bda988e35e Reviewed-on: https://gerrit.instructure.com/62005 Tested-by: Jenkins Reviewed-by: Jeremy Stanley Product-Review: James Williams QA-Review: James Williams --- app/models/rubric_assessment.rb | 5 ++- app/models/rubric_association.rb | 1 - config/initializers/dropped_columns.rb | 1 + ...8_drop_comments_from_rubric_assessments.rb | 13 ++++++++ .../rubric_assessments_controller_spec.rb | 32 +++++++------------ 5 files changed, 28 insertions(+), 24 deletions(-) create mode 100644 db/migrate/20150828114628_drop_comments_from_rubric_assessments.rb diff --git a/app/models/rubric_assessment.rb b/app/models/rubric_assessment.rb index 88a085d08f1..5e800a730b0 100644 --- a/app/models/rubric_assessment.rb +++ b/app/models/rubric_assessment.rb @@ -23,7 +23,7 @@ class RubricAssessment < ActiveRecord::Base include TextHelper include HtmlTextHelper - attr_accessible :rubric, :rubric_association, :user, :score, :data, :comments, :assessor, :artifact, :assessment_type + attr_accessible :rubric, :rubric_association, :user, :score, :data, :assessor, :artifact, :assessment_type belongs_to :rubric belongs_to :rubric_association belongs_to :user @@ -35,11 +35,10 @@ class RubricAssessment < ActiveRecord::Base simply_versioned - EXPORTABLE_ATTRIBUTES = [:id, :user_id, :rubric_id, :rubric_association_id, :score, :data, :comments, :created_at, :updated_at, :artifact_id, :artifact_type, :assessment_type, :assessor_id, :artifact_attempt] + EXPORTABLE_ATTRIBUTES = [:id, :user_id, :rubric_id, :rubric_association_id, :score, :data, :created_at, :updated_at, :artifact_id, :artifact_type, :assessment_type, :assessor_id, :artifact_attempt] EXPORTABLE_ASSOCIATIONS = [:rubric, :rubric_association, :user, :assessor, :artifact, :assessment_requests] validates_presence_of :assessment_type, :rubric_id, :artifact_id, :artifact_type, :assessor_id - validates_length_of :comments, :maximum => maximum_text_length, :allow_nil => true, :allow_blank => true before_save :update_artifact_parameters before_save :htmlify_rating_comments diff --git a/app/models/rubric_association.rb b/app/models/rubric_association.rb index 73d289de9eb..6e0523f2d22 100644 --- a/app/models/rubric_association.rb +++ b/app/models/rubric_association.rb @@ -310,7 +310,6 @@ class RubricAssociation < ActiveRecord::Base assessment ||= association.rubric_assessments.build(:assessor => opts[:assessor], :artifact => artifact, :user => artifact.student, :rubric => self.rubric, :assessment_type => params[:assessment_type]) assessment.score = score if replace_ratings assessment.data = ratings if replace_ratings - assessment.comments = params[:comments] if params[:comments] assessment.save assessment_to_return = assessment if assessment.artifact == opts[:artifact] diff --git a/config/initializers/dropped_columns.rb b/config/initializers/dropped_columns.rb index 08f13514a51..c5e1c336713 100644 --- a/config/initializers/dropped_columns.rb +++ b/config/initializers/dropped_columns.rb @@ -101,6 +101,7 @@ class ActiveRecord::Base crypted_webdav_access_code type).freeze, 'role_overrides' => %w(context_code enrollment_type).freeze, + 'rubric_assessments' => %w{comments}.freeze, 'users' => %w(type creation_unique_id creation_sis_batch_id diff --git a/db/migrate/20150828114628_drop_comments_from_rubric_assessments.rb b/db/migrate/20150828114628_drop_comments_from_rubric_assessments.rb new file mode 100644 index 00000000000..c80d0ca9437 --- /dev/null +++ b/db/migrate/20150828114628_drop_comments_from_rubric_assessments.rb @@ -0,0 +1,13 @@ +class DropCommentsFromRubricAssessments < ActiveRecord::Migration + tag :postdeploy + + def up + def up + remove_column :rubric_assessments, :comments + end + + def down + add_column :rubric_assessments, :comments, :text + end + end +end diff --git a/spec/controllers/rubric_assessments_controller_spec.rb b/spec/controllers/rubric_assessments_controller_spec.rb index 95fa8aa018f..60ed911c1cb 100644 --- a/spec/controllers/rubric_assessments_controller_spec.rb +++ b/spec/controllers/rubric_assessments_controller_spec.rb @@ -43,7 +43,7 @@ describe RubricAssessmentsController do end end end - + describe "PUT 'update'" do it "should require authorization" do course_with_teacher(:active_all => true) @@ -57,16 +57,8 @@ describe RubricAssessmentsController do put 'update', :course_id => @course.id, :rubric_association_id => @rubric_association.id, :id => @rubric_assessment.id, :rubric_assessment => {:user_id => @user.to_param, :assessment_type => "no_reason"} expect(response).to be_success end - it "should update the assessment" do - course_with_teacher_logged_in(:active_all => true) - rubric_assessment_model(:user => @user, :context => @course, :purpose => 'grading') - put 'update', :course_id => @course.id, :rubric_association_id => @rubric_association.id, :id => @rubric_assessment.id, :rubric_assessment => {:comments => "dude!", :user_id => @user.to_param, :assessment_type => "no_reason"} - expect(response).to be_success - expect(assigns[:assessment]).not_to be_nil - expect(assigns[:assessment].comments).to eql("dude!") - end end - + describe "POST 'remind'" do before do course_with_teacher(:active_all => true) @@ -106,7 +98,7 @@ describe RubricAssessmentsController do expect(assigns[:assessment]).to be_frozen end end - + describe "Assignment assessments" do it "should follow: actions from two teachers should only create one assessment" do setup_course_assessment @@ -119,7 +111,7 @@ describe RubricAssessmentsController do expect(response).to be_success expect(assigns[:assessment]).to eql(@assessment) end - + it "should follow: multiple peer reviews for the same submission should work fine" do setup_course_assessment user_session(@student2) @@ -127,18 +119,18 @@ describe RubricAssessmentsController do expect(response).to be_success @assessment = assigns[:assessment] expect(@assessment).not_to be_nil - + user_session(@student3) post 'create', :course_id => @course.id, :rubric_association_id => @rubric_association.id, :rubric_assessment => {:user_id => @student1.to_param, :assessment_type => "peer_review"} expect(response).to be_success expect(assigns[:assessment]).not_to eql(@assessment) - + user_session(@teacher2) post 'create', :course_id => @course.id, :rubric_association_id => @rubric_association.id, :rubric_assessment => {:user_id => @student1.to_param, :assessment_type => "peer_review"} expect(response).to be_success expect(assigns[:assessment]).not_to eql(@assessment) end - + it "should follow: multiple peer reviews for the same submission should work fine, even with a teacher assessment in play" do setup_course_assessment user_session(@teacher2) @@ -152,25 +144,25 @@ describe RubricAssessmentsController do expect(response).to be_success @assessment = assigns[:assessment] expect(@assessment).not_to be_nil - + user_session(@student3) post 'create', :course_id => @course.id, :rubric_association_id => @rubric_association.id, :rubric_assessment => {:user_id => @student1.to_param, :assessment_type => "peer_review"} expect(response).to be_success expect(assigns[:assessment]).not_to eql(@assessment) - + user_session(@teacher2) post 'create', :course_id => @course.id, :rubric_association_id => @rubric_association.id, :rubric_assessment => {:user_id => @student1.to_param, :assessment_type => "peer_review"} expect(response).to be_success expect(assigns[:assessment]).not_to eql(@assessment) expect(assigns[:assessment]).not_to eql(@grading_assessment) end - + it "should not allow assessing fellow students for a submission" do setup_course_assessment user_session(@student1) post 'create', :course_id => @course.id, :rubric_association_id => @rubric_association.id, :rubric_assessment => {:user_id => @student2.to_param, :assessment_type => 'peer_review'} assert_unauthorized - + @assignment.submit_homework(@student1, :url => "http://www.google.com") @assignment.submit_homework(@student2, :url => "http://www.google.com") @assignment.submit_homework(@student3, :url => "http://www.google.com") @@ -180,7 +172,7 @@ describe RubricAssessmentsController do # two of the six possible combinations have already been created expect(res.length).to eql(4) expect(res.to_a.find{|r| r.assessor == @student1 && r.user == @student2}).not_to be_nil - + post 'create', :course_id => @course.id, :rubric_association_id => @rubric_association.id, :rubric_assessment => {:user_id => @student2.to_param, :assessment_type => 'peer_review'} expect(response).to be_success end