From 7b1a379a2dacae3c4bbe218e9992c963d2708e08 Mon Sep 17 00:00:00 2001 From: Rolf Schmidt Date: Fri, 5 May 2017 11:16:26 +0200 Subject: [PATCH] Revert "Squashed commit of the following:" This reverts commit 7d2409bddcbeaa0d400c6deb7012cec97ee53008. --- .rubocop.yml | 14 -- .../_ui_element/ticket_perform_action.coffee | 10 -- app/models/concerns/has_karma_activity_log.rb | 21 --- app/models/concerns/has_links.rb | 24 --- .../concerns/has_online_notifications.rb | 21 --- app/models/karma/activity_log.rb | 24 --- app/models/link.rb | 73 ++++----- app/models/ticket.rb | 38 ++--- spec/factories/online_notification.rb | 12 -- spec/factories/tag.rb | 7 - spec/factories/ticket/article.rb | 14 -- spec/models/ticket_spec.rb | 142 ------------------ 12 files changed, 40 insertions(+), 360 deletions(-) delete mode 100644 app/models/concerns/has_karma_activity_log.rb delete mode 100644 app/models/concerns/has_links.rb delete mode 100644 app/models/concerns/has_online_notifications.rb delete mode 100644 spec/factories/online_notification.rb delete mode 100644 spec/factories/tag.rb delete mode 100644 spec/factories/ticket/article.rb diff --git a/.rubocop.yml b/.rubocop.yml index 1f7ed20d9..347ed9964 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -210,17 +210,3 @@ 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" \ No newline at end of file diff --git a/app/assets/javascripts/app/controllers/_ui_element/ticket_perform_action.coffee b/app/assets/javascripts/app/controllers/_ui_element/ticket_perform_action.coffee index e64d6edf4..fa8a00d79 100644 --- a/app/assets/javascripts/app/controllers/_ui_element/ticket_perform_action.coffee +++ b/app/assets/javascripts/app/controllers/_ui_element/ticket_perform_action.coffee @@ -32,16 +32,6 @@ 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 = {}) -> diff --git a/app/models/concerns/has_karma_activity_log.rb b/app/models/concerns/has_karma_activity_log.rb deleted file mode 100644 index 70768a309..000000000 --- a/app/models/concerns/has_karma_activity_log.rb +++ /dev/null @@ -1,21 +0,0 @@ -# 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 diff --git a/app/models/concerns/has_links.rb b/app/models/concerns/has_links.rb deleted file mode 100644 index 4a8e0dc5f..000000000 --- a/app/models/concerns/has_links.rb +++ /dev/null @@ -1,24 +0,0 @@ -# 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 diff --git a/app/models/concerns/has_online_notifications.rb b/app/models/concerns/has_online_notifications.rb deleted file mode 100644 index cb2ee529f..000000000 --- a/app/models/concerns/has_online_notifications.rb +++ /dev/null @@ -1,21 +0,0 @@ -# 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 diff --git a/app/models/karma/activity_log.rb b/app/models/karma/activity_log.rb index a97a379e8..40ef9ba81 100644 --- a/app/models/karma/activity_log.rb +++ b/app/models/karma/activity_log.rb @@ -5,14 +5,6 @@ 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) @@ -55,22 +47,6 @@ add karma activity log of an object 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) diff --git a/app/models/link.rb b/app/models/link.rb index 01186d59e..c3b714cbc 100644 --- a/app/models/link.rb +++ b/app/models/link.rb @@ -4,8 +4,6 @@ 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', @@ -133,65 +131,52 @@ class Link < ApplicationModel linktype = link_type_get(name: data[:link_type]) data[:link_type_id] = linktype.id end - Link.where( + links = 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 - - Link.where( + links = 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 - -=begin - - Link.remove_all( - link_object: 'Ticket', - link_object_value: 6, - ) - -=end - - 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, ) + + # 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, + ) + } end def self.link_type_get(data) diff --git a/app/models/ticket.rb b/app/models/ticket.rb index d34688823..a95f5467d 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -7,9 +7,6 @@ class Ticket < ApplicationModel include HasHistory include HasTags include HasSearchIndexBackend - include HasOnlineNotifications - include HasKarmaActivityLog - include HasLinks include Ticket::Escalation include Ticket::Subject @@ -26,6 +23,7 @@ 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 @@ -57,7 +55,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, dependent: :destroy + has_many :articles, class_name: 'Ticket::Article', after_add: :cache_update, after_remove: :cache_update has_many :ticket_time_accounting, class_name: 'Ticket::TimeAccounting', dependent: :destroy belongs_to :organization, class_name: 'Organization' belongs_to :state, class_name: 'Ticket::State' @@ -750,19 +748,6 @@ 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) @@ -921,16 +906,6 @@ 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/ @@ -1071,6 +1046,15 @@ 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 diff --git a/spec/factories/online_notification.rb b/spec/factories/online_notification.rb deleted file mode 100644 index 37ef435f9..000000000 --- a/spec/factories/online_notification.rb +++ /dev/null @@ -1,12 +0,0 @@ -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 diff --git a/spec/factories/tag.rb b/spec/factories/tag.rb deleted file mode 100644 index 04e00c15d..000000000 --- a/spec/factories/tag.rb +++ /dev/null @@ -1,7 +0,0 @@ -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 diff --git a/spec/factories/ticket/article.rb b/spec/factories/ticket/article.rb deleted file mode 100644 index be26f9c3f..000000000 --- a/spec/factories/ticket/article.rb +++ /dev/null @@ -1,14 +0,0 @@ -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 diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index 3ef97f67e..0c3cf3006 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -31,147 +31,5 @@ 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