From 87ebbec9b2dea4116ee6c2f5fe62a4874b3eeac0 Mon Sep 17 00:00:00 2001
From: Selase Krakani <849886+s3lase@users.noreply.github.com>
Date: Fri, 18 Aug 2023 15:30:59 +0000
Subject: [PATCH] FIX: Pending post deletion by creator (#23130)

`ReviewableQueuedPost` got refactored a while back to use the more
appropriate `target_created_by` for the user of the post being queued
instead of `created_by`. The change was not extended to the `DELETE
/review/:id` endpoint leading to error responses for a user attempting
to deleting their own queued post.

This fix extends the `Reviewable` lookup implementation in
`ReviewablesController#destroy` and Guardian implementation to account
for this change.
---
 app/controllers/reviewables_controller.rb    | 6 +++++-
 app/models/reviewable.rb                     | 9 +++++++++
 lib/guardian.rb                              | 2 +-
 spec/lib/guardian_spec.rb                    | 2 +-
 spec/requests/reviewables_controller_spec.rb | 6 +++---
 5 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/app/controllers/reviewables_controller.rb b/app/controllers/reviewables_controller.rb
index 112e8980929..a00b65a5a85 100644
--- a/app/controllers/reviewables_controller.rb
+++ b/app/controllers/reviewables_controller.rb
@@ -166,7 +166,11 @@ class ReviewablesController < ApplicationController
         current_user
       end
 
-    reviewable = Reviewable.find_by(id: params[:reviewable_id], created_by: user)
+    reviewable =
+      Reviewable.find_by_flagger_or_queued_post_creator(
+        id: params[:reviewable_id],
+        user_id: user.id,
+      )
     raise Discourse::NotFound.new if reviewable.blank?
 
     reviewable.perform(current_user, :delete, { guardian: @guardian })
diff --git a/app/models/reviewable.rb b/app/models/reviewable.rb
index 6f7efb86f88..c0fdb2d00c4 100644
--- a/app/models/reviewable.rb
+++ b/app/models/reviewable.rb
@@ -717,6 +717,15 @@ class Reviewable < ActiveRecord::Base
     end
   end
 
+  def self.find_by_flagger_or_queued_post_creator(id:, user_id:)
+    Reviewable.find_by(
+      "id = :id AND (created_by_id = :user_id
+       OR (target_created_by_id = :user_id AND type = 'ReviewableQueuedPost'))",
+      id: id,
+      user_id: user_id,
+    )
+  end
+
   private
 
   def update_flag_stats(status:, user_ids:)
diff --git a/lib/guardian.rb b/lib/guardian.rb
index 8e3add75cb2..602b0e42f2c 100644
--- a/lib/guardian.rb
+++ b/lib/guardian.rb
@@ -236,7 +236,7 @@ class Guardian
     return false if !authenticated?
     return true if is_api? && is_admin?
 
-    reviewable.created_by_id == @user.id
+    reviewable.target_created_by_id == @user.id
   end
 
   def can_see_group?(group)
diff --git a/spec/lib/guardian_spec.rb b/spec/lib/guardian_spec.rb
index 82d85785504..0b59e291729 100644
--- a/spec/lib/guardian_spec.rb
+++ b/spec/lib/guardian_spec.rb
@@ -4389,7 +4389,7 @@ RSpec.describe Guardian do
 
     context "when attempting to destroy your own reviewable" do
       it "returns true" do
-        queued_post = Fabricate(:reviewable_queued_post, created_by: user)
+        queued_post = Fabricate(:reviewable_queued_post, target_created_by: user)
         env =
           create_request_env(path: "/review/#{queued_post.id}.json").merge(
             { "REQUEST_METHOD" => "DELETE" },
diff --git a/spec/requests/reviewables_controller_spec.rb b/spec/requests/reviewables_controller_spec.rb
index 853750ad2f6..ba6c826be5e 100644
--- a/spec/requests/reviewables_controller_spec.rb
+++ b/spec/requests/reviewables_controller_spec.rb
@@ -805,7 +805,7 @@ RSpec.describe ReviewablesController do
 
       it "returns 200 if the user can delete the reviewable" do
         sign_in(user)
-        queued_post = Fabricate(:reviewable_queued_post, created_by: user)
+        queued_post = Fabricate(:reviewable_queued_post, target_created_by: user)
         delete "/review/#{queued_post.id}.json"
         expect(response.code).to eq("200")
         expect(queued_post.reload).to be_deleted
@@ -813,7 +813,7 @@ RSpec.describe ReviewablesController do
 
       it "denies attempts to destroy unowned reviewables" do
         sign_in(admin)
-        queued_post = Fabricate(:reviewable_queued_post, created_by: user)
+        queued_post = Fabricate(:reviewable_queued_post, target_created_by: user)
         delete "/review/#{queued_post.id}.json"
         expect(response.status).to eq(404)
         # Reviewable is not deleted because request is not via API
@@ -823,7 +823,7 @@ RSpec.describe ReviewablesController do
       shared_examples "for a passed user" do
         it "deletes reviewable" do
           api_key = Fabricate(:api_key).key
-          queued_post = Fabricate(:reviewable_queued_post, created_by: recipient)
+          queued_post = Fabricate(:reviewable_queued_post, target_created_by: recipient)
           delete "/review/#{queued_post.id}.json",
                  params: {
                    username: recipient.username,