Add need_grading_count scope to Submission

refs OUT-426

Counts submissions made by enrolled users in a pending grading state.
Includes partial index to support that aggregation

Test Plan
In canvas console or g/97755, ensure Submission.needs_grading_count:
Does count
  submission that has not been graded
  submission to quiz that is partially graded
  submission by student in the course
  submission by user that has been un- then re-enrolled in the course
  submission by user enrolled in >1 sections in the course *only once*
Does not count
  submission that has been graded
  submission that has been autograded
  submission by unenrolled user

Change-Id: I031bd3692c4dd4aaf4218a11a31c0debc61128ab
Reviewed-on: https://gerrit.instructure.com/97774
Reviewed-by: Augusto Callejas <acallejas@instructure.com>
QA-Review: Alex Ortiz-Rosado <aortiz@instructure.com>
Tested-by: Jenkins
Product-Review: Christian Prescott <cprescott@instructure.com>

app/models/assignments/needs_grading_count_query.rb

@@ -120,10 +120,7 @@ module Assignments
           AND e.course_id = ?
           AND e.type IN ('StudentEnrollment', 'StudentViewEnrollment')
           AND e.workflow_state = 'active'
-          AND submissions.submission_type IS NOT NULL
-          AND (submissions.workflow_state = 'pending_review'
-            OR (submissions.workflow_state = 'submitted'
-              AND (submissions.score IS NULL OR NOT submissions.grade_matches_current_submission)))
+          AND #{Submission.needs_grading_conditions}
         SQL
 
       string += <<-SQL

app/models/submission.rb

@@ -130,6 +130,8 @@ class Submission < ActiveRecord::Base
 
 
   # see #needs_grading?
+  # When changing these conditions, consider updating index_submissions_on_assignment_id
+  # to maintain performance.
   def self.needs_grading_conditions
     conditions = <<-SQL
       submissions.submission_type IS NOT NULL
@@ -159,8 +161,19 @@ class Submission < ActiveRecord::Base
     needs_grading? != needs_grading?(:was)
   end
 
-  scope :needs_grading, -> { where(needs_grading_conditions) }
+  scope :needs_grading, -> {
+    s_name = quoted_table_name
+    e_name = Enrollment.quoted_table_name
+    joins("INNER JOIN #{e_name} ON #{s_name}.user_id=#{e_name}.user_id")
+    .where(needs_grading_conditions)
+    .where(Enrollment.active_student_conditions)
+    .distinct
+  }
 
+  scope :needs_grading_count, -> {
+    select("COUNT(#{quoted_table_name}.id)")
+    .needs_grading
+  }
 
   sanitize_field :body, CanvasSanitize::SANITIZE
 

config/brakeman.ignore

@@ -0,0 +1,23 @@
+{
+    "ignored_warnings": [
+      {
+        "note": "Enrollment.active_student_conditions accepts no user input",
+        "warning_type": "SQL Injection",
+        "warning_code": 0,
+        "fingerprint": "f4e920699a6767e36d0e54f5c1ccd7a638a3654e8bef6e3ee6fbccffba76c345",
+        "message": "Possible SQL injection",
+        "file": "app/models/submission.rb",
+        "line": 169,
+        "link": "http://brakemanscanner.org/docs/warning_types/sql_injection/",
+        "code": "joins(\"INNER JOIN #{Enrollment.quoted_table_name} ON #{quoted_table_name}.user_id=#{Enrollment.quoted_table_name}.user_id\").where(needs_grading_conditions).where(Enrollment.active_student_conditions)",
+        "render_path": null,
+        "location": {
+          "type": "method",
+          "class": "Submission",
+          "method": "needs_grading"
+        },
+        "user_input": "Enrollment.active_student_conditions",
+        "confidence": "High"
+      }
+    ]
+}

db/migrate/20161128205000_index_submissions_on_assignment_id_where_needs_grading.rb

@@ -0,0 +1,17 @@
+class IndexSubmissionsOnAssignmentIdWhereNeedsGrading < ActiveRecord::Migration[4.2]
+  tag :predeploy
+  disable_ddl_transaction!
+
+  def change
+    # from Submission.needs_grading_conditions
+    conditions = <<-SQL
+      submission_type IS NOT NULL
+      AND (workflow_state = 'pending_review'
+        OR (workflow_state = 'submitted'
+          AND (score IS NULL OR NOT grade_matches_current_submission)
+        )
+      )
+    SQL
+    add_index :submissions, :assignment_id, where: conditions, algorithm: :concurrently
+  end
+end

spec/models/submission_spec.rb

@@ -2327,9 +2327,41 @@ describe Submission do
       OriginalityReport.create!(submission: submission, attachment: attachment, originality_score: 1.0, workflow_state:'pending')
       expect(submission.submission_history.first.turnitin_data[attachment.asset_string]).to be_nil
     end
-
   end
 
+  describe ".needs_grading" do
+    before :once do
+      @submission = @assignment.submit_homework(@student, submission_type: 'online_text_entry', body: 'asdf')
+    end
+
+    it "includes submission that has not been graded" do
+      expect(Submission.needs_grading.count).to eq(1)
+    end
+
+    it "includes submission by enrolled student" do
+      @student.enrollments.take!.complete
+      expect(Submission.needs_grading.count).to eq(0)
+      @course.enroll_student(@student).accept
+      expect(Submission.needs_grading.count).to eq(1)
+    end
+
+    it "includes submission by user with multiple enrollments in the course only once" do
+      another_section = @course.course_sections.create(name: 'two')
+      @course.enroll_student(@student, section: another_section, allow_multiple_enrollments: true).accept
+      expect(Submission.needs_grading.count).to eq(1)
+    end
+
+    it "does not include submission that has been graded" do
+      @assignment.grade_student(@student, grade: '100', grader: @teacher)
+      expect(Submission.needs_grading.count).to eq(0)
+    end
+
+    it "does not include submission by non-student user" do
+      @student.enrollments.take!.complete
+      @course.enroll_user(@student, 'TaEnrollment').accept
+      expect(Submission.needs_grading.count).to eq(0)
+    end
+  end
 end
 
 def submission_spec_model(opts={})