From 57b1f3409ed1fcb31906c8495c0996d9b344783f Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Mon, 22 Feb 2016 08:58:33 +0100 Subject: [PATCH] Improved auto seen feature for online notifications. --- .../online_notifications_controller.rb | 4 +- .../ticket/notification/background_job.rb | 17 +- .../ticket/online_notification_seen.rb | 5 + app/models/online_notification.rb | 53 +- lib/notification_factory.rb | 25 + test/unit/online_notifiaction_test.rb | 596 +++++++++--------- test/unit/ticket_notification_test.rb | 136 ++-- 7 files changed, 471 insertions(+), 365 deletions(-) diff --git a/app/controllers/online_notifications_controller.rb b/app/controllers/online_notifications_controller.rb index 594218858..fc1a3a30e 100644 --- a/app/controllers/online_notifications_controller.rb +++ b/app/controllers/online_notifications_controller.rb @@ -48,11 +48,11 @@ curl http://localhost/api/v1/online_notifications.json -v -u #{login}:#{password def index if params[:full] - render json: OnlineNotification.list_full(current_user, 75) + render json: OnlineNotification.list_full(current_user, 100) return end - notifications = OnlineNotification.list(current_user, 75) + notifications = OnlineNotification.list(current_user, 100) model_index_render_result(notifications) end diff --git a/app/models/observer/ticket/notification/background_job.rb b/app/models/observer/ticket/notification/background_job.rb index 289af9656..dd5cbe060 100644 --- a/app/models/observer/ticket/notification/background_job.rb +++ b/app/models/observer/ticket/notification/background_job.rb @@ -2,6 +2,15 @@ class Observer::Ticket::Notification::BackgroundJob def initialize(params, via_web = false) + +=begin + :type => 'update', + :ticket_id => 123, + :changes => { + 'attribute1' => [before,now], + 'attribute2' => [before,now], + } +=end @p = params @via_web = via_web end @@ -141,13 +150,19 @@ class Observer::Ticket::Notification::BackgroundJob used_channels = [] if channels['online'] used_channels.push 'online' - seen = ticket.online_notification_seen_state(user.id) # delete old notifications if @p[:type] == 'reminder_reached' || @p[:type] == 'escalation' seen = false OnlineNotification.remove_by_type('Ticket', ticket.id, @p[:type]) + + # on updates without state changes create unseen messages + elsif @p[:type] != 'create' && (!@p[:changes] || @p[:changes].empty? || !@p[:changes]['state_id']) + seen = false + else + seen = ticket.online_notification_seen_state(user.id) end + OnlineNotification.add( type: @p[:type], object: 'Ticket', diff --git a/app/models/observer/ticket/online_notification_seen.rb b/app/models/observer/ticket/online_notification_seen.rb index fe84057cc..9196bac21 100644 --- a/app/models/observer/ticket/online_notification_seen.rb +++ b/app/models/observer/ticket/online_notification_seen.rb @@ -18,6 +18,11 @@ class Observer::Ticket::OnlineNotificationSeen < ActiveRecord::Observer # return if we run import mode return if Setting.get('import_mode') + # set seen only if state has changes + return if !record.changes + return if record.changes.empty? + return if !record.changes['state_id'] + # check if existing online notifications for this ticket should be set to seen return true if !record.online_notification_seen_state diff --git a/app/models/online_notification.rb b/app/models/online_notification.rb index 1a639b223..7f96a2a54 100644 --- a/app/models/online_notification.rb +++ b/app/models/online_notification.rb @@ -44,9 +44,9 @@ add a new online notification for this user seen: data[:seen], user_id: data[:user_id], created_by_id: data[:created_by_id], - updated_by_id: data[:updated_by_id], - created_at: data[:created_at], - updated_at: data[:updated_at], + updated_by_id: data[:updated_by_id] || data[:created_by_id], + created_at: data[:created_at] || Time.zone.now, + updated_at: data[:updated_at] || Time.zone.now, } OnlineNotification.create(record) @@ -205,6 +205,53 @@ returns: =begin +check if all notifications are seed for dedecated object + + OnlineNotification.all_seen?('Ticket', 123) + +returns: + + true # false + +=end + + def self.all_seen?(object, o_id) + notifications = OnlineNotification.list_by_object(object, o_id) + notifications.each {|onine_notification| + return false if !onine_notification['seen'] + } + true + end + +=begin + +check if notification was created for certain user + + OnlineNotification.exists?(for_user, object, o_id, type, created_by_user, seen) + +returns: + + true # false + +=end + + # rubocop:disable Metrics/ParameterLists + def self.exists?(user, object, o_id, type, created_by_user, seen) + # rubocop:enable Metrics/ParameterLists + notifications = OnlineNotification.list(user, 10) + notifications.each {|notification| + next if notification['o_id'] != o_id + next if notification['object'] != object + next if notification['type'] != type + next if notification['created_by_id'] != created_by_user.id + next if notification['seen'] != seen + return true + } + false + end + +=begin + cleanup old online notifications OnlineNotification.cleanup diff --git a/lib/notification_factory.rb b/lib/notification_factory.rb index 41141b8e5..e6db05475 100644 --- a/lib/notification_factory.rb +++ b/lib/notification_factory.rb @@ -167,6 +167,31 @@ module NotificationFactory ) end +=begin + +get count of already sent notifications + + count = NotificationFactory.already_sent?(ticket, recipient_user, type) + +retunes + + 8 + +=end + + def self.already_sent?(ticket, recipient, type) + 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 + next if item['value_to'] !~ /#{type}/i + count += 1 + } + count + end + =begin result = NotificationFactory.template( diff --git a/test/unit/online_notifiaction_test.rb b/test/unit/online_notifiaction_test.rb index d9c39f986..f43d31fff 100644 --- a/test/unit/online_notifiaction_test.rb +++ b/test/unit/online_notifiaction_test.rb @@ -30,294 +30,345 @@ class OnlineNotificationTest < ActiveSupport::TestCase ) customer_user = User.lookup(email: 'nicole.braun@zammad.org') + Rails.configuration.webserver_is_active = true + test 'ticket notification' do - tests = [ - # test 1 - { - create: { - ticket: { - group_id: Group.lookup( name: 'Users' ).id, - customer_id: customer_user.id, - owner_id: User.lookup( login: '-' ).id, - title: 'Unit Test 1 (äöüß)!', - state_id: Ticket::State.lookup( name: 'closed' ).id, - priority_id: Ticket::Priority.lookup( name: '2 normal' ).id, - updated_by_id: agent_user1.id, - created_by_id: agent_user1.id, - }, - article: { - updated_by_id: agent_user1.id, - created_by_id: agent_user1.id, - type_id: Ticket::Article::Type.lookup( name: 'phone' ).id, - sender_id: Ticket::Article::Sender.lookup( name: 'Customer' ).id, - from: 'Unit Test ', - body: 'Unit Test 123', - internal: false - }, - online_notification: { - seen_only_exists: true, - }, - }, - update: { - ticket: { - title: 'Unit Test 1 (äöüß) - update!', - state_id: Ticket::State.lookup( name: 'open' ).id, - priority_id: Ticket::Priority.lookup( name: '1 low' ).id, - updated_by_id: customer_user.id, - }, - online_notification: { - seen_only_exists: false, - }, - }, - check: [ - { - type: 'create', - object: 'Ticket', - created_by_id: agent_user1.id, - }, - { - type: 'update', - object: 'Ticket', - created_by_id: customer_user.id, - }, - ], - }, + # case #1 + ticket1 = Ticket.create( + group_id: Group.lookup(name: 'Users').id, + customer_id: customer_user.id, + owner_id: User.lookup(login: '-').id, + title: 'Unit Test 1 (äöüß)!', + state_id: Ticket::State.lookup(name: 'closed').id, + priority_id: Ticket::Priority.lookup(name: '2 normal').id, + updated_by_id: agent_user1.id, + created_by_id: agent_user1.id, + ) + article1 = Ticket::Article.create( + ticket_id: ticket1.id, + updated_by_id: agent_user1.id, + created_by_id: agent_user1.id, + type_id: Ticket::Article::Type.lookup(name: 'phone').id, + sender_id: Ticket::Article::Sender.lookup(name: 'Customer').id, + from: 'Unit Test ', + body: 'Unit Test 123', + internal: false + ) - # test 2 - { - create: { - ticket: { - group_id: Group.lookup( name: 'Users' ).id, - customer_id: customer_user.id, - owner_id: User.lookup( login: '-' ).id, - title: 'Unit Test 2 (äöüß)!', - state_id: Ticket::State.lookup( name: 'new' ).id, - priority_id: Ticket::Priority.lookup( name: '2 normal' ).id, - updated_by_id: agent_user1.id, - created_by_id: agent_user1.id, - }, - article: { - updated_by_id: agent_user1.id, - created_by_id: agent_user1.id, - type_id: Ticket::Article::Type.lookup( name: 'phone' ).id, - sender_id: Ticket::Article::Sender.lookup( name: 'Customer' ).id, - from: 'Unit Test ', - body: 'Unit Test 123', - internal: false - }, - online_notification: { - seen_only_exists: false, - }, - }, - update: { - ticket: { - title: 'Unit Test 2 (äöüß) - update!', - state_id: Ticket::State.lookup( name: 'closed' ).id, - priority_id: Ticket::Priority.lookup( name: '1 low' ).id, - updated_by_id: customer_user.id, - }, - online_notification: { - seen_only_exists: true, - }, - }, - check: [ - { - type: 'create', - object: 'Ticket', - created_by_id: agent_user1.id, - }, - { - type: 'update', - object: 'Ticket', - created_by_id: customer_user.id, - }, - ], - }, - - # test 3 - { - create: { - ticket: { - group_id: Group.lookup( name: 'Users' ).id, - customer_id: customer_user.id, - owner_id: User.lookup( login: '-' ).id, - title: 'Unit Test 3 (äöüß)!', - state_id: Ticket::State.lookup( name: 'new' ).id, - priority_id: Ticket::Priority.lookup( name: '2 normal' ).id, - updated_by_id: agent_user1.id, - created_by_id: agent_user1.id, - }, - article: { - updated_by_id: agent_user1.id, - created_by_id: agent_user1.id, - type_id: Ticket::Article::Type.lookup( name: 'phone' ).id, - sender_id: Ticket::Article::Sender.lookup( name: 'Customer' ).id, - from: 'Unit Test ', - body: 'Unit Test 123', - internal: false - }, - online_notification: { - seen_only_exists: false, - }, - }, - update: { - ticket: { - title: 'Unit Test 3 (äöüß) - update!', - state_id: Ticket::State.lookup( name: 'open' ).id, - priority_id: Ticket::Priority.lookup( name: '1 low' ).id, - updated_by_id: customer_user.id, - }, - online_notification: { - seen_only_exists: false, - }, - }, - check: [ - { - type: 'create', - object: 'Ticket', - created_by_id: agent_user1.id, - }, - { - type: 'update', - object: 'Ticket', - created_by_id: customer_user.id, - }, - ], - }, - - # test 4 - { - create: { - ticket: { - group_id: Group.lookup( name: 'Users' ).id, - customer_id: customer_user.id, - owner_id: User.lookup( login: '-' ).id, - title: 'Unit Test 4 (äöüß)!', - state_id: Ticket::State.lookup( name: 'new' ).id, - priority_id: Ticket::Priority.lookup( name: '2 normal' ).id, - updated_by_id: agent_user1.id, - created_by_id: agent_user1.id, - }, - article: { - updated_by_id: agent_user1.id, - created_by_id: agent_user1.id, - type_id: Ticket::Article::Type.lookup( name: 'phone' ).id, - sender_id: Ticket::Article::Sender.lookup( name: 'Customer' ).id, - from: 'Unit Test ', - body: 'Unit Test 123', - internal: false - }, - online_notification: { - seen_only_exists: false, - }, - }, - update: { - ticket: { - title: 'Unit Test 4 (äöüß) - update!', - state_id: Ticket::State.lookup( name: 'open' ).id, - priority_id: Ticket::Priority.lookup( name: '1 low' ).id, - updated_by_id: customer_user.id, - }, - online_notification: { - seen_only_exists: false, - }, - }, - check: [ - { - type: 'create', - object: 'Ticket', - created_by_id: agent_user1.id, - }, - { - type: 'update', - object: 'Ticket', - created_by_id: customer_user.id, - }, - ], - }, - ] + # remember ticket tickets = [] - tests.each { |test| + tickets.push ticket1 - ticket = Ticket.create( test[:create][:ticket] ) - test[:check][0][:o_id] = ticket.id - test[:check][1][:o_id] = ticket.id + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off - test[:create][:article][:ticket_id] = ticket.id - article = Ticket::Article.create( test[:create][:article] ) + # because it's already closed + assert(OnlineNotification.all_seen?('Ticket', ticket1.id)) + assert(!OnlineNotification.exists?(agent_user1, 'Ticket', ticket1.id, 'create', agent_user1, false)) + assert(!OnlineNotification.exists?(agent_user1, 'Ticket', ticket1.id, 'create', agent_user1, true)) + assert(!OnlineNotification.exists?(agent_user2, 'Ticket', ticket1.id, 'create', agent_user1, false)) + assert(OnlineNotification.exists?(agent_user2, 'Ticket', ticket1.id, 'create', agent_user1, true)) - assert_equal( ticket.class.to_s, 'Ticket' ) + ticket1.update_attributes( + title: 'Unit Test 1 (äöüß) - update!', + state_id: Ticket::State.lookup(name: 'open').id, + priority_id: Ticket::Priority.lookup(name: '1 low').id, + updated_by_id: customer_user.id, + ) - # execute ticket events - Observer::Ticket::Notification.transaction - #puts Delayed::Job.all.inspect - Delayed::Worker.new.work_off + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off - # check online notifications - if test[:create][:online_notification] - if test[:create][:online_notification][:seen_only_exists] - notifications = OnlineNotification.list_by_object( 'Ticket', ticket.id ) - assert( notification_seen_only_exists_exists( notifications ), 'not seen notifications for ticket available') - else - notifications = OnlineNotification.list_by_object( 'Ticket', ticket.id ) - assert( !notification_seen_only_exists_exists( notifications ), 'seen notifications for ticket available') - end - end + # because it's already open + assert(!OnlineNotification.all_seen?('Ticket', ticket1.id)) + assert(!OnlineNotification.exists?(agent_user1, 'Ticket', ticket1.id, 'update', customer_user, true)) + assert(OnlineNotification.exists?(agent_user1, 'Ticket', ticket1.id, 'update', customer_user, false)) + assert(!OnlineNotification.exists?(agent_user2, 'Ticket', ticket1.id, 'update', customer_user, true)) + assert(OnlineNotification.exists?(agent_user2, 'Ticket', ticket1.id, 'update', customer_user, false)) - # update ticket - if test[:update][:ticket] - ticket.update_attributes( test[:update][:ticket] ) - end + # case #2 + ticket2 = Ticket.create( + group_id: Group.lookup(name: 'Users').id, + customer_id: customer_user.id, + owner_id: agent_user1.id, + title: 'Unit Test 1 (äöüß)!', + state_id: Ticket::State.lookup(name: 'closed').id, + priority_id: Ticket::Priority.lookup(name: '2 normal').id, + updated_by_id: customer_user.id, + created_by_id: customer_user.id, + ) + article2 = Ticket::Article.create( + ticket_id: ticket2.id, + updated_by_id: customer_user.id, + created_by_id: customer_user.id, + type_id: Ticket::Article::Type.lookup(name: 'phone').id, + sender_id: Ticket::Article::Sender.lookup(name: 'Customer').id, + from: 'Unit Test ', + body: 'Unit Test 123', + internal: false + ) - # execute ticket events - Observer::Ticket::Notification.transaction - #puts Delayed::Job.all.inspect - Delayed::Worker.new.work_off + # remember ticket + tickets = [] + tickets.push ticket2 - # remember ticket - tickets.push ticket + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off - # check online notifications - notification_check( OnlineNotification.list(agent_user2, 10), test[:check] ) + # because it's already closed + assert(!OnlineNotification.all_seen?('Ticket', ticket2.id)) + assert(OnlineNotification.exists?(agent_user1, 'Ticket', ticket2.id, 'create', customer_user, false)) + assert(!OnlineNotification.exists?(agent_user1, 'Ticket', ticket2.id, 'create', customer_user, true)) + assert(!OnlineNotification.exists?(agent_user2, 'Ticket', ticket2.id, 'create', customer_user, false)) + assert(!OnlineNotification.exists?(agent_user2, 'Ticket', ticket2.id, 'create', customer_user, true)) - # check online notifications - next if !test[:update][:online_notification] + ticket2.update_attributes( + title: 'Unit Test 1 (äöüß) - update!', + state_id: Ticket::State.lookup(name: 'open').id, + priority_id: Ticket::Priority.lookup(name: '1 low').id, + updated_by_id: customer_user.id, + ) - if test[:update][:online_notification][:seen_only_exists] - notifications = OnlineNotification.list_by_object( 'Ticket', ticket.id ) - assert( notification_seen_only_exists_exists( notifications ), 'not seen notifications for ticket available') - else - notifications = OnlineNotification.list_by_object( 'Ticket', ticket.id ) - assert( !notification_seen_only_exists_exists( notifications ), 'seen notifications for ticket available') - end - } + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + + # because it's already open + assert(!OnlineNotification.all_seen?('Ticket', ticket2.id)) + assert(OnlineNotification.exists?(agent_user1, 'Ticket', ticket2.id, 'update', customer_user, false)) + assert(!OnlineNotification.exists?(agent_user1, 'Ticket', ticket2.id, 'update', customer_user, true)) + assert(!OnlineNotification.exists?(agent_user2, 'Ticket', ticket2.id, 'update', customer_user, true)) + assert(!OnlineNotification.exists?(agent_user2, 'Ticket', ticket2.id, 'update', customer_user, false)) + + # case #3 + ticket3 = Ticket.create( + group_id: Group.lookup(name: 'Users').id, + customer_id: customer_user.id, + owner_id: User.lookup(login: '-').id, + title: 'Unit Test 2 (äöüß)!', + state_id: Ticket::State.lookup(name: 'new').id, + priority_id: Ticket::Priority.lookup(name: '2 normal').id, + updated_by_id: agent_user1.id, + created_by_id: agent_user1.id, + ) + article3 = Ticket::Article.create( + ticket_id: ticket3.id, + updated_by_id: agent_user1.id, + created_by_id: agent_user1.id, + type_id: Ticket::Article::Type.lookup(name: 'phone').id, + sender_id: Ticket::Article::Sender.lookup(name: 'Customer').id, + from: 'Unit Test ', + body: 'Unit Test 123', + internal: false, + ) + + # remember ticket + tickets.push ticket3 + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + + # because it's already new + assert(!OnlineNotification.all_seen?('Ticket', ticket3.id)) + assert(!OnlineNotification.exists?(agent_user1, 'Ticket', ticket3.id, 'create', agent_user1, false)) + assert(!OnlineNotification.exists?(agent_user1, 'Ticket', ticket3.id, 'create', agent_user1, true)) + assert(OnlineNotification.exists?(agent_user2, 'Ticket', ticket3.id, 'create', agent_user1, false)) + assert(!OnlineNotification.exists?(agent_user2, 'Ticket', ticket3.id, 'create', agent_user1, true)) + + ticket3.update_attributes( + title: 'Unit Test 2 (äöüß) - update!', + state_id: Ticket::State.lookup(name: 'closed').id, + priority_id: Ticket::Priority.lookup(name: '1 low').id, + updated_by_id: customer_user.id, + ) + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + + # because it's already closed + assert(OnlineNotification.all_seen?('Ticket', ticket3.id)) + assert_equal(1, NotificationFactory.already_sent?(ticket3, agent_user1, 'update')) + assert_equal(1, NotificationFactory.already_sent?(ticket3, agent_user2, 'update')) + assert(!OnlineNotification.exists?(agent_user1, 'Ticket', ticket3.id, 'update', customer_user, false)) + assert(OnlineNotification.exists?(agent_user1, 'Ticket', ticket3.id, 'update', customer_user, true)) + assert(!OnlineNotification.exists?(agent_user2, 'Ticket', ticket3.id, 'update', customer_user, false)) + assert(OnlineNotification.exists?(agent_user2, 'Ticket', ticket3.id, 'update', customer_user, true)) + + article3 = Ticket::Article.create( + ticket_id: ticket3.id, + updated_by_id: customer_user.id, + created_by_id: customer_user.id, + type_id: Ticket::Article::Type.lookup(name: 'phone').id, + sender_id: Ticket::Article::Sender.lookup(name: 'Customer').id, + from: 'Unit Test ', + body: 'Unit Test 123 # 2', + internal: false + ) + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + + # because it's already closed but an follow up arrived later + assert(!OnlineNotification.all_seen?('Ticket', ticket3.id)) + assert(OnlineNotification.exists?(agent_user1, 'Ticket', ticket3.id, 'update', customer_user, false)) + assert(OnlineNotification.exists?(agent_user1, 'Ticket', ticket3.id, 'update', customer_user, true)) + assert(OnlineNotification.exists?(agent_user2, 'Ticket', ticket3.id, 'update', customer_user, false)) + assert(OnlineNotification.exists?(agent_user2, 'Ticket', ticket3.id, 'update', customer_user, true)) + assert_equal(2, NotificationFactory.already_sent?(ticket3, agent_user1, 'update')) + assert_equal(2, NotificationFactory.already_sent?(ticket3, agent_user2, 'update')) + + # case #4 + ticket4 = Ticket.create( + group_id: Group.lookup(name: 'Users').id, + customer_id: customer_user.id, + owner_id: agent_user1.id, + title: 'Unit Test 3 (äöüß)!', + state_id: Ticket::State.lookup(name: 'new').id, + priority_id: Ticket::Priority.lookup(name: '2 normal').id, + updated_by_id: customer_user.id, + created_by_id: customer_user.id, + ) + article4 = Ticket::Article.create( + ticket_id: ticket4.id, + updated_by_id: customer_user.id, + created_by_id: customer_user.id, + type_id: Ticket::Article::Type.lookup(name: 'phone').id, + sender_id: Ticket::Article::Sender.lookup(name: 'Customer').id, + from: 'Unit Test ', + body: 'Unit Test 123', + internal: false, + ) + + # remember ticket + tickets.push ticket4 + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + + # because it's already new + assert(!OnlineNotification.all_seen?('Ticket', ticket4.id)) + assert(OnlineNotification.exists?(agent_user1, 'Ticket', ticket4.id, 'create', customer_user, false)) + assert(!OnlineNotification.exists?(agent_user1, 'Ticket', ticket4.id, 'create', customer_user, true)) + assert(!OnlineNotification.exists?(agent_user2, 'Ticket', ticket4.id, 'create', customer_user, false)) + assert(!OnlineNotification.exists?(agent_user2, 'Ticket', ticket4.id, 'create', customer_user, true)) + + ticket4.update_attributes( + title: 'Unit Test 3 (äöüß) - update!', + state_id: Ticket::State.lookup(name: 'open').id, + priority_id: Ticket::Priority.lookup(name: '1 low').id, + updated_by_id: customer_user.id, + ) + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + + # because it's already open + assert(!OnlineNotification.all_seen?('Ticket', ticket4.id)) + assert(OnlineNotification.exists?(agent_user1, 'Ticket', ticket4.id, 'update', customer_user, false)) + assert(!OnlineNotification.exists?(agent_user1, 'Ticket', ticket4.id, 'update', customer_user, true)) + assert(!OnlineNotification.exists?(agent_user2, 'Ticket', ticket4.id, 'update', customer_user, false)) + assert(!OnlineNotification.exists?(agent_user2, 'Ticket', ticket4.id, 'update', customer_user, true)) + + # case #5 + ticket5 = Ticket.create( + group_id: Group.lookup(name: 'Users').id, + customer_id: customer_user.id, + owner_id: User.lookup(login: '-').id, + title: 'Unit Test 4 (äöüß)!', + state_id: Ticket::State.lookup(name: 'new').id, + priority_id: Ticket::Priority.lookup( name: '2 normal').id, + updated_by_id: agent_user1.id, + created_by_id: agent_user1.id, + ) + article5 = Ticket::Article.create( + ticket_id: ticket5.id, + updated_by_id: agent_user1.id, + created_by_id: agent_user1.id, + type_id: Ticket::Article::Type.lookup(name: 'phone').id, + sender_id: Ticket::Article::Sender.lookup(name: 'Customer').id, + from: 'Unit Test ', + body: 'Unit Test 123', + internal: false, + ) + + # remember ticket + tickets.push ticket5 + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + + # because it's already new + assert(!OnlineNotification.all_seen?('Ticket', ticket5.id)) + assert(!OnlineNotification.exists?(agent_user1, 'Ticket', ticket5.id, 'create', agent_user1, true)) + assert(!OnlineNotification.exists?(agent_user1, 'Ticket', ticket5.id, 'create', agent_user1, false)) + assert(OnlineNotification.exists?(agent_user2, 'Ticket', ticket5.id, 'create', agent_user1, false)) + assert(!OnlineNotification.exists?(agent_user2, 'Ticket', ticket5.id, 'create', agent_user1, true)) + + ticket5.update_attributes( + title: 'Unit Test 4 (äöüß) - update!', + state_id: Ticket::State.lookup(name: 'open').id, + priority_id: Ticket::Priority.lookup(name: '1 low').id, + updated_by_id: customer_user.id, + ) + + # execute ticket events + Observer::Ticket::Notification.transaction + #puts Delayed::Job.all.inspect + Delayed::Worker.new.work_off + + # because it's already open + assert(!OnlineNotification.all_seen?('Ticket', ticket5.id)) + assert(OnlineNotification.exists?(agent_user1, 'Ticket', ticket5.id, 'update', customer_user, false)) + assert(!OnlineNotification.exists?(agent_user1, 'Ticket', ticket5.id, 'update', customer_user, true)) + assert(OnlineNotification.exists?(agent_user2, 'Ticket', ticket5.id, 'update', customer_user, false)) + assert(!OnlineNotification.exists?(agent_user2, 'Ticket', ticket5.id, 'update', customer_user, true)) # merge tickets - also remove notifications of merged tickets - tickets[2].merge_to( - ticket_id: tickets[3].id, + tickets[0].merge_to( + ticket_id: tickets[1].id, user_id: 1, ) Delayed::Worker.new.work_off - notifications = OnlineNotification.list_by_object( 'Ticket', tickets[2].id ) - assert( !notifications.empty?, 'should have notifications') - assert( notification_seen_only_exists_exists(notifications), 'still not seen notifications for merged ticket available') - notifications = OnlineNotification.list_by_object( 'Ticket', tickets[3].id ) - assert( !notifications.empty?, 'should have notifications') - assert( !notification_seen_only_exists_exists(notifications), 'no notifications for master ticket available') + notifications = OnlineNotification.list_by_object('Ticket', tickets[0].id) + assert(!notifications.empty?, 'should have notifications') + assert(OnlineNotification.all_seen?('Ticket', tickets[0].id), 'still not seen notifications for merged ticket available') + + notifications = OnlineNotification.list_by_object('Ticket', tickets[1].id) + assert(!notifications.empty?, 'should have notifications') + assert(!OnlineNotification.all_seen?('Ticket', tickets[1].id), 'no notifications for master ticket available') # delete tickets tickets.each { |ticket| ticket_id = ticket.id ticket.destroy - found = Ticket.where( id: ticket_id ).first - assert( !found, 'Ticket destroyed') + found = Ticket.find_by(id: ticket_id) + assert(!found, 'Ticket destroyed') # check if notifications for ticket still exist Delayed::Worker.new.work_off - notifications = OnlineNotification.list_by_object( 'Ticket', ticket_id ) - assert( notifications.empty?, 'still notifications for destroyed ticket available') + notifications = OnlineNotification.list_by_object('Ticket', ticket_id) + assert(notifications.empty?, 'still notifications for destroyed ticket available') } end @@ -325,7 +376,7 @@ class OnlineNotificationTest < ActiveSupport::TestCase ticket1 = Ticket.create( title: 'some title', group: Group.lookup(name: 'Users'), - customer_id: 2, + customer_id: customer_user.id, state: Ticket::State.lookup(name: 'new'), priority: Ticket::Priority.lookup(name: '2 normal'), updated_by_id: 1, @@ -531,29 +582,4 @@ class OnlineNotificationTest < ActiveSupport::TestCase OnlineNotification.destroy_all end - def notification_check(online_notifications, checks) - checks.each { |check_item| - hit = false - online_notifications.each {|onine_notification| - - next if onine_notification['o_id'] != check_item[:o_id] - next if onine_notification['object'] != check_item[:object] - next if onine_notification['type'] != check_item[:type] - next if onine_notification['created_by_id'] != check_item[:created_by_id] - - hit = true - - break - } - #puts "--- #{online_notifications.inspect}" - assert( hit, "online notification exists not #{check_item.inspect}" ) - } - end - - def notification_seen_only_exists_exists(online_notifications) - online_notifications.each {|onine_notification| - return false if !onine_notification['seen'] - } - true - end end diff --git a/test/unit/ticket_notification_test.rb b/test/unit/ticket_notification_test.rb index 5f0885bac..7f65dfa61 100644 --- a/test/unit/ticket_notification_test.rb +++ b/test/unit/ticket_notification_test.rb @@ -92,8 +92,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(1, notification_check(ticket1, agent1, 'email'), ticket1.id) - assert_equal(1, notification_check(ticket1, agent2, 'email'), ticket1.id) + assert_equal(1, NotificationFactory.already_sent?(ticket1, agent1, 'email'), ticket1.id) + assert_equal(1, NotificationFactory.already_sent?(ticket1, agent2, 'email'), ticket1.id) # create ticket in group ticket1 = Ticket.create( @@ -127,8 +127,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(0, notification_check(ticket1, agent1, 'email'), ticket1.id) - assert_equal(1, notification_check(ticket1, agent2, 'email'), ticket1.id) + assert_equal(0, NotificationFactory.already_sent?(ticket1, agent1, 'email'), ticket1.id) + assert_equal(1, NotificationFactory.already_sent?(ticket1, agent2, 'email'), ticket1.id) end test 'ticket notification - simple' do @@ -165,8 +165,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(1, notification_check(ticket1, agent1, 'email'), ticket1.id) - assert_equal(1, notification_check(ticket1, agent2, 'email'), ticket1.id) + assert_equal(1, NotificationFactory.already_sent?(ticket1, agent1, 'email'), ticket1.id) + assert_equal(1, NotificationFactory.already_sent?(ticket1, agent2, 'email'), ticket1.id) # update ticket attributes ticket1.title = "#{ticket1.title} - #2" @@ -179,8 +179,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(2, notification_check(ticket1, agent1, 'email'), ticket1.id) - assert_equal(2, notification_check(ticket1, agent2, 'email'), ticket1.id) + assert_equal(2, NotificationFactory.already_sent?(ticket1, agent1, 'email'), ticket1.id) + assert_equal(2, NotificationFactory.already_sent?(ticket1, agent2, 'email'), ticket1.id) # add article to ticket Ticket::Article.create( @@ -201,8 +201,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to not to agent1 but to agent2 - assert_equal(2, notification_check(ticket1, agent1, 'email'), ticket1.id) - assert_equal(3, notification_check(ticket1, agent2, 'email'), ticket1.id) + assert_equal(2, NotificationFactory.already_sent?(ticket1, agent1, 'email'), ticket1.id) + assert_equal(3, NotificationFactory.already_sent?(ticket1, agent2, 'email'), ticket1.id) # update ticket by user ticket1.owner_id = agent1.id @@ -226,8 +226,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to not to agent1 but to agent2 - assert_equal(2, notification_check(ticket1, agent1, 'email'), ticket1.id) - assert_equal(3, notification_check(ticket1, agent2, 'email'), ticket1.id) + assert_equal(2, NotificationFactory.already_sent?(ticket1, agent1, 'email'), ticket1.id) + assert_equal(3, NotificationFactory.already_sent?(ticket1, agent2, 'email'), ticket1.id) # create ticket with agent1 as owner ticket2 = Ticket.create( @@ -261,8 +261,8 @@ class TicketNotificationTest < ActiveSupport::TestCase assert(ticket2, 'ticket created') # verify notifications to no one - assert_equal(0, notification_check(ticket2, agent1, 'email'), ticket2.id) - assert_equal(0, notification_check(ticket2, agent2, 'email'), ticket2.id) + assert_equal(0, NotificationFactory.already_sent?(ticket2, agent1, 'email'), ticket2.id) + assert_equal(0, NotificationFactory.already_sent?(ticket2, agent2, 'email'), ticket2.id) # update ticket ticket2.title = "#{ticket2.title} - #2" @@ -276,8 +276,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to no one - assert_equal(0, notification_check(ticket2, agent1, 'email'), ticket2.id) - assert_equal(0, notification_check(ticket2, agent2, 'email'), ticket2.id) + assert_equal(0, NotificationFactory.already_sent?(ticket2, agent1, 'email'), ticket2.id) + assert_equal(0, NotificationFactory.already_sent?(ticket2, agent2, 'email'), ticket2.id) # update ticket ticket2.title = "#{ticket2.title} - #3" @@ -291,8 +291,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 and not to agent2 - assert_equal(1, notification_check(ticket2, agent1, 'email'), ticket2.id) - assert_equal(0, notification_check(ticket2, agent2, 'email'), ticket2.id) + assert_equal(1, NotificationFactory.already_sent?(ticket2, agent1, 'email'), ticket2.id) + assert_equal(0, NotificationFactory.already_sent?(ticket2, agent2, 'email'), ticket2.id) # create ticket with agent2 and agent1 as owner ticket3 = Ticket.create( @@ -326,8 +326,8 @@ class TicketNotificationTest < ActiveSupport::TestCase assert(ticket3, 'ticket created') # verify notifications to agent1 and not to agent2 - assert_equal(1, notification_check(ticket3, agent1, 'email'), ticket3.id) - assert_equal(0, notification_check(ticket3, agent2, 'email'), ticket3.id) + assert_equal(1, NotificationFactory.already_sent?(ticket3, agent1, 'email'), ticket3.id) + assert_equal(0, NotificationFactory.already_sent?(ticket3, agent2, 'email'), ticket3.id) # update ticket ticket3.title = "#{ticket3.title} - #2" @@ -341,8 +341,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to no one - assert_equal(1, notification_check(ticket3, agent1, 'email'), ticket3.id) - assert_equal(0, notification_check(ticket3, agent2, 'email'), ticket3.id) + assert_equal(1, NotificationFactory.already_sent?(ticket3, agent1, 'email'), ticket3.id) + assert_equal(0, NotificationFactory.already_sent?(ticket3, agent2, 'email'), ticket3.id) # update ticket ticket3.title = "#{ticket3.title} - #3" @@ -356,8 +356,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 and not to agent2 - assert_equal(2, notification_check(ticket3, agent1, 'email'), ticket3.id) - assert_equal(0, notification_check(ticket3, agent2, 'email'), ticket3.id) + assert_equal(2, NotificationFactory.already_sent?(ticket3, agent1, 'email'), ticket3.id) + assert_equal(0, NotificationFactory.already_sent?(ticket3, agent2, 'email'), ticket3.id) # update article / not notification should be sent article_inbound.internal = true @@ -369,8 +369,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications not to agent1 and not to agent2 - assert_equal(2, notification_check(ticket3, agent1, 'email'), ticket3.id) - assert_equal(0, notification_check(ticket3, agent2, 'email'), ticket3.id) + assert_equal(2, NotificationFactory.already_sent?(ticket3, agent1, 'email'), ticket3.id) + assert_equal(0, NotificationFactory.already_sent?(ticket3, agent2, 'email'), ticket3.id) delete = ticket1.destroy assert(delete, 'ticket1 destroy') @@ -432,8 +432,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(0, notification_check(ticket1, agent1, 'email'), ticket1.id) - assert_equal(1, notification_check(ticket1, agent2, 'email'), ticket1.id) + assert_equal(0, NotificationFactory.already_sent?(ticket1, agent1, 'email'), ticket1.id) + assert_equal(1, NotificationFactory.already_sent?(ticket1, agent2, 'email'), ticket1.id) # update ticket attributes ticket1.title = "#{ticket1.title} - #2" @@ -446,8 +446,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(0, notification_check(ticket1, agent1, 'email'), ticket1.id) - assert_equal(2, notification_check(ticket1, agent2, 'email'), ticket1.id) + assert_equal(0, NotificationFactory.already_sent?(ticket1, agent1, 'email'), ticket1.id) + assert_equal(2, NotificationFactory.already_sent?(ticket1, agent2, 'email'), ticket1.id) # create ticket in group ticket2 = Ticket.create( @@ -480,8 +480,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(1, notification_check(ticket2, agent1, 'email'), ticket2.id) - assert_equal(1, notification_check(ticket2, agent2, 'email'), ticket2.id) + assert_equal(1, NotificationFactory.already_sent?(ticket2, agent1, 'email'), ticket2.id) + assert_equal(1, NotificationFactory.already_sent?(ticket2, agent2, 'email'), ticket2.id) # update ticket attributes ticket2.title = "#{ticket2.title} - #2" @@ -494,8 +494,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(2, notification_check(ticket2, agent1, 'email'), ticket2.id) - assert_equal(2, notification_check(ticket2, agent2, 'email'), ticket2.id) + assert_equal(2, NotificationFactory.already_sent?(ticket2, agent1, 'email'), ticket2.id) + assert_equal(2, NotificationFactory.already_sent?(ticket2, agent2, 'email'), ticket2.id) # create ticket in group ticket3 = Ticket.create( @@ -528,8 +528,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(0, notification_check(ticket3, agent1, 'email'), ticket3.id) - assert_equal(1, notification_check(ticket3, agent2, 'email'), ticket3.id) + assert_equal(0, NotificationFactory.already_sent?(ticket3, agent1, 'email'), ticket3.id) + assert_equal(1, NotificationFactory.already_sent?(ticket3, agent2, 'email'), ticket3.id) # update ticket attributes ticket3.title = "#{ticket3.title} - #2" @@ -542,8 +542,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(0, notification_check(ticket3, agent1, 'email'), ticket3.id) - assert_equal(2, notification_check(ticket3, agent2, 'email'), ticket3.id) + assert_equal(0, NotificationFactory.already_sent?(ticket3, agent1, 'email'), ticket3.id) + assert_equal(2, NotificationFactory.already_sent?(ticket3, agent2, 'email'), ticket3.id) agent1.preferences['notification_config']['matrix']['create']['criteria']['owned_by_me'] = true agent1.preferences['notification_config']['matrix']['create']['criteria']['owned_by_nobody'] = false @@ -594,8 +594,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(1, notification_check(ticket4, agent1, 'email'), ticket4.id) - assert_equal(1, notification_check(ticket4, agent2, 'email'), ticket4.id) + assert_equal(1, NotificationFactory.already_sent?(ticket4, agent1, 'email'), ticket4.id) + assert_equal(1, NotificationFactory.already_sent?(ticket4, agent2, 'email'), ticket4.id) # update ticket attributes ticket4.title = "#{ticket4.title} - #2" @@ -608,8 +608,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(2, notification_check(ticket4, agent1, 'email'), ticket4.id) - assert_equal(2, notification_check(ticket4, agent2, 'email'), ticket4.id) + assert_equal(2, NotificationFactory.already_sent?(ticket4, agent1, 'email'), ticket4.id) + assert_equal(2, NotificationFactory.already_sent?(ticket4, agent2, 'email'), ticket4.id) agent1.preferences['notification_config']['matrix']['create']['criteria']['owned_by_me'] = true agent1.preferences['notification_config']['matrix']['create']['criteria']['owned_by_nobody'] = false @@ -660,8 +660,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(1, notification_check(ticket5, agent1, 'email'), ticket5.id) - assert_equal(0, notification_check(ticket5, agent2, 'email'), ticket5.id) + assert_equal(1, NotificationFactory.already_sent?(ticket5, agent1, 'email'), ticket5.id) + assert_equal(0, NotificationFactory.already_sent?(ticket5, agent2, 'email'), ticket5.id) # update ticket attributes ticket5.title = "#{ticket5.title} - #2" @@ -674,8 +674,8 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(2, notification_check(ticket5, agent1, 'email'), ticket5.id) - assert_equal(0, notification_check(ticket5, agent2, 'email'), ticket5.id) + assert_equal(2, NotificationFactory.already_sent?(ticket5, agent1, 'email'), ticket5.id) + assert_equal(0, NotificationFactory.already_sent?(ticket5, agent2, 'email'), ticket5.id) agent1.preferences['notification_config']['matrix']['create']['criteria']['owned_by_me'] = true agent1.preferences['notification_config']['matrix']['create']['criteria']['owned_by_nobody'] = false @@ -727,10 +727,10 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(1, notification_check(ticket6, agent1, 'email'), ticket6.id) - assert_equal(1, notification_check(ticket6, agent1, 'online'), ticket6.id) - assert_equal(0, notification_check(ticket6, agent2, 'email'), ticket6.id) - assert_equal(0, notification_check(ticket6, agent2, 'online'), ticket6.id) + assert_equal(1, NotificationFactory.already_sent?(ticket6, agent1, 'email'), ticket6.id) + assert_equal(1, NotificationFactory.already_sent?(ticket6, agent1, 'online'), ticket6.id) + assert_equal(0, NotificationFactory.already_sent?(ticket6, agent2, 'email'), ticket6.id) + assert_equal(0, NotificationFactory.already_sent?(ticket6, agent2, 'online'), ticket6.id) # update ticket attributes ticket6.title = "#{ticket6.title} - #2" @@ -743,10 +743,10 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(2, notification_check(ticket6, agent1, 'email'), ticket6.id) - assert_equal(2, notification_check(ticket6, agent1, 'online'), ticket6.id) - assert_equal(0, notification_check(ticket6, agent2, 'email'), ticket6.id) - assert_equal(0, notification_check(ticket6, agent2, 'online'), ticket6.id) + assert_equal(2, NotificationFactory.already_sent?(ticket6, agent1, 'email'), ticket6.id) + assert_equal(2, NotificationFactory.already_sent?(ticket6, agent1, 'online'), ticket6.id) + assert_equal(0, NotificationFactory.already_sent?(ticket6, agent2, 'email'), ticket6.id) + assert_equal(0, NotificationFactory.already_sent?(ticket6, agent2, 'online'), ticket6.id) agent1.preferences['notification_config']['matrix']['create']['criteria']['owned_by_me'] = true agent1.preferences['notification_config']['matrix']['create']['criteria']['owned_by_nobody'] = false @@ -806,10 +806,10 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(0, notification_check(ticket7, agent1, 'email'), ticket7.id) - assert_equal(1, notification_check(ticket7, agent1, 'online'), ticket7.id) - assert_equal(0, notification_check(ticket7, agent2, 'email'), ticket7.id) - assert_equal(0, notification_check(ticket7, agent2, 'online'), ticket7.id) + assert_equal(0, NotificationFactory.already_sent?(ticket7, agent1, 'email'), ticket7.id) + assert_equal(1, NotificationFactory.already_sent?(ticket7, agent1, 'online'), ticket7.id) + assert_equal(0, NotificationFactory.already_sent?(ticket7, agent2, 'email'), ticket7.id) + assert_equal(0, NotificationFactory.already_sent?(ticket7, agent2, 'online'), ticket7.id) # update ticket attributes ticket7.title = "#{ticket7.title} - #2" @@ -822,10 +822,10 @@ class TicketNotificationTest < ActiveSupport::TestCase Delayed::Worker.new.work_off # verify notifications to agent1 + agent2 - assert_equal(0, notification_check(ticket7, agent1, 'email'), ticket7.id) - assert_equal(2, notification_check(ticket7, agent1, 'online'), ticket7.id) - assert_equal(0, notification_check(ticket7, agent2, 'email'), ticket7.id) - assert_equal(0, notification_check(ticket7, agent2, 'online'), ticket7.id) + assert_equal(0, NotificationFactory.already_sent?(ticket7, agent1, 'email'), ticket7.id) + assert_equal(2, NotificationFactory.already_sent?(ticket7, agent1, 'online'), ticket7.id) + assert_equal(0, NotificationFactory.already_sent?(ticket7, agent2, 'email'), ticket7.id) + assert_equal(0, NotificationFactory.already_sent?(ticket7, agent2, 'online'), ticket7.id) end @@ -1066,16 +1066,4 @@ class TicketNotificationTest < ActiveSupport::TestCase end - def notification_check(ticket, recipient, type) - 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 - next if item['value_to'] !~ /#{type}/i - count += 1 - } - count - end end