diff --git a/app/models/observer/ticket/notification/background_job.rb b/app/models/observer/ticket/notification/background_job.rb index 6f3ef8294..b4b277e13 100644 --- a/app/models/observer/ticket/notification/background_job.rb +++ b/app/models/observer/ticket/notification/background_job.rb @@ -64,11 +64,12 @@ class Observer::Ticket::Notification::BackgroundJob # create desktop notification # create online notification + seen = ticket.online_notification_seen_state OnlineNotification.add( :type => @p[:type], :object => 'Ticket', :o_id => ticket.id, - :seen => false, + :seen => seen, :created_by_id => ticket.updated_by_id || 1, :user_id => user.id, ) diff --git a/app/models/observer/ticket/online_notification_seen.rb b/app/models/observer/ticket/online_notification_seen.rb new file mode 100644 index 000000000..df1e9ee02 --- /dev/null +++ b/app/models/observer/ticket/online_notification_seen.rb @@ -0,0 +1,26 @@ +# Copyright (C) 2012-2014 Zammad Foundation, http://zammad-foundation.org/ + +class Observer::Ticket::OnlineNotificationSeen < ActiveRecord::Observer + observe 'ticket' + + def after_create(record) + _check(record) + end + + def after_update(record) + _check(record) + end + + private + def _check(record) + + # return if we run import mode + return if Setting.get('import_mode') + + # check if existing online notifications for this ticket should be set to seen + return true if !record.online_notification_seen_state + + # set all online notifications to seen + OnlineNotification.seen_by_object( 'Ticket', record.id ) + end +end \ No newline at end of file diff --git a/app/models/online_notification.rb b/app/models/online_notification.rb index e53bea887..1b645250e 100644 --- a/app/models/online_notification.rb +++ b/app/models/online_notification.rb @@ -108,31 +108,16 @@ return all online notifications of an object notifications = OnlineNotification.list_by_object( 'Ticket', 123 ) -optional with seend attribute - - notifications = OnlineNotification.list_by_object( 'Ticket', 123, false ) - - =end - def self.list_by_object( object_name, o_id, seen = nil) + def self.list_by_object( object_name, o_id) object_id = ObjectLookup.by_name( object_name ) - if seen == nil - notifications = OnlineNotification.where( - :object_lookup_id => object_id, - :o_id => o_id, - ). - order( 'created_at DESC, id DESC' ). - limit( 10_000 ) - else - notifications = OnlineNotification.where( - :object_lookup_id => object_id, - :o_id => o_id, - :seen => seen, - ). - order( 'created_at DESC, id DESC' ). - limit( 10_000 ) - end + notifications = OnlineNotification.where( + :object_lookup_id => object_id, + :o_id => o_id, + ). + order( 'created_at DESC, id DESC' ). + limit( 10_000 ) list = [] notifications.each do |item| data = item.attributes @@ -164,6 +149,7 @@ mark online notification as seen by object notification.seen = true notification.save end + true end =begin diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 612d03d4e..20de3f9b6 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -170,11 +170,27 @@ returns # save ticket self.save + end - # set all online notifications to seen - OnlineNotification.seen_by_object( 'Ticket', self.id ) +=begin - true +know if online notifcation should be shown as already seen + + ticket = Ticket.find(1) + seen = ticket.online_notification_seen_state + +returns + + result = [user1, user2, ...] + +=end + + def online_notification_seen_state + state = Ticket::State.lookup( :id => self.state_id ) + state_type = Ticket::StateType.lookup( :id => state.state_type_id ) + return true if state_type.name == 'closed' + return true if state_type.name == 'merged' + false end private diff --git a/config/application.rb b/config/application.rb index 556d87080..16a2b70ac 100644 --- a/config/application.rb +++ b/config/application.rb @@ -41,6 +41,7 @@ module Zammad 'observer::_ticket::_reset_new_state', 'observer::_ticket::_escalation_calculation', 'observer::_ticket::_ref_object_touch', + 'observer::_ticket::_online_notification_seen', 'observer::_tag::_ticket_history', 'observer::_user::_ref_object_touch', 'observer::_user::_geo', diff --git a/test/unit/online_notifiaction_test.rb b/test/unit/online_notifiaction_test.rb index 66b14e023..b7d85ef05 100644 --- a/test/unit/online_notifiaction_test.rb +++ b/test/unit/online_notifiaction_test.rb @@ -41,7 +41,7 @@ class OnlineNotificationTest < ActiveSupport::TestCase :customer_id => customer_user.id, :owner_id => User.lookup( :login => '-' ).id, :title => 'Unit Test 1 (äöüß)!', - :state_id => Ticket::State.lookup( :name => 'new' ).id, + :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, @@ -55,6 +55,9 @@ class OnlineNotificationTest < ActiveSupport::TestCase :body => 'Unit Test 123', :internal => false }, + :online_notification => { + :seen_only_exists => true, + }, }, :update => { :ticket => { @@ -63,6 +66,9 @@ class OnlineNotificationTest < ActiveSupport::TestCase :priority_id => Ticket::Priority.lookup( :name => '1 low' ).id, :updated_by_id => customer_user.id, }, + :online_notification => { + :seen_only_exists => false, + }, }, :check => [ { @@ -75,7 +81,7 @@ class OnlineNotificationTest < ActiveSupport::TestCase :object => 'Ticket', :created_by_id => customer_user.id, }, - ] + ], }, # test 2 @@ -100,14 +106,20 @@ class OnlineNotificationTest < ActiveSupport::TestCase :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 => 'open' ).id, + :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 => [ { @@ -120,7 +132,109 @@ class OnlineNotificationTest < ActiveSupport::TestCase :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, + }, + ], }, ] tickets = [] @@ -140,6 +254,17 @@ class OnlineNotificationTest < ActiveSupport::TestCase #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 + # update ticket if test[:update][:ticket] ticket.update_attributes( test[:update][:ticket] ) @@ -155,19 +280,31 @@ class OnlineNotificationTest < ActiveSupport::TestCase # check online notifications notification_check( OnlineNotification.list(agent_user2, 10), test[:check] ) + + # check online notifications + if test[:update][:online_notification] + 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 + end } # merge tickets - also remove notifications of merged tickets - tickets[0].merge_to( - :ticket_id => tickets[1].id, + tickets[2].merge_to( + :ticket_id => tickets[3].id, :user_id => 1, ) - notifications = OnlineNotification.list_by_object( 'Ticket', tickets[0].id, false ) - assert( notifications.empty?, "still not seen notifications for merged ticket available") - - notifications = OnlineNotification.list_by_object( 'Ticket', tickets[1].id, true ) - assert( notifications.empty?, "no notifications for master ticket available") + 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") # delete tickets tickets.each { |ticket| @@ -196,8 +333,15 @@ class OnlineNotificationTest < ActiveSupport::TestCase end end } - assert( hit, "online notification exists #{ check_item.inspect }" ) + #puts "--- #{onine_notifications.inspect}" + assert( hit, "online notification exists not #{ check_item.inspect }" ) } end + def notification_seen_only_exists_exists( onine_notifications ) + onine_notifications.each {|onine_notification| + return false if !onine_notification['seen'] + } + true + end end \ No newline at end of file