Fixes #3105 - Merging tickets doesn’t trigger notification for target ticket

app/assets/javascripts/app/controllers/_ui_element/ticket_selector.coffee

@@ -73,8 +73,10 @@ class App.UiElement.ticket_selector
         null: false
         translate: true
         options:
-          create: 'created'
-          update: 'updated'
+          create:                  'created'
+          update:                  'updated'
+          'update.merged_into':    'merged into'
+          'update.received_merge': 'received merge'
         operator: ['is', 'is not']
 
     for groupKey, groupMeta of groups

app/assets/javascripts/app/models/ticket.coffee

@@ -103,17 +103,23 @@ class App.Ticket extends App.Model
     return if !item
     return if !item.created_by
 
-    if item.type is 'create'
-      return App.i18n.translateContent('%s created Ticket |%s|', item.created_by.displayName(), item.title)
-    else if item.type is 'update'
-      return App.i18n.translateContent('%s updated Ticket |%s|', item.created_by.displayName(), item.title)
-    else if item.type is 'reminder_reached'
-      return App.i18n.translateContent('Pending reminder reached for Ticket |%s|', item.title)
-    else if item.type is 'escalation'
-      return App.i18n.translateContent('Ticket |%s| is escalated!', item.title)
-    else if item.type is 'escalation_warning'
-      return App.i18n.translateContent('Ticket |%s| will escalate soon!', item.title)
-    return "Unknow action for (#{@objectDisplayName()}/#{item.type}), extend activityMessage() of model."
+    switch item.type
+      when 'create'
+        App.i18n.translateContent('%s created Ticket |%s|', item.created_by.displayName(), item.title)
+      when 'update'
+        App.i18n.translateContent('%s updated Ticket |%s|', item.created_by.displayName(), item.title)
+      when 'reminder_reached'
+        App.i18n.translateContent('Pending reminder reached for Ticket |%s|', item.title)
+      when 'escalation'
+        App.i18n.translateContent('Ticket |%s| is escalated!', item.title)
+      when 'escalation_warning'
+        App.i18n.translateContent('Ticket |%s| will escalate soon!', item.title)
+      when 'update.merged_into'
+        App.i18n.translateContent('Ticket |%s| was merged into another ticket', item.title)
+      when 'update.received_merge'
+        App.i18n.translateContent('Another ticket was merged into ticket |%s|', item.title)
+      else
+        "Unknow action for (#{@objectDisplayName()}/#{item.type}), extend activityMessage() of model."
 
   # apply macro
   @macro: (params) ->

app/models/ticket.rb

@@ -376,6 +376,26 @@ returns
 
       # touch new ticket (to broadcast change)
       target_ticket.touch # rubocop:disable Rails/SkipsModelValidations
+
+      EventBuffer.add('transaction', {
+                        object:     target_ticket.class.name,
+                        type:       'update.received_merge',
+                        data:       target_ticket,
+                        changes:    {},
+                        id:         target_ticket.id,
+                        user_id:    UserInfo.current_user_id,
+                        created_at: Time.zone.now,
+                      })
+
+      EventBuffer.add('transaction', {
+                        object:     self.class.name,
+                        type:       'update.merged_into',
+                        data:       self,
+                        changes:    {},
+                        id:         id,
+                        user_id:    UserInfo.current_user_id,
+                        created_at: Time.zone.now,
+                      })
     end
     true
   end

app/models/transaction/notification.rb

@@ -182,21 +182,24 @@ class Transaction::Notification
 
       # get user based notification template
       # if create, send create message / block update messages
-      template = nil
-      case @item[:type]
-      when 'create'
-        template = 'ticket_create'
-      when 'update'
-        template = 'ticket_update'
-      when 'reminder_reached'
-        template = 'ticket_reminder_reached'
-      when 'escalation'
-        template = 'ticket_escalation'
-      when 'escalation_warning'
-        template = 'ticket_escalation_warning'
-      else
-        raise "unknown type for notification #{@item[:type]}"
-      end
+      template = case @item[:type]
+                 when 'create'
+                   'ticket_create'
+                 when 'update'
+                   'ticket_update'
+                 when 'reminder_reached'
+                   'ticket_reminder_reached'
+                 when 'escalation'
+                   'ticket_escalation'
+                 when 'escalation_warning'
+                   'ticket_escalation_warning'
+                 when 'update.merged_into'
+                   'ticket_update_merged_into'
+                 when 'update.received_merge'
+                   'ticket_update_received_merge'
+                 else
+                   raise "unknown type for notification #{@item[:type]}"
+                 end
 
       current_user = User.lookup(id: @item[:user_id])
       if !current_user

app/views/mailer/ticket_update_merged_into/en.html.erb

@@ -0,0 +1,11 @@
+Ticket (#{ticket.title}) was merged into another ticket
+
+<div>Hi #{recipient.firstname},</div>
+<br>
+<div>
+Ticket (#{ticket.title}) was merged into another ticket by "<b>#{current_user.longname}</b>".
+</div>
+<br>
+<div>
+  <a href="#{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}" target="zammad_app">#{t('View this in Zammad')}</a>
+</div>

app/views/mailer/ticket_update_received_merge/en.html.erb

@@ -0,0 +1,11 @@
+Another ticket was merged into ticket (#{ticket.title})
+
+<div>Hi #{recipient.firstname},</div>
+<br>
+<div>
+Another ticket was merged into ticket (#{ticket.title}) by "<b>#{current_user.longname}</b>".
+</div>
+<br>
+<div>
+  <a href="#{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}" target="zammad_app">#{t('View this in Zammad')}</a>
+</div>

lib/notification_factory/mailer.rb

@@ -28,6 +28,9 @@ returns
     map = {
       'escalation_warning' => 'escalation'
     }
+
+    type = type.split('.').first # pick parent type of a subtype. Eg. update vs update.merged_into
+
     if map[type]
       type = map[type]
     end

spec/factories/trigger.rb

@@ -9,4 +9,25 @@ FactoryBot.define do
     created_by_id   { 1 }
     updated_by_id   { 1 }
   end
+
+  trait :conditionable do
+    transient do
+      condition_ticket_action { nil }
+    end
+
+    condition { {} }
+
+    callback(:after_stub, :before_create) do |object, context|
+      hash = object.condition
+
+      hash['ticket.action'] = { 'operator' => 'is', 'value' => context.condition_ticket_action.to_s } if context.condition_ticket_action
+
+      object.condition = hash
+    end
+  end
+
+  # empty trigger to help to test atomically
+  trait :no_perform do
+    perform { { null: true } }
+  end
 end

spec/factories/user.rb

@@ -57,6 +57,26 @@ FactoryBot.define do
       password_plain = user.password
       user.define_singleton_method(:password_plain, -> { password_plain })
     end
+
+    trait :preferencable do
+      transient do
+        notification_group_ids { [] }
+      end
+
+      preferences do
+        {
+          'notification_config' => {
+            'matrix'    => {
+              'create'           => { 'criteria' => { 'owned_by_me' => true, 'owned_by_nobody' => true }, 'channel' => { 'email' => true, 'online' => true } },
+              'update'           => { 'criteria' => { 'owned_by_me' => true, 'owned_by_nobody' => true }, 'channel' => { 'email' => true, 'online' => true } },
+              'reminder_reached' => { 'criteria' => { 'owned_by_me' => true, 'owned_by_nobody' => true }, 'channel' => { 'email' => true, 'online' => true } },
+              'escalation'       => { 'criteria' => { 'owned_by_me' => true, 'owned_by_nobody' => true }, 'channel' => { 'email' => true, 'online' => true } },
+            },
+            'group_ids' => notification_group_ids
+          }
+        }
+      end
+    end
   end
 
   sequence(:password_valid) do |n|

spec/models/ticket_spec.rb

@@ -274,6 +274,150 @@ RSpec.describe Ticket, type: :model do
           end
         end
       end
+
+      # https://github.com/zammad/zammad/issues/3105
+      context 'when merge actions triggers exist', :performs_jobs do
+        before do
+          ticket && target_ticket
+          merged_into_trigger && received_merge_trigger && update_trigger
+
+          allow_any_instance_of(described_class).to receive(:perform_changes) do |ticket, trigger| # rubocop:disable RSpec/AnyInstance
+            log << { ticket: ticket.id, trigger: trigger.id }
+          end
+
+          perform_enqueued_jobs do
+            ticket.merge_to(ticket_id: target_ticket.id, user_id: 1)
+          end
+        end
+
+        let(:merged_into_trigger)    { create(:trigger, :conditionable, condition_ticket_action: 'update.merged_into') }
+        let(:received_merge_trigger) { create(:trigger, :conditionable, condition_ticket_action: 'update.received_merge') }
+        let(:update_trigger)         { create(:trigger, :conditionable, condition_ticket_action: 'update') }
+
+        let(:log) { [] }
+
+        it 'merge_into triggered with source ticket' do
+          expect(log).to include({ ticket: ticket.id, trigger: merged_into_trigger.id })
+        end
+
+        it 'received_merge not triggered with source ticket' do
+          expect(log).not_to include({ ticket: ticket.id, trigger: received_merge_trigger.id })
+        end
+
+        it 'update not triggered with source ticket' do
+          expect(log).not_to include({ ticket: ticket.id, trigger: update_trigger.id })
+        end
+
+        it 'merge_into not triggered with target ticket' do
+          expect(log).not_to include({ ticket: target_ticket.id, trigger: merged_into_trigger.id })
+        end
+
+        it 'received_merge triggered with target ticket' do
+          expect(log).to include({ ticket: target_ticket.id, trigger: received_merge_trigger.id })
+        end
+
+        it 'update not triggered with target ticket' do
+          expect(log).not_to include({ ticket: target_ticket.id, trigger: update_trigger.id })
+        end
+      end
+
+      # https://github.com/zammad/zammad/issues/3105
+      context 'when user has notifications enabled', :performs_jobs do
+        before do
+          user
+
+          allow(OnlineNotification).to receive(:add) do |**args|
+            next if args[:object] != 'Ticket'
+
+            log << { type: :online, event: args[:type], ticket_id: args[:o_id], user_id: args[:user_id] }
+          end
+
+          allow(NotificationFactory::Mailer).to receive(:notification) do |**args|
+            log << { type: :email, event: args[:template], ticket_id: args[:objects][:ticket].id, user_id: args[:user].id }
+          end
+
+          perform_enqueued_jobs do
+            ticket.merge_to(ticket_id: target_ticket.id, user_id: 1)
+          end
+        end
+
+        let(:user) { create(:agent, :preferencable, notification_group_ids: [ticket, target_ticket].map(&:group_id), groups: [ticket, target_ticket].map(&:group)) }
+        let(:log)  { [] }
+
+        it 'merge_into notification sent with source ticket' do
+          expect(log).to include({ type: :online, event: 'update.merged_into', ticket_id: ticket.id, user_id: user.id })
+        end
+
+        it 'received_merge notification not sent with source ticket' do
+          expect(log).not_to include({ type: :online, event: 'update.received_merge', ticket_id: ticket.id, user_id: user.id })
+        end
+
+        it 'update notification not sent with source ticket' do
+          expect(log).not_to include({ type: :online, event: 'update', ticket_id: ticket.id, user_id: user.id })
+        end
+
+        it 'merge_into notification not sent with target ticket' do
+          expect(log).not_to include({ type: :online, event: 'update.merged_into', ticket_id: target_ticket.id, user_id: user.id })
+        end
+
+        it 'received_merge notification sent with target ticket' do
+          expect(log).to include({ type: :online, event: 'update.received_merge', ticket_id: target_ticket.id, user_id: user.id })
+        end
+
+        it 'update notification not sent with target ticket' do
+          expect(log).not_to include({ type: :online, event: 'update', ticket_id: target_ticket.id, user_id: user.id })
+        end
+
+        it 'merge_into email sent with source ticket' do
+          expect(log).to include({ type: :email, event: 'ticket_update_merged_into', ticket_id: ticket.id, user_id: user.id })
+        end
+
+        it 'received_merge email not sent with source ticket' do
+          expect(log).not_to include({ type: :email, event: 'ticket_update_received_merge', ticket_id: ticket.id, user_id: user.id })
+        end
+
+        it 'update email not sent with source ticket' do
+          expect(log).not_to include({ type: :email, event: 'ticket_update', ticket_id: ticket.id, user_id: user.id })
+        end
+
+        it 'merge_into email not sent with target ticket' do
+          expect(log).not_to include({ type: :email, event: 'ticket_update_merged_into', ticket_id: target_ticket.id, user_id: user.id })
+        end
+
+        it 'received_merge email sent with target ticket' do
+          expect(log).to include({ type: :email, event: 'ticket_update_received_merge', ticket_id: target_ticket.id, user_id: user.id })
+        end
+
+        it 'update email not sent with target ticket' do
+          expect(log).not_to include({ type: :email, event: 'ticket_update', ticket_id: target_ticket.id, user_id: user.id })
+        end
+      end
+
+      # https://github.com/zammad/zammad/issues/3105
+      context 'when sending notification email correct template', :performs_jobs do
+        before do
+          user
+
+          allow(NotificationFactory::Mailer).to receive(:send) do |**args|
+            log << args[:subject]
+          end
+
+          perform_enqueued_jobs do
+            ticket.merge_to(ticket_id: target_ticket.id, user_id: 1)
+          end
+        end
+
+        let(:user) { create(:agent, :preferencable, notification_group_ids: [ticket, target_ticket].map(&:group_id), groups: [ticket, target_ticket].map(&:group)) }
+        let(:log)  { [] }
+
+        it 'is used for merged_into' do
+          expect(log).to include(start_with("Ticket (#{ticket.title}) was merged into another ticket"))
+        end
+
+        it 'is used for received_merge' do
+          expect(log).to include(start_with("Another ticket was merged into ticket (#{target_ticket.title})"))
+        end
+      end
     end
 
     describe '#perform_changes' do

spec/support/active_job.rb

@@ -1,7 +1,6 @@
 # Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
 
 module ZammadActiveJobHelper
-
   delegate :enqueued_jobs, :performed_jobs, to: :queue_adapter
 
   def queue_adapter

spec/system/ticket/zoom_spec.rb

@@ -1706,4 +1706,60 @@ RSpec.describe 'Ticket zoom', type: :system do
       end
     end
   end
+
+  describe 'merging', authenticated_as: :user do
+    before do
+      merged_into_trigger && received_merge_trigger && update_trigger
+
+      visit "ticket/zoom/#{ticket.id}"
+      visit "ticket/zoom/#{target_ticket.id}"
+
+      await_empty_ajax_queue
+
+      ensure_websocket do
+        visit 'dashboard'
+      end
+    end
+
+    let(:merged_into_trigger)    { create(:trigger, :conditionable, condition_ticket_action: :merged_into) }
+    let(:received_merge_trigger) { create(:trigger, :conditionable, condition_ticket_action: :received_merge) }
+    let(:update_trigger)         { create(:trigger, :conditionable, condition_ticket_action: :update) }
+
+    let(:ticket)                 { create(:ticket) }
+    let(:target_ticket)          { create(:ticket) }
+
+    let(:user)                   { create(:agent, :preferencable, notification_group_ids: [ticket, target_ticket].map(&:group_id), groups: [ticket, target_ticket].map(&:group)) }
+
+    context 'when merging ticket' do
+      before do
+        ticket.merge_to(ticket_id: target_ticket.id, user_id: 1)
+      end
+
+      it 'pulses source ticket' do
+        expect(page).to have_css("#navigation a.is-modified[data-key=\"Ticket-#{ticket.id}\"]")
+      end
+
+      it 'pulses target ticket' do
+        expect(page).to have_css("#navigation a.is-modified[data-key=\"Ticket-#{target_ticket.id}\"]")
+      end
+    end
+
+    context 'when merging and looking at online notifications', :performs_jobs do
+      before do
+        perform_enqueued_jobs do
+          ticket.merge_to(ticket_id: target_ticket.id, user_id: 1)
+        end
+
+        find('.js-toggleNotifications').click
+      end
+
+      it 'shows online notification for source ticket' do
+        expect(page).to have_text("Ticket #{ticket.title} was merged into another ticket")
+      end
+
+      it 'shows online notification for target ticket' do
+        expect(page).to have_text("Another ticket was merged into ticket #{ticket.title}")
+      end
+    end
+  end
 end