From ccc61277f81ac8e9198f6f1e6edf804c0f585a9f Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Sun, 21 Feb 2016 12:43:16 +0100 Subject: [PATCH] Improved OnlineNotification.cleanup, just delete notification after 35 min. if user read it by him self. If auto seen is set (by some other user), just delete the online notification after 8 hours. --- .../widget/online_notification.coffee | 10 +- .../online_notifications_controller.rb | 4 +- .../ticket/online_notification_seen.rb | 2 +- .../background_job.rb | 6 +- app/models/online_notification.rb | 34 +++++-- db/migrate/20120101000001_create_base.rb | 1 + ...0002_add_updated_by_online_notification.rb | 6 ++ test/unit/online_notifiaction_test.rb | 91 +++++++++++++++++++ 8 files changed, 138 insertions(+), 16 deletions(-) create mode 100644 db/migrate/20160220000002_add_updated_by_online_notification.rb diff --git a/app/assets/javascripts/app/controllers/widget/online_notification.coffee b/app/assets/javascripts/app/controllers/widget/online_notification.coffee index e46a6b6ac..7bc4fb792 100644 --- a/app/assets/javascripts/app/controllers/widget/online_notification.coffee +++ b/app/assets/javascripts/app/controllers/widget/online_notification.coffee @@ -11,7 +11,7 @@ class App.OnlineNotificationWidget extends App.Controller @bind 'OnlineNotification::changed', => @delay( => @fetch() - 1600 + 2600 'online-notification-changed' ) @@ -38,12 +38,12 @@ class App.OnlineNotificationWidget extends App.Controller if @access() @createContainer() - @subscribeId = App.OnlineNotification.subscribe( @updateContent ) + @subscribeId = App.OnlineNotification.subscribe(@updateContent) release: -> @removeContainer() $(window).off 'click.notifications' - App.OnlineNotification.unsubscribe( @subscribeId ) + App.OnlineNotification.unsubscribe(@subscribeId) access: -> return false if !@Session.get() @@ -105,12 +105,12 @@ class App.OnlineNotificationWidget extends App.Controller fetch: => load = (items) => @fetchedData = true - App.OnlineNotification.refresh( items, { clear: true } ) + App.OnlineNotification.refresh(items, { clear: true }) @updateContent() App.OnlineNotification.fetchFull(load) updateContent: => - items = App.OnlineNotification.search( sortBy: 'created_at', order: 'DESC' ) + items = App.OnlineNotification.search(sortBy: 'created_at', order: 'DESC') counter = 0 for item in items if !item.seen diff --git a/app/controllers/online_notifications_controller.rb b/app/controllers/online_notifications_controller.rb index ae20a6981..594218858 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, 50) + render json: OnlineNotification.list_full(current_user, 75) return end - notifications = OnlineNotification.list(current_user, 50) + notifications = OnlineNotification.list(current_user, 75) model_index_render_result(notifications) end diff --git a/app/models/observer/ticket/online_notification_seen.rb b/app/models/observer/ticket/online_notification_seen.rb index 4ed8e791c..fe84057cc 100644 --- a/app/models/observer/ticket/online_notification_seen.rb +++ b/app/models/observer/ticket/online_notification_seen.rb @@ -23,6 +23,6 @@ class Observer::Ticket::OnlineNotificationSeen < ActiveRecord::Observer # set all online notifications to seen # send background job - Delayed::Job.enqueue( Observer::Ticket::OnlineNotificationSeen::BackgroundJob.new( record.id ) ) + Delayed::Job.enqueue( Observer::Ticket::OnlineNotificationSeen::BackgroundJob.new( record.id, record.updated_by_id ) ) end end 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 1b5b4dbab..4494fd0e0 100644 --- a/app/models/observer/ticket/online_notification_seen/background_job.rb +++ b/app/models/observer/ticket/online_notification_seen/background_job.rb @@ -1,6 +1,7 @@ class Observer::Ticket::OnlineNotificationSeen::BackgroundJob - def initialize(id) - @ticket_id = id + def initialize(ticket_id, user_id) + @ticket_id = ticket_id + @user_id = user_id || 1 end def perform @@ -14,6 +15,7 @@ class Observer::Ticket::OnlineNotificationSeen::BackgroundJob next if !seen next if seen == notification.seen notification.seen = true + notification.updated_by_id = @user_id notification.save } end diff --git a/app/models/online_notification.rb b/app/models/online_notification.rb index be83064bd..1a639b223 100644 --- a/app/models/online_notification.rb +++ b/app/models/online_notification.rb @@ -18,8 +18,11 @@ add a new online notification for this user object: 'Ticket', o_id: ticket.id, seen: false, - created_by_id: 1, user_id: 2, + created_by_id: 1, + updated_by_id: 1, + created_at: Time.zone.now, + updated_at: Time.zone.now, ) =end @@ -40,7 +43,10 @@ add a new online notification for this user type_lookup_id: type_id, seen: data[:seen], user_id: data[:user_id], - created_by_id: data[:created_by_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], } OnlineNotification.create(record) @@ -203,12 +209,28 @@ cleanup old online notifications OnlineNotification.cleanup +with dedicated times + + max_age = Time.zone.now - 9.months + max_own_seen = Time.zone.now - 35.minutes + max_auto_seen = Time.zone.now - 8.hours + + OnlineNotification.cleanup(max_age, max_own_seen, max_auto_seen) + =end - def self.cleanup - OnlineNotification.where('created_at < ?', Time.zone.now - 6.months).delete_all - OnlineNotification.where('seen = ? AND created_at < ?', true, Time.zone.now - 4.hours).delete_all - OnlineNotification.where('seen = ? AND updated_at < ?', true, Time.zone.now - 1.hour).delete_all + def self.cleanup(max_age = Time.zone.now - 9.months, max_own_seen = Time.zone.now - 35.minutes, max_auto_seen = Time.zone.now - 8.hours) + OnlineNotification.where('created_at < ?', max_age).delete_all + OnlineNotification.where('seen = ? AND updated_at < ?', true, max_own_seen).each {|notification| + + # delete own "seen" notificatons after 1 hour + next if notification.user_id == notification.updated_by_id && notification.updated_at > max_own_seen + + # delete notificatons which are set to "seen" by somebody else after 8 hour + next if notification.user_id != notification.updated_by_id && notification.updated_at > max_auto_seen + + notification.delete + } # notify all agents User.of_role('Agent').each {|user| diff --git a/db/migrate/20120101000001_create_base.rb b/db/migrate/20120101000001_create_base.rb index 0d0614c2b..7322ec71c 100644 --- a/db/migrate/20120101000001_create_base.rb +++ b/db/migrate/20120101000001_create_base.rb @@ -394,6 +394,7 @@ class CreateBase < ActiveRecord::Migration t.integer :type_lookup_id, null: false t.integer :user_id, null: false t.boolean :seen, null: false, default: false + t.integer :updated_by_id, null: false t.integer :created_by_id, null: false t.timestamps null: false end diff --git a/db/migrate/20160220000002_add_updated_by_online_notification.rb b/db/migrate/20160220000002_add_updated_by_online_notification.rb new file mode 100644 index 000000000..efc8975d6 --- /dev/null +++ b/db/migrate/20160220000002_add_updated_by_online_notification.rb @@ -0,0 +1,6 @@ +class AddUpdatedByOnlineNotification < ActiveRecord::Migration + def up + return if OnlineNotification.column_names.include?('updated_by_id') + add_column :online_notifications, :updated_by_id, :integer + end +end diff --git a/test/unit/online_notifiaction_test.rb b/test/unit/online_notifiaction_test.rb index 4abaed355..c8d0bcac9 100644 --- a/test/unit/online_notifiaction_test.rb +++ b/test/unit/online_notifiaction_test.rb @@ -440,6 +440,97 @@ class OnlineNotificationTest < ActiveSupport::TestCase end + test 'cleanup check' do + online_notification1 = OnlineNotification.add( + type: 'create', + object: 'Ticket', + o_id: 123, + seen: false, + user_id: agent_user1.id, + created_by_id: 1, + updated_by_id: 1, + created_at: Time.zone.now - 10.months, + updated_at: Time.zone.now - 10.months, + ) + online_notification2 = OnlineNotification.add( + type: 'create', + object: 'Ticket', + o_id: 123, + seen: true, + user_id: agent_user1.id, + created_by_id: 1, + updated_by_id: 1, + created_at: Time.zone.now - 10.months, + updated_at: Time.zone.now - 10.months, + ) + online_notification3 = OnlineNotification.add( + type: 'create', + object: 'Ticket', + o_id: 123, + seen: false, + user_id: agent_user1.id, + created_by_id: 1, + updated_by_id: 1, + created_at: Time.zone.now - 2.days, + updated_at: Time.zone.now - 2.days, + ) + online_notification4 = OnlineNotification.add( + type: 'create', + object: 'Ticket', + o_id: 123, + seen: true, + user_id: agent_user1.id, + created_by_id: agent_user1.id, + updated_by_id: agent_user1.id, + created_at: Time.zone.now - 2.days, + updated_at: Time.zone.now - 2.days, + ) + online_notification5 = OnlineNotification.add( + type: 'create', + object: 'Ticket', + o_id: 123, + seen: true, + user_id: agent_user1.id, + created_by_id: agent_user2.id, + updated_by_id: agent_user2.id, + created_at: Time.zone.now - 2.days, + updated_at: Time.zone.now - 2.days, + ) + online_notification6 = OnlineNotification.add( + type: 'create', + object: 'Ticket', + o_id: 123, + seen: true, + user_id: agent_user1.id, + created_by_id: agent_user1.id, + updated_by_id: agent_user1.id, + created_at: Time.zone.now - 10.minutes, + updated_at: Time.zone.now - 10.minutes, + ) + online_notification7 = OnlineNotification.add( + type: 'create', + object: 'Ticket', + o_id: 123, + seen: true, + user_id: agent_user1.id, + created_by_id: agent_user2.id, + updated_by_id: agent_user2.id, + created_at: Time.zone.now - 10.minutes, + updated_at: Time.zone.now - 10.minutes, + ) + + OnlineNotification.cleanup + + assert(!OnlineNotification.find_by(id: online_notification1.id)) + assert(!OnlineNotification.find_by(id: online_notification2.id)) + assert(OnlineNotification.find_by(id: online_notification3.id)) + assert(!OnlineNotification.find_by(id: online_notification4.id)) + assert(!OnlineNotification.find_by(id: online_notification5.id)) + assert(OnlineNotification.find_by(id: online_notification6.id)) + assert(OnlineNotification.find_by(id: online_notification7.id)) + + end + def notification_check(online_notifications, checks) checks.each { |check_item| hit = false