DEV: Unify reviewable action definition. (#34166)

This is a step towards unifying how reviewable types are defined.

In this change, we add a new `ReviewableActionBuilder` concern, which can be included by the various reviewable types to give them a standard helper for adding actions that can be performed on the reviewable.

The first feature for this concern takes the `build_action()` helper that some reviewables used, and makes it available to all reviewables. This is done in a backward compatible fashion, so it will work even whilst we transition to the new reviewable UI.

app/models/concerns/reviewable_action_builder.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+module ReviewableActionBuilder
+  extend ActiveSupport::Concern
+
+  # Build a single reviewable action and add it to the provided actions list.
+  # This is the canonical API used by both the legacy and refreshed UI code paths.
+  #
+  # @param actions [Reviewable::Actions] Actions instance to add to.
+  # @param id [Symbol] Symbol for the action, used to derive I18n keys.
+  # @param icon [String] Optional name of the icon to display with the action. Ignored in the refreshed UI.
+  # @param button_class [String] Optional CSS class for buttons in clients that render it.
+  # @param bundle [Reviewable::Actions::Bundle] Optional bundle object returned by add_bundle to group actions.
+  # @param client_action [String] Optional client-side action identifier (e.g. "edit").
+  # @param confirm [Boolean] When true, uses "reviewables.actions.<id>.confirm" for confirm_message.
+  # @param require_reject_reason [Boolean] When true, requires a rejection reason for the action.
+  #
+  # @return [Reviewable::Actions] The updated actions instance.
+  def build_action(
+    actions,
+    id,
+    icon: nil,
+    button_class: nil,
+    bundle: nil,
+    client_action: nil,
+    confirm: false,
+    require_reject_reason: false
+  )
+    actions.add(id, bundle: bundle) do |action|
+      prefix = "reviewables.actions.#{id}"
+      action.icon = icon if icon
+      action.button_class = button_class if button_class
+      action.label = "#{prefix}.title"
+      action.description = "#{prefix}.description"
+      action.client_action = client_action if client_action
+      action.confirm_message = "#{prefix}.confirm" if confirm
+      action.completed_message = "#{prefix}.complete"
+      action.require_reject_reason = require_reject_reason
+    end
+  end
+end

app/models/reviewable_flagged_post.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class ReviewableFlaggedPost < Reviewable
+  include ReviewableActionBuilder
+
   scope :pending_and_default_visible, -> { pending.default_visible }
 
   # Penalties are handled by the modal after the action is performed
@@ -311,27 +313,6 @@ class ReviewableFlaggedPost < Reviewable
     end
   end
 
-  def build_action(
-    actions,
-    id,
-    icon:,
-    button_class: nil,
-    bundle: nil,
-    client_action: nil,
-    confirm: false
-  )
-    actions.add(id, bundle: bundle) do |action|
-      prefix = "reviewables.actions.#{id}"
-      action.icon = icon
-      action.button_class = button_class
-      action.label = "#{prefix}.title"
-      action.description = "#{prefix}.description"
-      action.client_action = client_action
-      action.confirm_message = "#{prefix}.confirm" if confirm
-      action.completed_message = "#{prefix}.complete"
-    end
-  end
-
   def unassign_topic(performed_by, post)
     topic = post.topic
     return unless topic && performed_by && SiteSetting.reviewable_claiming != "disabled"

app/models/reviewable_post.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class ReviewablePost < Reviewable
+  include ReviewableActionBuilder
+
   def self.action_aliases
     { reject_and_silence: :reject_and_suspend }
   end
@@ -108,27 +110,6 @@ class ReviewablePost < Reviewable
     @post ||= (target || Post.with_deleted.find_by(id: target_id))
   end
 
-  def build_action(
-    actions,
-    id,
-    icon:,
-    button_class: nil,
-    bundle: nil,
-    client_action: nil,
-    confirm: false
-  )
-    actions.add(id, bundle: bundle) do |action|
-      prefix = "reviewables.actions.#{id}"
-      action.icon = icon
-      action.button_class = button_class
-      action.label = "#{prefix}.title"
-      action.description = "#{prefix}.description"
-      action.client_action = client_action
-      action.confirm_message = "#{prefix}.confirm" if confirm
-      action.completed_message = "#{prefix}.complete"
-    end
-  end
-
   def successful_transition(to_state, recalculate_score: true)
     create_result(:success, to_state) do |result|
       result.recalculate_score = recalculate_score

app/models/reviewable_queued_post.rb

@@ -1,6 +1,12 @@
 # frozen_string_literal: true
 
 class ReviewableQueuedPost < Reviewable
+  include ReviewableActionBuilder
+
+  def self.action_aliases
+    { discard_post: :reject_post }
+  end
+
   after_create do
     # Backwards compatibility, new code should listen for `reviewable_created`
     DiscourseEvent.trigger(:queued_post_created, self)
@@ -35,20 +41,9 @@ class ReviewableQueuedPost < Reviewable
   def build_actions(actions, guardian, args)
     unless approved?
       if topic&.closed?
-        actions.add(:approve_post_closed) do |a|
-          a.icon = "check"
-          a.label = "reviewables.actions.approve_post.title"
-          a.confirm_message = "reviewables.actions.approve_post.confirm_closed"
-          a.completed_message = "reviewables.actions.approve_post.complete"
-        end
+        build_action(actions, :approve_post_closed, icon: "check", confirm: true)
       else
-        if target_created_by.present?
-          actions.add(:approve_post) do |a|
-            a.icon = "check"
-            a.label = "reviewables.actions.approve_post.title"
-            a.completed_message = "reviewables.actions.approve_post.complete"
-          end
-        end
+        build_action(actions, :approve_post, icon: "check") if target_created_by.present?
       end
     end
 
@@ -57,27 +52,22 @@ class ReviewableQueuedPost < Reviewable
         reject_bundle =
           actions.add_bundle("#{id}-reject", label: "reviewables.actions.reject_post.title")
 
-        actions.add(:reject_post, bundle: reject_bundle) do |a|
-          a.icon = "xmark"
-          a.label = "reviewables.actions.discard_post.title"
-          a.button_class = "reject-post"
-        end
+        build_action(
+          actions,
+          :discard_post,
+          bundle: reject_bundle,
+          icon: "xmark",
+          button_class: "reject-post",
+        )
         delete_user_actions(actions, reject_bundle)
       else
-        actions.add(:reject_post) do |a|
-          a.icon = "xmark"
-          a.label = "reviewables.actions.reject_post.title"
-        end
+        build_action(actions, :reject_post, icon: "xmark")
       end
 
-      actions.add(:revise_and_reject_post) do |a|
-        a.label = "reviewables.actions.revise_and_reject_post.title"
-      end
+      build_action(actions, :revise_and_reject_post)
     end
 
-    actions.add(:delete) do |a|
-      a.label = "reviewables.actions.delete_single.title"
-    end if guardian.can_delete?(self)
+    build_action(actions, :delete) if guardian.can_delete?(self)
   end
 
   def build_editable_fields(fields, guardian, args)

app/models/reviewable_user.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class ReviewableUser < Reviewable
+  include ReviewableActionBuilder
+
   def self.create_for(user)
     create(created_by_id: Discourse.system_user.id, target: user)
   end
@@ -12,12 +14,7 @@ class ReviewableUser < Reviewable
   def build_actions(actions, guardian, args)
     return unless pending?
 
-    if guardian.can_approve?(target)
-      actions.add(:approve_user) do |a|
-        a.icon = "user-plus"
-        a.label = "reviewables.actions.approve_user.title"
-      end
-    end
+    build_action(actions, :approve_user, icon: "user-plus") if guardian.can_approve?(target)
 
     delete_user_actions(actions, require_reject_reason: !is_a_suspect_user?)
   end

config/locales/server.en.yml

@@ -5730,11 +5730,9 @@ en:
         title: "Agree and Edit Post"
         description: "Agree with flag and open a composer window to edit the post."
         complete: "Flag acknowledged, you can now edit the post."
-      delete_single:
+      delete:
         title: "Delete"
         complete: "Post deleted."
-      delete:
-        title: "Delete…"
       delete_and_ignore:
         title: "Ignore flag and delete post"
         description: "Ignore the flag by removing it from the queue and delete the post; if the first post, delete the topic as well. "
@@ -5778,7 +5776,10 @@ en:
         complete: "Post approved."
       approve_post:
         title: "Approve Post"
-        confirm_closed: "This topic is closed. Would you like to create the post anyway?"
+        complete: "Post approved."
+      approve_post_closed:
+        title: "Approve Post"
+        confirm: "This topic is closed. Would you like to create the post anyway?"
         complete: "Post approved."
       reject_post:
         title: "Reject Post"

lib/reviewable/actions.rb

@@ -17,7 +17,7 @@ class Reviewable < ActiveRecord::Base
       {
         approve: Action.new(:approve, "thumbs-up", "reviewables.actions.approve.title"),
         reject: Action.new(:reject, "thumbs-down", "reviewables.actions.reject.title"),
-        delete: Action.new(:delete, "trash-can", "reviewables.actions.delete_single.title"),
+        delete: Action.new(:delete, "trash-can", "reviewables.actions.delete.title"),
       }
     end
 

spec/models/concerns/reviewable_action_builder_spec.rb

@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+RSpec.describe ReviewableActionBuilder do
+  fab!(:admin)
+  fab!(:guardian) { Guardian.new(admin) }
+  fab!(:user)
+
+  describe "#build_action" do
+    fab!(:reviewable_user) { ReviewableUser.create_for(user) }
+
+    it "adds an action with i18n-derived defaults" do
+      user_actions = Reviewable::Actions.new(reviewable_user, guardian)
+
+      reviewable_user.build_action(user_actions, :approve_user)
+
+      action = user_actions.to_a.first
+      expect(action).to be_present
+      expect(action.label).to eq("reviewables.actions.approve_user.title")
+      expect(action.description).to eq("reviewables.actions.approve_user.description")
+      expect(action.completed_message).to eq("reviewables.actions.approve_user.complete")
+      expect(action.confirm_message).to be_nil
+      expect(action.icon).to be_nil
+      expect(action.button_class).to be_nil
+      expect(action.client_action).to be_nil
+      expect(action.require_reject_reason).to be(false)
+
+      # It should attach to a bundle automatically matching the full action id
+      bundle_ids = user_actions.bundles.map(&:id)
+      expect(bundle_ids).to include(action.id)
+    end
+
+    it "sets optional fields when provided" do
+      user_actions = Reviewable::Actions.new(reviewable_user, guardian)
+
+      reviewable_user.build_action(
+        user_actions,
+        :approve_user,
+        icon: "user-plus",
+        button_class: "btn-primary",
+        client_action: "go",
+        confirm: true,
+        require_reject_reason: true,
+      )
+
+      action = user_actions.to_a.first
+      expect(action.icon).to eq("user-plus")
+      expect(action.button_class).to eq("btn-primary")
+      expect(action.client_action).to eq("go")
+      expect(action.confirm_message).to eq("reviewables.actions.approve_user.confirm")
+      expect(action.require_reject_reason).to be(true)
+    end
+
+    it "adds the action to the provided bundle" do
+      user_actions = Reviewable::Actions.new(reviewable_user, guardian)
+
+      bundle = user_actions.add_bundle("custom-bundle", icon: "x", label: "Custom")
+      reviewable_user.build_action(user_actions, :approve_user, bundle: bundle)
+
+      action = user_actions.to_a.first
+      expect(user_actions.bundles).to include(bundle)
+      expect(bundle.actions).to include(action)
+    end
+  end
+end