make 'MI' shortcut work with set default grade

closes EVAL-2839
flag=assignment_missing_shortcut

Test Plan:
1. Enable the 'Keyboard Shortcut for Missing Assignment' SiteAdmin flag.
2. Go to Gradebook, click on an assignment header, and select 'Set Default
   Grade'.
3. Verify that you can enter "MI" (case insensitive) into the input to
   mark students as missing.
   - If "Overwrite already-entered grades" is not checked, students with
     grades and students marked as excused for the assignment will not be
     marked as missing.
   - If "Overwrite already-entered grades" is checked, all students for
     the assignment will be marked as missing.
   - If a missing policy is enabled, then ungraded students will be graded
     according to the missing policy in addition to being marked as
     missing.

Change-Id: I352a991c18bb82567e98bdac74800526ba0c9ec1
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/309136
Reviewed-by: Aaron Shafovaloff <ashafovaloff@instructure.com>
Reviewed-by: Derek Williams <derek.williams@instructure.com>
QA-Review: Cameron Ray <cameron.ray@instructure.com>
Product-Review: Jody Sailor
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>

app/controllers/gradebooks_controller.rb

@@ -710,7 +710,8 @@ class GradebooksController < ApplicationController
 
         submission = submission.permit(:grade, :score, :excuse, :excused,
                                        :graded_anonymously, :provisional, :final, :set_by_default_grade,
-                                       :comment, :media_comment_id, :media_comment_type, :group_comment).to_unsafe_h
+                                       :comment, :media_comment_id, :media_comment_type, :group_comment,
+                                       :late_policy_status).to_unsafe_h
         is_default_grade_for_missing = value_to_boolean(submission.delete(:set_by_default_grade)) && submission_record.missing? && submission_record.late_policy_status.nil?
 
         submission[:grader] = @current_user unless is_default_grade_for_missing
@@ -726,6 +727,7 @@ class GradebooksController < ApplicationController
           end
         end
         begin
+          dont_overwrite_grade = value_to_boolean(params[:dont_overwrite_grades])
           if %i[grade score excuse excused].any? { |k| submission.key? k }
             # if it's a percentage graded assignment, we need to ensure there's a
             # percent sign on the end. eventually this will probably be done in
@@ -734,7 +736,7 @@ class GradebooksController < ApplicationController
               submission[:grade] = "#{submission[:grade]}%"
             end
 
-            submission[:dont_overwrite_grade] = value_to_boolean(params[:dont_overwrite_grades])
+            submission[:dont_overwrite_grade] = dont_overwrite_grade
             submission.delete(:final) if submission[:final] && !@assignment.permits_moderation?(@current_user)
             subs = @assignment.grade_student(@user, submission.merge(skip_grader_check: is_default_grade_for_missing))
             apply_provisional_grade_filters!(submissions: subs, final: submission[:final]) if submission[:provisional]
@@ -748,6 +750,13 @@ class GradebooksController < ApplicationController
             apply_provisional_grade_filters!(submissions: subs, final: submission[:final]) if submission[:provisional]
             @submissions += subs
           end
+
+          if submission.key?(:late_policy_status) && submission_record.present? && (!dont_overwrite_grade || (submission_record.grade.blank? && !submission_record.excused?))
+            submission_record.update(late_policy_status: submission[:late_policy_status])
+            if submission_record.saved_change_to_late_policy_status?
+              @submissions << submission_record
+            end
+          end
         rescue Assignment::GradeError => e
           logger.info "GRADES: grade_student failed because '#{e.message}'"
           error = e
@@ -799,12 +808,11 @@ class GradebooksController < ApplicationController
     submissions.map do |submission|
       assignment = assignments[submission[:assignment_id].to_i]
       omitted_field = assignment.anonymize_students? ? :user_id : :anonymous_id
-      json_params = {
-        include: { submission_history: { methods: %i[late missing], except: omitted_field } },
+      json_params = Submission.json_serialization_full_parameters(methods: [:late, :missing]).merge(
+        include: { submission_history: { methods: %i[late missing word_count], except: omitted_field } },
         except: [omitted_field, :submission_comments]
-      }
-      json_params[:include][:submission_history][:methods] << :word_count
-      json = submission.as_json(Submission.json_serialization_full_parameters.merge(json_params))
+      )
+      json = submission.as_json(json_params)
 
       json[:submission].tap do |submission_json|
         submission_json[:assignment_visible] = submission.assignment_visible_to_user?(submission.user)

app/models/submission.rb

@@ -2506,6 +2506,9 @@ class Submission < ActiveRecord::Base
     includes = { quiz_submission: {} }
     methods = %i[submission_history attachments entered_score entered_grade word_count]
     methods << (additional_parameters.delete(:comments) || :submission_comments)
+    if additional_parameters[:methods]
+      methods.concat(Array(additional_parameters.delete(:methods)))
+    end
     excepts = additional_parameters.delete :except
 
     res = { methods: methods, include: includes }.merge(additional_parameters)

spec/controllers/gradebooks_controller_spec.rb

@@ -2086,12 +2086,22 @@ describe GradebooksController do
           expect(submissions).to all include("assignment_visible" => true)
         end
 
+        it "includes missing in base submission object" do
+          submission = json.first["submission"]
+          expect(submission).to include("missing" => false)
+        end
+
         it "includes missing in submission history" do
           submission_history = json.first["submission"]["submission_history"]
           submissions = submission_history.map { |submission| submission["submission"] }
           expect(submissions).to all include("missing" => false)
         end
 
+        it "includes late in base submission object" do
+          submission = json.first["submission"]
+          expect(submission).to include("late" => false)
+        end
+
         it "includes late in submission history" do
           submission_history = json.first["submission"]["submission_history"]
           submissions = submission_history.map { |submission| submission["submission"] }
@@ -2109,50 +2119,94 @@ describe GradebooksController do
           user_session(@teacher)
         end
 
-        let(:post_params) do
-          {
-            course_id: @course.id,
-            submission: {
-              assignment_id: @assignment.id,
-              user_id: @student.id,
-              grade: 10,
-              set_by_default_grade: true
+        context "setting grades" do
+          let(:post_params) do
+            {
+              course_id: @course.id,
+              submission: {
+                assignment_id: @assignment.id,
+                user_id: @student.id,
+                grade: 10,
+                set_by_default_grade: true
+              }
             }
-          }
+          end
+
+          it "does not set the grader_id on missing submissions if set_by_default_grade is true" do
+            @assignment.update!(due_at: 10.days.ago, submission_types: "online_text_entry")
+
+            expect { post(:update_submission, params: post_params, format: :json) }.not_to change {
+              @submission.reload.grader_id
+            }.from(nil)
+          end
+
+          it "sets the grader_id on missing submissions if set_by_default_grade is false" do
+            post_params[:submission][:set_by_default_grade] = false
+            @assignment.update!(due_at: 10.days.ago, submission_types: "online_text_entry")
+
+            expect { post(:update_submission, params: post_params, format: :json) }.to change {
+              @submission.reload.grader_id
+            }.from(nil).to(@teacher.id)
+          end
+
+          it "sets the grader_id on missing submissions when set_by_default_grade is true and the late policy status is missing" do
+            @assignment.update!(due_at: 10.days.ago, submission_types: "online_text_entry")
+            @submission.update!(late_policy_status: "missing")
+
+            expect { post(:update_submission, params: post_params, format: :json) }.to change {
+              @submission.reload.grader_id
+            }.from(nil).to(@teacher.id)
+          end
+
+          it "sets the grader_id on non missing submissions when set_by_default_grade is true" do
+            @assignment.update!(due_at: 10.days.from_now, submission_types: "online_text_entry")
+
+            expect { post(:update_submission, params: post_params, format: :json) }.to change {
+              @submission.reload.grader_id
+            }.from(nil).to(@teacher.id)
+          end
         end
 
-        it "does not set the grader_id on missing submissions if set_by_default_grade is true" do
-          @assignment.update!(due_at: 10.days.ago, submission_types: "online_text_entry")
+        context "marking students as missing" do
+          let(:post_params) do
+            {
+              course_id: @course.id,
+              submission: {
+                assignment_id: @assignment.id,
+                user_id: @student.id,
+                late_policy_status: "missing",
+                set_by_default_grade: true
+              }
+            }
+          end
 
-          expect { post(:update_submission, params: post_params, format: :json) }.not_to change {
-            @submission.reload.grader_id
-          }.from(nil)
-        end
+          it "marks not-yet-graded students as missing" do
+            expect { post(:update_submission, params: post_params, format: :json) }.to change {
+              @submission.reload.missing?
+            }.from(false).to(true)
+          end
 
-        it "sets the grader_id on missing submissions if set_by_default_grade is false" do
-          post_params[:submission][:set_by_default_grade] = false
-          @assignment.update!(due_at: 10.days.ago, submission_types: "online_text_entry")
+          it "marks already-graded students as missing" do
+            @assignment.grade_student(@student, grade: 2, grader: @teacher)
+            expect { post(:update_submission, params: post_params, format: :json) }.to change {
+              @submission.reload.missing?
+            }.from(false).to(true)
+          end
 
-          expect { post(:update_submission, params: post_params, format: :json) }.to change {
-            @submission.reload.grader_id
-          }.from(nil).to(@teacher.id)
-        end
+          it "marks not-yet-graded students as missing when passed dont_overwrite_grades param" do
+            params = post_params.merge(dont_overwrite_grades: true)
+            expect { post(:update_submission, params: params, format: :json) }.to change {
+              @submission.reload.missing?
+            }.from(false).to(true)
+          end
 
-        it "sets the grader_id on missing submissions when set_by_default_grade is true and the late policy status is missing" do
-          @assignment.update!(due_at: 10.days.ago, submission_types: "online_text_entry")
-          @submission.update!(late_policy_status: "missing")
-
-          expect { post(:update_submission, params: post_params, format: :json) }.to change {
-            @submission.reload.grader_id
-          }.from(nil).to(@teacher.id)
-        end
-
-        it "sets the grader_id on non missing submissions when set_by_default_grade is true" do
-          @assignment.update!(due_at: 10.days.from_now, submission_types: "online_text_entry")
-
-          expect { post(:update_submission, params: post_params, format: :json) }.to change {
-            @submission.reload.grader_id
-          }.from(nil).to(@teacher.id)
+          it "does not modify already-graded students when passed dont_overwrite_grades param" do
+            @assignment.grade_student(@student, grade: 2, grader: @teacher)
+            params = post_params.merge(dont_overwrite_grades: true)
+            expect { post(:update_submission, params: params, format: :json) }.not_to change {
+              @submission.reload.missing?
+            }.from(false)
+          end
         end
       end
 

spec/javascripts/jsx/gradebook/shared/SetDefaultGradeDialogManagerSpec.js

@@ -76,6 +76,7 @@ test('returns true when submissions are loaded', () => {
     createAssignmentProp(),
     createStudentsProp(),
     'contextId',
+    true,
     'selectedSection',
     false,
     true
@@ -89,6 +90,7 @@ test('returns false when submissions are not loaded', () => {
     createAssignmentProp(),
     createStudentsProp(),
     'contextId',
+    true,
     'selectedSection',
     false,
     false
@@ -102,6 +104,7 @@ test('returns false when grades are not published', () => {
     {...createAssignmentProp(), grades_published: false},
     createStudentsProp(),
     'contextId',
+    true,
     'selectedSection',
     false,
     true
@@ -122,6 +125,7 @@ QUnit.module('SetDefaultGradeDialogManager#showDialog', {
       assignment,
       createStudentsProp(),
       'contextId',
+      true,
       'selectedSection',
       opts.isAdmin,
       true

spec/javascripts/jsx/shared/SetDefaultGradeDialogSpec.js

@@ -106,14 +106,20 @@ QUnit.module('Shared > SetDefaultGradeDialog', suiteHooks => {
       sandbox.stub($, 'publish')
     })
 
-    test('submit reports number of students', async () => {
+    test('submit reports number of students scored', async () => {
       const payload = [
         {submission: {id: '11', assignment_id: '2', user_id: '3'}},
         {submission: {id: '22', assignment_id: '2', user_id: '4'}},
       ]
       respondWithPayload(payload)
       const students = [{id: '3'}, {id: '4'}]
-      dialog = new SetDefaultGradeDialog({assignment, students, context_id, alert})
+      dialog = new SetDefaultGradeDialog({
+        missing_shortcut_enabled: true,
+        assignment,
+        students,
+        context_id,
+        alert,
+      })
       dialog.show()
       clickSetDefaultGrade()
       const {
@@ -121,7 +127,57 @@ QUnit.module('Shared > SetDefaultGradeDialog', suiteHooks => {
           args: [message],
         },
       } = alert
-      strictEqual(message, '2 Student scores updated')
+      strictEqual(message, '2 student scores updated')
+    })
+
+    test('submit reports number of students marked as missing', async () => {
+      const payload = [
+        {submission: {id: '11', assignment_id: '2', user_id: '3'}},
+        {submission: {id: '22', assignment_id: '2', user_id: '4'}},
+      ]
+      respondWithPayload(payload)
+      const students = [{id: '3'}, {id: '4'}]
+      dialog = new SetDefaultGradeDialog({
+        missing_shortcut_enabled: true,
+        assignment,
+        students,
+        context_id,
+        alert,
+      })
+      dialog.show()
+      document.querySelector('input[name="default_grade"]').value = 'mi'
+      clickSetDefaultGrade()
+      const {
+        firstCall: {
+          args: [message],
+        },
+      } = alert
+      strictEqual(message, '2 students marked as missing')
+    })
+
+    test('submit ignores the missing shortcut when the shortcut feature flag is disabled', async () => {
+      const payload = [
+        {submission: {id: '11', assignment_id: '2', user_id: '3'}},
+        {submission: {id: '22', assignment_id: '2', user_id: '4'}},
+      ]
+      respondWithPayload(payload)
+      const students = [{id: '3'}, {id: '4'}]
+      dialog = new SetDefaultGradeDialog({
+        missing_shortcut_enabled: false,
+        assignment,
+        students,
+        context_id,
+        alert,
+      })
+      dialog.show()
+      document.querySelector('input[name="default_grade"]').value = 'mi'
+      clickSetDefaultGrade()
+      const {
+        firstCall: {
+          args: [message],
+        },
+      } = alert
+      strictEqual(message, '2 student scores updated')
     })
 
     test('submit reports number of students when api includes duplicates due to group assignments', async () => {
@@ -134,7 +190,14 @@ QUnit.module('Shared > SetDefaultGradeDialog', suiteHooks => {
       respondWithPayload(payload)
       const students = [{id: '3'}, {id: '4'}, {id: '5'}, {id: '6'}]
       // adjust page size so that we generate two requests
-      dialog = new SetDefaultGradeDialog({assignment, students, context_id, page_size: 2, alert})
+      dialog = new SetDefaultGradeDialog({
+        missing_shortcut_enabled: true,
+        assignment,
+        students,
+        context_id,
+        page_size: 2,
+        alert,
+      })
       dialog.show()
       clickSetDefaultGrade()
       const {
@@ -142,7 +205,7 @@ QUnit.module('Shared > SetDefaultGradeDialog', suiteHooks => {
           args: [message],
         },
       } = alert
-      strictEqual(message, '4 Student scores updated')
+      strictEqual(message, '4 student scores updated')
     })
   })
 })

spec/models/submission_spec.rb

@@ -138,6 +138,18 @@ describe Submission do
     end
   end
 
+  describe ".json_serialization_full_parameters" do
+    it "can be provided additional methods to include in the params" do
+      params = Submission.json_serialization_full_parameters(methods: [:missing])
+      expect(params[:methods]).to include :missing
+    end
+
+    it "can provide an additional method with singular form (no array)" do
+      params = Submission.json_serialization_full_parameters(methods: :missing)
+      expect(params[:methods]).to include :missing
+    end
+  end
+
   describe "#anonymous_id" do
     subject { submission.anonymous_id }
 

ui/features/gradebook/react/default_gradebook/Gradebook.tsx

@@ -4216,6 +4216,7 @@ class Gradebook extends React.Component<GradebookProps, GradebookState> {
       assignment,
       this.visibleStudentsThatCanSeeAssignment(assignmentId),
       this.options.context_id,
+      this.options.assignment_missing_shortcut,
       this.getFilterRowsBySetting('sectionId'),
       isAdmin(),
       this.contentLoadStates.submissionsLoaded

ui/features/gradebook/react/default_gradebook/gradebook.d.ts

@@ -86,6 +86,7 @@ export type GradebookOptions = {
   allow_apply_score_to_ungraded: boolean
   allow_separate_first_last_names: boolean
   allow_view_ungraded_as_zero: boolean
+  assignment_missing_shortcut: boolean
   attachment_url: null | string
   attachment: null | AttachmentData
   change_gradebook_version_url: string

ui/features/gradebook/react/shared/SetDefaultGradeDialogManager.ts

@@ -31,6 +31,8 @@ class SetDefaultGradeDialogManager {
 
   contextId: string
 
+  missingShortcutEnabled: boolean
+
   submissionsLoaded: boolean
 
   selectedSection: string | null
@@ -39,6 +41,7 @@ class SetDefaultGradeDialogManager {
     assignment: Assignment,
     students,
     contextId: string,
+    missingShortcutEnabled: boolean,
     selectedSection: string | null,
     isAdmin = false,
     submissionsLoaded = false
@@ -46,6 +49,7 @@ class SetDefaultGradeDialogManager {
     this.assignment = assignment
     this.students = students
     this.contextId = contextId
+    this.missingShortcutEnabled = missingShortcutEnabled
     this.selectedSection = selectedSection
     this.isAdmin = isAdmin
     this.submissionsLoaded = submissionsLoaded
@@ -58,6 +62,7 @@ class SetDefaultGradeDialogManager {
       assignment: this.assignment,
       students: this.students,
       context_id: this.contextId,
+      missing_shortcut_enabled: this.missingShortcutEnabled,
       selected_section: this.selectedSection,
     }
   }

ui/shared/grading/jquery/SetDefaultGradeDialog.coffee

@@ -36,7 +36,7 @@ noop = ->
 alertProxy = (message) -> alert(message)
 
 export default class SetDefaultGradeDialog
-  constructor: ({@assignment, @students, @context_id, @selected_section, @onClose = noop, @page_size = 50, @alert = alertProxy}) ->
+  constructor: ({@assignment, @students, @context_id, @missing_shortcut_enabled, @selected_section, @onClose = noop, @page_size = 50, @alert = alertProxy}) ->
 
   show: (onClose = @onClose) =>
     templateLocals =
@@ -78,12 +78,18 @@ export default class SetDefaultGradeDialog
           responses = [responses] if postDfds.length == 1
           submissions = getSubmissions(responses)
           $.publish 'submissions_updated', [submissions]
-          @alert(I18n.t 'alerts.scores_updated'
-          ,
-            one: '1 Student score updated'
-            other: '%{count} Student scores updated'
-          ,
-            count: submissions.length)
+          if @gradeIsMissingShortcut(formData.default_grade)
+            @alert(I18n.t
+              one: '1 student marked as missing'
+              other: '%{count} students marked as missing'
+            ,
+              count: submissions.length)
+          else
+            @alert(I18n.t
+              one: '1 student score updated'
+              other: '%{count} student scores updated'
+            ,
+              count: submissions.length)
           submittingDfd.resolve()
           @$dialog.dialog('close')
 
@@ -98,10 +104,17 @@ export default class SetDefaultGradeDialog
       _.chain(page)
         .map (s) =>
           prefix = "submissions[submission_#{s.id}]"
-          [["#{prefix}[assignment_id]", @assignment.id],
-          ["#{prefix}[user_id]", s.id],
-          ["#{prefix}[grade]", grade],
-          ["#{prefix}[set_by_default_grade]", true]]
+          params = [
+            ["#{prefix}[assignment_id]", @assignment.id],
+            ["#{prefix}[user_id]", s.id],
+            ["#{prefix}[set_by_default_grade]", true]
+          ]
+          if @gradeIsMissingShortcut(grade)
+            params.push(["#{prefix}[late_policy_status]", 'missing'])
+          else
+            params.push(["#{prefix}[grade]", grade])
+
+          params
         .flatten(true)
         .object()
         .value()
@@ -118,3 +131,6 @@ export default class SetDefaultGradeDialog
 
   gradeIsExcused: (grade) ->
     _.isString(grade) && grade.toUpperCase() == 'EX'
+
+  gradeIsMissingShortcut: (grade) ->
+    @missing_shortcut_enabled && grade?.toUpperCase() == 'MI'