provisional grades and comments in speed_grader_json

test plan: test in conjunction with g/61618,
which builds on this commit

fixes CNVS-22010
fixes CNVS-22011

Change-Id: I231069edf83c616293236afcfed2021c1f87d5b6
Reviewed-on: https://gerrit.instructure.com/60633
Reviewed-by: James Williams  <jamesw@instructure.com>
Tested-by: Jenkins
QA-Review: Jahnavi Yetukuri <jyetukuri@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>

app/controllers/gradebooks_controller.rb

@@ -431,6 +431,12 @@ class GradebooksController < ApplicationController
       return redirect_to polymorphic_url([@context, @assignment])
     end
 
+    grading_role = if @assignment.moderated_grading? && !@assignment.grades_published?
+      @context.grants_right?(@current_user, :moderate_grades) ? :moderator : :provisional_grader
+    else
+      :grader
+    end
+
     respond_to do |format|
       format.html do
         @headers = false
@@ -440,6 +446,7 @@ class GradebooksController < ApplicationController
           :CONTEXT_ACTION_SOURCE => :speed_grader,
           :settings_url => speed_grader_settings_course_gradebook_path,
           :force_anonymous_grading => force_anonymous_grading?(@assignment),
+          :grading_role => grading_role
         }
         if @assignment.quiz
           env[:quiz_history_url] = course_quiz_history_path @context.id,
@@ -452,7 +459,9 @@ class GradebooksController < ApplicationController
       end
 
       format.json do
-        render :json => @assignment.speed_grader_json(@current_user, service_enabled?(:avatars))
+        render :json => @assignment.speed_grader_json(@current_user,
+                                                      avatars: service_enabled?(:avatars),
+                                                      grading_role: grading_role)
       end
     end
   end

app/models/assignment.rb

@@ -1286,20 +1286,26 @@ class Assignment < ActiveRecord::Base
     json
   end
 
-  def speed_grader_json(user, avatars=false)
+  def grades_published?
+    # TODO: this will, of course, change once grade publishing is implemented.
+    #       we will probably need to add a column for grade publishing status
+    !moderated_grading?
+  end
+
+  def speed_grader_json(user, avatars: false, grading_role: :grader)
     Attachment.skip_thumbnails = true
-    submission_fields = [ :user_id, :id, :submitted_at, :workflow_state,
-                          :grade, :grade_matches_current_submission,
-                          :graded_at, :turnitin_data, :submission_type, :score,
-                          :assignment_id, :submission_comments, :excused ]
+    submission_fields = [:user_id, :id, :submitted_at, :workflow_state,
+                         :grade, :grade_matches_current_submission,
+                         :graded_at, :turnitin_data, :submission_type, :score,
+                         :assignment_id, :submission_comments, :excused].freeze
 
     comment_fields = [:comment, :id, :author_name, :created_at, :author_id,
                       :media_comment_type, :media_comment_id,
-                      :cached_attachments, :attachments]
+                      :cached_attachments, :attachments].freeze
 
     attachment_fields = [:id, :comment_id, :content_type, :context_id, :context_type,
                          :display_name, :filename, :mime_class,
-                         :size, :submitter_id, :workflow_state]
+                         :size, :submitter_id, :workflow_state].freeze
 
     res = as_json(
       :include => {
@@ -1337,10 +1343,9 @@ class Assignment < ActiveRecord::Base
         map{|s| s.as_json(:include_root => false, :only => [:user_id, :course_section_id]) }
     res[:context][:quiz] = self.quiz.as_json(:include_root => false, :only => [:anonymous_submissions])
 
-    submissions = self.submissions.where(:user_id => students)
-                  .includes(:submission_comments,
-                            :versions,
-                            :quiz_submission)
+    includes = [:versions, :quiz_submission]
+    includes << (grading_role == :grader ? :submission_comments : :all_submission_comments)
+    submissions = self.submissions.where(:user_id => students).includes(*includes)
 
     res[:too_many_quiz_submissions] = too_many = too_many_qs_versions?(submissions)
     qs_versions = quiz_submission_versions(submissions, too_many)
@@ -1349,16 +1354,20 @@ class Assignment < ActiveRecord::Base
 
     res[:submissions] = submissions.map do |sub|
       json = sub.as_json(:include_root => false,
-        :include => {
-          :submission_comments => {
-            :methods => avatar_methods,
-            :only => comment_fields
-          }
-        },
         :methods => [:submission_history, :late],
         :only => submission_fields
       ).merge("from_enrollment_type" => enrollment_types_by_id[sub.user_id])
 
+      if grading_role == :provisional_grader || grading_role == :moderator
+        provisional_grade = sub.provisional_grades.where(scorer_id: user).first
+        provisional_grade ||= ModeratedGrading::NullProvisionalGrade.new(user.id)
+        json.merge! provisional_grade.grade_attributes
+      end
+
+      json[:submission_comments] = (provisional_grade || sub).submission_comments.as_json(:include_root => false,
+                                                                                          :methods => avatar_methods,
+                                                                                          :only => comment_fields)
+
       json['attachments'] = sub.attachments.map{|att| att.as_json(
           :only => [:mime_class, :comment_id, :id, :submitter_id ]
       )}
@@ -1366,9 +1375,6 @@ class Assignment < ActiveRecord::Base
       json['submission_history'] = if json['submission_history'] && (quiz.nil? || too_many)
                                      json['submission_history'].map do |version|
                                        version.as_json(
-                                         :include => {
-                                           :submission_comments => { :only => comment_fields }
-                                         },
                                          :only => submission_fields,
                                          :methods => [:versioned_attachments, :late]
                                        ).tap do |version_json|
@@ -1398,6 +1404,17 @@ class Assignment < ActiveRecord::Base
                                        }}
                                      end
                                    end
+
+      if grading_role == :moderator
+        json['provisional_grades'] = sub.provisional_grades.where('scorer_id<>?', user.id).map do |pg|
+          pg.grade_attributes.tap do |json|
+            json[:submission_comments] = pg.submission_comments.as_json(:include_root => false,
+                                                                        :methods => avatar_methods,
+                                                                        :only => comment_fields)
+          end
+        end
+      end
+
       json
     end
     res[:GROUP_GRADING_MODE] = grade_as_group?

app/models/moderated_grading/provisional_grade.rb

@@ -1,7 +1,7 @@
 class ModeratedGrading::ProvisionalGrade < ActiveRecord::Base
   include Canvas::GradeValidations
 
-  attr_accessible :grade, :score, :position
+  attr_accessible :grade, :score
 
   belongs_to :submission
   belongs_to :scorer, class_name: 'User'
@@ -14,8 +14,47 @@ class ModeratedGrading::ProvisionalGrade < ActiveRecord::Base
     super
   end
 
+  def grade_attributes
+    self.as_json(:only => [:grade, :score, :graded_at, :scorer_id],
+                 :methods => [:provisional_grade_id, :grade_matches_current_submission],
+                 :include_root => false)
+  end
+
+  def grade_matches_current_submission
+    submission.submitted_at.nil? || self.graded_at.nil? || submission.submitted_at <= self.graded_at
+  end
+
+  def provisional_grade_id
+    self.id
+  end
+
+  def submission_comments
+    submission.all_submission_comments.for_provisional_grade(self.id)
+  end
+
   private
   def set_graded_at
     self.graded_at = Time.zone.now
   end
 end
+
+class ModeratedGrading::NullProvisionalGrade
+  def initialize(scorer_id)
+    @scorer_id = scorer_id
+  end
+
+  def grade_attributes
+    {
+      'provisional_grade_id' => nil,
+      'grade' => nil,
+      'score' => nil,
+      'graded_at' => nil,
+      'scorer_id' => @scorer_id,
+      'grade_matches_current_submission' => true
+    }
+  end
+
+  def submission_comments
+    SubmissionComment.none
+  end
+end

spec/models/assignment_spec.rb

@@ -2846,6 +2846,54 @@ describe Assignment do
         end
       end
     end
+
+    describe "with moderated grading" do
+      before(:once) do
+        course_with_ta :course => @course, :active_all => true
+        assignment_model(:course => @course, :submission_types => 'online_text_entry')
+        @submission = @assignment.submit_homework(@student, :submission_type => 'online_text_entry', :body => 'ahem')
+        @assignment.grade_student(@student, :comment => 'real comment', :score => 1)
+        @submission.add_comment(:author => @teacher, :comment => 'provisional comment', :provisional => true)
+        @submission.provisional_grade(@teacher).update_attribute(:score, 2)
+        @submission.add_comment(:author => @ta, :comment => 'other provisional comment', :provisional => true)
+        @submission.provisional_grade(@ta).update_attribute(:score, 3)
+      end
+
+      describe "for provisional grader" do
+        before(:once) do
+          @json = @assignment.speed_grader_json(@ta, :grading_role => :provisional_grader)
+        end
+
+        it "should include only the grader's provisional grades" do
+          expect(@json['submissions'][0]['score']).to eq 3
+          expect(@json['submissions'][0]['provisional_grades']).to be_nil
+        end
+
+        it "should include only the grader's provisional comments" do
+          expect(@json['submissions'][0]['submission_comments'].map { |comment| comment['comment'] }).to eq ['other provisional comment']
+        end
+      end
+
+      describe "for moderator" do
+        before(:once) do
+          @json = @assignment.speed_grader_json(@teacher, :grading_role => :moderator)
+        end
+
+        it "should include the moderator's provisional grades and comments" do
+          expect(@json['submissions'][0]['score']).to eq 2
+          expect(@json['submissions'][0]['submission_comments'].map { |comment| comment['comment'] }).to eq ['provisional comment']
+        end
+
+        it "should list all other provisional grades" do
+          pgs = @json['submissions'][0]['provisional_grades']
+          expect(pgs.size).to eq 1
+          expect(pgs.map { |pg| [pg['score'], pg['scorer_id'], pg['submission_comments'][0]['comment']] }).to eq(
+            [[3.0, @ta.id, "other provisional comment"]]
+          )
+        end
+      end
+
+    end
   end
 
   describe "#too_many_qs_versions" do

spec/models/moderated_grading/provisional_grade_spec.rb

@@ -7,15 +7,52 @@ describe ModeratedGrading::ProvisionalGrade do
     end
   end
   let(:submission) { assignment.submissions.create!(user: user) }
-  let(:assignment) { course.assignments.create! }
+  let(:assignment) { course.assignments.create! submission_types: 'online_text_entry' }
   let(:course) { Course.create! }
   let(:user) { User.create! }
+  let(:student) { u = User.create!; course.enroll_student(u); u }
   let(:now) { Time.zone.now }
 
   it { is_expected.to be_valid }
   it { is_expected.to validate_presence_of(:scorer) }
   it { is_expected.to validate_presence_of(:submission) }
 
+  describe 'grade_attributes' do
+    it "returns the proper format" do
+      json = provisional_grade.grade_attributes
+      expect(json).to eq({
+                           'provisional_grade_id' => provisional_grade.id,
+                           'grade' => 'A',
+                           'score' => 100.0,
+                           'graded_at' => nil,
+                           'scorer_id' => provisional_grade.scorer_id,
+                           'grade_matches_current_submission' => true
+                         })
+    end
+  end
+
+  describe "grade_matches_current_submission" do
+    it "returns true if the grade is newer than the submission" do
+      sub = nil
+      Timecop.freeze(10.minutes.ago) do
+        sub = assignment.submit_homework(student, :submission_type => 'online_text_entry', :body => 'hallo')
+      end
+      pg = sub.find_or_create_provisional_grade! scorer: user, score: 1
+      expect(pg.reload.grade_matches_current_submission).to eq true
+    end
+
+    it "returns false if the submission is newer than the grade" do
+      sub = nil
+      pg = nil
+      Timecop.freeze(10.minutes.ago) do
+        sub = assignment.submit_homework(student, :submission_type => 'online_text_entry', :body => 'hallo')
+        pg = sub.find_or_create_provisional_grade! scorer: user, score: 1
+      end
+      assignment.submit_homework(student, :submission_type => 'online_text_entry', :body => 'resubmit')
+      expect(pg.reload.grade_matches_current_submission).to eq false
+    end
+  end
+
   describe 'unique constraint' do
     it "disallows multiple provisional grades from the same user" do
       pg1 = submission.provisional_grades.build(score: 75)
@@ -43,3 +80,18 @@ describe ModeratedGrading::ProvisionalGrade do
     end
   end
 end
+
+describe ModeratedGrading::NullProvisionalGrade do
+  describe 'grade_attributes' do
+    it "returns the proper format" do
+      expect(ModeratedGrading::NullProvisionalGrade.new(1).grade_attributes).to eq({
+          'provisional_grade_id' => nil,
+          'grade' => nil,
+          'score' => nil,
+          'graded_at' => nil,
+          'scorer_id' => 1,
+          'grade_matches_current_submission' => true
+        })
+    end
+  end
+end