honor gradebook inactive/concluded settings in export

Gradebook export now honors the settings from gradebook related to
showing active and inactive enrollments.

fixes CNVS-30640

test plan:
 - Create a course with an assignment, and three
   students, one active, one inactive, and one concluded.
 - In gradebook turn off show inactive and show concluded.
 - Export the gradebook and note that only the active student shows.
 - In gradebook turn on show inactive.
 - Export the gradebook and note that both the active and inactive
   students show.
 - In gradebook turn off show inactive and turn on show concluded.
 - Export the gradebook and note that both the active and concluded
   students show.
 - In gradebook turn on show inactive and turn on show concluded.
 - Export the gradebook and note that all the students show
 - Edit the export and change the grade for all students.
 - Upload the export and note that only the active and inactive students
   are graded

Change-Id: I40dfe08991049f0689e05588f189b33a59cf9813
Reviewed-on: https://gerrit.instructure.com/86570
Reviewed-by: Derek Bender <djbender@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Tested-by: Jenkins
QA-Review: KC Naegle <knaegle@instructure.com>
Product-Review: Christi Wruck

app/coffeescripts/gradebook2/Gradebook.coffee

@@ -1165,10 +1165,8 @@ define [
         $('#download_csv').prop('disabled', true)
         $('.icon-import').parent().focus()
         loading_interval = self.exportingGradebookStatus()
-        include_priors = $('#show_concluded_enrollments').prop('checked')
 
         params =
-          include_priors: include_priors
           grading_period_id: @getGradingPeriodToShow()
 
         $.ajaxJSON(

app/controllers/gradebook_csvs_controller.rb

@@ -28,8 +28,7 @@ class GradebookCsvsController < ApplicationController
 
       csv_options = {
         include_sis_id: @context.grants_any_right?(@current_user, session, :read_sis, :manage_sis),
-        grading_period_id: params[:grading_period_id],
-        include_priors: Canvas::Plugin.value_to_boolean(params[:include_priors])
+        grading_period_id: params[:grading_period_id]
       }
 
       attachment_progress = @context.gradebook_to_csv_in_background(filename, @current_user, csv_options)

app/models/assignment/speed_grader.rb

@@ -2,8 +2,11 @@ require_relative '../assignment'
 
 class Assignment
   class SpeedGrader
+    include GradebookSettingsHelpers
+
     def initialize(assignment, user, avatars: false, grading_role: :grader)
       @assignment = assignment
+      @course = @assignment.context
       @user = user
       @avatars = avatars
       @grading_role = grading_role
@@ -44,9 +47,8 @@ class Assignment
         others.each { |s| res[:context][:rep_for_student][s.id] = rep.id }
       end
 
-      enrollments =
-        @assignment.context.apply_enrollment_visibility(enrollment_scope(gradebook_includes),
-                                                        @user, nil, include: gradebook_includes)
+      enrollments = @course.apply_enrollment_visibility(gradebook_enrollment_scope, @user, nil,
+                                                        include: gradebook_includes)
 
       is_provisional = @grading_role == :provisional_grader || @grading_role == :moderator
       rubric_assmnts = @assignment.visible_rubric_assessments_for(@user, :provisional_grader => is_provisional) || []
@@ -240,25 +242,5 @@ class Assignment
       Attachment.skip_thumbnails = nil
     end
 
-    private
-
-    def gradebook_includes
-      @gradebook_includes ||= begin
-        context_id = @assignment.context.id
-        gb_settings = @user.preferences.fetch(:gradebook_settings, {}).fetch(context_id, {})
-
-        includes = []
-        includes << :inactive if gb_settings.fetch('show_inactive_enrollments', "false") == "true"
-        includes << :completed if gb_settings.fetch('show_concluded_enrollments', "false") == "true"
-        includes
-      end
-    end
-
-    def enrollment_scope(includes)
-      scope = @assignment.context.all_accepted_student_enrollments
-      scope = scope.where("enrollments.workflow_state <> 'inactive'") unless includes.include?(:inactive)
-      scope = scope.where("enrollments.workflow_state <> 'completed'") unless includes.include?(:completed)
-      scope
-    end
   end
 end

app/models/course.rb

@@ -106,6 +106,7 @@ class Course < ActiveRecord::Base
   has_many :prior_enrollments, -> { preload(:user, :course).where(workflow_state: 'completed') }, class_name: 'Enrollment'
   has_many :prior_users, :through => :prior_enrollments, :source => :user
   has_many :students, :through => :student_enrollments, :source => :user
+  has_many :gradable_students, through: :gradable_student_enrollments, source: :user
   has_many :admin_visible_students, :through => :admin_visible_student_enrollments, :source => :user
 
   has_many :self_enrolled_students, -> { where("self_enrolled") }, through: :student_enrollments, source: :user
@@ -114,6 +115,7 @@ class Course < ActiveRecord::Base
   has_many :participating_students, -> { where(enrollments: { type: ['StudentEnrollment', 'StudentViewEnrollment'], workflow_state: 'active' }) }, through: :enrollments, source: :user
   has_many :student_enrollments, -> { where("enrollments.workflow_state NOT IN ('rejected', 'completed', 'deleted', 'inactive') AND enrollments.type IN ('StudentEnrollment', 'StudentViewEnrollment')").preload(:user) }, class_name: 'Enrollment'
   has_many :admin_visible_student_enrollments, -> { where("enrollments.workflow_state NOT IN ('rejected', 'completed', 'deleted') AND enrollments.type IN ('StudentEnrollment', 'StudentViewEnrollment')").preload(:user) }, class_name: 'Enrollment'
+  has_many :gradable_student_enrollments, -> { where(enrollments: { workflow_state: ['active', 'inactive'], type: ['StudentEnrollment', 'StudentViewEnrollment'] }).preload(:user) }, class_name: 'Enrollment'
   has_many :all_student_enrollments, -> { where("enrollments.workflow_state<>'deleted' AND enrollments.type IN ('StudentEnrollment', 'StudentViewEnrollment')").preload(:user) }, class_name: 'Enrollment'
   has_many :all_accepted_student_enrollments, -> { where("enrollments.workflow_state NOT IN ('rejected', 'deleted') AND enrollments.type IN ('StudentEnrollment', 'StudentViewEnrollment')").preload(:user) }, class_name: 'Enrollment'
   has_many :all_real_users, :through => :all_real_enrollments, :source => :user

lib/gradebook_exporter.rb

@@ -18,6 +18,7 @@
 
 class GradebookExporter
   include GradebookTransformer
+  include GradebookSettingsHelpers
 
   def initialize(course, user, options = {})
     @course  = course
@@ -26,9 +27,9 @@ class GradebookExporter
   end
 
   def to_csv
-    collection = @options[:include_priors] ? @course.all_student_enrollments : @course.admin_visible_student_enrollments
-    enrollments_scope = @course.apply_enrollment_visibility(collection, @user)
-    student_enrollments = enrollments_for_csv(enrollments_scope, @options)
+    enrollment_scope = @course.apply_enrollment_visibility(gradebook_enrollment_scope, @user, nil,
+                                                           include: gradebook_includes)
+    student_enrollments = enrollments_for_csv(enrollment_scope, @options)
 
     student_section_names = {}
     student_enrollments.each do |enrollment|

lib/gradebook_importer.rb

@@ -170,7 +170,7 @@ class GradebookImporter
 
     # remove concluded enrollments
     prior_enrollment_ids = (
-      @all_students.keys - @context.students.pluck(:user_id).map(&:to_i)
+      @all_students.keys - @context.gradable_students.pluck(:user_id).map(&:to_i)
     ).to_set
     @students.delete_if { |s| prior_enrollment_ids.include? s.id }
 

lib/gradebook_settings_helpers.rb

@@ -0,0 +1,41 @@
+#
+# Copyright (C) 2016 Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+module GradebookSettingsHelpers
+  private
+
+  def gradebook_includes
+    @gradebook_includes ||= begin
+      course_id = @course.id
+      gb_settings = @user.preferences.fetch(:gradebook_settings, {}).fetch(course_id, {})
+
+      includes = []
+      includes << :inactive if gb_settings.fetch('show_inactive_enrollments', "false") == "true"
+      includes << :completed if gb_settings.fetch('show_concluded_enrollments', "false") == "true"
+      includes
+    end
+  end
+
+  def gradebook_enrollment_scope(course = @course)
+    scope = course.all_accepted_student_enrollments
+    scope = scope.where("enrollments.workflow_state <> 'inactive'") unless gradebook_includes.include?(:inactive)
+    scope = scope.where("enrollments.workflow_state <> 'completed'") unless gradebook_includes.include?(:completed)
+    scope
+  end
+
+end

spec/controllers/gradebooks_controller_spec.rb

@@ -454,7 +454,7 @@ describe GradebooksController do
           exporter = GradebookExporter.new(
             @course,
             @teacher,
-            { include_priors: false, include_sis_id: true }
+            { include_sis_id: true }
           )
           raw_csv = exporter.to_csv
           expect(raw_csv).to include("Déjà vu")

spec/lib/gradebook_exporter_spec.rb

@@ -26,7 +26,8 @@ describe GradebookExporter do
   end
 
   describe "#to_csv" do
-    let(:course)    { @course }
+    let(:course) { @course }
+    let(:teacher) { @teacher }
 
     def exporter(opts = {})
       GradebookExporter.new(course, @teacher, opts)
@@ -194,6 +195,15 @@ describe GradebookExporter do
       student1_enrollment.deactivate
       student2_enrollment.deactivate
 
+      teacher.preferences[:gradebook_settings] =
+      { course.id =>
+        {
+          'show_inactive_enrollments' => 'true',
+          'show_concluded_enrollments' => 'false'
+        }
+      }
+      teacher.save!
+
       csv = exporter.to_csv
       rows = CSV.parse(csv, headers: true)
 

spec/lib/gradebook_importer_spec.rb

@@ -121,28 +121,28 @@ describe GradebookImporter do
     it "should Lookup with either Student Name, ID, SIS User ID, or SIS Login ID" do
       course_model
 
-      student_in_course(:name => "Some Name")
+      student_in_course(:name => "Some Name", active_all: true)
       @u1 = @user
 
       user_with_pseudonym(:active_all => true)
       @user.pseudonym.sis_user_id = "SISUSERID"
       @user.pseudonym.save!
-      student_in_course(:user => @user)
+      student_in_course(user: @user, active_all: true)
       @u2 = @user
 
       user_with_pseudonym(:active_all => true, :username => "something_that_has_not_been_taken")
-      student_in_course(:user => @user)
+      student_in_course(user: @user, active_all: true)
       @u3 = @user
 
       user_with_pseudonym(:active_all => true, :username => "inactive_login")
       @user.pseudonym.destroy
-      student_in_course(:user => @user)
+      student_in_course(user: @user, active_all: true)
       @u4 = @user
 
       user_with_pseudonym(:active_all => true, :username => "inactive_login")
       @user.pseudonym.destroy
       @user.pseudonyms.create!(:unique_id => 'active_login', :account => Account.default)
-      student_in_course(:user => @user)
+      student_in_course(user: @user, active_all: true)
       @u5 = @user
 
       uploaded_csv = CSV.generate do |csv|
@@ -183,7 +183,7 @@ describe GradebookImporter do
     it "should Lookup by root account" do
       course_model
 
-      student_in_course(:name => "Some Name")
+      student_in_course(name: "Some Name", active_all: true)
       @u1 = @user
 
       account2 = Account.create!
@@ -212,14 +212,14 @@ describe GradebookImporter do
       user_with_pseudonym(:active_all => true)
       @user.pseudonym.sis_user_id = "0123456"
       @user.pseudonym.save!
-      student_in_course(:user => @user)
+      student_in_course(user: @user, active_all: true)
       @u0 = @user
 
       # user with an sis-id that is a number
       user_with_pseudonym(:active_all => true, :username => "octal_ud")
       @user.pseudonym.destroy
       @user.pseudonyms.create!(:unique_id => '0231163', :account => Account.default)
-      student_in_course(:user => @user)
+      student_in_course(user: @user, active_all: true)
       @u1 = @user
 
       uploaded_csv = CSV.generate do |csv|
@@ -327,7 +327,7 @@ describe GradebookImporter do
   end
 
   it "should parse new and existing users" do
-    course_with_student
+    course_with_student(active_all: true)
     @student1 = @student
     e = student_in_course
     e.update_attribute :workflow_state, 'completed'
@@ -359,7 +359,7 @@ describe GradebookImporter do
   end
 
   it "should include assignments that the grade changed for an existing user" do
-    course_with_student
+    course_with_student(active_all: true)
     @assignment1 = @course.assignments.create!(:name => 'Assignment 1', :points_possible => 10)
     @assignment1.grade_student(@student, :grade => 8)
     importer_with_rows(

spec/models/course_spec.rb

@@ -1275,7 +1275,16 @@ describe Course, "gradebook_to_csv" do
     e.update_attribute :workflow_state, 'completed'
 
     expect(GradebookExporter.new(@course, @teacher).to_csv).not_to include @student.name
-    expect(GradebookExporter.new(@course, @teacher, include_priors: true).to_csv).to include @student.name
+
+    @teacher.preferences[:gradebook_settings] =
+      { @course.id =>
+        {
+          'show_inactive_enrollments' => 'false',
+          'show_concluded_enrollments' => 'true'
+        }
+      }
+    @teacher.save!
+    expect(GradebookExporter.new(@course, @teacher).to_csv).to include @student.name
   end
 
   context "accumulated points" do