use correct id for merging observation links

when merging a user if done from the source shard the target user.id is
global, but from target shard is is local and the update call will fail
and update the record to a local id. This could potentially be a user
that should not be associated to that record

test plan
 - specs should pass
 - merge users with observation links from target shard

fixes VICE-734
flag=none

Change-Id: Iad6007bfb4bc596f21a69a59555d5622f08d4721
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/245751
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ben Nelson <bnelson@instructure.com>
QA-Review: Caleb Guanzon <cguanzon@instructure.com>
Product-Review: Caleb Guanzon <cguanzon@instructure.com>
This commit is contained in:
Rob Orton 2020-08-21 21:07:21 -06:00
parent 9bb6ac03dd
commit 4e6aa43759
3 changed files with 15 additions and 3 deletions

View File

@ -44,7 +44,7 @@ class UserObservationLink < ActiveRecord::Base
MISSING_ROOT_ACCOUNT_ID = -1
# shadow_record param is private
# cross_shard_record param is private
def self.create_or_restore(student: , observer: , root_account: , cross_shard_record: false)
raise ArgumentError, 'student, observer and root_account are required' unless student && observer && root_account
shard = cross_shard_record ? observer.shard : student.shard

View File

@ -411,9 +411,9 @@ class UserMerge
from_user.as_observer_observation_links.where(user_id: target_user).destroy_all
target_user.as_observer_observation_links.where(user_id: from_user).destroy_all
from_user.as_observer_observation_links.update_all(observer_id: target_user.id)
xor_observer_ids = UserObservationLink.where(student: [from_user, target_user]).distinct.pluck(:observer_id)
xor_observer_ids = UserObservationLink.shard(from_user, target_user).where(student: [from_user, target_user]).distinct.pluck(:observer_id)
from_user.as_student_observation_links.where(observer_id: target_user.as_student_observation_links.map(&:observer_id)).destroy_all
from_user.as_student_observation_links.update_all(user_id: target_user.id)
from_user.shard.activate { from_user.as_student_observation_links.update_all(user_id: target_user.id) }
# for any observers not already watching both users, make sure they have
# any missing observer enrollments added
if from_user.shard != target_user.shard

View File

@ -520,6 +520,18 @@ describe SplitUsers do
expect(shard1_source_user.reload.linked_observers).to eq [observer2]
end
it 'should handle user_observers cross shard from target shard' do
observer1 = user_model
add_linked_observer(restored_user, observer1)
@shard1.activate do
UserMerge.from(restored_user).into(shard1_source_user)
end
expect(restored_user.linked_observers).to eq []
expect(shard1_source_user.linked_observers.pluck(:id).sort).to eq [observer1.id].sort
SplitUsers.split_db_users(shard1_source_user)
expect(restored_user.reload.linked_observers).to eq [observer1]
end
it 'should handle conflicting submissions for cross shard users' do
course1.enroll_student(restored_user, enrollment_state: 'active')
course1.enroll_student(shard1_source_user, enrollment_state: 'active')