From c5401679825b739b541aad3d7c31258cb9f5849e Mon Sep 17 00:00:00 2001
From: Blake Erickson <o.blakeerickson@gmail.com>
Date: Mon, 6 Feb 2023 11:55:52 -0700
Subject: [PATCH] FIX: Remove action buttons if post has already been reviewed
 (#20126)

* FIX: Remove action buttons if post has already been reviewed

* Change the approve to reject test to expect an error

* Adds a controller spec to ensure you can't edit a non-pending review item

* Remove unnessary conditional
---
 app/models/reviewable_queued_post.rb         | 22 +++++++++++---------
 spec/models/reviewable_spec.rb               | 13 ++++++------
 spec/requests/reviewables_controller_spec.rb | 16 ++++++++++++++
 3 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/app/models/reviewable_queued_post.rb b/app/models/reviewable_queued_post.rb
index ecf6297cdcd..873d425e919 100644
--- a/app/models/reviewable_queued_post.rb
+++ b/app/models/reviewable_queued_post.rb
@@ -32,7 +32,7 @@ class ReviewableQueuedPost < Reviewable
       end
     end
 
-    unless rejected?
+    if pending?
       actions.add(:reject_post) do |a|
         a.icon = "times"
         a.label = "reviewables.actions.reject_post.title"
@@ -45,17 +45,19 @@ class ReviewableQueuedPost < Reviewable
   end
 
   def build_editable_fields(fields, guardian, args)
-    # We can edit category / title if it's a new topic
-    if topic_id.blank?
-      # Only staff can edit category for now, since in theory a category group reviewer could
-      # post in a category they don't have access to.
-      fields.add("category_id", :category) if guardian.is_staff?
+    if pending?
+      # We can edit category / title if it's a new topic
+      if topic_id.blank?
+        # Only staff can edit category for now, since in theory a category group reviewer could
+        # post in a category they don't have access to.
+        fields.add("category_id", :category) if guardian.is_staff?
 
-      fields.add("payload.title", :text)
-      fields.add("payload.tags", :tags)
+        fields.add("payload.title", :text)
+        fields.add("payload.tags", :tags)
+      end
+
+      fields.add("payload.raw", :editor)
     end
-
-    fields.add("payload.raw", :editor)
   end
 
   def create_options
diff --git a/spec/models/reviewable_spec.rb b/spec/models/reviewable_spec.rb
index 1f9d8e6b0b8..a9b0f887097 100644
--- a/spec/models/reviewable_spec.rb
+++ b/spec/models/reviewable_spec.rb
@@ -394,14 +394,15 @@ RSpec.describe Reviewable, type: :model do
     it "triggers a notification on approve -> reject to update status" do
       reviewable = Fabricate(:reviewable_queued_post, status: Reviewable.statuses[:approved])
 
-      expect do reviewable.perform(moderator, :reject_post) end.to change {
-        Jobs::NotifyReviewable.jobs.size
-      }.by(1)
+      expect { reviewable.perform(moderator, :reject_post) }.to raise_error(
+        Reviewable::InvalidAction,
+      )
+    end
 
-      job = Jobs::NotifyReviewable.jobs.last
+    it "triggers a notification on approve -> edit to update status" do
+      reviewable = Fabricate(:reviewable_queued_post, status: Reviewable.statuses[:approved])
 
-      expect(job["args"].first["reviewable_id"]).to eq(reviewable.id)
-      expect(job["args"].first["updated_reviewable_ids"]).to contain_exactly(reviewable.id)
+      expect { reviewable.perform(moderator, :edit_post) }.to raise_error(Reviewable::InvalidAction)
     end
 
     it "triggers a notification on reject -> approve to update status" do
diff --git a/spec/requests/reviewables_controller_spec.rb b/spec/requests/reviewables_controller_spec.rb
index f0cbcc6400d..d40131b3d72 100644
--- a/spec/requests/reviewables_controller_spec.rb
+++ b/spec/requests/reviewables_controller_spec.rb
@@ -663,6 +663,9 @@ RSpec.describe ReviewablesController do
       fab!(:reviewable_post) { Fabricate(:reviewable_queued_post) }
       fab!(:reviewable_topic) { Fabricate(:reviewable_queued_post_topic) }
       fab!(:moderator) { Fabricate(:moderator) }
+      fab!(:reviewable_approved_post) do
+        Fabricate(:reviewable_queued_post, status: Reviewable.statuses[:approved])
+      end
 
       before { sign_in(moderator) }
 
@@ -740,6 +743,19 @@ RSpec.describe ReviewablesController do
         expect(json["version"] > 0).to eq(true)
       end
 
+      it "prevents you from updating an approved post" do
+        put "/review/#{reviewable_approved_post.id}.json?version=#{reviewable_approved_post.version}",
+            params: {
+              reviewable: {
+                payload: {
+                  raw: "new raw content",
+                },
+              },
+            }
+
+        expect(response.code).to eq("403")
+      end
+
       it "allows you to update a queued post (for new topic)" do
         new_category_id = Fabricate(:category).id