Fixes #3123: Merging Tickets/Users/... does not update ExternalSync references.

app/models/external_sync.rb

@@ -46,6 +46,15 @@ class ExternalSync < ApplicationModel
       result
     end
 
+    def migrate(object, from_id, to_id)
+      where(
+        object: object,
+        o_id:   from_id,
+      ).update_all( # rubocop:disable Rails/SkipsModelValidations
+        o_id: to_id,
+      )
+    end
+
     private
 
     def extract(remote_key, structure)

app/models/ticket.rb

@@ -356,6 +356,9 @@ returns
         link_object_target_value: id
       )
 
+      # external sync references
+      ExternalSync.migrate('Ticket', id, target_ticket.id)
+
       # set state to 'merged'
       self.state_id = Ticket::State.lookup(name: 'merged').id
 

db/migrate/20200716124141_issue_3123_external_sync_ticket_merge.rb

@@ -0,0 +1,31 @@
+class Issue3123ExternalSyncTicketMerge < ActiveRecord::Migration[5.2]
+  def change
+
+    # return if it's a new setup
+    return if !Setting.find_by(name: 'system_init_done')
+
+    merged_ticket_ids_with_external_sync.each do |id_from|
+      id_to = merged_ticket_ids_map[id_from]
+      ExternalSync.migrate('Ticket', id_from, id_to)
+    end
+  end
+
+  private
+
+  # reduce to the ones with an ExternalSync entry
+  def merged_ticket_ids_with_external_sync
+    @merged_ticket_ids_with_external_sync ||= ExternalSync.where(
+      object: 'Ticket',
+      o_id:   merged_ticket_ids_map.keys,
+    ).pluck(:o_id).uniq
+  end
+
+  # get all merged tickets
+  def merged_ticket_ids_map
+    @merged_ticket_ids_map ||= History.where(
+      history_type_id:   History.type_lookup('merged_into').id,
+      history_object_id: History.object_lookup('Ticket').id,
+    ).pluck(:id_from, :id_to).to_h
+  end
+
+end

lib/models.rb

@@ -266,6 +266,9 @@ returns
         items_to_update.each_value(&:save!)
       end
     end
+
+    ExternalSync.migrate(object_name, object_id_primary, object_id_to_merge)
+
     true
   end
 end

spec/db/migrate/issue3123_external_sync_ticket_merge_spec.rb

@@ -0,0 +1,37 @@
+require 'rails_helper'
+
+RSpec.describe Issue3123ExternalSyncTicketMerge, type: :db_migration do
+
+  let(:user) { create(:agent) }
+  let(:source_ticket) { create(:ticket) }
+  let(:target_ticket) { create(:ticket) }
+
+  before do
+    source_ticket.merge_to(
+      ticket_id: target_ticket.id,
+      user_id:   user.id,
+    )
+  end
+
+  context 'when no ExternalSync entries' do
+
+    it "doesn't send ExternalSync.migrate" do
+      allow(ExternalSync).to receive(:migrate)
+      migrate
+      expect(ExternalSync).not_to have_received(:migrate)
+    end
+  end
+
+  context 'when ExternalSync entries present' do
+
+    before do
+      create(:external_sync, object: 'Ticket', o_id: source_ticket.id)
+    end
+
+    it 'sends ExternalSync.migrate' do
+      allow(ExternalSync).to receive(:migrate)
+      migrate
+      expect(ExternalSync).to have_received(:migrate).with('Ticket', source_ticket.id, target_ticket.id)
+    end
+  end
+end

spec/factories/external_sync.rb

@@ -1,4 +1,14 @@
 FactoryBot.define do
   factory :external_sync do
+    source    { 'Some3rdPartyService' }
+    source_id { SecureRandom.uuid }
+    object    { 'Ticket' }
+    o_id      { 1 }
+
+    # https://github.com/thoughtbot/factory_bot/issues/1142
+    after :build do |record, options|
+      record.source_id = options.source_id
+      record.source    = options.source
+    end
   end
 end

spec/lib/external_sync_spec.rb

@@ -172,4 +172,25 @@ RSpec.describe ExternalSync do
       expect(source.destroyed?).to be false
     end
   end
+
+  describe '.migrate' do
+
+    let(:model) { 'Ticket' }
+    let(:factory_name) { model.downcase.to_sym }
+    let(:source) { create(factory_name) }
+    let(:target) { create(factory_name) }
+    let(:entries) do
+      create_list(:external_sync, 2,
+                  object: model,
+                  o_id:   source.id,)
+    end
+
+    it 'migrates entries' do
+      expect do
+        described_class.migrate('Ticket', source.id, target.id)
+      end.to change {
+        entries.each(&:reload).map(&:o_id).uniq
+      }.from([source.id]).to([target.id])
+    end
+  end
 end

spec/lib/models_spec.rb

@@ -0,0 +1,25 @@
+require 'rails_helper'
+
+RSpec.describe Models do
+
+  describe '.merge' do
+
+    context 'when ExternalSync references are present' do
+
+      shared_examples 'migrates entries' do |model|
+
+        let(:factory_name) { model.downcase.to_sym }
+        let(:source) { create(factory_name) }
+        let(:target) { create(factory_name) }
+
+        it 'sends ExternalSync.migrate' do
+          allow(ExternalSync).to receive(:migrate)
+          described_class.merge(model, source.id, target.id)
+          expect(ExternalSync).to have_received(:migrate).with(model, source.id, target.id)
+        end
+      end
+
+      it_behaves_like 'migrates entries', 'User'
+    end
+  end
+end

spec/models/ticket_spec.rb

@@ -116,6 +116,14 @@ RSpec.describe Ticket, type: :model do
           expect(origin_history['id_to']).to eq target_ticket.id
           expect(origin_history['id_from']).to eq ticket.id
         end
+
+        it 'sends ExternalSync.migrate' do
+          allow(ExternalSync).to receive(:migrate)
+
+          ticket.merge_to(ticket_id: target_ticket.id, user_id: merge_user.id)
+
+          expect(ExternalSync).to have_received(:migrate).with('Ticket', ticket.id, target_ticket.id)
+        end
       end
     end