From 464673594c7c6ad63d7d42a52cb159e6eced4dac Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Fri, 2 Jan 2015 16:50:31 +0100 Subject: [PATCH] Init version of improved notifications. --- app/models/observer/ticket/notification.rb | 254 +++++++++------- .../ticket/notification/background_job.rb | 15 +- test/unit/history_test.rb | 3 + test/unit/ticket_notification_test.rb | 276 ++++++++++++++++++ 4 files changed, 439 insertions(+), 109 deletions(-) create mode 100644 test/unit/ticket_notification_test.rb diff --git a/app/models/observer/ticket/notification.rb b/app/models/observer/ticket/notification.rb index 7f0eca37f..98026c81d 100644 --- a/app/models/observer/ticket/notification.rb +++ b/app/models/observer/ticket/notification.rb @@ -17,35 +17,18 @@ class Observer::Ticket::Notification < ActiveRecord::Observer # reset buffer EventBuffer.reset - list.each { |event| + # get uniq objects + listObjects = get_uniq_changes(list) + listObjects.each {|ticket_id, item| + ticket = Ticket.lookup( :id => ticket_id ) + article = ticket.articles[-1] - # get current state of objects - if event[:name] == 'Ticket::Article' - article = Ticket::Article.lookup( :id => event[:id] ) - - # next if article is already deleted - next if !article - - ticket = article.ticket - elsif event[:name] == 'Ticket' - ticket = Ticket.lookup( :id => event[:id] ) - - # next if ticket is already deleted - next if !ticket - - article = ticket.articles[-1] - next if !article - else - raise "unknown object for notification #{event[:name]}" - end - - # send new ticket notification to agents - if event[:name] == 'Ticket' && event[:type] == 'create' - - puts 'send new ticket notify to agent' + # if create, send create message / block update messages + if item[:type] == 'create' + puts 'send ticket create notify to agent' send_notify( { - :event => event, + :event => item[:type], :recipient => 'to_work_on', # group|owner|to_work_on|customer :subject => 'New Ticket (#{ticket.title})', :body => 'Hi #{recipient.firstname}, @@ -70,105 +53,102 @@ class Observer::Ticket::Notification < ActiveRecord::Observer ) end - # send new ticket notification to customers - if event[:name] == 'Ticket' && event[:type] == 'create' - - # only for incoming emails - next if article.type.name != 'email' - - puts 'send new ticket notify to customer' + # if update, send update message, add changes to message + if item[:type] == 'update' + #puts "CHANGESSS #{item[:changes].inspect}" + puts 'send ticket update notify to agent' + changes = '' + item[:changes].each {|key,value| + changes = "#{key}: #{value[0]} -> #{value[1]}" + } send_notify( { - :event => event, - :recipient => 'customer', # group|owner|to_work_on|customer - :subject => 'New Ticket has been created! (#{ticket.title})', - :body => 'Thanks for your email. A new ticket has been created. + :event => item[:type], + :recipient => 'to_work_on', # group|owner|to_work_on + :subject => 'Updated (#{ticket.title})', + :body => 'Hi #{recipient.firstname}, - You wrote: + updated (#{ticket.title}) via i18n(#{article.type.name}). + + Changes: + ' + changes + ' + + From: #{article.from} #{article.body} - Your email will be answered by a human ASAP - - Have fun with Zammad! :-) - - Your Zammad Team + #{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}/#{article.id} ' }, ticket, article, - 'new ticket' + 'update ticket', ) end + } + end - # send follow up notification - if event[:name] == 'Ticket::Article' && event[:type] == 'create' +=begin - # only send article notifications after init article is created (handled by ticket create event) - next if ticket.articles.count.to_i <= 1 + result = get_uniq_changes(events) - puts 'send new ticket::article notify' + result = { + :1 => { + :type => 'create', + }, + :9 = { + :type => 'update', + :changes => { + :attribute1 => [before,now], + :attribute2 => [before,now], + } + }, + } - if article.sender.name == 'Customer' - send_notify( - { - :event => event, - :recipient => 'to_work_on', # group|owner|to_work_on|customer - :subject => 'Follow Up (#{ticket.title})', - :body => 'Hi #{recipient.firstname}, +=end - a follow Up (#{ticket.title}) via i18n(#{article.type.name}). + def self.get_uniq_changes(events) + listObjects = {} + events.each { |event| - Group: #{ticket.group.name} - Owner: #{ticket.owner.firstname} #{ticket.owner.lastname} - State: i18n(#{ticket.state.name}) + # get changes - From: #{article.from} - - #{article.body} - + # send notify - #{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}/#{article.id} - ' - }, - ticket, - article, - 'follow up' - ) + # get current state of objects + #if event[:name] == 'Ticket::Article' + # article = Ticket::Article.lookup( :id => event[:id] ) + # + # # next if article is already deleted + # next if !article + # + # ticket = article.ticket + # #listObjects[ticket.id] = event + #elsif event[:name] == 'Ticket' + if event[:name] == 'Ticket' + ticket = Ticket.lookup( :id => event[:id] ) + + # next if ticket is already deleted + next if !ticket + + article = ticket.articles[-1] + next if !article + + if !listObjects[ticket.id] + listObjects[ticket.id] = {} end - - # send new note notification to owner - # if agent == created.id - if article.sender.name == 'Agent' && article.created_by_id != article.ticket.owner_id - send_notify( - { - :event => event, - :recipient => 'owner', # group|owner|to_work_on - :subject => 'Updated (#{ticket.title})', - :body => 'Hi #{recipient.firstname}, - - updated (#{ticket.title}) via i18n(#{article.type.name}). - - Group: #{ticket.group.name} - Owner: #{ticket.owner.firstname} #{ticket.owner.lastname} - State: i18n(#{ticket.state.name}) - - From: #{article.from} - - #{article.body} - - - #{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}/#{article.id} - ' - }, - ticket, - article, - 'follow up', - ) + if !listObjects[ticket.id][:type] || listObjects[ticket.id][:type] == 'update' + listObjects[ticket.id][:type] = event[:type] end + if event[:changes] + listObjects[ticket.id][:changes] = event[:changes] + end + #else + # raise "unknown object for notification #{event[:name]}" end } + listObjects end def self.send_notify(data, ticket, article, type) @@ -205,22 +185,84 @@ class Observer::Ticket::Notification < ActiveRecord::Observer return if Setting.get('import_mode') #puts 'before_update' - current = record.class.find(record.id) + #current = record.class.find(record.id) + + real_changes = {} + record.changes.each {|key, value| + next if key == 'updated_at' + next if key == 'first_response' + next if key == 'last_contact_agent' + next if key == 'last_contact_customer' + next if key == 'last_contact' + next if key == 'article_count' + next if key == 'create_article_type_id' + next if key == 'create_article_sender_id' + real_changes[key] = value + } + + return if real_changes.empty? + + human_changes = {} + real_changes.each {|key, value| + + # get attribute name + attribute_name = key.to_s + if attribute_name[-3,3] == '_id' + attribute_name = attribute_name[ 0, attribute_name.length-3 ] + end + if key == attribute_name + human_changes[key] = value + end + + value_id = [] + value_str = [ value[0], value[1] ] + if key.to_s[-3,3] == '_id' + value_id[0] = value[0] + value_id[1] = value[1] + + if record.respond_to?( attribute_name ) && record.send(attribute_name) + relation_class = record.send(attribute_name).class + if relation_class && value_id[0] + relation_model = relation_class.lookup( :id => value_id[0] ) + if relation_model + if relation_model['name'] + value_str[0] = relation_model['name'] + elsif relation_model.respond_to?('fullname') + value_str[0] = relation_model.send('fullname') + end + end + end + if relation_class && value_id[1] + relation_model = relation_class.lookup( :id => value_id[1] ) + if relation_model + if relation_model['name'] + value_str[1] = relation_model['name'] + elsif relation_model.respond_to?('fullname') + value_str[1] = relation_model.send('fullname') + end + end + end + end + end + human_changes[attribute_name] = [value_str[0].to_s, value_str[1].to_s] + } # do not send anything if nothing has changed - return if current.attributes == record.attributes + return if human_changes.empty? # puts 'UPDATE!!!!!!!!' + # puts "changes #{record.changes.inspect}" # puts 'current' # puts current.inspect # puts 'record' # puts record.inspect e = { - :name => record.class.name, - :type => 'update', - :data => record, - :id => record.id, + :name => record.class.name, + :type => 'update', + :data => record, + :changes => human_changes, + :id => record.id, } EventBuffer.add(e) end diff --git a/app/models/observer/ticket/notification/background_job.rb b/app/models/observer/ticket/notification/background_job.rb index 5c1c621dd..0058b3d26 100644 --- a/app/models/observer/ticket/notification/background_job.rb +++ b/app/models/observer/ticket/notification/background_job.rb @@ -17,20 +17,20 @@ class Observer::Ticket::Notification::BackgroundJob if data[:recipient] == 'group' recipients = ticket.agent_of_group() - # owner + # owner elsif data[:recipient] == 'owner' if ticket.owner_id != 1 recipients.push ticket.owner end - # customer + # customer elsif data[:recipient] == 'customer' if ticket.customer_id != 1 # temporarily disabled # recipients.push ticket.customer end - # owner or group of agents to work on + # owner or group of agents to work on elsif data[:recipient] == 'to_work_on' if ticket.owner_id != 1 recipients.push ticket.owner @@ -43,6 +43,13 @@ class Observer::Ticket::Notification::BackgroundJob recipient_list = '' notification_subject = '' recipients.each do |user| + + next if ticket.updated_by_id == ticket.owner_id + + # create desktop notification + + + # create online notification OnlineNotification.add( :type => @type, :object => 'Ticket', @@ -52,6 +59,7 @@ class Observer::Ticket::Notification::BackgroundJob :user_id => user.id, ) + # create email notification next if !user.email || user.email == '' # add recipient_list @@ -88,6 +96,7 @@ class Observer::Ticket::Notification::BackgroundJob # add history record if recipient_list != '' + puts "send... #{recipient_list} #{ticket.id}" History.add( :o_id => ticket.id, :history_type => 'notification', diff --git a/test/unit/history_test.rb b/test/unit/history_test.rb index b709f577e..622d5a630 100644 --- a/test/unit/history_test.rb +++ b/test/unit/history_test.rb @@ -174,6 +174,9 @@ class HistoryTest < ActiveSupport::TestCase # execute ticket events Observer::Ticket::Notification.transaction + # execute background jobs + Delayed::Worker.new.work_off + # remember ticket tickets.push ticket diff --git a/test/unit/ticket_notification_test.rb b/test/unit/ticket_notification_test.rb new file mode 100644 index 000000000..ccfc34f79 --- /dev/null +++ b/test/unit/ticket_notification_test.rb @@ -0,0 +1,276 @@ +# encoding: utf-8 +require 'test_helper' + +class TicketNotificationTest < ActiveSupport::TestCase + test 'ticket create' do + + # create agent1 & agent2 + groups = Group.where( :name => 'Users' ) + roles = Role.where( :name => 'Agent' ) + agent1 = User.create_or_update( + :login => 'ticket-notification-agent1@example.com', + :firstname => 'Notification', + :lastname => 'Agent1', + :email => 'ticket-notification-agent1@example.com', + :password => 'agentpw', + :active => true, + :roles => roles, + :groups => groups, + :updated_by_id => 1, + :created_by_id => 1, + ) + agent2 = User.create_or_update( + :login => 'ticket-notification-agent2@example.com', + :firstname => 'Notification', + :lastname => 'Agent2', + :email => 'ticket-notification-agent2@example.com', + :password => 'agentpw', + :active => true, + :roles => roles, + :groups => groups, + :updated_by_id => 1, + :created_by_id => 1, + ) + Group.create_if_not_exists( + :name => 'WithoutAccess', + :note => 'Test for notification check.', + :updated_by_id => 1, + :created_by_id => 1 + ) + + # create customer + roles = Role.where( :name => 'Customer' ) + customer = User.create_or_update( + :login => 'ticket-notification-customer@example.com', + :firstname => 'Notification', + :lastname => 'Customer', + :email => 'ticket-notification-customer@example.com', + :password => 'agentpw', + :active => true, + :roles => roles, + :groups => groups, + :updated_by_id => 1, + :created_by_id => 1, + ) + + # create ticket in group + ticket1 = Ticket.create( + :title => 'some notification test 1', + :group => Group.lookup( :name => 'Users'), + :customer => customer, + :state => Ticket::State.lookup( :name => 'new' ), + :priority => Ticket::Priority.lookup( :name => '2 normal' ), + :updated_by_id => customer.id, + :created_by_id => customer.id, + ) + article_inbound = Ticket::Article.create( + :ticket_id => ticket1.id, + :from => 'some_sender@example.com', + :to => 'some_recipient@example.com', + :subject => 'some subject', + :message_id => 'some@id', + :body => 'some message', + :internal => false, + :sender => Ticket::Article::Sender.where(:name => 'Customer').first, + :type => Ticket::Article::Type.where(:name => 'email').first, + :updated_by_id => customer.id, + :created_by_id => customer.id, + ) + assert( ticket1, "ticket created" ) + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + + # verify notifications to agent1 + agent2 + assert_equal( 1, notification_check(ticket1, agent1), 'agent 1 notification count check' ) + assert_equal( 1, notification_check(ticket1, agent2), 'agent 2 notification count check' ) + + # update ticket attributes + ticket1.title = "#{ticket1.title} - #2" + ticket1.priority = Ticket::Priority.lookup( :name => '3 high' ) + ticket1.save + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + + # verify notifications to agent1 + agent2 + assert_equal( 2, notification_check(ticket1, agent1), 'agent 1 notification count check' ) + assert_equal( 2, notification_check(ticket1, agent2), 'agent 2 notification count check' ) + + # add article to ticket + article_note = Ticket::Article.create( + :ticket_id => ticket1.id, + :from => 'some person', + :subject => 'some note', + :body => 'some message', + :internal => true, + :sender => Ticket::Article::Sender.where(:name => 'Agent').first, + :type => Ticket::Article::Type.where(:name => 'note').first, + :updated_by_id => 1, + :created_by_id => 1, + ) + + # verify notifications to agent1 + agent2 + + + + # create ticket with agent1 as owner + ticket2 = Ticket.create( + :title => 'some notification test 2', + :group => Group.lookup( :name => 'Users'), + :customer_id => 2, + :owner_id => agent1.id, + :state => Ticket::State.lookup( :name => 'new' ), + :priority => Ticket::Priority.lookup( :name => '2 normal' ), + :updated_by_id => agent1.id, + :created_by_id => agent1.id, + ) + article_inbound = Ticket::Article.create( + :ticket_id => ticket2.id, + :from => 'some_sender@example.com', + :to => 'some_recipient@example.com', + :subject => 'some subject', + :message_id => 'some@id', + :body => 'some message', + :internal => false, + :sender => Ticket::Article::Sender.where(:name => 'Agent').first, + :type => Ticket::Article::Type.where(:name => 'phone').first, + :updated_by_id => agent1.id, + :created_by_id => agent1.id, + ) + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + assert( ticket2, "ticket created" ) + + # verify notifications to no one + assert_equal( 0, notification_check(ticket2, agent1), 'agent 1 notification count check' ) + assert_equal( 0, notification_check(ticket2, agent2), 'agent 2 notification count check' ) + + # update ticket + ticket2.title = "#{ticket2.title} - #2" + ticket2.updated_by_id = agent1.id + ticket2.priority = Ticket::Priority.lookup( :name => '3 high' ) + ticket2.save + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + + # verify notifications to no one + assert_equal( 0, notification_check(ticket2, agent1), 'agent 1 notification count check' ) + assert_equal( 0, notification_check(ticket2, agent2), 'agent 2 notification count check' ) + + # update ticket + ticket2.title = "#{ticket2.title} - #3" + ticket2.updated_by_id = agent2.id + ticket2.priority = Ticket::Priority.lookup( :name => '2 normal' ) + ticket2.save + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + + # verify notifications to agent1 and not to agent2 + assert_equal( 1, notification_check(ticket2, agent1), 'agent 1 notification count check' ) + assert_equal( 0, notification_check(ticket2, agent2), 'agent 2 notification count check' ) + + + + # create ticket with agent2 and agent1 as owner + ticket3 = Ticket.create( + :title => 'some notification test 3', + :group => Group.lookup( :name => 'Users'), + :customer_id => 2, + :owner_id => agent1.id, + :state => Ticket::State.lookup( :name => 'new' ), + :priority => Ticket::Priority.lookup( :name => '2 normal' ), + :updated_by_id => agent2.id, + :created_by_id => agent2.id, + ) + article_inbound = Ticket::Article.create( + :ticket_id => ticket3.id, + :from => 'some_sender@example.com', + :to => 'some_recipient@example.com', + :subject => 'some subject', + :message_id => 'some@id', + :body => 'some message', + :internal => false, + :sender => Ticket::Article::Sender.where(:name => 'Agent').first, + :type => Ticket::Article::Type.where(:name => 'phone').first, + :updated_by_id => agent2.id, + :created_by_id => agent2.id, + ) + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + assert( ticket3, "ticket created" ) + + # verify notifications to agent1 and not to agent2 + assert_equal( 1, notification_check(ticket3, agent1), 'agent 1 notification count check' ) + assert_equal( 0, notification_check(ticket3, agent2), 'agent 2 notification count check' ) + + # update ticket + ticket3.title = "#{ticket3.title} - #2" + ticket3.updated_by_id = agent1.id + ticket3.priority = Ticket::Priority.lookup( :name => '3 high' ) + ticket3.save + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + + # verify notifications to no one + assert_equal( 1, notification_check(ticket3, agent1), 'agent 1 notification count check' ) + assert_equal( 0, notification_check(ticket3, agent2), 'agent 2 notification count check' ) + + # update ticket + ticket3.title = "#{ticket3.title} - #3" + ticket3.updated_by_id = agent2.id + ticket3.priority = Ticket::Priority.lookup( :name => '2 normal' ) + ticket3.save + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + + # verify notifications to agent1 and not to agent2 + assert_equal( 2, notification_check(ticket3, agent1), 'agent 1 notification count check' ) + assert_equal( 0, notification_check(ticket3, agent2), 'agent 2 notification count check' ) + + + delete = ticket1.destroy + assert( delete, "ticket1 destroy" ) + + delete = ticket2.destroy + assert( delete, "ticket2 destroy" ) + + delete = ticket3.destroy + assert( delete, "ticket3 destroy" ) + + end + + def notification_check(ticket, recipient) + result = ticket.history_get() + count = 0 + result.each {|item| + next if item['type'] != 'notification' + next if item['object'] != 'Ticket' + next if item['value_to'] !~ /#{recipient.email}/i + count += 1 + } + count + end +end \ No newline at end of file