UX: return correct error message if reviewable user is deleted already. (#12977)

Currently, when the target is not available we're returning the error message "`You are not permitted to view the requested resource`" which is not clear.
This commit is contained in:
Vinoth Kannan 2021-05-07 22:00:04 +05:30 committed by GitHub
parent efe055c273
commit 10449ff794
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 2 deletions

View File

@ -199,8 +199,12 @@ class ReviewablesController < ApplicationController
result = reviewable.perform(current_user, params[:action_id].to_sym, args)
rescue Reviewable::InvalidAction => e
# Consider InvalidAction an InvalidAccess
raise Discourse::InvalidAccess.new(e.message)
if reviewable.type == 'ReviewableUser' && !reviewable.pending? && reviewable.target.blank?
raise Discourse::NotFound.new(e.message, custom_message: "reviewables.already_handled_and_user_not_exist")
else
# Consider InvalidAction an InvalidAccess
raise Discourse::InvalidAccess.new(e.message)
end
rescue Reviewable::UpdateConflict
return render_json_error(I18n.t('reviewables.conflict'), status: 409)
end

View File

@ -4916,6 +4916,7 @@ en:
reviewables:
already_handled: "Thanks, but we've already reviewed that post and determined it does not need to be flagged again."
already_handled_and_user_not_exist: "Thanks, but someone already reviewed and that user no longer exists."
priorities:
low: "Low"
medium: "Medium"

View File

@ -174,6 +174,25 @@ describe ReviewablesController do
expect(json_review['user_id']).to eq(user.id)
end
it "returns correct error message if ReviewableUser not found" do
sign_in(admin)
Jobs.run_immediately!
SiteSetting.must_approve_users = true
user = Fabricate(:user)
user.activate
reviewable = ReviewableUser.find_by(target: user)
put "/review/#{reviewable.id}/perform/reject_user_delete.json?version=0"
expect(response.code).to eq("200")
put "/review/#{reviewable.id}/perform/reject_user_delete.json?version=0&index=2"
expect(response.code).to eq("404")
json = response.parsed_body
expect(json["error_type"]).to eq("not_found")
expect(json["errors"][0]).to eq(I18n.t("reviewables.already_handled_and_user_not_exist"))
end
context "supports filtering by range" do
let(:from) { 3.days.ago.strftime('%F') }
let(:to) { 1.day.ago.strftime('%F') }