add sis_pseudonym_id to enrollment object

fixes CORE-667
fixes GRADE-250
fixes GRADE-251

test plan
 - have a user with multiple sis pseudonyms
 - enroll user into courses through sis import
   using both sis_ids for different enrollments
 - each enrollment should return the correct
   pseudonym in grade export, Enrollment API,
   Section API, Submission API

Change-Id: I2693851b6b65fe8266b3a4e6e8cefc30e3d6f214
Reviewed-on: https://gerrit.instructure.com/136804
Tested-by: Jenkins
QA-Review: Anju Reddy <[email protected]>
Reviewed-by: Cody Cutrer <[email protected]>
Product-Review: Rob Orton <[email protected]>

app/controllers/courses_controller.rb

@@ -991,7 +991,7 @@ def user
         ActiveRecord::Associations::Preloader.new.preload(users, {:not_ended_enrollments => :course})
       end
       user = users.first or raise ActiveRecord::RecordNotFound
-      enrollments = user.not_ended_enrollments if includes.include?('enrollments')
+      enrollments = user.not_ended_enrollments.preload(:root_account, :sis_pseudonym) if includes.include?('enrollments')
       render :json => user_json(user, @current_user, session, includes, @context, enrollments)
     end
   end

app/controllers/enrollments_api_controller.rb

@@ -389,7 +389,7 @@ def index
       enrollments,
       self, send("api_v1_#{endpoint_scope}_enrollments_url"))
 
-    ActiveRecord::Associations::Preloader.new.preload(enrollments, [:user, :course, :course_section])
+    ActiveRecord::Associations::Preloader.new.preload(enrollments, [:user, :course, :course_section, :root_account, :sis_pseudonym])
 
     include_group_ids = Array(params[:include]).include?("group_ids")
     includes = [:user] + Array(params[:include])

app/controllers/submissions_api_controller.rb

@@ -413,7 +413,7 @@ def for_students
 
     if params[:grouped].present?
       scope = (@section || @context).all_student_enrollments.
-        preload(:user => :pseudonyms).
+        preload(:root_account, :sis_pseudonym, :user => :pseudonyms).
         where(:user_id => student_ids)
       student_enrollments = Api.paginate(scope, self, polymorphic_url([:api_v1, @section || @context, :student_submissions]))
 
@@ -434,7 +434,7 @@ def for_students
         seen_users << student.id
         hash = { :user_id => student.id, :section_id => enrollment.course_section_id, :submissions => [] }
 
-        pseudonym = SisPseudonym.for(student, context)
+        pseudonym = SisPseudonym.for(student, enrollment)
         if pseudonym && show_sis_info
           hash[:integration_id] = pseudonym.integration_id
           hash[:sis_user_id] = pseudonym.sis_user_id

app/models/enrollment.rb

@@ -30,10 +30,11 @@ class Enrollment < ActiveRecord::Base
 
   include Workflow
 
-  belongs_to :course, :touch => true, :inverse_of => :enrollments
-  belongs_to :course_section
-  belongs_to :root_account, :class_name => 'Account'
-  belongs_to :user
+  belongs_to :course, touch: true, inverse_of: :enrollments
+  belongs_to :course_section, inverse_of: :enrollments
+  belongs_to :root_account, class_name: 'Account', inverse_of: :enrollments
+  belongs_to :user, inverse_of: :enrollments
+  belongs_to :sis_pseudonym, class_name: 'Pseudonym', inverse_of: :sis_enrollments
   belongs_to :associated_user, :class_name => 'User'
 
   belongs_to :role

app/models/pseudonym.rb

@@ -23,6 +23,7 @@ class Pseudonym < ActiveRecord::Base
   belongs_to :account
   belongs_to :user
   has_many :communication_channels, -> { order(:position) }
+  has_many :sis_enrollments, class_name: 'Enrollment', inverse_of: :sis_pseudonym
   belongs_to :communication_channel
   belongs_to :sis_communication_channel, :class_name => 'CommunicationChannel'
   belongs_to :authentication_provider, class_name: 'AccountAuthorizationConfig'

app/models/sis_pseudonym.rb

@@ -33,7 +33,9 @@ def initialize(user, context, type, require_sis, include_deleted)
   end
 
   def pseudonym
-    result = find_in_home_account || find_in_other_accounts
+    result = @context.sis_pseudonym if @context.class <= Enrollment
+    result ||= find_in_home_account
+    result ||= find_in_other_accounts
     if result
       result.account = root_account if result.account_id == root_account.id
     end

db/migrate/20171017211555_add_sis_psuedonym_id_to_enrollments.rb

@@ -0,0 +1,25 @@
+#
+# Copyright (C) 2017 - present 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/>.
+#
+class AddSisPsuedonymIdToEnrollments < ActiveRecord::Migration[5.0]
+  tag :predeploy
+
+  def change
+    add_column :enrollments, :sis_pseudonym_id, :integer, limit: 8
+    add_reference :enrollments, :pseudonym, column: :sis_pseudonym_id, index: true, foreign_key: true
+  end
+end

gems/plugins/account_reports/lib/account_reports/sis_exporter.rb

@@ -412,7 +412,7 @@ def enrollments_to_csv(enrol, headers, include_other_roots)
         # rather than a cursor for this iteration
         # because it often is big enough that the slave
         # kills it mid-run (http://www.postgresql.org/docs/9.0/static/hot-standby.html)
-        enrol.find_in_batches(start: 0) do |batch|
+        enrol.preload(:root_account, :sis_pseudonym).find_in_batches(start: 0) do |batch|
           users = batch.map {|e| User.new(id: e.user_id)}.compact
           users += batch.map {|e| User.new(id: e.associated_user_id) unless e.associated_user_id.nil?}.compact
           users.uniq!
@@ -422,7 +422,8 @@ def enrollments_to_csv(enrol, headers, include_other_roots)
           batch.each do |e|
             p = loaded_pseudonym(pseudonyms,
                                  users_by_id[e.user_id],
-                                 include_deleted: @include_deleted)
+                                 include_deleted: @include_deleted,
+                                 enrollment: e)
             next unless p
             p2 = nil
             row = enrollment_row(e, include_other_roots, p, p2, pseudonyms, users_by_id)
@@ -528,10 +529,11 @@ def enrollment_headers(include_other_roots)
       headers
     end
 
-    def loaded_pseudonym(pseudonyms, u, include_deleted: false)
+    def loaded_pseudonym(pseudonyms, u, include_deleted: false, enrollment: nil)
+      context = enrollment || root_account
       user_pseudonyms = pseudonyms[u.id] || []
       u.instance_variable_set(include_deleted ? :@all_pseudonyms : :@all_active_pseudonyms, user_pseudonyms)
-      SisPseudonym.for(u, root_account, {type: :trusted, require_sis: false, include_deleted: include_deleted})
+      SisPseudonym.for(u, context, {type: :trusted, require_sis: false, include_deleted: include_deleted})
     end
 
     def load_cross_shard_logins(users, include_deleted: false)

gems/plugins/account_reports/spec_canvas/sis_provisioning_reports_spec.rb

@@ -946,8 +946,8 @@ def create_some_group_memberships_n_stuff()
             @user2 = user_with_managed_pseudonym(active_all: true, account: @root, name: 'James John',
                                                  username: '[email protected]', sis_user_id: 'other_shard2')
           end
-          allow(@account).to receive(:trusted_account_ids).and_return([@account.id, @root.id])
-          allow(@account).to receive(:trust_exists?).and_return(true)
+          allow_any_instantiation_of(@account).to receive(:trusted_account_ids).and_return([@account.id, @root.id])
+          allow_any_instantiation_of(@account).to receive(:trust_exists?).and_return(true)
           @e1 = @course1.enroll_user(@user1)
           @course1.enroll_user(@user2)
 

lib/api/v1/section.rb

@@ -30,18 +30,14 @@ def section_json(section, user, session, includes, options = {})
     end
     res['sis_import_id'] = section.sis_batch_id if section.course.grants_right?(user, session, :manage_sis)
     if includes.include?('students')
-      proxy = section.enrollments
-      if user_json_is_admin?
-        proxy = proxy.preload(user: :pseudonyms)
-      else
-        proxy = proxy.preload(:user)
-      end
+      proxy = section.enrollments.preload(:root_account, :sis_pseudonym, user: :pseudonyms)
       include_enrollments = includes.include?('enrollments')
-      res['students'] = proxy.where(:type => 'StudentEnrollment').
-        map { |e|
-          enrollments = include_enrollments ? [e] : nil
-          user_json(e.user, user, session, includes, @context, enrollments)
-        }
+      res['students'] = []
+      proxy.where(:type => 'StudentEnrollment').preload(:root_account, :sis_pseudonym).find_each do |e|
+        enrollments = include_enrollments ? [e] : nil
+        res['students'] << user_json(e.user, user, session, includes, @context, enrollments, [], e)
+      end
+      res['students'] = nil if res['students'].empty?
     end
     res['total_students'] = section.students.not_fake_student.count if includes.include?('total_students')
 

lib/api/v1/user.rb

@@ -42,14 +42,15 @@ def user_json_preloads(users, preload_email=false, opts={})
     end
   end
 
-  def user_json(user, current_user, session, includes = [], context = @context, enrollments = nil, excludes = [])
+  def user_json(user, current_user, session, includes = [], context = @context, enrollments = nil, excludes = [], enrollment=nil)
     includes ||= []
     excludes ||= []
     api_json(user, current_user, session, API_USER_JSON_OPTS).tap do |json|
       enrollment_json_opts = { current_grading_period_scores: includes.include?('current_grading_period_scores') }
       if !excludes.include?('pseudonym') && user_json_is_admin?(context, current_user)
         include_root_account = @domain_root_account.trust_exists?
-        pseudonym = SisPseudonym.for(user, @domain_root_account, type: :implicit, require_sis: false)
+        sis_context = enrollment || @domain_root_account
+        pseudonym = SisPseudonym.for(user, sis_context, type: :implicit, require_sis: false)
         enrollment_json_opts[:sis_pseudonym] = pseudonym if pseudonym&.sis_user_id
         # the sis fields on pseudonym are poorly named -- sis_user_id is
         # the id in the SIS import data, where on every other table
@@ -241,7 +242,7 @@ def enrollment_json(enrollment, user, session, includes = [], opts = {})
         json[:course_integration_id] = enrollment.course.integration_id
         json[:sis_section_id] = enrollment.course_section.sis_source_id
         json[:section_integration_id] = enrollment.course_section.integration_id
-        pseudonym = opts.key?(:sis_pseudonym) ? opts[:sis_pseudonym] : sis_pseudonym_for(enrollment.user)
+        pseudonym = opts.key?(:sis_pseudonym) ? opts[:sis_pseudonym] : sis_pseudonym_for(enrollment.user, enrollment)
         json[:sis_user_id] = pseudonym.try(:sis_user_id)
       end
       json[:html_url] = course_user_url(enrollment.course_id, enrollment.user_id)
@@ -320,8 +321,8 @@ def user_can_read_sis_data?(user, context)
     sis_id_context(context).grants_right?(user, :read_sis) || @domain_root_account.grants_right?(user, :manage_sis)
   end
 
-  def sis_pseudonym_for(user)
-    SisPseudonym.for(user, @domain_root_account, type: :trusted)
+  def sis_pseudonym_for(user, context=@domain_root_account)
+    SisPseudonym.for(user, context, type: :trusted)
   end
 
   def group_ids(user)

lib/gradebook_exporter.rb

@@ -58,7 +58,7 @@ def determine_column_separator
 
   def csv_data
     enrollment_scope = @course.apply_enrollment_visibility(gradebook_enrollment_scope, @user, nil,
-                                                           include: gradebook_includes)
+                                                           include: gradebook_includes).preload(:root_account, :sis_pseudonym)
     student_enrollments = enrollments_for_csv(enrollment_scope)
 
     student_section_names = {}
@@ -183,7 +183,7 @@ def csv_data
             end
           end
           row = [student_name(student), student.id]
-          pseudonym = SisPseudonym.for(student, @course, type: :implicit, require_sis: false)
+          pseudonym = SisPseudonym.for(student, student_enrollment, type: :implicit, require_sis: false)
           row << pseudonym.try(:sis_user_id) if include_sis_id
           row << pseudonym.try(:unique_id)
           row << (pseudonym && HostUrl.context_host(pseudonym.account)) if include_sis_id && include_root_account

lib/sis/enrollment_importer.rb

@@ -283,6 +283,7 @@ def process_batch
                 @update_account_association_user_ids.add(user.id)
               end
             end
+            enrollment.sis_pseudonym_id = pseudo.id
             if enrollment.changed?
               @users_to_touch_ids.add(user.id)
               courses_to_recache_due_dates << enrollment.course_id if enrollment.workflow_state_changed?

spec/models/sis_pseudonym_spec.rb

@@ -82,6 +82,26 @@ def pseud_params(unique_id, account = Account.default)
     expect(SisPseudonym.for(u, course1)).to eq @p
   end
 
+  it "should return pseudonym for specfic enrollment" do
+    @p = u.pseudonyms.create!(pseud_params("[email protected]")) do |x|
+      x.workflow_state = 'active'
+      x.sis_user_id = "user2"
+    end
+    @p2 = u.pseudonyms.create!(pseud_params("[email protected]")) do |x|
+      x.workflow_state = 'active'
+      x.sis_user_id = "user2b"
+    end
+    e = course1.enroll_user(u, 'StudentEnrollment', enrollment_state: 'active')
+    e.sis_pseudonym_id = @p.id
+    e.save!
+    section = course1.course_sections.create
+    e2 = course1.enroll_user(u, 'StudentEnrollment', enrollment_state: 'active', section: section, allow_multiple_enrollments: true)
+    e2.sis_pseudonym_id = @p2.id
+    e2.save!
+    expect(SisPseudonym.for(u, e)).to eq @p
+    expect(SisPseudonym.for(u, e2)).to eq @p2
+  end
+
   it "should find the right root account for a course" do
     pseudonym = account2.pseudonyms.create!(user: u, unique_id: 'user') do |p|
       p.sis_user_id = 'abc'