create ProvisionalGrade table and model

- extracted some shared behavior between submissions and provisional grades
- improved organization in submissions

refs: CNVS-22006
Change-Id: I4207893362e4548d1d76d066322f3b333357807a
Reviewed-on: https://gerrit.instructure.com/59643
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Jenkins
QA-Review: Jahnavi Yetukuri <jyetukuri@instructure.com>
Product-Review: Derek Bender <dbender@instructure.com>

Gemfile.d/app.rb

@@ -1,4 +1,3 @@
-
 if CANVAS_RAILS3
   gem 'rails', '3.2.22'
   gem 'rack', '1.4.5'

Gemfile.d/test.rb

@@ -17,6 +17,7 @@ group :test do
   gem 'rspec-rails', '3.2.3'
   gem 'rspec-legacy_formatters', '1.0.0'
   gem 'rspec-collection_matchers', '1.1.2'
+  gem 'shoulda-matchers', '2.8.0'
 
   gem 'rubocop', require: false
   gem 'rubocop-rspec', require: false

app/models/grading_period.rb

@@ -70,7 +70,6 @@ class GradingPeriod < ActiveRecord::Base
       .last == self
   end
 
-
   def overlapping?
     overlaps.active.exists?
   end

app/models/moderated_grading.rb

@@ -0,0 +1,5 @@
+module ModeratedGrading
+  def self.table_name_prefix
+    'moderated_grading_'
+  end
+end

app/models/moderated_grading/provisional_grade.rb

@@ -0,0 +1,23 @@
+class ModeratedGrading::ProvisionalGrade < ActiveRecord::Base
+  include Canvas::GradeValidations
+
+  attr_accessible :grade, :score, :position
+
+  belongs_to :submission
+  belongs_to :scorer, class_name: 'User'
+
+  validates :position, uniqueness: { scope: :submission_id }
+  validates :position, presence: true
+  validates :scorer, presence: true
+  validates :submission, presence: true
+
+  def valid?(*)
+    set_graded_at if grade_changed?
+    super
+  end
+
+  private
+  def set_graded_at
+    self.graded_at = Time.zone.now
+  end
+end

app/models/submission.rb

@@ -19,11 +19,16 @@
 require 'atom'
 
 class Submission < ActiveRecord::Base
+  include Canvas::GradeValidations
+  include CustomValidations
   include SendToStream
+  include Workflow
+
   attr_protected :submitted_at
   attr_readonly :assignment_id
   attr_accessor :visible_to_user,
                 :skip_grade_calc
+
   belongs_to :attachment # this refers to the screenshot of the submission if it is a url submission
   belongs_to :assignment
   belongs_to :user
@@ -31,15 +36,16 @@ class Submission < ActiveRecord::Base
   belongs_to :group
   belongs_to :media_object
   belongs_to :student, :class_name => 'User', :foreign_key => :user_id
+  belongs_to :quiz_submission, :class_name => 'Quizzes::QuizSubmission'
   has_many :submission_comments, :order => 'created_at', :dependent => :destroy
   has_many :visible_submission_comments, :class_name => 'SubmissionComment', :order => 'created_at, id', :conditions => { :hidden => false }
   has_many :hidden_submission_comments, :class_name => 'SubmissionComment', :order => 'created_at, id', :conditions => { :hidden => true }
   has_many :assessment_requests, :as => :asset
   has_many :assigned_assessments, :class_name => 'AssessmentRequest', :as => :assessor_asset
-  belongs_to :quiz_submission, :class_name => 'Quizzes::QuizSubmission'
-  has_one :rubric_assessment, :as => :artifact, :conditions => {:assessment_type => "grading"}
   has_many :rubric_assessments, :as => :artifact
   has_many :attachment_associations, :as => :context
+  has_many :provisional_grades, class_name: 'ModeratedGrading::ProvisionalGrade'
+  has_one :rubric_assessment, :as => :artifact, :conditions => {:assessment_type => "grading"}
 
   # we no longer link submission comments and conversations, but we haven't fixed up existing
   # linked conversations so this relation might be useful
@@ -49,23 +55,29 @@ class Submission < ActiveRecord::Base
   has_many :content_participations, :as => :content
 
   EXPORTABLE_ATTRIBUTES = [
-    :id, :body, :url, :attachment_id, :grade, :score, :submitted_at, :assignment_id, :user_id, :submission_type, :workflow_state, :created_at, :updated_at, :group_id,
-    :attachment_ids, :processed, :process_attempts, :grade_matches_current_submission, :published_score, :published_grade, :graded_at, :student_entered_score, :grader_id,
-    :media_comment_id, :media_comment_type, :quiz_submission_id, :submission_comments_count, :has_rubric_assessment, :attempt, :context_code, :media_object_id,
-    :turnitin_data, :has_admin_comment, :cached_due_date
-  ]
+    :id, :body, :url, :attachment_id, :grade, :score, :submitted_at,
+    :assignment_id, :user_id, :submission_type, :workflow_state,
+    :created_at, :updated_at, :group_id, :attachment_ids, :processed,
+    :process_attempts, :grade_matches_current_submission, :published_score,
+    :published_grade, :graded_at, :student_entered_score, :grader_id,
+    :media_comment_id, :media_comment_type, :quiz_submission_id,
+    :submission_comments_count, :has_rubric_assessment, :attempt,
+    :context_code, :media_object_id, :turnitin_data, :has_admin_comment,
+    :cached_due_date
+  ].freeze
 
   EXPORTABLE_ASSOCIATIONS = [
-    :attachment, :assignment, :user, :grader, :group, :media_object, :student, :submission_comments, :assessment_requests, :assigned_assessments, :quiz_submission,
-    :rubric_assessment, :rubric_assessments, :attachments, :content_participations
-  ]
+    :attachment, :assignment, :user, :grader, :group, :media_object,
+    :student, :submission_comments, :assessment_requests,
+    :assigned_assessments, :quiz_submission, :rubric_assessment,
+    :rubric_assessments, :attachments, :content_participations
+  ].freeze
 
   serialize :turnitin_data, Hash
+
   validates_presence_of :assignment_id, :user_id
   validates_length_of :body, :maximum => maximum_long_text_length, :allow_nil => true, :allow_blank => true
   validates_length_of :published_grade, :maximum => maximum_string_length, :allow_nil => true, :allow_blank => true
-  validates_length_of :grade, :maximum => maximum_string_length, :allow_nil => true, :allow_blank => true
-  include CustomValidations
   validates_as_url :url
 
   scope :with_comments, -> { includes(:submission_comments) }
@@ -93,6 +105,16 @@ class Submission < ActiveRecord::Base
     where("submissions.assignment_id IN (SELECT assignments.id FROM assignments WHERE assignments.context_id = ? AND assignments.context_type = 'Course')", course)
   }
 
+  workflow do
+    state :submitted do
+      event :grade_it, :transitions_to => :graded
+    end
+    state :unsubmitted
+    state :pending_review
+    state :graded
+  end
+
+
   # see #needs_grading?
   def self.needs_grading_conditions
     conditions = <<-SQL
@@ -790,17 +812,6 @@ class Submission < ActiveRecord::Base
     connection.after_transaction_commit { Auditors::GradeChange.record(self) }
   end
 
-  include Workflow
-
-  workflow do
-    state :submitted do
-      event :grade_it, :transitions_to => :graded
-    end
-    state :unsubmitted
-    state :pending_review
-    state :graded
-  end
-
   scope :with_assignment, -> { joins(:assignment).where("assignments.workflow_state <> 'deleted'")}
 
   scope :graded, -> { where("submissions.grade IS NOT NULL") }

db/migrate/20150730170646_create_moderated_grading_provisional_grades.rb

@@ -0,0 +1,30 @@
+class CreateModeratedGradingProvisionalGrades < ActiveRecord::Migration
+  tag :predeploy
+
+  def up
+    create_table :moderated_grading_provisional_grades do |t|
+      t.string     :grade
+      t.float      :score
+      t.timestamp  :graded_at
+      t.integer    :position,   null: false, limit: 8
+      t.references :scorer,     null: false, limit: 8
+      t.references :submission, null: false, limit: 8
+
+      t.timestamps
+    end
+
+    add_index :moderated_grading_provisional_grades, :submission_id
+    add_index :moderated_grading_provisional_grades,
+              [:submission_id, :position],
+              unique: true,
+              name: :idx_mg_provisional_grades_unique_submission_position
+    add_foreign_key :moderated_grading_provisional_grades, :submissions
+    add_foreign_key :moderated_grading_provisional_grades,
+                    :users,
+                    column: :scorer_id
+  end
+
+  def down
+    drop_table :moderated_grading_provisional_grades
+  end
+end

lib/canvas.rb

@@ -8,7 +8,10 @@ module Canvas
 
   def self.active_record_foreign_key_check(name, type, options)
     if name.to_s =~ /_id\z/ && type.to_s == 'integer' && options[:limit].to_i < 8
-      raise ArgumentError, "All foreign keys need to be 8-byte integers. #{name} looks like a foreign key to me, please add this option: `:limit => 8`"
+      raise ArgumentError, <<-EOS
+        All foreign keys need to be at least 8-byte integers. #{name}
+        looks like a foreign key, please add this option: `:limit => 8`
+      EOS
     end
   end
 
@@ -51,12 +54,21 @@ module Canvas
 
       # sanity check the file
       unless configs.is_a?(Hash)
-        raise "Invalid config/cache_store.yml: Root is not a hash. See comments in config/cache_store.yml.example"
+        raise <<-EOS
+          Invalid config/cache_store.yml: Root is not a hash. See comments in
+          config/cache_store.yml.example
+        EOS
       end
 
       non_hashes = configs.keys.select { |k| !configs[k].is_a?(Hash) }
       non_hashes.reject! { |k| configs[k].is_a?(String) && configs[configs[k]].is_a?(Hash) }
-      raise "Invalid config/cache_store.yml: Some keys are not hashes: #{non_hashes.join(', ')}. See comments in config/cache_store.yml.example" unless non_hashes.empty?
+      unless non_hashes.empty?
+        raise <<-EOS
+          Invalid config/cache_store.yml: Some keys are not hashes:
+          #{non_hashes.join(', ')}. See comments in
+          config/cache_store.yml.example
+        EOS
+      end
 
       configs.each do |env, config|
         if config.is_a?(String)

lib/canvas/grade_validations.rb

@@ -0,0 +1,9 @@
+require 'active_support/concern'
+
+module Canvas::GradeValidations
+  extend ActiveSupport::Concern
+
+  included do
+    validates_length_of :grade, :maximum => maximum_string_length, :allow_nil => true, :allow_blank => true
+  end
+end

spec/models/moderated_grading/provisional_grade_spec.rb

@@ -0,0 +1,30 @@
+require 'spec_helper'
+
+describe ModeratedGrading::ProvisionalGrade do
+  subject(:provisional_grade) do
+    submission.provisional_grades.new(grade: 'A', score: 100.0, position: 1, scorer: user).tap do |grade|
+      grade.scorer = user
+    end
+  end
+  let(:submission) { assignment.submissions.create!(user: user) }
+  let(:assignment) { course.assignments.create! }
+  let(:course) { Course.create! }
+  let(:user) { User.create! }
+  let(:now) { Time.zone.now }
+
+  it { is_expected.to be_valid }
+  it { is_expected.to validate_presence_of(:position) }
+  it { is_expected.to validate_presence_of(:scorer) }
+  it { is_expected.to validate_presence_of(:submission) }
+  it { is_expected.to validate_uniqueness_of(:position).scoped_to(:submission_id) }
+
+  describe '#graded_at when a grade changes' do
+    it { expect(provisional_grade.graded_at).to be_nil }
+    it 'updates the graded_at timestamp' do
+      Timecop.freeze(now) do
+        provisional_grade.update_attributes(grade: 'B')
+        expect(provisional_grade.graded_at).to eql now
+      end
+    end
+  end
+end

spec/models/submission_spec.rb

@@ -815,19 +815,19 @@ describe Submission do
 
   describe "graded?" do
     it "is false before graded" do
-      s, _ = @assignment.find_or_create_submission(@user)
-      expect(s.graded?).to eql false
+      submission, _ = @assignment.find_or_create_submission(@user)
+      expect(submission).to_not be_graded
     end
 
     it "is true for graded assignments" do
-      s, _ = @assignment.grade_student(@user, grade: 1)
-      expect(s.graded?).to eql true
+      submission, _ = @assignment.grade_student(@user, grade: 1)
+      expect(submission).to be_graded
     end
 
     it "is also true for excused assignments" do
-      s, _ = @assignment.find_or_create_submission(@user)
-      s.excused = true
-      expect(s.graded?).to eql true
+      submission, _ = @assignment.find_or_create_submission(@user)
+      submission.excused = true
+      expect(submission).to be_graded
     end
   end
 
@@ -835,25 +835,22 @@ describe Submission do
     let(:submission) { Submission.new }
 
     it "returns false when its not autograded" do
-      assignment = stub(:muted? => false)
-      @submission = Submission.new
-      expect(@submission.autograded?).to eq false
+      submission = Submission.new
+      expect(submission).to_not be_autograded
 
-      @submission.grader_id = Shard.global_id_for(@user.id)
-      expect(@submission.autograded?).to eq false
+      submission.grader_id = Shard.global_id_for(@user.id)
+      expect(submission).to_not be_autograded
     end
 
     it "returns true when its autograded" do
-      assignment = stub(:muted? => false)
-      @submission = Submission.new
-      @submission.grader_id = -1
-      expect(@submission.autograded?).to eq true
+      submission = Submission.new
+      submission.grader_id = -1
+      expect(submission).to be_autograded
     end
   end
 
   describe "past_due" do
     before :once do
-      u1 = @user
       submission_spec_model
       @submission1 = @submission