From 10449ff79416001b36e3557a808a0c87ec09dccd Mon Sep 17 00:00:00 2001
From: Vinoth Kannan <svkn.87@gmail.com>
Date: Fri, 7 May 2021 22:00:04 +0530
Subject: [PATCH] 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.
---
 app/controllers/reviewables_controller.rb    |  8 ++++++--
 config/locales/server.en.yml                 |  1 +
 spec/requests/reviewables_controller_spec.rb | 19 +++++++++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/app/controllers/reviewables_controller.rb b/app/controllers/reviewables_controller.rb
index 9afb716ae6d..f9249991b09 100644
--- a/app/controllers/reviewables_controller.rb
+++ b/app/controllers/reviewables_controller.rb
@@ -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
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 880ff97155d..23b159e73ad 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -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"
diff --git a/spec/requests/reviewables_controller_spec.rb b/spec/requests/reviewables_controller_spec.rb
index 89639a38e65..5a2abc23528 100644
--- a/spec/requests/reviewables_controller_spec.rb
+++ b/spec/requests/reviewables_controller_spec.rb
@@ -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') }