return deleted enrollments if requested in enrollments index

also refactor enrollments_visible_to away because it is bad

test plan:
* add people to a course
* remove them
* calls to /api/v1/courses/X/enrollments?state[]=deleted
 and /api/v1/sections/Y/enrollments?state[]=deleted
 should include the deleted enrollments

closes #CNVS-23729

Change-Id: I29d6995064dc6cf79af44b940f55dc0269c2a891
Reviewed-on: https://gerrit.instructure.com/65756
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Ryan Allen <rallen@instructure.com>
Product-Review: James Williams  <jamesw@instructure.com>

app/controllers/assignments_controller.rb

@@ -143,7 +143,7 @@ class AssignmentsController < ApplicationController
       @can_view_grades = @context.grants_right?(@current_user, session, :view_all_grades)
       @can_grade = @assignment.grants_right?(@current_user, session, :grade)
       if @can_view_grades || @can_grade
-        visible_student_ids = @context.enrollments_visible_to(@current_user, :include_priors => true).pluck(:user_id)
+        visible_student_ids = @context.apply_enrollment_visibility(@context.all_student_enrollments, @current_user).pluck(:user_id)
         @current_student_submissions = @assignment.submissions.where("submissions.submission_type IS NOT NULL").where(:user_id => visible_student_ids).to_a
       end
 

app/controllers/enrollments_api_controller.rb

@@ -503,7 +503,7 @@ class EnrollmentsApiController < ApplicationController
     end
 
     if authorized_action(@context, @current_user, [:read_roster, :view_all_grades, :manage_grades])
-      scope = @context.enrollments_visible_to(@current_user, :type => :all, :include_priors => true).where(enrollment_index_conditions)
+      scope = @context.apply_enrollment_visibility(@context.all_enrollments, @current_user).where(enrollment_index_conditions)
       unless params[:state].present?
         scope = scope.active_or_pending
       end

app/controllers/quizzes/quiz_submissions_api_controller.rb

@@ -167,7 +167,8 @@ class Quizzes::QuizSubmissionsApiController < ApplicationController
   def index
     quiz_submissions = if @context.grants_any_right?(@current_user, session, :manage_grades, :view_all_grades)
       # teachers have access to all student submissions
-      Api.paginate @quiz.quiz_submissions.where(:user_id => visible_user_ids),
+      visible_student_ids = @context.apply_enrollment_visibility(@context.student_enrollments, @current_user).pluck(:user_id)
+      Api.paginate @quiz.quiz_submissions.where(:user_id => visible_student_ids),
         self,
         api_v1_course_quiz_submissions_url(@context, @quiz)
     elsif @quiz.grants_right?(@current_user, session, :submit)
@@ -409,11 +410,6 @@ class Quizzes::QuizSubmissionsApiController < ApplicationController
     !!params[:preview]
   end
 
-  def visible_user_ids(opts = {})
-    scope = @context.enrollments_visible_to(@current_user, opts)
-    scope.pluck(:user_id)
-  end
-
   def serialize_and_render(quiz_submissions)
     quiz_submissions = [ quiz_submissions ] unless quiz_submissions.is_a? Array
 

app/controllers/submissions_api_controller.rb

@@ -173,7 +173,8 @@ class SubmissionsApiController < ApplicationController
   def index
     if authorized_action(@context, @current_user, [:manage_grades, :view_all_grades])
       @assignment = @context.assignments.active.find(params[:assignment_id])
-      submissions = @assignment.submissions.where(:user_id => visible_user_ids)
+      visible_student_ids = @context.apply_enrollment_visibility(@context.student_enrollments, @current_user, section_ids).pluck(:user_id)
+      submissions = @assignment.submissions.where(:user_id => visible_student_ids)
       includes = Array.wrap(params[:include])
 
       if includes.include?("visibility") && @context.feature_enabled?(:differentiated_assignments)
@@ -247,14 +248,11 @@ class SubmissionsApiController < ApplicationController
 
     can_view_all = @context.grants_any_right?(@current_user, session, :manage_grades, :view_all_grades)
     if all && can_view_all
-      opts = { include_priors: true }
-      if @section
-        opts[:section_ids] = [@section.id]
-      end
       # this is a scope, and will generate subqueries
-      student_ids = @context.enrollments_visible_to(@current_user, opts).select(:user_id)
+      student_ids = @context.apply_enrollment_visibility(@context.all_student_enrollments, @current_user, section_ids).select(:user_id)
     elsif can_view_all
-      inaccessible_students = student_ids - visible_user_ids(:include_priors => true)
+      visible_student_ids = @context.apply_enrollment_visibility(@context.all_student_enrollments, @current_user, section_ids).pluck(:user_id)
+      inaccessible_students = student_ids - visible_student_ids
       if !inaccessible_students.empty?
         return render_unauthorized_action
       end
@@ -830,12 +828,8 @@ class SubmissionsApiController < ApplicationController
     api_find(students, user_id)
   end
 
-  def visible_user_ids(opts = {})
-    if @section
-      opts[:section_ids] = [@section.id]
-    end
-    scope = @context.enrollments_visible_to(@current_user, opts)
-    scope.pluck(:user_id)
+  def section_ids
+    @section ? [@section.id] : nil
   end
 
   def bulk_load_attachments_and_previews(submissions)

app/controllers/users_controller.rb

@@ -1737,8 +1737,8 @@ class UsersController < ApplicationController
         enrollments = student.student_enrollments.active.preload(:course).shard(student).to_a
         enrollments.each do |enrollment|
           should_include = enrollment.course.user_has_been_instructor?(@teacher) &&
-                           enrollment.course.enrollments_visible_to(@teacher, :include_priors => true).where(id: enrollment).first &&
-                           enrollment.course.grants_right?(@current_user, :read_reports)
+                           enrollment.course.grants_right?(@current_user, :read_reports) &&
+                           enrollment.course.apply_enrollment_visibility(enrollment.course.all_student_enrollments, @teacher).where(id: enrollment).first
           if should_include
             Enrollment.recompute_final_score_if_stale(enrollment.course, student) { enrollment.reload }
             @courses[enrollment.course] = teacher_activity_report(@teacher, enrollment.course, [enrollment])
@@ -1757,7 +1757,8 @@ class UsersController < ApplicationController
           redirect_to_referrer_or_default(root_url)
         elsif authorized_action(course, @current_user, :read_reports)
           Enrollment.recompute_final_score_if_stale(course)
-          @courses[course] = teacher_activity_report(@teacher, course, course.enrollments_visible_to(@teacher, :include_priors => true))
+          enrollments = course.apply_enrollment_visibility(course.all_student_enrollments, @teacher)
+          @courses[course] = teacher_activity_report(@teacher, course, enrollments)
         end
       end
 

app/models/assignment.rb

@@ -1394,7 +1394,7 @@ class Assignment < ActiveRecord::Base
       }
     end
 
-    enrollments = context.enrollments_visible_to(user)
+    enrollments = context.apply_enrollment_visibility(context.student_enrollments, user)
 
     is_provisional = grading_role == :provisional_grader || grading_role == :moderator
     rubric_assmnts = visible_rubric_assessments_for(user, :provisional_grader => is_provisional) || []

app/models/course.rb

@@ -2047,11 +2047,12 @@ class Course < ActiveRecord::Base
 
   # returns a scope, not an array of users/enrollments
   def students_visible_to(user, include_priors=false)
-    enrollments_visible_to(user, :include_priors => include_priors, :return_users => true)
+    scope = include_priors ? self.all_students : self.students
+    self.apply_enrollment_visibility(scope, user)
   end
 
-  def enrollments_visible_to(user, opts = {})
-    visibilities = section_visibilities_for(user)
+  def enrollments_visible_to(user, opts={})
+    # because of course it's used in a plugin
     relation = []
     relation << 'all' if opts[:include_priors]
     if opts[:type] == :all
@@ -2066,15 +2067,23 @@ class Course < ActiveRecord::Base
     end
     relation = relation.join('_')
     # our relations don't all follow the same pattern
-    relation = case relation
-                 when 'all_enrollments'; 'enrollments'
-                 when 'enrollments'; 'current_enrollments'
-                 else; relation
-               end
-    scope = self.send(relation.to_sym)
-    if opts[:section_ids]
-      scope = scope.where('enrollments.course_section_id' => opts[:section_ids].to_a)
+    unless opts[:include_deleted]
+      relation = case relation
+                   when 'all_enrollments'; 'enrollments'
+                   when 'enrollments'; 'current_enrollments'
+                   else; relation
+                 end
     end
+    self.apply_enrollment_visibility(self.send(relation.to_sym), user, opts[:section_ids])
+  end
+
+  # can apply to user scopes as well if through enrollments (e.g. students, teachers)
+  def apply_enrollment_visibility(scope, user, section_ids=nil)
+    if section_ids
+      scope = scope.where('enrollments.course_section_id' => section_ids.to_a)
+    end
+
+    visibilities = section_visibilities_for(user)
     visibility_level = enrollment_visibility_level_for(user, visibilities)
     account_admin = visibility_level == :full && visibilities.empty?
     # teachers, account admins, and student view students can see student view students
@@ -2747,7 +2756,7 @@ class Course < ActiveRecord::Base
   end
 
   def re_send_invitations!(from_user)
-    self.enrollments_visible_to(from_user).invited.except(:preload).preload(user: :communication_channels).find_each do |e|
+    self.apply_enrollment_visibility(self.student_enrollments, from_user).invited.except(:preload).preload(user: :communication_channels).find_each do |e|
       e.re_send_confirmation! if e.invited?
     end
   end

app/models/quizzes/quiz_submission_zipper.rb

@@ -70,7 +70,7 @@ class Quizzes::QuizSubmissionZipper < ContentZipper
   def find_submissions
     submissions = quiz.quiz_submissions
     if zip_attachment.user && quiz.context.enrollment_visibility_level_for(zip_attachment.user) != :full
-      visible_student_ids = quiz.context.enrollments_visible_to(zip_attachment.user).pluck(:user_id)
+      visible_student_ids = quiz.context.apply_enrollment_visibility(quiz.context.student_enrollments, zip_attachment.user).pluck(:user_id)
       submissions = submissions.where(:user_id => visible_student_ids)
     end
     @submissions = submissions.map(&:latest_submitted_attempt).compact

lib/gradebook_exporter.rb

@@ -26,6 +26,8 @@ class GradebookExporter
   end
 
   def to_csv
+    collection = @options[:include_priors] ? @course.all_student_enrollments : @course.student_enrollments
+    enrollments_scope = @course.apply_enrollment_visibility(collection, @user)
     student_enrollments = enrollments_for_csv(enrollments_scope, @options)
 
     student_section_names = {}
@@ -170,11 +172,6 @@ class GradebookExporter
     enrollments.partition { |e| e.type != "StudentViewEnrollment" }.flatten
   end
 
-  def enrollments_scope
-    enrollment_opts = @options.slice(:include_priors)
-    @course.enrollments_visible_to(@user, enrollment_opts)
-  end
-
   def name_method
     @course.list_students_by_sortable_name? ? :sortable_name : :name
   end

spec/apis/v1/enrollments_api_spec.rb

@@ -988,6 +988,25 @@ describe EnrollmentsApiController, type: :request do
         expect(json.all?{ |r| r["course_section_id"] == @section.id }).to be_truthy
       end
 
+      it "should list deleted section enrollments properly" do
+        enrollment = @student.enrollments.first
+        enrollment.course_section = @section
+        enrollment.save!
+        enrollment.destroy
+
+        @path = "/api/v1/sections/#{@section.id}/enrollments?state[]=deleted"
+        @params = { :controller => "enrollments_api", :action => "index", :section_id => @section.id.to_param, :format => "json", :state => ["deleted"] }
+        json = api_call(:get, @path, @params)
+
+        expect(json.length).to eql 1
+        expect(json.all?{ |r| r["course_section_id"] == @section.id }).to be_truthy
+
+        @path = "/api/v1/sections/#{@section.id}/enrollments"
+        @params = { :controller => "enrollments_api", :action => "index", :section_id => @section.id.to_param, :format => "json" }
+        json = api_call(:get, @path, @params)
+        expect(json.length).to eql 0
+      end
+
       describe "custom roles" do
         context "user context" do
           before :once do

spec/models/course_spec.rb

@@ -3178,11 +3178,13 @@ describe Course, "section_visibility" do
     it "should return student view students to account admins" do
       @course.student_view_student
       @admin = account_admin_user
-      expect(@course.enrollments_visible_to(@admin).map(&:user)).to be_include(@course.student_view_student)
+      visible_enrollments = @course.apply_enrollment_visibility(@course.student_enrollments, @admin)
+      expect(visible_enrollments.map(&:user)).to be_include(@course.student_view_student)
     end
 
     it "should return student view students to student view students" do
-      expect(@course.enrollments_visible_to(@course.student_view_student).map(&:user)).to be_include(@course.student_view_student)
+      visible_enrollments = @course.apply_enrollment_visibility(@course.student_enrollments, @course.student_view_student)
+      expect(visible_enrollments.map(&:user)).to be_include(@course.student_view_student)
     end
   end
 
@@ -3196,7 +3198,7 @@ describe Course, "section_visibility" do
     end
 
     it "should return non-limited admins from other sections" do
-      expect(@course.enrollments_visible_to(@ta, :type => :teacher, :return_users => true)).to eq [@teacher]
+      expect(@course.apply_enrollment_visibility(@course.teachers, @ta)).to eq [@teacher]
     end
   end