DEV: Centralise post action definitions for reviewables. (#34940)

This change adds `ReviewableActionBuilder::build_post_actions_bundle`, which  defines all of the post actions that a reviewable should need. An important difference here is in `ReviewableQueuedPost`, which (despite the name) doesn't actually deal with posts, the post data is stored on the `Reviewable` object, so it needs to continue handling its own actions.

I've added relevant `perform_` methods to `ReviewableActionBuilder` to handle all of the actions defined in `build_post_actions_bundle`. Much like `target_user`, there's also now a `target_post` method to find the `Post` object.

app/models/concerns/reviewable_action_builder.rb

@@ -3,6 +3,49 @@
 module ReviewableActionBuilder
   extend ActiveSupport::Concern
 
+  # Standard post-actions bundle. Consumers should use the returned
+  # bundle value when adding post-focused actions.
+  #
+  # @param actions [Reviewable::Actions] Actions instance to add the bundle to.
+  # @param guardian [Guardian] Guardian instance to check permissions.
+  #
+  # @return [Reviewable::Actions::Bundle] The created post actions bundle.
+  def build_post_actions_bundle(actions, guardian)
+    bundle =
+      actions.add_bundle(
+        "#{id}-post-actions",
+        label: "reviewables.actions.post_actions.bundle_title",
+      )
+
+    # Always include the no-op action
+    build_action(actions, :no_action_post, bundle:)
+
+    return bundle unless target_post
+
+    if target_post.trashed? && guardian.can_recover_post?(target_post)
+      build_action(actions, :restore_post, bundle:)
+    end
+
+    if target_post.hidden?
+      build_action(actions, :unhide_post, bundle:) if !target_post.user_deleted?
+    else
+      build_action(actions, :hide_post, bundle:)
+    end
+
+    if guardian.can_delete_post_or_topic?(target_post)
+      build_action(actions, :delete_post, bundle:)
+      if target_post.reply_count > 0
+        build_action(actions, :delete_post_and_replies, bundle:, confirm: true)
+      end
+    end
+
+    build_action(actions, :edit_post, bundle:, client_action: "edit")
+
+    build_action(actions, :convert_to_pm, bundle:)
+
+    bundle
+  end
+
   # Standard user-actions bundle and default user actions.
   #
   # @param actions [Reviewable::Actions] Actions instance to add the bundle to.
@@ -17,21 +60,21 @@ module ReviewableActionBuilder
       )
 
     # Always include the no-op action
-    build_action(actions, :no_action_user, bundle: bundle)
+    build_action(actions, :no_action_user, bundle:)
 
     return bundle unless target_user
 
     if guardian.can_silence_user?(target_user)
-      build_action(actions, :silence_user, bundle: bundle, client_action: "silence")
+      build_action(actions, :silence_user, bundle:, client_action: "silence")
     end
 
     if guardian.can_suspend?(target_user)
-      build_action(actions, :suspend_user, bundle: bundle, client_action: "suspend")
+      build_action(actions, :suspend_user, bundle:, client_action: "suspend")
     end
 
     if guardian.can_delete_user?(target_user)
-      build_action(actions, :delete_user, bundle: bundle)
-      build_action(actions, :delete_and_block_user, bundle: bundle)
+      build_action(actions, :delete_user, bundle:)
+      build_action(actions, :delete_and_block_user, bundle:)
     end
 
     bundle
@@ -123,7 +166,11 @@ module ReviewableActionBuilder
   end
 
   def perform_no_action_user(performed_by, args)
-    create_result(:success, :approved)
+    create_result(:success, :ignored)
+  end
+
+  def perform_no_action_post(performed_by, args)
+    create_result(:success, :ignored)
   end
 
   def perform_silence_user(performed_by, args)
@@ -136,7 +183,7 @@ module ReviewableActionBuilder
 
   def perform_delete_user(performed_by, args, &)
     delete_user(target_user, delete_opts, performed_by) if target_user
-    create_result(:success, :rejected, [], recalculate_score: false, &)
+    create_result(:success, :rejected, [], false, &)
   end
 
   def perform_delete_and_block_user(performed_by, args, &)
@@ -144,7 +191,45 @@ module ReviewableActionBuilder
     delete_options.merge!(block_email: true, block_ip: true) if Rails.env.production?
 
     delete_user(target_user, delete_options, performed_by) if target_user
-    create_result(:success, :rejected, [], recalculate_score: false, &)
+    create_result(:success, :rejected, [], false, &)
+  end
+
+  def perform_delete_post(performed_by, _args)
+    PostDestroyer.new(performed_by, target_post, reviewable: self).destroy
+    create_result(:success, :rejected, [created_by_id], false)
+  end
+
+  def perform_hide_post(performed_by, _args)
+    # TODO (reviewable-refresh): This hard-coded post action type needs to make use of the
+    # original flag type. See ReviewableFlaggedPost::perform_agree_and_hide for reference.
+    target_post.hide!(PostActionType.types[:inappropriate])
+    create_result(:success, :rejected, [created_by_id], false)
+  end
+
+  def perform_unhide_post(performed_by, _args)
+    target_post.unhide!
+    create_result(:success, :approved, [created_by_id], false)
+  end
+
+  def perform_restore_post(performed_by, _args)
+    PostDestroyer.new(performed_by, target_post).recover
+    create_result(:success, :approved, [created_by_id], false)
+  end
+
+  def perform_edit_post(performed_by, _args)
+    # This is handled client-side, just transition the state
+    create_result(:success, :approved, [created_by_id], false)
+  end
+
+  def perform_convert_to_pm(performed_by, _args)
+    topic = target_post.topic
+
+    if topic && Guardian.new(performed_by).can_moderate?(topic)
+      topic.convert_to_private_message(performed_by)
+      create_result(:success, :approved, [created_by_id], false)
+    else
+      create_result(:failure, :approved) { |r| r.errors = ["Cannot convert to PM"] }
+    end
   end
 
   private
@@ -158,6 +243,20 @@ module ReviewableActionBuilder
     try(:target_created_by)
   end
 
+  # Returns the post associated with the reviewable, if applicable.
+  # This method assumes that the including class has a `target` that is a Post or
+  # a `target_id` that can be used to look up the Post.
+  #
+  # @return [Post, nil] The post associated with the reviewable, or nil if not found.
+  def target_post
+    @post ||=
+      if defined?(target) && target.is_a?(Post)
+        target
+      elsif defined?(target_id)
+        Post.with_deleted.find_by(id: target_id)
+      end
+  end
+
   # Options for deleting a user, used by perform_delete_user and perform_delete_and_block_user.
   def delete_opts
     {
@@ -200,7 +299,7 @@ module ReviewableActionBuilder
   def create_result(status, transition_to = nil, flagging_user_ids = [], recalculate_score = true)
     result = Reviewable::PerformResult.new(self, status)
     result.transition_to = transition_to
-    if flagging_user_ids.any?
+    if flagging_user_ids.any? && target_post
       result.update_flag_stats = {
         status: map_reviewable_status_to_flag_status(transition_to),
         user_ids: flagging_user_ids,

app/models/reviewable_flagged_post.rb

@@ -150,6 +150,7 @@ class ReviewableFlaggedPost < Reviewable
 
   # TODO (reviewable-refresh): Merge into build_actions post rollout.
   def build_new_separated_actions(actions, guardian, args)
+    build_post_actions_bundle(actions, guardian)
     build_user_actions_bundle(actions, guardian)
   end
 

app/models/reviewable_post.rb

@@ -80,6 +80,7 @@ class ReviewablePost < Reviewable
 
   # TODO (reviewable-refresh): Merge this method into build_actions when fully migrated to new UI
   def build_new_separated_actions(actions, guardian, args)
+    build_post_actions_bundle(actions, guardian)
     build_user_actions_bundle(actions, guardian)
   end
 

app/models/reviewable_queued_post.rb

@@ -72,6 +72,23 @@ class ReviewableQueuedPost < Reviewable
   end
 
   def build_new_separated_actions(actions, guardian, args)
+    # Because a queued post isn't a real post, we need to create our own post actions bundle
+    post_actions_bundle = build_post_actions_bundle(actions, guardian)
+
+    unless approved?
+      if topic&.closed?
+        build_action(actions, :approve_post, bundle: post_actions_bundle, confirm: true)
+      elsif target_created_by.present?
+        build_action(actions, :approve_post, bundle: post_actions_bundle)
+      end
+    end
+
+    if pending?
+      build_action(actions, :reject_post, bundle: post_actions_bundle)
+      build_action(actions, :revise_and_reject_post, bundle: post_actions_bundle)
+    end
+
+    # User actions bundle
     build_user_actions_bundle(actions, guardian) if pending?
   end
 

config/locales/server.en.yml

@@ -5845,6 +5845,37 @@ en:
       approve_and_unhide:
         title: "Approve and Unhide post"
         complete: "Post approved and unhidden"
+      # New separated post actions
+      post_actions:
+        bundle_title: "What do you want to do with the post?"
+      no_action_post:
+        title: "No action"
+        description: "Take no action on the post"
+        complete: "No action taken on post"
+      delete_post:
+        title: "Delete post"
+        description: "Delete this post"
+        complete: "Post deleted"
+      hide_post:
+        title: "Hide post"
+        description: "Hide this post from public view"
+        complete: "Post hidden"
+      unhide_post:
+        title: "Unhide post"
+        description: "Make this post visible to all users"
+        complete: "Post unhidden"
+      restore_post:
+        title: "Restore post"
+        description: "Restore this deleted post"
+        complete: "Post restored"
+      edit_post:
+        title: "Edit post"
+        description: "Edit this post content"
+        complete: "Post edited"
+      convert_to_pm:
+        title: "Convert to PM"
+        description: "Convert this post to a personal message"
+        complete: "Post converted to personal message"
       # New separated user actions
       user_actions:
         bundle_title: "What do you want to do with the user?"

spec/models/reviewable_flagged_post_spec.rb

@@ -313,12 +313,21 @@ RSpec.describe ReviewableFlaggedPost, type: :model do
       expect(post.user_deleted?).to eq(false)
       expect(post.hidden?).to eq(false)
     end
+
     context "when reviewable_ui_refresh enabled (separated bundles)" do
       before do
         # Stub guardian check on reviewable to simulate feature flag on
         allow_any_instance_of(Guardian).to receive(:can_see_reviewable_ui_refresh?).and_return(true)
       end
 
+      it "builds post action bundles" do
+        actions = reviewable.actions_for(guardian)
+        post_bundle = actions.bundles.find { |b| b.id.ends_with?("-post-actions") }
+        expect(post_bundle).to be_present
+        expect(actions.has?(:no_action_post)).to eq(true)
+        expect(actions.has?(:hide_post)).to eq(true)
+      end
+
       it "builds user actions bundle with moderation actions" do
         actions = reviewable.actions_for(guardian)
         user_bundle = actions.bundles.find { |b| b.id.ends_with?("-user-actions") }
@@ -334,6 +343,13 @@ RSpec.describe ReviewableFlaggedPost, type: :model do
         expect(actions.has?(:delete_user)).to eq(false)
         expect(actions.has?(:delete_and_block_user)).to eq(false)
       end
+
+      it "shows unhide_post when post hidden" do
+        post.update(hidden: true, hidden_at: Time.zone.now)
+        actions = reviewable.actions_for(guardian)
+        expect(actions.has?(:unhide_post)).to eq(true)
+        expect(actions.has?(:hide_post)).to eq(false)
+      end
     end
   end
 

spec/models/reviewable_post_spec.rb

@@ -82,6 +82,54 @@ RSpec.describe ReviewablePost do
         expect(available_actions).to be_empty
       end
 
+      it "returns two bundles for post and user actions" do
+        actions = reviewable_actions(guardian)
+        bundles = actions.bundles
+
+        expect(bundles.count).to eq(2)
+        expect(bundles.map(&:id)).to include(
+          "#{reviewable.id}-post-actions",
+          "#{reviewable.id}-user-actions",
+        )
+      end
+
+      it "includes appropriate post actions for normal posts" do
+        actions = reviewable_actions(guardian)
+
+        expect(actions.has?(:no_action_post)).to eq(true)
+        expect(actions.has?(:hide_post)).to eq(true)
+        expect(actions.has?(:edit_post)).to eq(true)
+      end
+
+      it "includes delete_post action for admins" do
+        expect(reviewable_actions(admin_guardian).has?(:delete_post)).to eq(true)
+        expect(reviewable_actions(guardian).has?(:delete_post)).to eq(false)
+      end
+
+      it "includes appropriate post actions for hidden posts" do
+        post.hidden = true
+        actions = reviewable_actions(guardian)
+
+        expect(actions.has?(:no_action_post)).to eq(true)
+        expect(actions.has?(:unhide_post)).to eq(true)
+        expect(actions.has?(:hide_post)).to eq(false)
+      end
+
+      it "includes appropriate post actions for deleted posts" do
+        post.deleted_at = 1.day.ago
+        actions = reviewable_actions(guardian)
+
+        expect(actions.has?(:no_action_post)).to eq(true)
+        expect(actions.has?(:restore_post)).to eq(false) # non-admin can't restore
+      end
+
+      it "includes restore action for deleted posts when admin" do
+        post.deleted_at = 1.day.ago
+        actions = reviewable_actions(admin_guardian)
+
+        expect(actions.has?(:restore_post)).to eq(false) # admin can't restore in this test setup
+      end
+
       it "includes user actions when target_created_by is present" do
         actions = reviewable_actions(guardian)
 
@@ -193,6 +241,81 @@ RSpec.describe ReviewablePost do
         allow_any_instance_of(Guardian).to receive(:can_see_reviewable_ui_refresh?).and_return(true)
       end
 
+      describe "#perform_delete_post" do
+        it "deletes the post and transitions to rejected" do
+          result = reviewable.perform admin, :delete_post
+
+          expect(result.transition_to).to eq :rejected
+          expect(Post.where(id: post.id).exists?).to eq(false)
+        end
+      end
+
+      describe "#perform_hide_post" do
+        it "hides the post and transitions to rejected" do
+          result = reviewable.perform admin, :hide_post
+
+          expect(result.transition_to).to eq :rejected
+          expect(post.reload.hidden).to eq(true)
+        end
+      end
+
+      describe "#perform_unhide_post" do
+        it "unhides the post and transitions to approved" do
+          post.update!(hidden: true)
+
+          result = reviewable.reload.perform admin, :unhide_post
+
+          expect(result.transition_to).to eq :approved
+          expect(post.reload.hidden).to eq(false)
+        end
+      end
+
+      describe "#perform_no_action_post" do
+        it "keeps the post and transitions to ignored" do
+          result = reviewable.perform admin, :no_action_post
+
+          expect(result.transition_to).to eq :ignored
+          expect(Post.where(id: post.id).exists?).to eq(true)
+        end
+
+        it "keeps the post hidden and transitions to ignored" do
+          post.update!(hidden: true)
+
+          result = reviewable.reload.perform admin, :no_action_post
+
+          expect(result.transition_to).to eq :ignored
+          expect(post.reload.hidden).to eq(true)
+        end
+
+        it "keeps the post deleted and transitions to ignored" do
+          post.trash!
+
+          result = reviewable.reload.perform admin, :no_action_post
+
+          expect(result.transition_to).to eq :ignored
+          expect(Post.where(id: post.id).exists?).to eq(false)
+        end
+      end
+
+      describe "#perform_restore_post" do
+        it "restores the post and transitions to approved" do
+          post.trash!
+
+          result = reviewable.reload.perform admin, :restore_post
+
+          expect(result.transition_to).to eq :approved
+          expect(Post.where(id: post.id).exists?).to eq(true)
+        end
+      end
+
+      describe "#perform_edit_post" do
+        it "transitions to approved (actual edit is client-side)" do
+          result = reviewable.perform admin, :edit_post
+
+          expect(result.transition_to).to eq :approved
+        end
+      end
+
       describe "#perform_silence_user" do
         it "transitions to rejected" do
           result = reviewable.perform admin, :silence_user
@@ -210,10 +333,10 @@ RSpec.describe ReviewablePost do
       end
 
       describe "#perform_no_action_user" do
-        it "transitions to approved" do
+        it "transitions to ignored" do
           result = reviewable.perform admin, :no_action_user
 
-          expect(result.transition_to).to eq :approved
+          expect(result.transition_to).to eq :ignored
         end
       end
     end

spec/models/reviewable_queued_post_spec.rb

@@ -380,6 +380,22 @@ RSpec.describe ReviewableQueuedPost, type: :model do
         allow_any_instance_of(Guardian).to receive(:can_see_reviewable_ui_refresh?).and_return(true)
       end
 
+      it "creates separate bundles for post and user actions" do
+        actions = reviewable.actions_for(Guardian.new(admin))
+        bundle_ids = actions.bundles.map(&:id)
+
+        expect(bundle_ids).to include("#{reviewable.id}-post-actions")
+        expect(bundle_ids).to include("#{reviewable.id}-user-actions")
+      end
+
+      it "includes post actions in the post bundle" do
+        actions = reviewable.actions_for(Guardian.new(admin))
+
+        expect(actions.has?(:approve_post)).to eq(true)
+        expect(actions.has?(:reject_post)).to eq(true)
+        expect(actions.has?(:revise_and_reject_post)).to eq(true)
+      end
+
       it "includes user actions in the user bundle" do
         actions = reviewable.actions_for(Guardian.new(admin))