From 3e2648314d1a44d6783c65ddfce789a2001b55e1 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Fri, 9 Jan 2015 20:44:04 +0100 Subject: [PATCH] Added more unit tests. --- app/models/observer/ticket/notification.rb | 5 ++ .../ticket/notification/background_job.rb | 39 +++++++------ test/unit/ticket_notification_test.rb | 56 +++++++++++++++---- 3 files changed, 73 insertions(+), 27 deletions(-) diff --git a/app/models/observer/ticket/notification.rb b/app/models/observer/ticket/notification.rb index 557258c41..6306a819f 100644 --- a/app/models/observer/ticket/notification.rb +++ b/app/models/observer/ticket/notification.rb @@ -64,6 +64,11 @@ class Observer::Ticket::Notification < ActiveRecord::Observer listObjects[ticket.id] = {} end listObjects[ticket.id][:article_id] = article.id + listObjects[ticket.id][:ticket_id] = ticket.id + + if !listObjects[ticket.id][:type] + listObjects[ticket.id][:type] = 'update' + end elsif event[:name] == 'Ticket' ticket = Ticket.lookup( :id => event[:id] ) diff --git a/app/models/observer/ticket/notification/background_job.rb b/app/models/observer/ticket/notification/background_job.rb index 53a56e9e2..cafc133fd 100644 --- a/app/models/observer/ticket/notification/background_job.rb +++ b/app/models/observer/ticket/notification/background_job.rb @@ -2,15 +2,12 @@ class Observer::Ticket::Notification::BackgroundJob def initialize(params) - @ticket_id = params[:ticket_id] - @article_id = params[:article_id] - @type = params[:type] - @changes = params[:changes] + @p = params end def perform - ticket = Ticket.find(@ticket_id) - if @article_id - article = Ticket::Article.find(@article_id) + ticket = Ticket.find(@p[:ticket_id]) + if @p[:article_id] + article = Ticket::Article.find(@p[:article_id]) end # find recipients @@ -54,14 +51,21 @@ class Observer::Ticket::Notification::BackgroundJob recipient_list = '' recipients.each do |user| - next if ticket.updated_by_id == user.id + # ignore user who changed it by him self + if article + next if article.updated_by_id == user.id + else + next if ticket.updated_by_id == user.id + end + + # ignore inactive users next if !user.active # create desktop notification # create online notification OnlineNotification.add( - :type => @type, + :type => @p[:type], :object => 'Ticket', :o_id => ticket.id, :seen => false, @@ -78,17 +82,20 @@ class Observer::Ticket::Notification::BackgroundJob end recipient_list += user.email.to_s + # ignore if no changes has been done changes = self.human_changes(user, ticket) - next if !changes || changes.empty? + if @p[:type] == 'update' && !article && ( !changes || changes.empty? ) + next + end # get user based notification template # if create, send create message / block update messages - if @type == 'create' + if @p[:type] == 'create' template = self.template_create(user, ticket, article, changes) - elsif @type == 'update' + elsif @p[:type] == 'update' template = self.template_update(user, ticket, article, changes) else - raise "unknown type for notification #{@type}" + raise "unknown type for notification #{@p[:type]}" end # prepare subject & body @@ -109,7 +116,7 @@ class Observer::Ticket::Notification::BackgroundJob notification[:subject] = ticket.subject_build( notification[:subject] ) # send notification - puts "send ticket notifiaction to agent (#{@type}/#{ticket.id}/#{user.email})" + puts "send ticket notifiaction to agent (#{@p[:type]}/#{ticket.id}/#{user.email})" NotificationFactory.send( :recipient => user, @@ -133,13 +140,13 @@ class Observer::Ticket::Notification::BackgroundJob def human_changes(user, record) - return {} if !@changes + return {} if !@p[:changes] # only show allowed attributes attribute_list = ObjectManager::Attribute.by_object_as_hash('Ticket', user) #puts "AL #{attribute_list.inspect}" user_related_changes = {} - @changes.each {|key, value| + @p[:changes].each {|key, value| # if no config exists, use all attributes if !attribute_list || attribute_list.empty? diff --git a/test/unit/ticket_notification_test.rb b/test/unit/ticket_notification_test.rb index 816d3adf3..d5af43081 100644 --- a/test/unit/ticket_notification_test.rb +++ b/test/unit/ticket_notification_test.rb @@ -110,20 +110,50 @@ class TicketNotificationTest < ActiveSupport::TestCase # 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, + :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 => agent1.id, + :created_by_id => agent1.id, ) - # verify notifications to agent1 + agent2 + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + # verify notifications to not to agent1 but to agent2 + assert_equal( 2, notification_check(ticket1, agent1), ticket1.id ) + assert_equal( 3, notification_check(ticket1, agent2), ticket1.id ) + # update ticket by user + ticket1.owner_id = agent1.id + ticket1.updated_by_id = agent1.id + ticket1.save + 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 => agent1.id, + :created_by_id => agent1.id, + ) + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + + # verify notifications to not to agent1 but to agent2 + assert_equal( 2, notification_check(ticket1, agent1), ticket1.id ) + assert_equal( 3, notification_check(ticket1, agent2), ticket1.id ) # create ticket with agent1 as owner ticket2 = Ticket.create( @@ -348,7 +378,7 @@ class TicketNotificationTest < ActiveSupport::TestCase :to => 'some_recipient@example.com', :subject => 'some subject', :message_id => 'some@id', - :body => 'some message', + :body => "some message\nnewline1 abc\nnewline2", :internal => false, :sender => Ticket::Article::Sender.where(:name => 'Customer').first, :type => Ticket::Article::Type.where(:name => 'email').first, @@ -380,6 +410,7 @@ class TicketNotificationTest < ActiveSupport::TestCase assert_match( /Priority/, template[:body] ) assert_match( /1 low/, template[:body] ) assert_match( /2 normal/, template[:body] ) + assert_match( /update/, template[:subject] ) # en notification body = NotificationFactory.build( @@ -394,6 +425,7 @@ class TicketNotificationTest < ActiveSupport::TestCase assert_match( /Priority/, body ) assert_match( /1 low/, body ) assert_match( /2 normal/, body ) + assert_match( /update/, body ) # de template template = bg.template_update(agent1, ticket1, article, human_changes) @@ -402,6 +434,7 @@ class TicketNotificationTest < ActiveSupport::TestCase assert_match( /Priority/, template[:body] ) assert_match( /1 low/, template[:body] ) assert_match( /2 normal/, template[:body] ) + assert_match( /aktualis/, template[:subject] ) # de notification body = NotificationFactory.build( @@ -417,6 +450,7 @@ class TicketNotificationTest < ActiveSupport::TestCase assert_match( /Priorität/, body ) assert_match( /1 niedrig/, body ) assert_match( /2 normal/, body ) + assert_match( /aktualis/, body ) end