Refactoring: Use duck-typing for first argument of Ticket#perform_changes to enhance feature/implementation/extension capabilities.

This commit is contained in:
Thorsten Eckel 2020-09-30 14:28:07 +02:00
parent ffe5948699
commit b52e49e6e6
3 changed files with 24 additions and 12 deletions

View file

@ -86,7 +86,7 @@ job.run(true)
tickets&.each do |ticket| tickets&.each do |ticket|
Transaction.execute(disable_notification: disable_notification, reset_user_id: true) do Transaction.execute(disable_notification: disable_notification, reset_user_id: true) do
ticket.perform_changes(perform, 'job') ticket.perform_changes(self, 'job')
end end
end end

View file

@ -889,11 +889,17 @@ condition example
perform changes on ticket 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 =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})" } logger.debug { "Perform #{perform_origin} #{perform.inspect} on Ticket.find(#{id})" }
article = begin article = begin
@ -1253,7 +1259,7 @@ perform active triggers on ticket
local_options[:trigger_ids][ticket.id.to_s].push trigger.id 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]})" } 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 if recursive == true
Observer::Transaction.commit(local_options) Observer::Transaction.commit(local_options)

View file

@ -148,6 +148,12 @@ RSpec.describe Ticket, type: :model do
end end
describe '#perform_changes' do 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 # Regression test for https://github.com/zammad/zammad/issues/2001
describe 'argument handling' do describe 'argument handling' do
let(:perform) do let(:perform) do
@ -161,7 +167,7 @@ RSpec.describe Ticket, type: :model do
end end
it 'does not mutate contents of "perform" hash' do 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 } .not_to change { perform }
end end
end end
@ -176,7 +182,7 @@ RSpec.describe Ticket, type: :model do
end end
it 'changes #state to specified value' do 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') .to change { ticket.reload.state.name }.to('closed')
end end
end end
@ -198,7 +204,7 @@ RSpec.describe Ticket, type: :model do
it 'changes pending date to given date' do it 'changes pending date to given date' do
freeze_time 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)) .to change(ticket, :pending_time).to(be_within(1.minute).of(timestamp))
end end
end end
@ -211,7 +217,7 @@ RSpec.describe Ticket, type: :model do
freeze_time do freeze_time do
interval = relative_value.send(relative_range).from_now 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)) .to change(ticket, :pending_time).to(be_within(1.minute).of(interval))
end end
end end
@ -256,7 +262,7 @@ RSpec.describe Ticket, type: :model do
end end
it 'performs a ticket deletion on a ticket' do 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) .to change(ticket, :destroyed?).to(true)
end end
end end
@ -300,7 +306,7 @@ RSpec.describe Ticket, type: :model do
.not_to receive(:template) .not_to receive(:template)
.with(hash_including(objects: { ticket: ticket, article: new_article })) .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 end
end end
@ -329,12 +335,12 @@ RSpec.describe Ticket, type: :model do
shared_examples 'verify log visibility status' do shared_examples 'verify log visibility status' do
shared_examples 'notification trigger' do shared_examples 'notification trigger' do
it 'adds Ticket::Article' 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) .to change { ticket.articles.count }.by(1)
end end
it 'new Ticket::Article visibility reflects setting' do 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 new_article = ticket.articles.reload.last
expect(new_article.internal).to be target_internal_value expect(new_article.internal).to be target_internal_value
end end