Init version of improved notifications.

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)
-      # get current state of objects
+    listObjects.each {|ticket_id, item|
-      if event[:name] == 'Ticket::Article'
+      ticket  = Ticket.lookup( :id => ticket_id )
-        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 create, send create message / block update messages
-      if event[:name] == 'Ticket' && event[:type] == 'create'
+      if item[:type] == 'create'
-
+        puts 'send ticket create notify to agent'
-        puts 'send new ticket 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,89 +53,25 @@ class Observer::Ticket::Notification < ActiveRecord::Observer
         )
       end
 
-      # send new ticket notification to customers
+      # if update, send update message, add changes to message
-      if event[:name] == 'Ticket' && event[:type] == 'create'
+      if item[:type] == 'update'
-
+        #puts "CHANGESSS #{item[:changes].inspect}"
-        # only for incoming emails
+        puts 'send ticket update notify to agent'
-        next if article.type.name != 'email'
+        changes = ''
-
+        item[:changes].each {|key,value|
-        puts 'send new ticket notify to customer'
+          changes = "#{key}: #{value[0]} -> #{value[1]}"
+        }
         send_notify(
           {
-            :event     => event,
+            :event     => item[:type],
-            :recipient => 'customer', # group|owner|to_work_on|customer
+            :recipient => 'to_work_on', # group|owner|to_work_on
-            :subject   => 'New Ticket has been created! (#{ticket.title})',
-            :body      => 'Thanks for your email. A new ticket has been created.
-
-            You wrote:
-            <snip>
-            #{article.body}
-            </snip>
-
-            Your email will be answered by a human ASAP
-
-            Have fun with Zammad! :-)
-
-            Your Zammad Team
-            '
-          },
-          ticket,
-          article,
-          'new ticket'
-        )
-      end
-
-      # send follow up notification
-      if event[:name] == 'Ticket::Article' && event[:type] == 'create'
-
-        # only send article notifications after init article is created (handled by ticket create event)
-        next if ticket.articles.count.to_i <= 1
-
-        puts 'send new ticket::article notify'
-
-        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},
-
-              a follow Up (#{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}
-              <snip>
-              #{article.body}
-              </snip>
-
-              #{config.http_type}://#{config.fqdn}/#ticket/zoom/#{ticket.id}/#{article.id}
-              '
-            },
-            ticket,
-            article,
-            'follow up'
-          )
-        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}
+            Changes:
-              Owner: #{ticket.owner.firstname} #{ticket.owner.lastname}
+            ' + changes + '
-              State: i18n(#{ticket.state.name})
 
             From: #{article.from}
             <snip>
@@ -164,11 +83,72 @@ class Observer::Ticket::Notification < ActiveRecord::Observer
           },
           ticket,
           article,
-            'follow up',
+          'update ticket',
         )
       end
+    }
+  end
+
+=begin
+
+  result = get_uniq_changes(events)
+
+  result = {
+    :1 => {
+      :type => 'create',
+    },
+    :9 = {
+      :type => 'update',
+      :changes => {
+        :attribute1 => [before,now],
+        :attribute2 => [before,now],
+      }
+    },
+  }
+
+=end
+
+  def self.get_uniq_changes(events)
+    listObjects = {}
+    events.each { |event|
+
+      # get changes
+
+      # send notify
+
+      # 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
+        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,12 +185,73 @@ 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'
@@ -220,6 +261,7 @@ class Observer::Ticket::Notification < ActiveRecord::Observer
       :name    => record.class.name,
       :type    => 'update',
       :data    => record,
+      :changes => human_changes,
       :id      => record.id,
     }
     EventBuffer.add(e)

app/models/observer/ticket/notification/background_job.rb

@@ -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',

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
 

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