drop comments from rubric_assessments

Change-Id: I653ebae164ea588756b16091af9033bda988e35e
Reviewed-on: https://gerrit.instructure.com/62005
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Product-Review: James Williams  <jamesw@instructure.com>
QA-Review: James Williams  <jamesw@instructure.com>

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

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]

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

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

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