Browse Source

Fixes #4834 - When an update via API fails, a notification is sent to the agents that the change was made.

Co-authored-by: Rolf Schmidt <rolf.schmidt@zammad.com>
Florian Liebe 1 year ago
parent
commit
2616904efd

+ 2 - 2
app/models/organization.rb

@@ -17,8 +17,6 @@ class Organization < ApplicationModel
   include Organization::SearchIndex
   include Organization::TriggersSubscriptions
 
-  include HasTransactionDispatcher
-
   default_scope { order(:id) }
 
   has_many :members, class_name: 'User', after_add: :member_update, after_remove: :member_update
@@ -29,7 +27,9 @@ class Organization < ApplicationModel
   before_update :domain_cleanup
 
   # workflow checks should run after before_create and before_update callbacks
+  # the transaction dispatcher must be run after the workflow checks!
   include ChecksCoreWorkflow
+  include HasTransactionDispatcher
 
   core_workflow_screens 'create', 'edit'
 

+ 2 - 2
app/models/ticket.rb

@@ -38,10 +38,10 @@ class Ticket < ApplicationModel
   # This must be loaded late as it depends on the internal before_create and before_update handlers of ticket.rb.
   include Ticket::SetsLastOwnerUpdateTime
 
-  include HasTransactionDispatcher
-
   # workflow checks should run after before_create and before_update callbacks
+  # the transaction dispatcher must be run after the workflow checks!
   include ChecksCoreWorkflow
+  include HasTransactionDispatcher
 
   validates :group_id, presence: true
 

+ 2 - 2
app/models/user.rb

@@ -22,8 +22,6 @@ class User < ApplicationModel
   include User::PerformsGeoLookup
   include User::UpdatesTicketOrganization
 
-  include HasTransactionDispatcher
-
   has_and_belongs_to_many :organizations,          after_add: %i[cache_update create_organization_add_history], after_remove: %i[cache_update create_organization_remove_history], class_name: 'Organization'
   has_and_belongs_to_many :overviews,              dependent: :nullify
   has_many                :tokens,                 after_add: :cache_update, after_remove: :cache_update, dependent: :destroy
@@ -56,7 +54,9 @@ class User < ApplicationModel
   validate :ensure_uniq_email, unless: :skip_ensure_uniq_email
 
   # workflow checks should run after before_create and before_update callbacks
+  # the transaction dispatcher must be run after the workflow checks!
   include ChecksCoreWorkflow
+  include HasTransactionDispatcher
 
   core_workflow_screens 'create', 'edit'
 

+ 26 - 0
spec/models/ticket/has_transaction_dispatcher_spec.rb

@@ -0,0 +1,26 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe 'HasTransactionDispatcher', db_strategy: :reset, type: :model do
+  let(:ticket) { create(:ticket) }
+  let(:agent)  { create(:agent, groups: [ticket.group]) }
+
+  describe '#before_update' do
+    context 'when ticket is updated without sending required values' do
+      before do
+        UserInfo.current_user_id = agent.id
+
+        create(:object_manager_attribute_text, :required_screen)
+        ObjectManager::Attribute.migration_execute
+      end
+
+      it 'does not call the TransactionDispatcher before_update hook', :aggregate_failures do
+        allow(TransactionDispatcher).to receive(:before_update)
+
+        expect { ticket.update(title: 'New title', screen: 'edit') }.to raise_error(Exceptions::ApplicationModel)
+        expect(TransactionDispatcher).not_to have_received(:before_update)
+      end
+    end
+  end
+end