Refactoring: Migrate Rails Observers to Concerns.

.rubocop/todo.yml

@@ -138,6 +138,18 @@ Metrics/AbcSize:
     - 'app/models/concerns/has_rich_text.rb'
     - 'app/models/concerns/has_search_index_backend.rb'
     - 'app/models/concerns/has_search_sortable.rb'
+    - 'app/models/concerns/ticket/article/adds_metadata_email.rb'
+    - 'app/models/concerns/ticket/article/adds_metadata_general.rb'
+    - 'app/models/concerns/ticket/article/adds_metadata_origin_by_id.rb'
+    - 'app/models/concerns/ticket/article/enqueue_communicate_email_job.rb'
+    - 'app/models/concerns/ticket/article/enqueue_communicate_facebook_job.rb'
+    - 'app/models/concerns/ticket/article/enqueue_communicate_sms_job.rb'
+    - 'app/models/concerns/ticket/article/enqueue_communicate_telegram_job.rb'
+    - 'app/models/concerns/ticket/article/enqueue_communicate_twitter_job.rb'
+    - 'app/models/concerns/ticket/article/resets_ticket_state.rb'
+    - 'app/models/concerns/ticket/sets_last_owner_update_time.rb'
+    - 'app/models/concerns/ticket/touches_associations.rb'
+    - 'app/models/concerns/user/performs_geo_lookup.rb'
     - 'app/models/cti/caller_id.rb'
     - 'app/models/cti/driver/base.rb'
     - 'app/models/cti/driver/placetel.rb'
@@ -156,19 +168,7 @@ Metrics/AbcSize:
     - 'app/models/link.rb'
     - 'app/models/object_manager/attribute.rb'
     - 'app/models/observer/chat/leave/background_job.rb'
-    - 'app/models/observer/ticket/article/communicate_email.rb'
-    - 'app/models/observer/ticket/article/communicate_facebook.rb'
-    - 'app/models/observer/ticket/article/communicate_sms.rb'
-    - 'app/models/observer/ticket/article/communicate_telegram.rb'
-    - 'app/models/observer/ticket/article/communicate_twitter.rb'
-    - 'app/models/observer/ticket/article/fillup_from_email.rb'
-    - 'app/models/observer/ticket/article/fillup_from_general.rb'
-    - 'app/models/observer/ticket/article/fillup_from_origin_by_id.rb'
-    - 'app/models/observer/ticket/last_owner_update.rb'
-    - 'app/models/observer/ticket/ref_object_touch.rb'
-    - 'app/models/observer/ticket/reset_new_state.rb'
     - 'app/models/observer/transaction.rb'
-    - 'app/models/observer/user/geo.rb'
     - 'app/models/online_notification.rb'
     - 'app/models/online_notification/assets.rb'
     - 'app/models/organization/assets.rb'
@@ -533,6 +533,17 @@ Metrics/CyclomaticComplexity:
     - 'app/models/concerns/has_rich_text.rb'
     - 'app/models/concerns/has_search_index_backend.rb'
     - 'app/models/concerns/has_search_sortable.rb'
+    - 'app/models/concerns/ticket/article/adds_metadata_email.rb'
+    - 'app/models/concerns/ticket/article/adds_metadata_general.rb'
+    - 'app/models/concerns/ticket/article/adds_metadata_origin_by_id.rb'
+    - 'app/models/concerns/ticket/article/enqueue_communicate_email_job.rb'
+    - 'app/models/concerns/ticket/article/enqueue_communicate_facebook_job.rb'
+    - 'app/models/concerns/ticket/article/enqueue_communicate_sms_job.rb'
+    - 'app/models/concerns/ticket/article/enqueue_communicate_twitter_job.rb'
+    - 'app/models/concerns/ticket/article/resets_ticket_state.rb'
+    - 'app/models/concerns/ticket/sets_last_owner_update_time.rb'
+    - 'app/models/concerns/ticket/touches_associations.rb'
+    - 'app/models/concerns/user/performs_geo_lookup.rb'
     - 'app/models/cti/caller_id.rb'
     - 'app/models/cti/driver/base.rb'
     - 'app/models/cti/driver/placetel.rb'
@@ -545,18 +556,7 @@ Metrics/CyclomaticComplexity:
     - 'app/models/karma/activity_log.rb'
     - 'app/models/knowledge_base.rb'
     - 'app/models/object_manager/attribute.rb'
-    - 'app/models/observer/ticket/article/communicate_email.rb'
-    - 'app/models/observer/ticket/article/communicate_facebook.rb'
-    - 'app/models/observer/ticket/article/communicate_sms.rb'
-    - 'app/models/observer/ticket/article/communicate_twitter.rb'
-    - 'app/models/observer/ticket/article/fillup_from_email.rb'
-    - 'app/models/observer/ticket/article/fillup_from_general.rb'
-    - 'app/models/observer/ticket/article/fillup_from_origin_by_id.rb'
-    - 'app/models/observer/ticket/last_owner_update.rb'
-    - 'app/models/observer/ticket/ref_object_touch.rb'
-    - 'app/models/observer/ticket/reset_new_state.rb'
     - 'app/models/observer/transaction.rb'
-    - 'app/models/observer/user/geo.rb'
     - 'app/models/online_notification/assets.rb'
     - 'app/models/organization/assets.rb'
     - 'app/models/organization/search.rb'
@@ -763,6 +763,16 @@ Metrics/PerceivedComplexity:
     - 'app/models/concerns/has_rich_text.rb'
     - 'app/models/concerns/has_search_index_backend.rb'
     - 'app/models/concerns/has_search_sortable.rb'
+    - 'app/models/concerns/ticket/article/adds_metadata_email.rb'
+    - 'app/models/concerns/ticket/article/adds_metadata_general.rb'
+    - 'app/models/concerns/ticket/article/adds_metadata_origin_by_id.rb'
+    - 'app/models/concerns/ticket/article/enqueue_communicate_email_job.rb'
+    - 'app/models/concerns/ticket/article/enqueue_communicate_facebook_job.rb'
+    - 'app/models/concerns/ticket/article/enqueue_communicate_twitter_job.rb'
+    - 'app/models/concerns/ticket/article/resets_ticket_state.rb'
+    - 'app/models/concerns/ticket/sets_last_owner_update_time.rb'
+    - 'app/models/concerns/ticket/touches_associations.rb'
+    - 'app/models/concerns/user/performs_geo_lookup.rb'
     - 'app/models/cti/caller_id.rb'
     - 'app/models/cti/driver/base.rb'
     - 'app/models/cti/driver/placetel.rb'
@@ -775,16 +785,6 @@ Metrics/PerceivedComplexity:
     - 'app/models/karma/activity_log.rb'
     - 'app/models/knowledge_base.rb'
     - 'app/models/object_manager/attribute.rb'
-    - 'app/models/observer/ticket/article/communicate_email.rb'
-    - 'app/models/observer/ticket/article/communicate_facebook.rb'
-    - 'app/models/observer/ticket/article/communicate_sms.rb'
-    - 'app/models/observer/ticket/article/communicate_twitter.rb'
-    - 'app/models/observer/ticket/article/fillup_from_email.rb'
-    - 'app/models/observer/ticket/article/fillup_from_general.rb'
-    - 'app/models/observer/ticket/article/fillup_from_origin_by_id.rb'
-    - 'app/models/observer/ticket/last_owner_update.rb'
-    - 'app/models/observer/ticket/ref_object_touch.rb'
-    - 'app/models/observer/ticket/reset_new_state.rb'
     - 'app/models/observer/transaction.rb'
     - 'app/models/online_notification/assets.rb'
     - 'app/models/organization/assets.rb'

app/models/concerns/tag/writes_to_ticket_history.rb

@@ -0,0 +1,41 @@
+# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
+
+# Records added/removed tags also in the ticket history.
+module Tag::WritesToTicketHistory
+  extend ActiveSupport::Concern
+
+  included do
+    after_create  :write_tag_added_to_ticket_history
+    after_destroy :write_tag_removed_to_ticket_history
+  end
+
+  private
+
+  def write_tag_added_to_ticket_history
+
+    return true if tag_object.name != 'Ticket'
+
+    History.add(
+      o_id:              o_id,
+      history_type:      'added',
+      history_object:    'Ticket',
+      history_attribute: 'tag',
+      value_to:          tag_item.name,
+      created_by_id:     created_by_id,
+    )
+  end
+
+  def write_tag_removed_to_ticket_history
+
+    return true if tag_object.name != 'Ticket'
+
+    History.add(
+      o_id:              o_id,
+      history_type:      'removed',
+      history_object:    'Ticket',
+      history_attribute: 'tag',
+      value_to:          tag_item.name,
+      created_by_id:     created_by_id,
+    )
+  end
+end

app/models/observer/ticket/article/fillup_from_email.rb → app/models/concerns/ticket/article/adds_metadata_email.rb

@@ -1,9 +1,16 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 
-class Observer::Ticket::Article::FillupFromEmail < ActiveRecord::Observer
-  observe 'ticket::_article'
+# Adds certain (missing) meta data when creating email articles.
+module Ticket::Article::AddsMetadataEmail
+  extend ActiveSupport::Concern
 
-  def before_create(record)
+  included do
+    before_create :ticket_article_add_metadata_email
+  end
+
+  private
+
+  def ticket_article_add_metadata_email
 
     # return if we run import mode
     return true if Setting.get('import_mode')
@@ -13,36 +20,36 @@ class Observer::Ticket::Article::FillupFromEmail < ActiveRecord::Observer
     return if ApplicationHandleInfo.postmaster?
 
     # if sender is customer, do not change anything
-    return true if !record.sender_id
+    return true if !sender_id
 
-    sender = Ticket::Article::Sender.lookup(id: record.sender_id)
+    sender = Ticket::Article::Sender.lookup(id: sender_id)
     return true if sender.nil?
     return true if sender.name == 'Customer'
 
     # set email attributes
-    return true if !record.type_id
+    return true if !type_id
 
-    type = Ticket::Article::Type.lookup(id: record.type_id)
+    type = Ticket::Article::Type.lookup(id: type_id)
     return true if type.nil?
     return true if type.name != 'email'
 
     # set subject if empty
-    ticket = record.ticket
-    if !record.subject || record.subject == ''
-      record.subject = ticket.title
+    ticket = self.ticket
+    if !subject || subject == ''
+      self.subject = ticket.title
     end
 
     # clean subject
-    record.subject = ticket.subject_clean(record.subject)
+    self.subject = ticket.subject_clean(subject)
 
     # generate message id, force it in production, in test allow to set it for testing reasons
-    if !record.message_id || Rails.env.production?
+    if !message_id || Rails.env.production?
       fqdn = Setting.get('fqdn')
-      record.message_id = "<#{DateTime.current.to_s(:number)}.#{record.ticket_id}.#{rand(999_999_999_999)}@#{fqdn}>"
+      self.message_id = "<#{DateTime.current.to_s(:number)}.#{ticket_id}.#{rand(999_999_999_999)}@#{fqdn}>"
     end
 
     # generate message_id_md5
-    record.check_message_id_md5
+    check_message_id_md5
 
     # set sender
     email_address = ticket.group.email_address
@@ -51,20 +58,20 @@ class Observer::Ticket::Article::FillupFromEmail < ActiveRecord::Observer
     end
 
     # remember email address for background job
-    record.preferences['email_address_id'] = email_address.id
+    preferences['email_address_id'] = email_address.id
 
     # fill from
-    if record.created_by_id != 1 && Setting.get('ticket_define_email_from') == 'AgentNameSystemAddressName'
+    if created_by_id != 1 && Setting.get('ticket_define_email_from') == 'AgentNameSystemAddressName'
       separator   = Setting.get('ticket_define_email_from_separator')
-      sender      = User.find(record.created_by_id)
+      sender      = User.find(created_by_id)
       realname    = "#{sender.firstname} #{sender.lastname} #{separator} #{email_address.realname}"
-      record.from = Channel::EmailBuild.recipient_line(realname, email_address.email)
+      self.from = Channel::EmailBuild.recipient_line(realname, email_address.email)
     elsif Setting.get('ticket_define_email_from') == 'AgentName'
-      sender      = User.find(record.created_by_id)
+      sender      = User.find(created_by_id)
       realname    = "#{sender.firstname} #{sender.lastname}"
-      record.from = Channel::EmailBuild.recipient_line(realname, email_address.email)
+      self.from = Channel::EmailBuild.recipient_line(realname, email_address.email)
     else
-      record.from = Channel::EmailBuild.recipient_line(email_address.realname, email_address.email)
+      self.from = Channel::EmailBuild.recipient_line(email_address.realname, email_address.email)
     end
     true
   end

app/models/observer/ticket/article/fillup_from_general.rb → app/models/concerns/ticket/article/adds_metadata_general.rb

@@ -1,9 +1,17 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 
-class Observer::Ticket::Article::FillupFromGeneral < ActiveRecord::Observer
-  observe 'ticket::_article'
+# Adds certain (missing) meta data when creating articles.
+# This module depends on AddsMetadataOriginById to run before it.
+module Ticket::Article::AddsMetadataGeneral
+  extend ActiveSupport::Concern
 
-  def before_create(record)
+  included do
+    before_create :ticket_article_add_metadata_general
+  end
+
+  private
+
+  def ticket_article_add_metadata_general
 
     # return if we run import mode
     return true if Setting.get('import_mode')
@@ -13,9 +21,9 @@ class Observer::Ticket::Article::FillupFromGeneral < ActiveRecord::Observer
     return true if ApplicationHandleInfo.postmaster?
 
     # set from on all article types excluding email|twitter status|twitter direct-message|facebook feed post|facebook feed comment
-    return true if record.type_id.blank?
+    return true if type_id.blank?
 
-    type = Ticket::Article::Type.lookup(id: record.type_id)
+    type = Ticket::Article::Type.lookup(id: type_id)
 
     # from will be set by channel backend
     return true if type.nil?
@@ -26,31 +34,31 @@ class Observer::Ticket::Article::FillupFromGeneral < ActiveRecord::Observer
     return true if type.name == 'facebook feed comment'
     return true if type.name == 'sms'
 
-    user_id = record.created_by_id
+    user_id = created_by_id
 
-    if record.origin_by_id.present?
+    if origin_by_id.present?
 
       # in case the customer is using origin_by_id, force it to current session user
       # and set sender to Customer
-      if !record.created_by.permissions?('ticket.agent')
-        record.origin_by_id = record.created_by_id
-        record.sender_id = Ticket::Article::Sender.lookup(name: 'Customer').id
+      if !created_by.permissions?('ticket.agent')
+        self.origin_by_id = created_by_id
+        self.sender_id = Ticket::Article::Sender.lookup(name: 'Customer').id
       end
 
       # in case origin_by is different than created_by, set sender to Customer
       # Customer in context of this conversation, not as a permission
-      if record.origin_by != record.created_by_id
-        record.sender_id = Ticket::Article::Sender.lookup(name: 'Customer').id
-        user_id = record.origin_by_id
+      if origin_by != created_by_id
+        self.sender_id = Ticket::Article::Sender.lookup(name: 'Customer').id
+        user_id = origin_by_id
       end
     end
     return true if user_id.blank?
 
     user = User.find(user_id)
     if type.name == 'web' || type.name == 'phone'
-      record.from = "#{user.firstname} #{user.lastname} <#{user.email}>"
+      self.from = "#{user.firstname} #{user.lastname} <#{user.email}>"
       return
     end
-    record.from = "#{user.firstname} #{user.lastname}"
+    self.from = "#{user.firstname} #{user.lastname}"
   end
 end

app/models/concerns/ticket/article/adds_metadata_origin_by_id.rb

@@ -0,0 +1,34 @@
+# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
+
+# Adds origin_by_id field (if missing) when creating articles.
+module Ticket::Article::AddsMetadataOriginById
+  extend ActiveSupport::Concern
+
+  included do
+    before_create :ticket_article_add_metadata_origin_by_id
+  end
+
+  private
+
+  def ticket_article_add_metadata_origin_by_id
+
+    # return if we run import mode
+    return true if Setting.get('import_mode')
+
+    # only do fill origin_by_id if article got created via application_server (e. g. not
+    # if article and sender type is set via *.postmaster)
+    return true if ApplicationHandleInfo.postmaster?
+
+    # check if origin_by_id exists
+    return true if origin_by_id.present?
+    return true if ticket.blank?
+    return true if ticket.customer_id.blank?
+    return true if sender_id.blank?
+    return true if sender.name != 'Customer'
+
+    type_name = type.name
+    return true if type_name != 'phone' && type_name != 'note' && type_name != 'web'
+
+    self.origin_by_id = ticket.customer_id
+  end
+end

app/models/observer/ticket/article/communicate_email.rb → app/models/concerns/ticket/article/enqueue_communicate_email_job.rb

@@ -1,9 +1,16 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 
-class Observer::Ticket::Article::CommunicateEmail < ActiveRecord::Observer
-  observe 'ticket::_article'
+# Schedules a backgrond communication job for new email articles.
+module Ticket::Article::EnqueueCommunicateEmailJob
+  extend ActiveSupport::Concern
 
-  def after_create(record)
+  included do
+    after_create :ticket_article_enqueue_communicate_email_job
+  end
+
+  private
+
+  def ticket_article_enqueue_communicate_email_job
 
     # return if we run import mode
     return true if Setting.get('import_mode')
@@ -13,20 +20,20 @@ class Observer::Ticket::Article::CommunicateEmail < ActiveRecord::Observer
     return true if ApplicationHandleInfo.postmaster?
 
     # if sender is customer, do not communicate
-    return true if !record.sender_id
+    return true if !sender_id
 
-    sender = Ticket::Article::Sender.lookup(id: record.sender_id)
+    sender = Ticket::Article::Sender.lookup(id: sender_id)
     return true if sender.nil?
     return true if sender.name == 'Customer'
 
     # only apply on emails
-    return true if !record.type_id
+    return true if !type_id
 
-    type = Ticket::Article::Type.lookup(id: record.type_id)
+    type = Ticket::Article::Type.lookup(id: type_id)
     return true if type.nil?
     return true if type.name != 'email'
 
     # send background job
-    TicketArticleCommunicateEmailJob.perform_later(record.id)
+    TicketArticleCommunicateEmailJob.perform_later(id)
   end
 end

app/models/observer/ticket/article/communicate_facebook.rb → app/models/concerns/ticket/article/enqueue_communicate_facebook_job.rb

@@ -1,8 +1,16 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
-class Observer::Ticket::Article::CommunicateFacebook < ActiveRecord::Observer
-  observe 'ticket::_article'
 
-  def after_create(record)
+# Schedules a backgrond communication job for new facebook articles.
+module Ticket::Article::EnqueueCommunicateFacebookJob
+  extend ActiveSupport::Concern
+
+  included do
+    after_create :ticket_article_enqueue_communicate_facebook_job
+  end
+
+  private
+
+  def ticket_article_enqueue_communicate_facebook_job
 
     # return if we run import mode
     return true if Setting.get('import_mode')
@@ -12,20 +20,20 @@ class Observer::Ticket::Article::CommunicateFacebook < ActiveRecord::Observer
     return true if ApplicationHandleInfo.postmaster?
 
     # if sender is customer, do not communicate
-    return true if !record.sender_id
+    return true if !sender_id
 
-    sender = Ticket::Article::Sender.lookup(id: record.sender_id)
+    sender = Ticket::Article::Sender.lookup(id: sender_id)
     return true if sender.nil?
     return true if sender.name == 'Customer'
 
     # only apply for facebook
-    return true if !record.type_id
+    return true if !type_id
 
-    type = Ticket::Article::Type.lookup(id: record.type_id)
+    type = Ticket::Article::Type.lookup(id: type_id)
     return true if type.nil?
     return true if !type.name.start_with?('facebook')
 
-    CommunicateFacebookJob.perform_later(record.id)
+    CommunicateFacebookJob.perform_later(id)
   end
 
 end

app/models/concerns/ticket/article/enqueue_communicate_sms_job.rb

@@ -0,0 +1,34 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+# Schedules a backgrond communication job for new SMS articles.
+module Ticket::Article::EnqueueCommunicateSmsJob
+  extend ActiveSupport::Concern
+
+  included do
+    after_create :ticket_article_enqueue_communicate_sms_job
+  end
+
+  private
+
+  def ticket_article_enqueue_communicate_sms_job
+
+    # return if we run import mode
+    return true if Setting.get('import_mode')
+
+    # if sender is customer, do not communicate
+    return true if !sender_id
+
+    sender = Ticket::Article::Sender.lookup(id: sender_id)
+    return true if sender.nil?
+    return true if sender.name == 'Customer'
+
+    # only apply on sms
+    return true if !type_id
+
+    type = Ticket::Article::Type.lookup(id: type_id)
+    return true if type.nil?
+    return true if type.name != 'sms'
+
+    CommunicateSmsJob.perform_later(id)
+  end
+end

app/models/concerns/ticket/article/enqueue_communicate_telegram_job.rb

@@ -0,0 +1,34 @@
+# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
+
+# Schedules a backgrond communication job for new telegram articles.
+module Ticket::Article::EnqueueCommunicateTelegramJob
+  extend ActiveSupport::Concern
+
+  included do
+    after_create :ticket_article_enqueue_communicate_telegram_job
+  end
+
+  private
+
+  def ticket_article_enqueue_communicate_telegram_job
+
+    # return if we run import mode
+    return true if Setting.get('import_mode')
+
+    # if sender is customer, do not communicate
+    return true if !sender_id
+
+    sender = Ticket::Article::Sender.lookup(id: sender_id)
+    return true if sender.nil?
+    return true if sender.name == 'Customer'
+
+    # only apply on telegram messages
+    return true if !type_id
+
+    type = Ticket::Article::Type.lookup(id: type_id)
+    return true if !type.name.match?(/\Atelegram/i)
+
+    CommunicateTelegramJob.perform_later(id)
+  end
+
+end

app/models/observer/ticket/article/communicate_twitter.rb → app/models/concerns/ticket/article/enqueue_communicate_twitter_job.rb

@@ -1,9 +1,16 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 
-class Observer::Ticket::Article::CommunicateTwitter < ActiveRecord::Observer
-  observe 'ticket::_article'
+# Schedules a backgrond communication job for new twitter articles.
+module Ticket::Article::EnqueueCommunicateTwitterJob
+  extend ActiveSupport::Concern
 
-  def after_create(record)
+  included do
+    after_create :ticket_article_enqueue_communicate_twitter_job
+  end
+
+  private
+
+  def ticket_article_enqueue_communicate_twitter_job
 
     # return if we run import mode
     return true if Setting.get('import_mode')
@@ -13,22 +20,22 @@ class Observer::Ticket::Article::CommunicateTwitter < ActiveRecord::Observer
     return true if ApplicationHandleInfo.postmaster?
 
     # if sender is customer, do not communicate
-    return true if !record.sender_id
+    return true if !sender_id
 
-    sender = Ticket::Article::Sender.lookup(id: record.sender_id)
+    sender = Ticket::Article::Sender.lookup(id: sender_id)
     return true if sender.nil?
     return true if sender.name == 'Customer'
 
     # only apply on tweets
-    return true if !record.type_id
+    return true if !type_id
 
-    type = Ticket::Article::Type.lookup(id: record.type_id)
+    type = Ticket::Article::Type.lookup(id: type_id)
     return true if type.nil?
     return true if !type.name.match?(/\Atwitter/i)
 
-    raise Exceptions::UnprocessableEntity, 'twitter to: parameter is missing' if record.to.blank? && type['name'] == 'twitter direct-message'
+    raise Exceptions::UnprocessableEntity, 'twitter to: parameter is missing' if to.blank? && type['name'] == 'twitter direct-message'
 
-    CommunicateTwitterJob.perform_later(record.id)
+    CommunicateTwitterJob.perform_later(id)
   end
 
 end