Added more unit tests.

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] )

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]
+    @p = params
-    @article_id = params[:article_id]
-    @type       = params[:type]
-    @changes    = params[:changes]
   end
   def perform
-    ticket  = Ticket.find(@ticket_id)
+    ticket  = Ticket.find(@p[:ticket_id])
-    if @article_id
+    if @p[:article_id]
-      article = Ticket::Article.find(@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?

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,
+      :ticket_id     => ticket1.id,
-      :from           => 'some person',
+      :from          => 'some person',
-      :subject        => 'some note',
+      :subject       => 'some note',
-      :body           => 'some message',
+      :body          => 'some message',
-      :internal       => true,
+      :internal      => true,
-      :sender         => Ticket::Article::Sender.where(:name => 'Agent').first,
+      :sender        => Ticket::Article::Sender.where(:name => 'Agent').first,
-      :type           => Ticket::Article::Type.where(:name => 'note').first,
+      :type          => Ticket::Article::Type.where(:name => 'note').first,
-      :updated_by_id  => 1,
+      :updated_by_id => agent1.id,
-      :created_by_id  => 1,
+      :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