properly split communication channels

fixes CNVS-29149

test plan
 - have 2 users with communication channels
 - merge them
 - split them
 - it should work

Change-Id: I8ca1b73efb1898c2c060160bbdd5cce9ba8759e2
Reviewed-on: https://gerrit.instructure.com/90561
Tested-by: Jenkins
Reviewed-by: John Corrigan <jcorrigan@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>

app/models/split_users.rb

@@ -102,6 +102,7 @@ class SplitUsers
     end
 
     def move_records_to_old_user(source_user, user, records)
+      fix_communication_channels(source_user, user, records.where(context_type: 'CommunicationChannel'))
       move_user_observers(source_user, user, records.where(context_type: 'UserObserver', previous_user_id: user))
       move_attachments(source_user, user, records.where(context_type: 'Attachment'))
       enrollment_ids = records.where(context_type: 'Enrollment', previous_user_id: user).pluck(:context_id)
@@ -125,6 +126,23 @@ class SplitUsers
       restore_worklow_states_from_records(records)
     end
 
+    def fix_communication_channels(source_user, user, cc_records)
+      if source_user.shard != user.shard
+        user.shard.activate do
+          # remove communication channels that didn't exist prior to the merge
+          CommunicationChannel.where(id: cc_records.where(previous_workflow_state: 'non existent').pluck(:context_id)).delete_all
+        end
+      end
+      # move moved communication channels back
+      max_position = user.communication_channels.last.try(:position) || 0
+      scope = source_user.communication_channels.where(id: cc_records.where(previous_user_id: user).pluck(:context_id))
+      scope.update_all(["user_id=?, position=position+?", user, max_position]) unless scope.empty?
+
+      cc_records.where.not(previous_workflow_state: 'non existent').each do |cr|
+        CommunicationChannel.where(id: cr.context_id).update_all(workflow_state: cr.previous_workflow_state)
+      end
+    end
+
     def move_user_observers(source_user, user, records)
       # skip when the user observer is between the two users. Just undlete the record
       not_obs = UserObserver.where(user_id: [source_user, user], observer_id: [source_user, user])
@@ -160,13 +178,6 @@ class SplitUsers
     def move_pseudonyms_to_user(pseudonyms, target_user)
       pseudonyms.each do |pseudonym|
         pseudonym.update_attribute(:user_id, target_user.id)
-        # try to grab the email
-        pseudonym.sis_communication_channel.try(:update_attribute, :user_id, target_user.id)
-        next unless pseudonym.communication_channel_id
-        if pseudonym.communication_channel_id != pseudonym.sis_communication_channel_id
-          cc = CommunicationChannel.where(id: pseudonym.communication_channel_id).first
-          cc.update_attribute(:user_id, target_user.id) if cc
-        end
       end
     end
 

spec/models/split_user_spec.rb

@@ -181,6 +181,65 @@ describe SplitUsers do
         expect(enrollment4.reload.user).to eq user1
         expect(enrollment5.reload.user).to eq user2
       end
+
+      it "should move ccs to the new user (but only if they don't already exist)" do
+        # unconfirmed: active conflict
+        user1.communication_channels.create!(path: 'a@instructure.com')
+        user2.communication_channels.create!(path: 'A@instructure.com') { |cc| cc.workflow_state = 'active' }
+        # active: unconfirmed conflict
+        user1.communication_channels.create!(path: 'b@instructure.com') { |cc| cc.workflow_state = 'active' }
+        cc = user2.communication_channels.create!(path: 'B@instructure.com')
+        # active: active conflict
+        user1.communication_channels.create!(path: 'c@instructure.com') { |cc| cc.workflow_state = 'active' }
+        user2.communication_channels.create!(path: 'C@instructure.com') { |cc| cc.workflow_state = 'active' }
+        # unconfirmed: unconfirmed conflict
+        user1.communication_channels.create!(path: 'd@instructure.com')
+        user2.communication_channels.create!(path: 'D@instructure.com')
+        # retired: unconfirmed conflict
+        user1.communication_channels.create!(path: 'e@instructure.com') { |cc| cc.workflow_state = 'retired' }
+        user2.communication_channels.create!(path: 'E@instructure.com')
+        # unconfirmed: retired conflict
+        user1.communication_channels.create!(path: 'f@instructure.com')
+        user2.communication_channels.create!(path: 'F@instructure.com') { |cc| cc.workflow_state = 'retired' }
+        # retired: active conflict
+        user1.communication_channels.create!(path: 'g@instructure.com') { |cc| cc.workflow_state = 'retired' }
+        user2.communication_channels.create!(path: 'G@instructure.com') { |cc| cc.workflow_state = 'active' }
+        # active: retired conflict
+        user1.communication_channels.create!(path: 'h@instructure.com') { |cc| cc.workflow_state = 'active' }
+        user2.communication_channels.create!(path: 'H@instructure.com') { |cc| cc.workflow_state = 'retired' }
+        # retired: retired conflict
+        user1.communication_channels.create!(path: 'i@instructure.com') { |cc| cc.workflow_state = 'retired' }
+        user2.communication_channels.create!(path: 'I@instructure.com') { |cc| cc.workflow_state = 'retired' }
+        # <nothing>: active
+        user2.communication_channels.create!(path: 'J@instructure.com') { |cc| cc.workflow_state = 'active' }
+        # active: <nothing>
+        user1.communication_channels.create!(path: 'k@instructure.com') { |cc| cc.workflow_state = 'active' }
+        # <nothing>: unconfirmed
+        user2.communication_channels.create!(path: 'L@instructure.com')
+        # unconfirmed: <nothing>
+        user1.communication_channels.create!(path: 'm@instructure.com')
+        # <nothing>: retired
+        user2.communication_channels.create!(path: 'N@instructure.com') { |cc| cc.workflow_state = 'retired' }
+        # retired: <nothing>
+        user1.communication_channels.create!(path: 'o@instructure.com') { |cc| cc.workflow_state = 'retired' }
+
+        user1_ccs = user1.communication_channels.where.not(workflow_state: 'retired').
+          map { |cc| [cc.path, cc.workflow_state] }.sort
+        # cc will not be restored because it conflicted on merge and it was unconfirmed and it is frd deleted
+        user2_ccs = user2.communication_channels.where.not(id: cc, workflow_state: 'retired').
+          map { |cc| [cc.path, cc.workflow_state] }.sort
+
+        UserMerge.from(user1).into(user2)
+        SplitUsers.split_db_users(user2)
+        user1.reload
+        user2.reload
+
+        expect(user1.communication_channels.where.not(workflow_state: 'retired').
+          map { |cc| [cc.path, cc.workflow_state] }.sort).to eq user1_ccs
+        expect(user2.communication_channels.where.not(workflow_state: 'retired').
+          map { |cc| [cc.path, cc.workflow_state] }.sort).to eq user2_ccs
+      end
+
     end
 
     it 'should restore submissions' do
@@ -250,6 +309,28 @@ describe SplitUsers do
         expect(p1.reload.user).to eq user1
         expect(@user2.all_pseudonyms).to eq [@p2]
       end
+
+      it "should split a user across shards with ccs" do
+        user1 = user_with_pseudonym(username: 'user1@example.com', active_all: 1)
+        user1_ccs = user1.communication_channels.map { |cc| [cc.path, cc.workflow_state] }.sort
+
+        @shard1.activate do
+          account = Account.create!
+          @user2 = user_with_pseudonym(username: 'user2@example.com', active_all: 1, account: account)
+        end
+        user2_ccs = @user2.communication_channels.map { |cc| [cc.path, cc.workflow_state] }.sort
+
+        @shard1.activate do
+          UserMerge.from(user1).into(@user2)
+          SplitUsers.split_db_users(@user2)
+        end
+
+        user1.reload
+        @user2.reload
+        expect(user1.communication_channels.map { |cc| [cc.path, cc.workflow_state] }.sort).to eq user1_ccs
+        expect(@user2.communication_channels.map { |cc| [cc.path, cc.workflow_state] }.sort).to eq user2_ccs
+      end
+
     end
   end
 end