From 79e3d4e2bdc1e490e6c32aa47c1ad34d78f37aa5 Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Fri, 18 Aug 2023 10:55:17 +0800 Subject: [PATCH] FIX: Don't run post validations when hiding post (#23139) When hiding a post (essentially updating hidden, hidden_at, and hidden_reason_id) our callbacks are running the whole battery of post validations. This can cause the hiding to fail in a number of edge cases. The issue is similar to the one fixed in #11680, but applies to all post validations, none of which should apply when hiding a post. After some code reading and discussion, none of the validations in PostValidator seem to be relevant when hiding posts, so instead of just skipping unique check, we skip all post validator checks. --- app/models/post.rb | 9 +++------ spec/models/post_spec.rb | 15 +++++---------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 86cfa5962a4..31a9015fe0e 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -585,13 +585,10 @@ class Post < ActiveRecord::Base hiding_again = hidden_at.present? - self.hidden = true - self.hidden_at = Time.zone.now - self.hidden_reason_id = reason - self.skip_unique_check = true - Post.transaction do - save! + self.skip_validation = true + + update!(hidden: true, hidden_at: Time.zone.now, hidden_reason_id: reason) Topic.where( "id = :topic_id AND NOT EXISTS(SELECT 1 FROM POSTS WHERE topic_id = :topic_id AND NOT hidden)", diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 12a46d8f77c..09c78fa8d25 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -1441,17 +1441,12 @@ RSpec.describe Post do after { Discourse.redis.flushdb } - it "should ignore the unique post validator when hiding a post with similar content as a recent post" do - post_2 = Fabricate(:post, user: post.user) - SiteSetting.unique_posts_mins = 10 - post.store_unique_post_key + it "should not run post validations" do + PostValidator.any_instance.expects(:validate).never - expect(post_2.valid?).to eq(false) - expect(post_2.errors.full_messages.to_s).to include(I18n.t(:just_posted_that)) - - post_2.hide!(PostActionType.types[:off_topic]) - - expect(post_2.reload.hidden).to eq(true) + expect { post.hide!(PostActionType.types[:off_topic]) }.to change { post.reload.hidden }.from( + false, + ).to(true) end it "should decrease user_stat topic_count for first post" do