From d8be2e0aa79b285c42d65cd910ce544842a92002 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Tue, 25 Jul 2017 13:53:01 +0200 Subject: [PATCH] Small improvement for issue #1216 (moved same ticket check into model, show error message in ui). --- .../app/controllers/agent_ticket_merge.coffee | 8 ++++++-- app/controllers/tickets_controller.rb | 9 --------- app/models/ticket.rb | 7 +++++-- spec/models/ticket_spec.rb | 13 ++++++++++++- 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/app/assets/javascripts/app/controllers/agent_ticket_merge.coffee b/app/assets/javascripts/app/controllers/agent_ticket_merge.coffee index aad33ceb4649..d7108719889a 100644 --- a/app/assets/javascripts/app/controllers/agent_ticket_merge.coffee +++ b/app/assets/javascripts/app/controllers/agent_ticket_merge.coffee @@ -98,9 +98,13 @@ class App.TicketMerge extends App.ControllerModal type: 'error' msg: App.i18n.translateContent(data['message']) timeout: 6000 - @formEnable(e) - error: => + error: (data) => + details = data.responseJSON || {} + @notify + type: 'error' + msg: App.i18n.translateContent(details.error_human || details.error || 'Unable to merge!') + timeout: 6000 @formEnable(e) ) diff --git a/app/controllers/tickets_controller.rb b/app/controllers/tickets_controller.rb index b50a40c9ea68..60ac0d05e7a0 100644 --- a/app/controllers/tickets_controller.rb +++ b/app/controllers/tickets_controller.rb @@ -345,15 +345,6 @@ def ticket_merge # permission check ticket_permission(ticket_slave) - # check diffetent ticket ids - if ticket_slave.id == ticket_master.id - render json: { - result: 'failed', - message: 'Can\'t merge ticket with it self!', - } - return - end - # merge ticket ticket_slave.merge_to( ticket_id: ticket_master.id, diff --git a/app/models/ticket.rb b/app/models/ticket.rb index a2b042389c2f..eee5accc7fd9 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -276,9 +276,12 @@ def self.process_escalation def merge_to(data) # prevent cross merging tickets - target_ticket = Ticket.find(data[:ticket_id]) + target_ticket = Ticket.find_by(id: data[:ticket_id]) raise 'no target ticket given' if !target_ticket - raise 'invalid state for target ticket' if target_ticket.state.name == 'merged' + raise Exceptions::UnprocessableEntity, 'ticket already merged, no merge into merged ticket possible' if target_ticket.state.state_type.name == 'merged' + + # check different ticket ids + raise Exceptions::UnprocessableEntity, 'Can\'t merge ticket with it self!' if id == target_ticket.id # update articles Transaction.execute do diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index 9aa16440406c..7967a201fcc8 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -19,7 +19,18 @@ ticket_id: source_ticket.id, user_id: 1, ) - }.to raise_error('invalid state for target ticket') + }.to raise_error('ticket already merged, no merge into merged ticket possible') + end + + it 'prevents merging ticket in it self' do + source_ticket = create(:ticket) + + expect { + result = source_ticket.merge_to( + ticket_id: source_ticket.id, + user_id: 1, + ) + }.to raise_error('Can\'t merge ticket with it self!') end end