Fixed issue #214 - Delete tickets.

.rubocop.yml

@@ -210,3 +210,17 @@ Performance/Casecmp:
   Description: 'Use `casecmp` rather than `downcase ==`.'
   Reference: 'https://github.com/JuanitoFatas/fast-ruby#stringcasecmp-vs-stringdowncase---code'
   Enabled: false
+
+# RSpec tests
+Style/NumericPredicate:
+  Description: >-
+                 Checks for the use of predicate- or comparison methods for
+                 numeric comparisons.
+  StyleGuide: '#predicate-methods'
+  # This will change to a new method call which isn't guaranteed to be on the
+  # object. Switching these methods has to be done with knowledge of the types
+  # of the variables which rubocop doesn't have.
+  AutoCorrect: false
+  Enabled: true
+  Exclude:
+    - "**/*_spec.rb"

app/assets/javascripts/app/controllers/_ui_element/ticket_perform_action.coffee

@@ -32,6 +32,16 @@ class App.UiElement.ticket_perform_action
                 config.operator = ['add', 'remove']
               elements["#{groupKey}.#{config.name}"] = config
 
+    # add ticket deletion action
+    elements['ticket.action'] =
+      name: 'action'
+      display: 'Action'
+      tag: 'select'
+      null: false
+      translate: true
+      options:
+        delete: 'delete'
+
     [defaults, groups, elements]
 
   @render: (attribute, params = {}) ->

app/models/concerns/has_karma_activity_log.rb

@@ -0,0 +1,21 @@
+# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
+module HasKarmaActivityLog
+  extend ActiveSupport::Concern
+
+  included do
+    before_destroy :karma_activity_log_destroy
+  end
+
+=begin
+
+delete object online notification list, will be executed automatically
+
+  model = Model.find(123)
+  model.karma_activity_log_destroy
+
+=end
+
+  def karma_activity_log_destroy
+    Karma::ActivityLog.remove(self.class.to_s, id)
+  end
+end

app/models/concerns/has_links.rb

@@ -0,0 +1,24 @@
+# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
+module HasLinks
+  extend ActiveSupport::Concern
+
+  included do
+    before_destroy :links_destroy
+  end
+
+=begin
+
+delete object link list, will be executed automatically
+
+  model = Model.find(123)
+  model.links_destroy
+
+=end
+
+  def links_destroy
+    Link.remove_all(
+      link_object: self.class.to_s,
+      link_object_value: id,
+    )
+  end
+end

app/models/concerns/has_online_notifications.rb

@@ -0,0 +1,21 @@
+# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
+module HasOnlineNotifications
+  extend ActiveSupport::Concern
+
+  included do
+    before_destroy :online_notification_destroy
+  end
+
+=begin
+
+delete object online notification list, will be executed automatically
+
+  model = Model.find(123)
+  model.online_notification_destroy
+
+=end
+
+  def online_notification_destroy
+    OnlineNotification.remove(self.class.to_s, id)
+  end
+end

app/models/karma/activity_log.rb

@@ -5,6 +5,14 @@ class Karma::ActivityLog < ApplicationModel
 
   self.table_name = 'karma_activity_logs'
 
+=begin
+
+add karma activity log of an object
+
+  Karma::ActivityLog.add('ticket create', User.find(1), 'Ticket', 123)
+
+=end
+
   def self.add(action, user, object, o_id, force = false)
     activity = Karma::Activity.lookup(name: action)
 
@@ -47,6 +55,22 @@ class Karma::ActivityLog < ApplicationModel
     true
   end
 
+=begin
+
+remove whole karma activity log of an object
+
+  Karma::ActivityLog.remove('Ticket', 123)
+
+=end
+
+  def self.remove(object_name, o_id)
+    object_id = ObjectLookup.by_name(object_name)
+    Karma::ActivityLog.where(
+      object_lookup_id: object_id,
+      o_id: o_id,
+    ).destroy_all
+  end
+
   def self.latest(user, limit = 12)
     result = []
     logs = Karma::ActivityLog.where(user_id: user.id).order(id: :desc).limit(limit)

app/models/link.rb

@@ -4,6 +4,8 @@ class Link < ApplicationModel
   belongs_to :link_type,    class_name: 'Link::Type'
   belongs_to :link_object,  class_name: 'Link::Object'
 
+  after_destroy :touch_link_references
+
   @map = {
     'normal' => 'normal',
     'parent' => 'child',
@@ -131,52 +133,65 @@ class Link < ApplicationModel
       linktype = link_type_get(name: data[:link_type])
       data[:link_type_id] = linktype.id
     end
-    links = Link.where(
+    Link.where(
       link_type_id: data[:link_type_id],
       link_object_source_id: data[:link_object_source_id],
       link_object_source_value: data[:link_object_source_value],
       link_object_target_id: data[:link_object_target_id],
       link_object_target_value: data[:link_object_target_value]
-    )
+    ).destroy_all
-
-    # touch references
-    links.each { |link|
-      link.destroy
-      touch_reference_by_params(
-        object: Link::Object.lookup(id: link.link_object_source_id).name,
-        o_id: link.link_object_source_value,
-      )
-      touch_reference_by_params(
-        object: Link::Object.lookup(id: link.link_object_target_id).name,
-        o_id: link.link_object_target_value,
-      )
-    }
 
     # from the other site
     if data.key?(:link_type)
       linktype = link_type_get(name: @map[ data[:link_type] ])
       data[:link_type_id] = linktype.id
     end
-    links = Link.where(
+
+    Link.where(
       link_type_id: data[:link_type_id],
       link_object_target_id: data[:link_object_source_id],
       link_object_target_value: data[:link_object_source_value],
       link_object_source_id: data[:link_object_target_id],
       link_object_source_value: data[:link_object_target_value]
-    )
+    ).destroy_all
+  end
 
-    # touch references
+=begin
-    links.each { |link|
+
-      link.destroy
+   Link.remove_all(
-      touch_reference_by_params(
+    link_object: 'Ticket',
-        object: Link::Object.lookup(id: link.link_object_source_id).name,
+    link_object_value: 6,
-        o_id: link.link_object_source_value,
+  )
-      )
+
-      touch_reference_by_params(
+=end
-        object: Link::Object.lookup(id: link.link_object_target_id).name,
+
-        o_id: link.link_object_target_value,
+  def self.remove_all(data)
-      )
+    if data.key?(:link_object)
-    }
+      linkobject = link_object_get(name: data[:link_object])
+      data[:link_object_id] = linkobject.id
+    end
+
+    Link.where(
+      link_object_target_id: data[:link_object_id],
+      link_object_target_value: data[:link_object_value],
+    ).destroy_all
+    Link.where(
+      link_object_source_id: data[:link_object_id],
+      link_object_source_value: data[:link_object_value],
+    ).destroy_all
+
+    true
+  end
+
+  def touch_link_references
+    Link.touch_reference_by_params(
+      object: Link::Object.lookup(id: link_object_source_id).name,
+      o_id: link_object_source_value,
+    )
+    Link.touch_reference_by_params(
+      object: Link::Object.lookup(id: link_object_target_id).name,
+      o_id: link_object_target_value,
+    )
   end
 
   def self.link_type_get(data)

app/models/ticket.rb

@@ -7,6 +7,9 @@ class Ticket < ApplicationModel
   include HasHistory
   include HasTags
   include HasSearchIndexBackend
+  include HasOnlineNotifications
+  include HasKarmaActivityLog
+  include HasLinks
 
   include Ticket::Escalation
   include Ticket::Subject
@@ -23,7 +26,6 @@ class Ticket < ApplicationModel
   after_create   :check_escalation_update
   before_update  :check_defaults, :check_title, :reset_pending_time
   after_update   :check_escalation_update
-  before_destroy :destroy_dependencies
 
   validates :group_id, presence: true
 
@@ -55,7 +57,7 @@ class Ticket < ApplicationModel
                              :preferences
 
   belongs_to    :group,                  class_name: 'Group'
-  has_many      :articles,               class_name: 'Ticket::Article', after_add: :cache_update, after_remove: :cache_update
+  has_many      :articles,               class_name: 'Ticket::Article', after_add: :cache_update, after_remove: :cache_update, dependent: :destroy
   has_many      :ticket_time_accounting, class_name: 'Ticket::TimeAccounting', dependent: :destroy
   belongs_to    :organization,           class_name: 'Organization'
   belongs_to    :state,                  class_name: 'Ticket::State'
@@ -748,6 +750,19 @@ perform changes on ticket
 
   def perform_changes(perform, perform_origin, item = nil, current_user_id = nil)
     logger.debug "Perform #{perform_origin} #{perform.inspect} on Ticket.find(#{id})"
+
+    # if the configuration contains the deletion of the ticket then
+    # we skip all other ticket changes because they does not matter
+    if perform['ticket.action'].present? && perform['ticket.action']['value'] == 'delete'
+      perform.each do |key, _value|
+        (object_name, attribute) = key.split('.', 2)
+        next if object_name != 'ticket'
+        next if attribute == 'action'
+
+        perform.delete(key)
+      end
+    end
+
     changed = false
     perform.each do |key, value|
       (object_name, attribute) = key.split('.', 2)
@@ -906,6 +921,16 @@ perform changes on ticket
         next
       end
 
+      # delete ticket
+      if key == 'ticket.action'
+        next if value['value'].blank?
+        next if value['value'] != 'delete'
+
+        destroy
+
+        next
+      end
+
       # lookup pre_condition
       if value['pre_condition']
         if value['pre_condition'] =~ /^not_set/
@@ -1046,15 +1071,6 @@ result
     true
   end
 
-  def destroy_dependencies
-
-    # delete articles
-    articles.destroy_all
-
-    # destroy online notifications
-    OnlineNotification.remove(self.class.to_s, id)
-  end
-
   def set_default_state
     return if state_id
 

spec/factories/online_notification.rb

@@ -0,0 +1,12 @@
+FactoryGirl.define do
+  factory :online_notification do
+    object_lookup_id { ObjectLookup.by_name('Ticket') }
+    type_lookup_id { TypeLookup.by_name('Assigned to you') }
+    seen false
+    user_id 1
+    created_by_id 1
+    updated_by_id 1
+    created_at Time.zone.now
+    updated_at Time.zone.now
+  end
+end

spec/factories/tag.rb

@@ -0,0 +1,7 @@
+FactoryGirl.define do
+  factory :tag do
+    tag_object_id { Tag::Object.lookup_by_name_and_create('Ticket').id }
+    tag_item_id { Tag::Item.lookup_by_name_and_create('blub').id }
+    created_by_id 1
+  end
+end

spec/factories/ticket/article.rb

@@ -0,0 +1,14 @@
+FactoryGirl.define do
+  factory :ticket_article, class: Ticket::Article do
+    from 'factory-customer-1@example.com'
+    to 'factory-customer-1@example.com'
+    subject 'factory article'
+    message_id 'factory@id_com_1'
+    body 'some message 123'
+    internal false
+    sender { Ticket::Article::Sender.find_by(name: 'Customer') }
+    type { Ticket::Article::Type.find_by(name: 'email') }
+    updated_by_id 1
+    created_by_id 1
+  end
+end

spec/models/ticket_spec.rb

@@ -31,5 +31,147 @@ RSpec.describe Ticket do
 
       expect(check_ticket_ids).to match_array(expected_ticket_ids)
     end
+
   end
+
+  describe '.destroy' do
+
+    it 'deletes all related objects before destroy' do
+      ApplicationHandleInfo.current = 'application_server'
+
+      source_ticket = create(:ticket)
+
+      # create some links
+      important_ticket1 = create(:ticket)
+      important_ticket2 = create(:ticket)
+      important_ticket3 = create(:ticket)
+
+      # create some articles
+      create(:ticket_article, ticket_id: source_ticket.id)
+      create(:ticket_article, ticket_id: source_ticket.id)
+      create(:ticket_article, ticket_id: source_ticket.id)
+
+      create(:link, link_object_source_value: source_ticket.id, link_object_target_value: important_ticket1.id)
+      create(:link, link_object_source_value: important_ticket2.id, link_object_target_value: source_ticket.id)
+      create(:link, link_object_source_value: source_ticket.id, link_object_target_value: important_ticket3.id)
+
+      create(:online_notification, o_id: source_ticket.id)
+      create(:tag, o_id: source_ticket.id)
+
+      Observer::Transaction.commit
+      Scheduler.worker(true)
+
+      # get before destroy
+      activities = ActivityStream.where(
+        activity_stream_object_id: ObjectLookup.by_name('Ticket'),
+        o_id: source_ticket.id,
+      )
+      links = Link.list(
+        link_object: 'Ticket',
+        link_object_value: source_ticket.id
+      )
+      articles = Ticket::Article.where(ticket_id: source_ticket.id)
+      history = History.list('Ticket', source_ticket.id, nil, true)
+      karma_log = Karma::ActivityLog.where(
+        object_lookup_id: ObjectLookup.by_name('Ticket'),
+        o_id: source_ticket.id,
+      )
+      online_notifications = OnlineNotification.where(
+        object_lookup_id: ObjectLookup.by_name('Ticket'),
+        o_id: source_ticket.id,
+      )
+      recent_views = OnlineNotification.where(
+        object_lookup_id: ObjectLookup.by_name('Ticket'),
+        o_id: source_ticket.id,
+      )
+      tags = Tag.tag_list(
+        object: 'Ticket',
+        o_id: source_ticket.id,
+      )
+
+      # check before destroy
+      expect(activities.count).to be >= 0
+      expect(links.count).to be >= 0
+      expect(articles.count).to be >= 0
+      expect(history[:list].count).to be >= 0
+      expect(karma_log.count).to be >= 0
+      expect(online_notifications.count).to be >= 0
+      expect(recent_views.count).to be >= 0
+      expect(tags.count).to be >= 0
+
+      # destroy ticket
+      source_ticket.destroy
+
+      # get after destroy
+      activities = ActivityStream.where(
+        activity_stream_object_id: ObjectLookup.by_name('Ticket'),
+        o_id: source_ticket.id,
+      )
+      links = Link.list(
+        link_object: 'Ticket',
+        link_object_value: source_ticket.id
+      )
+      articles = Ticket::Article.where(ticket_id: source_ticket.id)
+      history = History.list('Ticket', source_ticket.id, nil, true)
+      karma_log = Karma::ActivityLog.where(
+        object_lookup_id: ObjectLookup.by_name('Ticket'),
+        o_id: source_ticket.id,
+      )
+      online_notifications = OnlineNotification.where(
+        object_lookup_id: ObjectLookup.by_name('Ticket'),
+        o_id: source_ticket.id,
+      )
+      recent_views = OnlineNotification.where(
+        object_lookup_id: ObjectLookup.by_name('Ticket'),
+        o_id: source_ticket.id,
+      )
+      tags = Tag.tag_list(
+        object: 'Ticket',
+        o_id: source_ticket.id,
+      )
+
+      # check after destroy
+      expect(activities.count).to be == 0
+      expect(links.count).to be == 0
+      expect(articles.count).to be == 0
+      expect(history[:list].count).to be == 0
+      expect(karma_log.count).to be == 0
+      expect(online_notifications.count).to be == 0
+      expect(recent_views.count).to be == 0
+      expect(tags.count).to be == 0
+
+    end
+
+  end
+
+  describe '.perform_changes' do
+
+    it 'performes a ticket state change on a ticket' do
+      source_ticket = create(:ticket)
+
+      changes = {
+        'ticket.state_id' => { 'value' => Ticket::State.lookup(name: 'closed').id.to_s },
+      }
+
+      source_ticket.perform_changes(changes, 'trigger', source_ticket, User.find(1))
+      source_ticket.reload
+
+      expect(source_ticket.state.name).to eq('closed')
+    end
+
+    it 'performes a ticket deletion on a ticket' do
+      source_ticket = create(:ticket)
+
+      changes = {
+        'ticket.state_id' => { 'value' => Ticket::State.lookup(name: 'closed').id.to_s },
+        'ticket.action' => { 'value' => 'delete' },
+      }
+
+      source_ticket.perform_changes(changes, 'trigger', source_ticket, User.find(1))
+      ticket_with_source_ids = Ticket.where(id: source_ticket.id)
+      expect(ticket_with_source_ids).to match_array([])
+    end
+
+  end
+
 end