From e2dff052a54d1e915265e3adcc946a22c319e82e Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Thu, 3 Sep 2015 11:14:09 +0200 Subject: [PATCH] Improved online notification. Just leave unseen if I'm owner and somebody else sets pending_reminder. --- .../ticket/notification/background_job.rb | 2 +- .../background_job.rb | 12 ++- app/models/online_notification.rb | 38 +++----- app/models/ticket.rb | 30 +++++-- test/unit/online_notifiaction_test.rb | 88 ++++++++++++++++++- 5 files changed, 135 insertions(+), 35 deletions(-) diff --git a/app/models/observer/ticket/notification/background_job.rb b/app/models/observer/ticket/notification/background_job.rb index a41108815..abaa86a7b 100644 --- a/app/models/observer/ticket/notification/background_job.rb +++ b/app/models/observer/ticket/notification/background_job.rb @@ -65,7 +65,7 @@ class Observer::Ticket::Notification::BackgroundJob # create desktop notification # create online notification - seen = ticket.online_notification_seen_state + seen = ticket.online_notification_seen_state(user.id) OnlineNotification.add( type: @p[:type], object: 'Ticket', diff --git a/app/models/observer/ticket/online_notification_seen/background_job.rb b/app/models/observer/ticket/online_notification_seen/background_job.rb index 6bf4c684b..1b5b4dbab 100644 --- a/app/models/observer/ticket/online_notification_seen/background_job.rb +++ b/app/models/observer/ticket/online_notification_seen/background_job.rb @@ -6,6 +6,16 @@ class Observer::Ticket::OnlineNotificationSeen::BackgroundJob def perform # set all online notifications to seen - OnlineNotification.seen_by_object( 'Ticket', @ticket_id ) + ActiveRecord::Base.transaction do + ticket = Ticket.lookup(id: @ticket_id) + OnlineNotification.list_by_object('Ticket', @ticket_id).each {|notification| + next if notification.seen + seen = ticket.online_notification_seen_state(notification.user_id) + next if !seen + next if seen == notification.seen + notification.seen = true + notification.save + } + end end end diff --git a/app/models/online_notification.rb b/app/models/online_notification.rb index 5ec4ae2ab..97a8e9e6d 100644 --- a/app/models/online_notification.rb +++ b/app/models/online_notification.rb @@ -14,12 +14,12 @@ class OnlineNotification < ApplicationModel add a new online notification for this user OnlineNotification.add( - :type => 'Assigned to you', - :object => 'Ticket', - :o_id => ticket.id, - :seen => false, - :created_by_id => 1, - :user_id => 2, + type: 'Assigned to you', + object: 'Ticket', + o_id: ticket.id, + seen: false, + created_by_id: 1, + user_id: 2, ) =end @@ -51,7 +51,7 @@ add a new online notification for this user mark online notification as seen OnlineNotification.seen( - :id => 2, + id: 2, ) =end @@ -93,9 +93,9 @@ return all online notifications of an user .limit( limit ) list = [] notifications.each do |item| - data = item.attributes - data['object'] = ObjectLookup.by_id( data['object_lookup_id'] ) - data['type'] = TypeLookup.by_id( data['type_lookup_id'] ) + data = item.attributes + data['object'] = ObjectLookup.by_id( data['object_lookup_id'] ) + data['type'] = TypeLookup.by_id( data['type_lookup_id'] ) data.delete('object_lookup_id') data.delete('type_lookup_id') list.push data @@ -119,24 +119,14 @@ return all online notifications of an object ) .order( 'created_at DESC, id DESC' ) # rubocop:disable Style/MultilineOperationIndentation .limit( 10_000 ) # rubocop:disable Style/MultilineOperationIndentation - - list = [] - notifications.each do |item| - data = item.attributes - data['object'] = ObjectLookup.by_id( data['object_lookup_id'] ) - data['type'] = TypeLookup.by_id( data['type_lookup_id'] ) - data.delete('object_lookup_id') - data.delete('type_lookup_id') - list.push data - end - list + notifications end =begin mark online notification as seen by object - OnlineNotification.seen_by_object( 'Ticket', 123 ) + OnlineNotification.seen_by_object( 'Ticket', 123, user_id ) =end @@ -163,8 +153,8 @@ return all online notifications of an user with assets returns: list = { - :stream => notifications, - :assets => assets + stream: notifications, + assets: assets, } =end diff --git a/app/models/ticket.rb b/app/models/ticket.rb index c2f22aee4..2ee46b9d3 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -165,8 +165,8 @@ merge tickets ticket = Ticket.find(123) result = ticket.merge_to( - :ticket_id => 123, - :user_id => 123, + ticket_id: 123, + user_id: 123, ) returns @@ -222,10 +222,19 @@ returns =begin -know if online notifcation should be shown as already seen with current state +check if online notifcation should be shown in general as already seen with current state ticket = Ticket.find(1) - seen = ticket.online_notification_seen_state + seen = ticket.online_notification_seen_state(user_id_check) + +returns + + result = true # or false + +check if online notifcation should be shown for this user as already seen with current state + + ticket = Ticket.find(1) + seen = ticket.online_notification_seen_state(check_user_id) returns @@ -233,7 +242,7 @@ returns =end - def online_notification_seen_state + def online_notification_seen_state(user_id_check = nil) state = Ticket::State.lookup( id: state_id ) state_type = Ticket::StateType.lookup( id: state.state_type_id ) @@ -244,7 +253,14 @@ returns end # set all to seen if new state is pending reminder state - return true if state_type.name == 'pending reminder' + if state_type.name == 'pending reminder' + if user_id_check + return false if owner_id == 1 + return false if updated_by_id != owner_id && user_id_check == owner_id + return true + end + return true + end # set all to seen if new state is a closed or merged state return true if state_type.name == 'closed' @@ -260,9 +276,7 @@ returns end def check_title - return if !title - title.gsub!(/\s|\t|\r/, ' ') end diff --git a/test/unit/online_notifiaction_test.rb b/test/unit/online_notifiaction_test.rb index dba6ca958..8d850a295 100644 --- a/test/unit/online_notifiaction_test.rb +++ b/test/unit/online_notifiaction_test.rb @@ -30,7 +30,7 @@ class OnlineNotificationTest < ActiveSupport::TestCase ) customer_user = User.lookup( login: 'nicole.braun@zammad.org' ) - test 'ticket notifiaction' do + test 'ticket notification' do tests = [ # test 1 @@ -321,6 +321,92 @@ class OnlineNotificationTest < ActiveSupport::TestCase } end + test 'ticket notification item check' do + ticket1 = Ticket.create( + title: "some title", + group: Group.lookup(name: 'Users'), + customer_id: 2, + state: Ticket::State.lookup(name: 'new'), + priority: Ticket::Priority.lookup(name: '2 normal'), + updated_by_id: 1, + created_by_id: 1, + ) + assert( ticket1, 'ticket created' ) + 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 article_inbound', + internal: false, + sender: Ticket::Article::Sender.lookup(name: 'Customer'), + type: Ticket::Article::Type.lookup(name: 'email'), + updated_by_id: 1, + created_by_id: 1, + ) + + assert_equal(ticket1.online_notification_seen_state, false) + assert_equal(ticket1.online_notification_seen_state(agent_user1), false) + assert_equal(ticket1.online_notification_seen_state(agent_user2), false) + + # pending reminder, just let new owner to unseed + ticket1.update_attributes( + owner_id: agent_user1.id, + state: Ticket::State.lookup(name: 'pending reminder'), + updated_by_id: agent_user2.id, + ) + + assert_equal(ticket1.online_notification_seen_state, true) + assert_equal(ticket1.online_notification_seen_state(agent_user1.id), false) + assert_equal(ticket1.online_notification_seen_state(agent_user2.id), true) + + # pending reminder, just let new owner to unseed + ticket1.update_attributes( + owner_id: 1, + state: Ticket::State.lookup(name: 'pending reminder'), + updated_by_id: agent_user2.id, + ) + + assert_equal(ticket1.online_notification_seen_state, true) + assert_equal(ticket1.online_notification_seen_state(agent_user1.id), false) + assert_equal(ticket1.online_notification_seen_state(agent_user2.id), false) + + # pending reminder, self done, all to unseed + ticket1.update_attributes( + owner_id: agent_user1.id, + state: Ticket::State.lookup(name: 'pending reminder'), + updated_by_id: agent_user1.id, + ) + + assert_equal(ticket1.online_notification_seen_state, true) + assert_equal(ticket1.online_notification_seen_state(agent_user1.id), true) + assert_equal(ticket1.online_notification_seen_state(agent_user2.id), true) + + # pending close, all to unseen + ticket1.update_attributes( + owner_id: agent_user1.id, + state: Ticket::State.lookup(name: 'pending close'), + updated_by_id: agent_user2.id, + ) + + assert_equal(ticket1.online_notification_seen_state, true) + assert_equal(ticket1.online_notification_seen_state(agent_user1.id), true) + assert_equal(ticket1.online_notification_seen_state(agent_user2.id), true) + + # to open, all to seen + ticket1.update_attributes( + owner_id: agent_user1.id, + state: Ticket::State.lookup(name: 'open'), + updated_by_id: agent_user2.id, + ) + + assert_equal(ticket1.online_notification_seen_state, false) + assert_equal(ticket1.online_notification_seen_state(agent_user1.id), false) + assert_equal(ticket1.online_notification_seen_state(agent_user2.id), false) + + end + def notification_check( online_notifications, checks ) checks.each { |check_item| hit = false