FIX: double reviewable items bug (#28161)

When `SiteSetting.review_every_post` is true and the category `require_topic_approval` system creates two reviewable items.
1. Firstly, because the category needs approval, the `ReviewableQueuePost` record` is created - at this stage, no topic is created.
2. Admin is approving the review. The topic and first post are created.
3. Because `review_every_post` is true `queue_for_review_if_possible` callback is evaluated and `ReviewablePost` is created.
4. Then `ReviewableQueuePost` is linked to the newly generated topic and post.

At the beginning, we were thinking about hooking to those guards:
```
  def self.queue_for_review_if_possible(post, created_or_edited_by)
    return unless SiteSetting.review_every_post
    return if post.post_type != Post.types[:regular] || post.topic.private_message?
    return if Reviewable.pending.where(target: post).exists?
...
```
And add something like
```
 return if Reviewable.approved.where(target: post).exists?
```

However, because the callback happens in point 3. before the `ReviewableQueuePost` is linked to the `Topic`, it was not possible.

Therefore, when `ReviewableQueuePost` is creating a `Topic`, a new option called `:reviewed_queued_post` is passed to `PostCreator` to avoid creating a second `Reviewable`.
This commit is contained in:
Krzysztof Kotlarek 2024-07-31 12:45:00 +10:00 committed by GitHub
parent e9c0a6dffe
commit 5830f2c9a1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 26 additions and 1 deletions

View File

@ -108,6 +108,7 @@ class ReviewableQueuedPost < Reviewable
skip_jobs: true, skip_jobs: true,
skip_events: true, skip_events: true,
skip_guardian: true, skip_guardian: true,
reviewed_queued_post: true,
) )
opts.merge!(guardian: Guardian.new(performed_by)) if performed_by.staff? opts.merge!(guardian: Guardian.new(performed_by)) if performed_by.staff?

View File

@ -222,7 +222,7 @@ class PostCreator
auto_close auto_close
end end
if !opts[:import_mode] if !opts[:import_mode] && !opts[:reviewed_queued_post]
handle_spam if (@spam || @post) handle_spam if (@spam || @post)
ReviewablePost.queue_for_review_if_possible(@post, @user) if !@spam && @post && errors.blank? ReviewablePost.queue_for_review_if_possible(@post, @user) if !@spam && @post && errors.blank?

View File

@ -2167,4 +2167,28 @@ RSpec.describe PostCreator do
expect { post_creator.create }.not_to change(ReviewablePost, :count) expect { post_creator.create }.not_to change(ReviewablePost, :count)
end end
end end
context "when the review_every_post setting is enabled and category requires topic approval" do
fab!(:category)
before do
category.require_topic_approval = true
category.save!
end
before { SiteSetting.review_every_post = true }
it "creates single reviewable item" do
manager =
NewPostManager.new(
user,
title: "this is a new title",
raw: "this is a new post",
category: category.id,
)
reviewable = manager.perform.reviewable
expect { reviewable.perform(admin, :approve_post) }.not_to change(ReviewablePost, :count)
end
end
end end