Small improvement for issue #1216 (moved same ticket check into model…

…, show error message in ui).
zammad · Jul 25, 2017 · d8be2e0 · d8be2e0
1 parent d636996
commit d8be2e0
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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