Browse Source

Fixed issue #2405 - Unable to load Zammad in web browser, because of online notification of ticket which was already deleted in the meantime.

Martin Edenhofer 6 years ago
parent
commit
7326122e28

+ 3 - 0
app/models/activity_stream.rb

@@ -49,6 +49,9 @@ add a new activity entry for an object
       permission_id = permission.id
     end
 
+    # check if object for online notification exists
+    exists_by_object_and_id?(data[:object], data[:o_id])
+
     # check newest entry - is needed
     result = ActivityStream.where(
       o_id: data[:o_id],

+ 1 - 0
app/models/application_model.rb

@@ -18,6 +18,7 @@ class ApplicationModel < ActiveRecord::Base
   include ApplicationModel::ChecksImport
   include ApplicationModel::CanTouchReferences
   include ApplicationModel::CanQueryCaseInsensitiveWhereOrSql
+  include ApplicationModel::HasExistsCheckByObjectAndId
 
   self.abstract_class = true
 end

+ 35 - 0
app/models/application_model/has_exists_check_by_object_and_id.rb

@@ -0,0 +1,35 @@
+# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
+module ApplicationModel::HasExistsCheckByObjectAndId
+  extend ActiveSupport::Concern
+
+  class_methods do
+
+=begin
+
+verify if referenced object exists
+
+  success = Model.exists_by_object_and_id('Ticket', 123)
+
+returns
+
+  # true or will raise an exception
+
+=end
+
+    def exists_by_object_and_id?(object, o_id)
+
+      begin
+        local_class = object.constantize
+      rescue => e
+        raise "Unable for get an instance of '#{object}': #{e.inspect}"
+      end
+      if !local_class.exists?(o_id)
+        raise ActiveRecord::RecordNotFound, "Unable for find reference object '#{object}.exists?(#{o_id})!"
+      end
+
+      true
+    end
+
+  end
+
+end

+ 3 - 0
app/models/online_notification.rb

@@ -41,6 +41,9 @@ add a new online notification for this user
       object_id = ObjectLookup.by_name(data[:object])
     end
 
+    # check if object for online notification exists
+    exists_by_object_and_id?(data[:object], data[:o_id])
+
     record = {
       o_id: data[:o_id],
       object_lookup_id: object_id,

+ 5 - 3
app/models/recent_view.rb

@@ -15,9 +15,11 @@ class RecentView < ApplicationModel
   def self.log(object, o_id, user)
     return if !access(object, o_id, user)
 
-    RecentView.create(o_id: o_id,
-                      recent_view_object_id: ObjectLookup.by_name(object),
-                      created_by_id: user.id)
+    exists_by_object_and_id?(object, o_id)
+
+    RecentView.create!(o_id: o_id,
+                       recent_view_object_id: ObjectLookup.by_name(object),
+                       created_by_id: user.id)
   end
 
   def self.log_destroy(requested_object, requested_object_id)

+ 47 - 7
test/unit/online_notifiaction_test.rb

@@ -538,10 +538,22 @@ class OnlineNotificationTest < ActiveSupport::TestCase
   end
 
   test 'cleanup check' do
+
+    ticket1 = Ticket.create(
+      group: @group,
+      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,
+    )
+
     online_notification1 = OnlineNotification.add(
       type:          'create',
       object:        'Ticket',
-      o_id:          123,
+      o_id:          ticket1.id,
       seen:          false,
       user_id:       @agent_user1.id,
       created_by_id: 1,
@@ -552,7 +564,7 @@ class OnlineNotificationTest < ActiveSupport::TestCase
     online_notification2 = OnlineNotification.add(
       type:          'create',
       object:        'Ticket',
-      o_id:          123,
+      o_id:          ticket1.id,
       seen:          true,
       user_id:       @agent_user1.id,
       created_by_id: 1,
@@ -563,7 +575,7 @@ class OnlineNotificationTest < ActiveSupport::TestCase
     online_notification3 = OnlineNotification.add(
       type:          'create',
       object:        'Ticket',
-      o_id:          123,
+      o_id:          ticket1.id,
       seen:          false,
       user_id:       @agent_user1.id,
       created_by_id: 1,
@@ -574,7 +586,7 @@ class OnlineNotificationTest < ActiveSupport::TestCase
     online_notification4 = OnlineNotification.add(
       type:          'create',
       object:        'Ticket',
-      o_id:          123,
+      o_id:          ticket1.id,
       seen:          true,
       user_id:       @agent_user1.id,
       created_by_id: @agent_user1.id,
@@ -585,7 +597,7 @@ class OnlineNotificationTest < ActiveSupport::TestCase
     online_notification5 = OnlineNotification.add(
       type:          'create',
       object:        'Ticket',
-      o_id:          123,
+      o_id:          ticket1.id,
       seen:          true,
       user_id:       @agent_user1.id,
       created_by_id: @agent_user2.id,
@@ -596,7 +608,7 @@ class OnlineNotificationTest < ActiveSupport::TestCase
     online_notification6 = OnlineNotification.add(
       type:          'create',
       object:        'Ticket',
-      o_id:          123,
+      o_id:          ticket1.id,
       seen:          true,
       user_id:       @agent_user1.id,
       created_by_id: @agent_user1.id,
@@ -607,7 +619,7 @@ class OnlineNotificationTest < ActiveSupport::TestCase
     online_notification7 = OnlineNotification.add(
       type:          'create',
       object:        'Ticket',
-      o_id:          123,
+      o_id:          ticket1.id,
       seen:          true,
       user_id:       @agent_user1.id,
       created_by_id: @agent_user2.id,
@@ -628,4 +640,32 @@ class OnlineNotificationTest < ActiveSupport::TestCase
     OnlineNotification.destroy_all
   end
 
+  test 'not existing object ref' do
+    assert_raises(RuntimeError) do
+      OnlineNotification.add(
+        type:          'create',
+        object:        'TicketNotExisting',
+        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,
+      )
+    end
+    assert_raises(ActiveRecord::RecordNotFound) do
+      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,
+      )
+    end
+  end
 end