FIX: do not remove uploads from posts that are still in review queue (#37372)

The "Remove upload markup from deleted posts" automation was incorrectly
processing posts that were deleted but still had pending reviewables in
the review queue.

Scenario: When a post is flagged (hidden from public) and the author
subsequently deletes it, the post has both `hidden: true` and
`deleted_at` set. The automation would strip upload markup from these
posts, causing data loss when moderators later restored them after
completing their review.

This fix adds a NOT EXISTS clause to exclude any post with a pending
reviewable targeting it. The automation will now wait until the review
is resolved (approved/rejected/ignored) before processing the post.
This commit is contained in:
Arpit Jalan
2026-01-29 17:00:33 +05:30
committed by GitHub
parent 4dd917c819
commit 01cdbcf413
2 changed files with 64 additions and 0 deletions
@@ -32,6 +32,15 @@ DiscourseAutomation::Scriptable.add(
"LEFT JOIN post_custom_fields ON posts.id = post_custom_fields.post_id AND post_custom_fields.name = 'uploads_removed_at'",
)
.where("post_custom_fields.post_id IS NULL")
.where(
"NOT EXISTS (
SELECT 1 FROM reviewables
WHERE reviewables.target_id = posts.id
AND reviewables.target_type = 'Post'
AND reviewables.status = ?
)",
Reviewable.statuses[:pending],
)
.distinct
.limit(DiscourseAutomation::REMOVE_UPLOAD_MARKUP_FROM_DELETED_POSTS_BATCH_SIZE)
.each do |post|
@@ -82,6 +82,61 @@ describe "RemoveUploadMarkupFromDeletedPosts" do
expect(file_upload_reference.reload).to be_present
end
it "does not remove uploads from flagged posts that are hidden and deleted but still pending review" do
flagged_hidden_deleted_post =
Fabricate(
:post,
topic: topic,
raw: raw,
hidden: true,
hidden_at: 1.month.ago,
deleted_at: 1.month.ago,
)
Fabricate(:upload_reference, upload: upload, target: flagged_hidden_deleted_post)
Fabricate(
:reviewable_flagged_post,
topic: topic,
target: flagged_hidden_deleted_post,
status: Reviewable.statuses[:pending],
)
expect {
automation.trigger!
flagged_hidden_deleted_post.reload
}.to_not change { flagged_hidden_deleted_post.raw }
expect(flagged_hidden_deleted_post.custom_fields["uploads_removed_at"]).to be_nil
end
it "removes uploads from deleted posts after reviewable is resolved" do
deleted_post_with_resolved_reviewable =
Fabricate(:post, topic: topic, raw: raw, deleted_at: 1.month.ago)
Fabricate(:upload_reference, upload: upload, target: deleted_post_with_resolved_reviewable)
Fabricate(
:upload_reference,
upload: nameless_upload,
target: deleted_post_with_resolved_reviewable,
)
Fabricate(
:upload_reference,
upload: file_upload,
target: deleted_post_with_resolved_reviewable,
)
Fabricate(
:reviewable_flagged_post,
topic: topic,
target: deleted_post_with_resolved_reviewable,
status: Reviewable.statuses[:approved],
)
expect {
automation.trigger!
deleted_post_with_resolved_reviewable.reload
}.to change { deleted_post_with_resolved_reviewable.raw }.from(raw).to(expected_raw)
end
it "adds a timestamp to the custom field uploads_removed_at" do
expect {
automation.trigger!