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.
This commit is contained in:
Selase Krakani 2023-08-18 15:30:59 +00:00 committed by GitHub
parent 4661bcb5ea
commit 87ebbec9b2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 19 additions and 6 deletions

View File

@ -166,7 +166,11 @@ class ReviewablesController < ApplicationController
current_user current_user
end 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? raise Discourse::NotFound.new if reviewable.blank?
reviewable.perform(current_user, :delete, { guardian: @guardian }) reviewable.perform(current_user, :delete, { guardian: @guardian })

View File

@ -717,6 +717,15 @@ class Reviewable < ActiveRecord::Base
end end
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 private
def update_flag_stats(status:, user_ids:) def update_flag_stats(status:, user_ids:)

View File

@ -236,7 +236,7 @@ class Guardian
return false if !authenticated? return false if !authenticated?
return true if is_api? && is_admin? return true if is_api? && is_admin?
reviewable.created_by_id == @user.id reviewable.target_created_by_id == @user.id
end end
def can_see_group?(group) def can_see_group?(group)

View File

@ -4389,7 +4389,7 @@ RSpec.describe Guardian do
context "when attempting to destroy your own reviewable" do context "when attempting to destroy your own reviewable" do
it "returns true" 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 = env =
create_request_env(path: "/review/#{queued_post.id}.json").merge( create_request_env(path: "/review/#{queued_post.id}.json").merge(
{ "REQUEST_METHOD" => "DELETE" }, { "REQUEST_METHOD" => "DELETE" },

View File

@ -805,7 +805,7 @@ RSpec.describe ReviewablesController do
it "returns 200 if the user can delete the reviewable" do it "returns 200 if the user can delete the reviewable" do
sign_in(user) 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" delete "/review/#{queued_post.id}.json"
expect(response.code).to eq("200") expect(response.code).to eq("200")
expect(queued_post.reload).to be_deleted expect(queued_post.reload).to be_deleted
@ -813,7 +813,7 @@ RSpec.describe ReviewablesController do
it "denies attempts to destroy unowned reviewables" do it "denies attempts to destroy unowned reviewables" do
sign_in(admin) 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" delete "/review/#{queued_post.id}.json"
expect(response.status).to eq(404) expect(response.status).to eq(404)
# Reviewable is not deleted because request is not via API # 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 shared_examples "for a passed user" do
it "deletes reviewable" do it "deletes reviewable" do
api_key = Fabricate(:api_key).key 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", delete "/review/#{queued_post.id}.json",
params: { params: {
username: recipient.username, username: recipient.username,