From e56ed8eecadcff392ca86be85546b0aa4e59ff07 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Fri, 22 Apr 2016 08:55:10 +0200 Subject: [PATCH] Moved signature detection to transaction module. --- .../ticket/article/signature_detection.rb | 33 ---- .../signature_detection/background_job.rb | 9 - app/models/observer/transaction.rb | 161 +++++++++--------- app/models/transaction/notification.rb | 10 +- app/models/transaction/signature_detection.rb | 58 +++++++ app/models/transaction/slack.rb | 10 +- config/application.rb | 1 - ...160422000001_update_signature_detection.rb | 13 ++ db/seeds.rb | 9 + lib/signature_detection.rb | 5 +- test/unit/email_signatur_detection_test.rb | 5 +- test/unit/ticket_notification_test.rb | 20 +-- 12 files changed, 192 insertions(+), 142 deletions(-) delete mode 100644 app/models/observer/ticket/article/signature_detection.rb delete mode 100644 app/models/observer/ticket/article/signature_detection/background_job.rb create mode 100644 app/models/transaction/signature_detection.rb create mode 100644 db/migrate/20160422000001_update_signature_detection.rb diff --git a/app/models/observer/ticket/article/signature_detection.rb b/app/models/observer/ticket/article/signature_detection.rb deleted file mode 100644 index a3b520215..000000000 --- a/app/models/observer/ticket/article/signature_detection.rb +++ /dev/null @@ -1,33 +0,0 @@ -# Copyright (C) 2012-2014 Zammad Foundation, http://zammad-foundation.org/ - -require 'signature_detection' - -class Observer::Ticket::Article::SignatureDetection < ActiveRecord::Observer - observe 'ticket::_article' - - def before_create(record) - - # return if we run import mode - return if Setting.get('import_mode') - - # if sender is not customer, do not change anything - sender = Ticket::Article::Sender.lookup(id: record.sender_id) - return if !sender - return if sender['name'] != 'Customer' - - # set email attributes - type = Ticket::Article::Type.lookup(id: record.type_id) - return if type['name'] != 'email' - - # add queue job to update current signature of user id - Delayed::Job.enqueue(Observer::Ticket::Article::SignatureDetection::BackgroundJob.new(record.created_by_id)) - - # user - user = User.lookup(id: record.created_by_id) - return if !user - return if !user.preferences - return if !user.preferences[:signature_detection] - - record.preferences[:signature_detection] = SignatureDetection.find_signature_line(user.preferences[:signature_detection], record.body) - end -end diff --git a/app/models/observer/ticket/article/signature_detection/background_job.rb b/app/models/observer/ticket/article/signature_detection/background_job.rb deleted file mode 100644 index 83eecbf23..000000000 --- a/app/models/observer/ticket/article/signature_detection/background_job.rb +++ /dev/null @@ -1,9 +0,0 @@ -class Observer::Ticket::Article::SignatureDetection::BackgroundJob - def initialize(id) - @user_id = id - end - - def perform - SignatureDetection.rebuild_user(@user_id) - end -end diff --git a/app/models/observer/transaction.rb b/app/models/observer/transaction.rb index 5644a23c8..8f403d38e 100644 --- a/app/models/observer/transaction.rb +++ b/app/models/observer/transaction.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2014 Zammad Foundation, http://zammad-foundation.org/ class Observer::Transaction < ActiveRecord::Observer - observe :ticket, 'ticket::_article' + observe :ticket, 'ticket::_article', :user, :organization def self.commit(params = {}) @@ -28,10 +28,10 @@ class Observer::Transaction < ActiveRecord::Observer # get uniq objects list_objects = get_uniq_changes(list) - list_objects.each {|_id, item| - - # send background job - Delayed::Job.enqueue(Transaction::BackgroundJob.new(item, params)) + list_objects.each {|_object, objects| + objects.each {|_id, item| + Delayed::Job.enqueue(Transaction::BackgroundJob.new(item, params)) + } } end @@ -40,33 +40,37 @@ class Observer::Transaction < ActiveRecord::Observer result = get_uniq_changes(events) result = { - 1 => { - object: 'Ticket', - type: 'create', - ticket_id: 123, - article_id: 123, - }, - 9 => { - object: 'Ticket', - type: 'update', - ticket_id: 123, - changes: { - attribute1: [before, now], - attribute2: [before, now], - } + 'Ticket' => + 1 => { + object: 'Ticket', + type: 'create', + object_id: 123, + article_id: 123, + }, + 9 => { + object: 'Ticket', + type: 'update', + object_id: 123, + changes: { + attribute1: [before, now], + attribute2: [before, now], + }, + }, }, } result = { - 9 => { - object: 'Ticket', - type: 'update', - ticket_id: 123, - article_id: 123, - changes: { - attribute1: [before, now], - attribute2: [before, now], - } + 'Ticket' => + 9 => { + object: 'Ticket', + type: 'update', + object_id: 123, + article_id: 123, + changes: { + attribute1: [before, now], + attribute2: [before, now], + }, + }, }, } @@ -76,57 +80,59 @@ class Observer::Transaction < ActiveRecord::Observer list_objects = {} events.each { |event| - # get current state of objects - if event[:name] == 'Ticket::Article' + # simulate article create as ticket update + article = nil + if event[:object] == 'Ticket::Article' article = Ticket::Article.lookup(id: event[:id]) - - # next if article is already deleted next if !article + next if event[:type] == 'update' - ticket = article.ticket - if !list_objects[ticket.id] - list_objects[ticket.id] = {} + # set new event infos + ticket = Ticket.lookup(id: article.ticket_id) + event[:object] = 'Ticket' + event[:id] = ticket.id + event[:type] = 'update' + event[:changes] = nil + end + + # get current state of objects + object = Kernel.const_get(event[:object]).lookup(id: event[:id]) + + # next if object is already deleted + next if !object + + if !list_objects[event[:object]] + list_objects[event[:object]] = {} + end + if !list_objects[event[:object]][object.id] + list_objects[event[:object]][object.id] = {} + end + store = list_objects[event[:object]][object.id] + store[:object] = event[:object] + store[:object_id] = object.id + + if !store[:type] || store[:type] == 'update' + store[:type] = event[:type] + end + + # merge changes + if event[:changes] + if !store[:changes] + store[:changes] = event[:changes] + else + event[:changes].each {|key, value| + if !store[:changes][key] + store[:changes][key] = value + else + store[:changes][key][1] = value[1] + end + } end - list_objects[ticket.id][:object] = 'Ticket' - list_objects[ticket.id][:article_id] = article.id - list_objects[ticket.id][:ticket_id] = ticket.id + end - if !list_objects[ticket.id][:type] - list_objects[ticket.id][:type] = 'update' - end - - elsif event[:name] == 'Ticket' - ticket = Ticket.lookup(id: event[:id]) - - # next if ticket is already deleted - next if !ticket - - if !list_objects[ticket.id] - list_objects[ticket.id] = {} - end - list_objects[ticket.id][:object] = 'Ticket' - list_objects[ticket.id][:ticket_id] = ticket.id - - if !list_objects[ticket.id][:type] || list_objects[ticket.id][:type] == 'update' - list_objects[ticket.id][:type] = event[:type] - end - - # merge changes - if event[:changes] - if !list_objects[ticket.id][:changes] - list_objects[ticket.id][:changes] = event[:changes] - else - event[:changes].each {|key, value| - if !list_objects[ticket.id][:changes][key] - list_objects[ticket.id][:changes][key] = value - else - list_objects[ticket.id][:changes][key][1] = value[1] - end - } - end - end - else - raise "unknown object for integration #{event[:name]}" + # remember article id if exists + if article + store[:article_id] = article.id end } list_objects @@ -138,7 +144,7 @@ class Observer::Transaction < ActiveRecord::Observer return if Setting.get('import_mode') e = { - name: record.class.name, + object: record.class.name, type: 'create', data: record, id: record.id, @@ -151,9 +157,6 @@ class Observer::Transaction < ActiveRecord::Observer # return if we run import mode return if Setting.get('import_mode') - # ignore updates on articles / we just want send integrations on ticket updates - return if record.class.name == 'Ticket::Article' - # ignore certain attributes real_changes = {} record.changes.each {|key, value| @@ -173,7 +176,7 @@ class Observer::Transaction < ActiveRecord::Observer return if real_changes.empty? e = { - name: record.class.name, + object: record.class.name, type: 'update', data: record, changes: real_changes, diff --git a/app/models/transaction/notification.rb b/app/models/transaction/notification.rb index b28d06b55..5c9cca837 100644 --- a/app/models/transaction/notification.rb +++ b/app/models/transaction/notification.rb @@ -6,7 +6,7 @@ class Transaction::Notification { object: 'Ticket', type: 'update', - ticket_id: 123, + object_id: 123, via_web: true, changes: { 'attribute1' => [before, now], @@ -21,9 +21,15 @@ class Transaction::Notification end def perform + + # return if we run import mode + return if Setting.get('import_mode') + + return if @item[:object] != 'Ticket' + return if @params[:disable_notification] - ticket = Ticket.find(@item[:ticket_id]) + ticket = Ticket.find(@item[:object_id]) if @item[:article_id] article = Ticket::Article.find(@item[:article_id]) end diff --git a/app/models/transaction/signature_detection.rb b/app/models/transaction/signature_detection.rb new file mode 100644 index 000000000..3c14b74e5 --- /dev/null +++ b/app/models/transaction/signature_detection.rb @@ -0,0 +1,58 @@ +# Copyright (C) 2012-2014 Zammad Foundation, http://zammad-foundation.org/ +require 'signature_detection' + +class Transaction::SignatureDetection + +=begin + { + object: 'Ticket', + type: 'update', + object_id: 123, + via_web: true, + changes: { + 'attribute1' => [before, now], + 'attribute2' => [before, now], + } + }, +=end + + def initialize(item, params = {}) + @item = item + @params = params + end + + def perform + + # return if we run import mode + return if Setting.get('import_mode') + + return if @item[:type] != 'create' + return if @item[:object] != 'Ticket' + + ticket = Ticket.lookup(id: @item[:object_id]) + return if !ticket + article = ticket.articles.first + return if !article + + # if sender is not customer, do not change anything + sender = Ticket::Article::Sender.lookup(id: article.sender_id) + return if !sender + return if sender['name'] != 'Customer' + + # set email attributes + type = Ticket::Article::Type.lookup(id: article.type_id) + return if type['name'] != 'email' + + # add queue job to update current signature of user id + SignatureDetection.rebuild_user(article.created_by_id) + + # user + user = User.lookup(id: article.created_by_id) + return if !user + return if !user.preferences + return if !user.preferences[:signature_detection] + article.preferences[:signature_detection] = SignatureDetection.find_signature_line(user.preferences[:signature_detection], article.body) + article.save + end + +end diff --git a/app/models/transaction/slack.rb b/app/models/transaction/slack.rb index 23138b83a..a4c331b00 100644 --- a/app/models/transaction/slack.rb +++ b/app/models/transaction/slack.rb @@ -6,14 +6,14 @@ class Transaction::Slack backend = Transaction::Slack.new( object: 'Ticket', type: 'create', - ticket_id: 1, + object_id: 1, ) backend.perform { object: 'Ticket', type: 'update', - ticket_id: 123, + object_id: 123, via_web: true, changes: { 'attribute1' => [before, now], @@ -27,6 +27,10 @@ backend.perform end def perform + + # return if we run import mode + return if Setting.get('import_mode') + return if @item[:object] != 'Ticket' return if !Setting.get('slack_integration') @@ -34,7 +38,7 @@ backend.perform return if !config return if !config['items'] - ticket = Ticket.find(@item[:ticket_id]) + ticket = Ticket.find(@item[:object_id]) if @item[:article_id] article = Ticket::Article.find(@item[:article_id]) end diff --git a/config/application.rb b/config/application.rb index 4228be027..e783fae9a 100644 --- a/config/application.rb +++ b/config/application.rb @@ -30,7 +30,6 @@ module Zammad 'observer::_ticket::_article::_communicate_email', 'observer::_ticket::_article::_communicate_facebook', 'observer::_ticket::_article::_communicate_twitter', - 'observer::_ticket::_article::_signature_detection', 'observer::_ticket::_reset_new_state', 'observer::_ticket::_escalation_calculation', 'observer::_ticket::_ref_object_touch', diff --git a/db/migrate/20160422000001_update_signature_detection.rb b/db/migrate/20160422000001_update_signature_detection.rb new file mode 100644 index 000000000..33ba97d08 --- /dev/null +++ b/db/migrate/20160422000001_update_signature_detection.rb @@ -0,0 +1,13 @@ +class UpdateSignatureDetection < ActiveRecord::Migration + def up + Setting.create_if_not_exists( + title: 'Define transaction backend.', + name: '1000_signature_detection', + area: 'Transaction::Backend', + description: 'Define the transaction backend to detect customers signature in email.', + options: {}, + state: 'Transaction::SignatureDetection', + frontend: false + ) + end +end diff --git a/db/seeds.rb b/db/seeds.rb index 9f29ba6d5..ae9c440ea 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -1778,6 +1778,15 @@ Setting.create_if_not_exists( state: 'Transaction::Notification', frontend: false ) +Setting.create_if_not_exists( + title: 'Define transaction backend.', + name: '1000_signature_detection', + area: 'Transaction::Backend', + description: 'Define the transaction backend to detect customers signature in email.', + options: {}, + state: 'Transaction::SignatureDetection', + frontend: false +) Setting.create_if_not_exists( title: 'Define transaction backend.', name: '6000_slack_webhook', diff --git a/lib/signature_detection.rb b/lib/signature_detection.rb index 2a21dda43..9b4187a9c 100644 --- a/lib/signature_detection.rb +++ b/lib/signature_detection.rb @@ -131,7 +131,6 @@ returns =end def self.by_user_id(user_id) - type = Ticket::Article::Type.lookup(name: 'email') sender = Ticket::Article::Sender.lookup(name: 'Customer') article_bodies = [] @@ -142,7 +141,7 @@ returns article_bodies.push article.body } - find_signature( article_bodies ) + find_signature(article_bodies) end =begin @@ -167,7 +166,7 @@ returns =begin -rebuild signature for user +rebuild signature detection for user SignatureDetection.rebuild_user(user_id) diff --git a/test/unit/email_signatur_detection_test.rb b/test/unit/email_signatur_detection_test.rb index 9d02fa785..c8844760c 100644 --- a/test/unit/email_signatur_detection_test.rb +++ b/test/unit/email_signatur_detection_test.rb @@ -73,6 +73,7 @@ class EmailSignaturDetectionTest < ActiveSupport::TestCase ticket1, article1, user1, mail = Channel::EmailParser.new.process({}, raw_email) assert(ticket1) assert(article1) + Delayed::Worker.new.work_off # process email II file = File.open("#{Rails.root}/test/fixtures/email_signature_detection/client_a_2.txt", 'rb') @@ -80,8 +81,6 @@ class EmailSignaturDetectionTest < ActiveSupport::TestCase ticket2, article2, user2, mail = Channel::EmailParser.new.process({}, raw_email) assert(ticket2) assert(article2) - - # process background jobs (user signature detection & article signature detection) Delayed::Worker.new.work_off # check if user2 has a signature_detection value @@ -94,8 +93,10 @@ class EmailSignaturDetectionTest < ActiveSupport::TestCase ticket3, article3, user3, mail = Channel::EmailParser.new.process({}, raw_email) assert(ticket3) assert(article3) + Delayed::Worker.new.work_off # check if article3 has a signature_detection value + article3 = Ticket::Article.find(article3.id) assert_equal(article3.preferences[:signature_detection], 6) # relbuild all diff --git a/test/unit/ticket_notification_test.rb b/test/unit/ticket_notification_test.rb index 4ae1a26a4..4de9a0d46 100644 --- a/test/unit/ticket_notification_test.rb +++ b/test/unit/ticket_notification_test.rb @@ -904,11 +904,11 @@ class TicketNotificationTest < ActiveSupport::TestCase list = EventBuffer.list('transaction') list_objects = Observer::Transaction.get_uniq_changes(list) - assert_equal('some notification event test 1', list_objects[ticket1.id][:changes]['title'][0]) - assert_equal('some notification event test 1 - #2', list_objects[ticket1.id][:changes]['title'][1]) - assert_not(list_objects[ticket1.id][:changes]['priority']) - assert_equal(2, list_objects[ticket1.id][:changes]['priority_id'][0]) - assert_equal(3, list_objects[ticket1.id][:changes]['priority_id'][1]) + assert_equal('some notification event test 1', list_objects['Ticket'][ticket1.id][:changes]['title'][0]) + assert_equal('some notification event test 1 - #2', list_objects['Ticket'][ticket1.id][:changes]['title'][1]) + assert_not(list_objects['Ticket'][ticket1.id][:changes]['priority']) + assert_equal(2, list_objects['Ticket'][ticket1.id][:changes]['priority_id'][0]) + assert_equal(3, list_objects['Ticket'][ticket1.id][:changes]['priority_id'][1]) # update ticket attributes ticket1.title = "#{ticket1.title} - #3" @@ -918,11 +918,11 @@ class TicketNotificationTest < ActiveSupport::TestCase list = EventBuffer.list('transaction') list_objects = Observer::Transaction.get_uniq_changes(list) - assert_equal('some notification event test 1', list_objects[ticket1.id][:changes]['title'][0]) - assert_equal('some notification event test 1 - #2 - #3', list_objects[ticket1.id][:changes]['title'][1]) - assert_not(list_objects[ticket1.id][:changes]['priority']) - assert_equal(2, list_objects[ticket1.id][:changes]['priority_id'][0]) - assert_equal(1, list_objects[ticket1.id][:changes]['priority_id'][1]) + assert_equal('some notification event test 1', list_objects['Ticket'][ticket1.id][:changes]['title'][0]) + assert_equal('some notification event test 1 - #2 - #3', list_objects['Ticket'][ticket1.id][:changes]['title'][1]) + assert_not(list_objects['Ticket'][ticket1.id][:changes]['priority']) + assert_equal(2, list_objects['Ticket'][ticket1.id][:changes]['priority_id'][0]) + assert_equal(1, list_objects['Ticket'][ticket1.id][:changes]['priority_id'][1]) end