diff --git a/app/models/job.rb b/app/models/job.rb index c4dd51812..d7982bbc0 100644 --- a/app/models/job.rb +++ b/app/models/job.rb @@ -86,7 +86,7 @@ job.run(true) tickets&.each do |ticket| Transaction.execute(disable_notification: disable_notification, reset_user_id: true) do - ticket.perform_changes(perform, 'job') + ticket.perform_changes(self, 'job') end end diff --git a/app/models/ticket.rb b/app/models/ticket.rb index a1cf82fb7..2879d21d0 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -889,11 +889,17 @@ condition example perform changes on ticket - ticket.perform_changes({}, 'trigger', item, current_user_id) + ticket.perform_changes(trigger, 'trigger', item, current_user_id) + + # or + + ticket.perform_changes(job, 'job', item, current_user_id) =end - def perform_changes(perform, perform_origin, item = nil, current_user_id = nil) + def perform_changes(performable, perform_origin, item = nil, current_user_id = nil) + + perform = performable.perform logger.debug { "Perform #{perform_origin} #{perform.inspect} on Ticket.find(#{id})" } article = begin @@ -1253,7 +1259,7 @@ perform active triggers on ticket local_options[:trigger_ids][ticket.id.to_s].push trigger.id logger.info { "Execute trigger (#{trigger.name}/#{trigger.id}) for this object (Ticket:#{ticket.id}/Loop:#{local_options[:loop_count]})" } - ticket.perform_changes(trigger.perform, 'trigger', item, user_id) + ticket.perform_changes(trigger, 'trigger', item, user_id) if recursive == true Observer::Transaction.commit(local_options) diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index 6c34d74b6..a08bcd646 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -148,6 +148,12 @@ RSpec.describe Ticket, type: :model do end describe '#perform_changes' do + + # a `performable` can be a Trigger or a Job + # we use DuckTyping and expect that a performable + # implements the following interface + let(:performable) { OpenStruct.new(id: 1, perform: perform) } + # Regression test for https://github.com/zammad/zammad/issues/2001 describe 'argument handling' do let(:perform) do @@ -161,7 +167,7 @@ RSpec.describe Ticket, type: :model do end it 'does not mutate contents of "perform" hash' do - expect { ticket.perform_changes(perform, 'trigger', {}, 1) } + expect { ticket.perform_changes(performable, 'trigger', {}, 1) } .not_to change { perform } end end @@ -176,7 +182,7 @@ RSpec.describe Ticket, type: :model do end it 'changes #state to specified value' do - expect { ticket.perform_changes(perform, 'trigger', ticket, User.first) } + expect { ticket.perform_changes(performable, 'trigger', ticket, User.first) } .to change { ticket.reload.state.name }.to('closed') end end @@ -198,7 +204,7 @@ RSpec.describe Ticket, type: :model do it 'changes pending date to given date' do freeze_time do - expect { ticket.perform_changes(perform, 'trigger', ticket, User.first) } + expect { ticket.perform_changes(performable, 'trigger', ticket, User.first) } .to change(ticket, :pending_time).to(be_within(1.minute).of(timestamp)) end end @@ -211,7 +217,7 @@ RSpec.describe Ticket, type: :model do freeze_time do interval = relative_value.send(relative_range).from_now - expect { ticket.perform_changes(perform, 'trigger', ticket, User.first) } + expect { ticket.perform_changes(performable, 'trigger', ticket, User.first) } .to change(ticket, :pending_time).to(be_within(1.minute).of(interval)) end end @@ -256,7 +262,7 @@ RSpec.describe Ticket, type: :model do end it 'performs a ticket deletion on a ticket' do - expect { ticket.perform_changes(perform, 'trigger', ticket, User.first) } + expect { ticket.perform_changes(performable, 'trigger', ticket, User.first) } .to change(ticket, :destroyed?).to(true) end end @@ -300,7 +306,7 @@ RSpec.describe Ticket, type: :model do .not_to receive(:template) .with(hash_including(objects: { ticket: ticket, article: new_article })) - ticket.perform_changes(trigger.perform, 'trigger', { article_id: article.id }, 1) + ticket.perform_changes(trigger, 'trigger', { article_id: article.id }, 1) end end end @@ -329,12 +335,12 @@ RSpec.describe Ticket, type: :model do shared_examples 'verify log visibility status' do shared_examples 'notification trigger' do it 'adds Ticket::Article' do - expect { ticket.perform_changes(perform, 'trigger', ticket, user) } + expect { ticket.perform_changes(performable, 'trigger', ticket, user) } .to change { ticket.articles.count }.by(1) end it 'new Ticket::Article visibility reflects setting' do - ticket.perform_changes(perform, 'trigger', ticket, User.first) + ticket.perform_changes(performable, 'trigger', ticket, User.first) new_article = ticket.articles.reload.last expect(new_article.internal).to be target_internal_value end