Skip to content

Commit

Permalink
Fixed issue #2405 - Unable to load Zammad in web browser, because of …
Browse files Browse the repository at this point in the history
…online notification of ticket which was already deleted in the meantime.
  • Loading branch information
znuny-robo authored and martini committed Dec 18, 2018
1 parent 9bcd07c commit 629c561
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 10 deletions.
3 changes: 3 additions & 0 deletions app/models/activity_stream.rb
Expand Up @@ -49,6 +49,9 @@ def self.add(data)
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],
Expand Down
1 change: 1 addition & 0 deletions app/models/application_model.rb
Expand Up @@ -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 changes: 35 additions & 0 deletions 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 changes: 3 additions & 0 deletions app/models/online_notification.rb
Expand Up @@ -41,6 +41,9 @@ def self.add(data)
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,
Expand Down
8 changes: 5 additions & 3 deletions app/models/recent_view.rb
Expand Up @@ -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)
Expand Down
54 changes: 47 additions & 7 deletions test/unit/online_notifiaction_test.rb
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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

0 comments on commit 629c561

Please sign in to comment.