Inactive student submissions do not show up for todo list

fixes ADMIN-2711

Test plan
- Set up a student who has two enrollments in different
  courses
- In one course, submit an assignment as the student
  and then mark that student's enrollment as inactive
- Make sure everything else for that assignment is
  graded
- Check /api/v1/users/self/todo and ensure you don't
  see the assignment in the list
- Ensure every other submission in
  /api/v1/users/self/todo_items_count has been graded
  and make sure you can't see a count for the submission

Change-Id: Id01333b990dbbef762b55ca2ba4f98382b0b2ad4
Reviewed-on: https://gerrit.instructure.com/197166
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>

app/controllers/users_controller.rb

@@ -952,7 +952,7 @@ class UsersController < ApplicationController
   #   }
   def todo_item_count
     return render_unauthorized_action unless @current_user
-    grading = @current_user.assignments_needing_grading_count
+    grading = @current_user.submissions_needing_grading_count
     submitting = @current_user.assignments_needing_submitting(include_ungraded: true, scope_only: true, limit: nil).size
     if Array(params[:include]).include? 'ungraded_quizzes'
       submitting += @current_user.ungraded_quizzes(:needing_submitting => true, scope_only: true, limit: nil).size

app/models/submission.rb

@@ -267,10 +267,12 @@ class Submission < ActiveRecord::Base
 
   scope :needs_grading, -> {
     all.primary_shard.activate do
-      joins("INNER JOIN #{Enrollment.quoted_table_name} ON submissions.user_id=enrollments.user_id")
-      .where(needs_grading_conditions)
-      .where(Enrollment.active_student_conditions)
-      .distinct
+      joins(:assignment).
+        joins("INNER JOIN #{Enrollment.quoted_table_name} ON submissions.user_id=enrollments.user_id
+                                                         AND assignments.context_id = enrollments.course_id").
+        where(needs_grading_conditions).
+        where(Enrollment.active_student_conditions).
+        distinct
     end
   }
 

app/models/user_learning_object_scopes.rb

@@ -276,11 +276,10 @@ module UserLearningObjectScopes
   end
 
   # opts forwaded to course_ids_for_todo_lists
-  def assignments_needing_grading_count(**opts)
+  def submissions_needing_grading_count(**opts)
     course_ids = course_ids_for_todo_lists(:instructor, **opts)
     Submission.active.
       needs_grading.
-      joins(:assignment).
       joins("INNER JOIN #{Enrollment.quoted_table_name} AS grader_enrollments ON assignments.context_id = grader_enrollments.course_id").
       where(assignments: {context_id: course_ids}).
       merge(Assignment.expecting_submission).
@@ -320,13 +319,17 @@ module UserLearningObjectScopes
   def grader_visible_submissions_sql
     "SELECT submissions.id
        FROM #{Submission.quoted_table_name}
+       INNER JOIN #{Enrollment.quoted_table_name} AS student_enrollments ON student_enrollments.user_id = submissions.user_id
+                                                                        AND student_enrollments.course_id = assignments.context_id
       WHERE submissions.assignment_id = assignments.id
         AND enrollments.limit_privileges_to_course_section = 'f'
         AND #{Submission.needs_grading_conditions}
+        AND student_enrollments.workflow_state = 'active'
       UNION
      SELECT submissions.id
        FROM #{Submission.quoted_table_name}
       INNER JOIN #{Enrollment.quoted_table_name} AS student_enrollments ON student_enrollments.user_id = submissions.user_id
+                                                                       AND student_enrollments.course_id = assignments.context_id
       WHERE submissions.assignment_id = assignments.id
         AND #{Submission.needs_grading_conditions}
         AND enrollments.limit_privileges_to_course_section = 't'

spec/models/submission_spec.rb

@@ -5437,6 +5437,12 @@ describe Submission do
       expect(Submission.needs_grading.count).to eq(0)
     end
 
+    it 'does not include submissions for inactive/concluded students who have other active enrollments somewhere' do
+      @course.enroll_student(@student).update_attribute(:workflow_state, 'inactive')
+      course_with_student(user: @student, active_all: true)
+      expect(Submission.needs_grading).not_to include @assignment.submissions.first
+    end
+
     context "sharding" do
       require_relative '../sharding_spec_helper'
       specs_require_sharding

spec/models/user_learning_object_scopes_spec.rb

@@ -667,6 +667,13 @@ describe UserLearningObjectScopes do
       expect(@ta.assignments_needing_grading(scope_only: true)).not_to include assignment
     end
 
+    it 'should not count submissions for inactive students when they have active enrollments in other courses' do
+      @course1.enroll_student(@student_b).update_attribute(:workflow_state, 'inactive')
+      assignment = @course1.assignments.first
+      assignment.grade_student(@student_a, grade: "1", grader: @teacher)
+      expect(@teacher.assignments_needing_grading(scope_only: true)).not_to include assignment
+    end
+
     it "should limit the number of returned assignments" do
       assignment_ids = create_records(Assignment, Array.new(20) do |x|
         {
@@ -758,7 +765,7 @@ describe UserLearningObjectScopes do
     end
   end
 
-  context "#assignments_needing_grading_count" do
+  context "#submissions_needing_grading_count" do
     before :once do
       course_with_teacher(active_all: true)
       @sectionb = @course.course_sections.create!(name: 'section B')
@@ -774,7 +781,7 @@ describe UserLearningObjectScopes do
         @assignment.submit_homework student, body: "submission for #{student.name}"
       end
 
-      expect(@teacher.assignments_needing_grading_count).to eq 2
+      expect(@teacher.submissions_needing_grading_count).to eq 2
     end
 
     it 'should not show counts for submissions that a grader can\'t see due to enrollment visibility' do
@@ -784,7 +791,7 @@ describe UserLearningObjectScopes do
         @assignment.submit_homework student, body: "submission for #{student.name}"
       end
 
-      expect(@teacher.assignments_needing_grading_count).to eq 1
+      expect(@teacher.submissions_needing_grading_count).to eq 1
     end
 
     it 'should not show counts for submissions in a section where the grader is enrolled but is not a grader' do
@@ -795,7 +802,7 @@ describe UserLearningObjectScopes do
         @assignment.submit_homework student, body: "submission for #{student.name}"
       end
 
-      expect(@teacher.assignments_needing_grading_count).to eq 1
+      expect(@teacher.submissions_needing_grading_count).to eq 1
     end
   end