From 3b55de90e517f7081006947cc4fabbe5651a65c0 Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Wed, 2 Sep 2020 05:40:42 +0530 Subject: [PATCH] FIX: skip pm view action log while generating webhook payload. Currently, while generating webhook payloads for a topic it's accidentally adding a personal message view log in 'system' user's history. --- lib/post_destroyer.rb | 9 +++++---- lib/topic_view.rb | 6 +++--- spec/components/post_destroyer_spec.rb | 7 +++++++ 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index 1fb5daeed47..2ecde9a5853 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -57,10 +57,11 @@ class PostDestroyer payload = WebHook.generate_payload(:post, @post) if WebHook.active_web_hooks(:post).exists? topic = @post.topic is_first_post = @post.is_first_post? && topic + has_topic_web_hooks = is_first_post && WebHook.active_web_hooks(:topic).exists? - if is_first_post - topic_view = TopicView.new(topic.id, Discourse.system_user) - topic_payload = WebHook.generate_payload(:topic, topic_view, WebHookTopicViewSerializer) if WebHook.active_web_hooks(:topic).exists? + if has_topic_web_hooks + topic_view = TopicView.new(topic.id, Discourse.system_user, skip_staff_action: true) + topic_payload = WebHook.generate_payload(:topic, topic_view, WebHookTopicViewSerializer) end delete_removed_posts_after = @opts[:delete_removed_posts_after] || SiteSetting.delete_removed_posts_after @@ -80,7 +81,7 @@ class PostDestroyer UserProfile.remove_featured_topic_from_all_profiles(@topic) UserActionManager.topic_destroyed(topic) DiscourseEvent.trigger(:topic_destroyed, topic, @user) - WebHook.enqueue_topic_hooks(:topic_destroyed, topic, topic_payload) + WebHook.enqueue_topic_hooks(:topic_destroyed, topic, topic_payload) if has_topic_web_hooks end end diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 3752ac30921..451f483f8fd 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -56,7 +56,7 @@ class TopicView @user = user @guardian = Guardian.new(@user) - check_and_raise_exceptions + check_and_raise_exceptions(options[:skip_staff_action]) @message_bus_last_id = MessageBus.last_id("/topic/#{@topic.id}") @print = options[:print].present? @@ -783,7 +783,7 @@ class TopicView end - def check_and_raise_exceptions + def check_and_raise_exceptions(skip_staff_action) raise Discourse::NotFound if @topic.blank? # Special case: If the topic is private and the user isn't logged in, ask them # to log in! @@ -793,7 +793,7 @@ class TopicView # can user see this topic? raise Discourse::InvalidAccess.new("can't see #{@topic}", @topic) unless @guardian.can_see?(@topic) # log personal message views - if SiteSetting.log_personal_messages_views && @topic.present? && @topic.private_message? && @topic.all_allowed_users.where(id: @user.id).blank? + if SiteSetting.log_personal_messages_views && !skip_staff_action && @topic.present? && @topic.private_message? && @topic.all_allowed_users.where(id: @user.id).blank? unless UserHistory.where(acting_user_id: @user.id, action: UserHistory.actions[:check_personal_message], topic_id: @topic.id).where("created_at > ?", 1.hour.ago).exists? StaffActionLogger.new(@user).log_check_personal_message(@topic) end diff --git a/spec/components/post_destroyer_spec.rb b/spec/components/post_destroyer_spec.rb index ff6417fe508..91ba651b115 100644 --- a/spec/components/post_destroyer_spec.rb +++ b/spec/components/post_destroyer_spec.rb @@ -611,6 +611,13 @@ describe PostDestroyer do expect(events[1][:event_name]).to eq(:topic_destroyed) expect(events[1][:params].first).to eq(first_post.topic) end + + it 'should not log a personal message view' do + SiteSetting.log_personal_messages_views = true + Fabricate(:topic_web_hook) + StaffActionLogger.any_instance.expects(:log_check_personal_message).never + PostDestroyer.new(admin, first_post).destroy + end end context 'deleting the second post in a topic' do