DEV: Improve the helper methods for defining reviewable actions (#35406)

This change adds a new `ReviewableActionBuilder#build_bundle` helper for
quickly defining action bundles that can be performed on reviewables.
`ReviewableActionBuilder#build_action` has also been updated to allow
plugin-defined actions to appear correctly.

The core reviewable types have been updated to use this new method, and
I've also added support for reviewable chat messages, to demonstrate
plugin support.

Co-authored-by: Krzysztof Kotlarek <kotlarek.krzysztof@gmail.com>

app/models/concerns/reviewable_action_builder.rb

@@ -3,81 +3,69 @@
 module ReviewableActionBuilder
   extend ActiveSupport::Concern
 
+  attr_accessor :actions, :guardian, :action_args
+
   # 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)
+  def build_post_actions_bundle
+    bundle_actions = { no_action_post: {} }
+    if target_post
+      if target_post.trashed? && @guardian.can_recover_post?(target_post)
+        bundle_actions[:restore_post] = {}
       end
+
+      if target_post.hidden?
+        bundle_actions[:unhide_post] = {} if !target_post.user_deleted?
+      else
+        bundle_actions[:hide_post] = {}
+      end
+
+      if @guardian.can_delete_post_or_topic?(target_post)
+        bundle_actions[:delete_post] = {}
+        bundle_actions[:delete_post_and_replies] = { confirm: true } if target_post.reply_count > 0
+      end
+
+      bundle_actions[:edit_post] = { client_action: "edit" }
+
+      bundle_actions[:convert_to_pm] = {}
     end
 
-    build_action(actions, :edit_post, bundle:, client_action: "edit")
-
-    build_action(actions, :convert_to_pm, bundle:)
-
-    bundle
+    build_bundle(
+      "#{id}-post-actions",
+      "reviewables.actions.post_actions.bundle_title",
+      bundle_actions,
+      source: "core",
+    )
   end
 
   # Standard user-actions bundle and default user 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 user actions bundle.
-  def build_user_actions_bundle(actions, guardian)
-    bundle =
-      actions.add_bundle(
-        "#{id}-user-actions",
-        label: "reviewables.actions.user_actions.bundle_title",
-      )
+  def build_user_actions_bundle
+    bundle_actions = { no_action_user: {} }
+    if target_user
+      if @guardian.can_silence_user?(target_user)
+        bundle_actions[:silence_user] = { client_action: "silence" }
+      end
 
-    # Always include the no-op action
-    build_action(actions, :no_action_user, bundle:)
+      if @guardian.can_suspend?(target_user)
+        bundle_actions[:suspend_user] = { client_action: "suspend" }
+      end
 
-    return bundle unless target_user
-
-    if guardian.can_silence_user?(target_user)
-      build_action(actions, :silence_user, bundle:, client_action: "silence")
+      if @guardian.can_delete_user?(target_user)
+        bundle_actions[:delete_user] = {}
+        bundle_actions[:delete_and_block_user] = {}
+      end
     end
 
-    if guardian.can_suspend?(target_user)
-      build_action(actions, :suspend_user, bundle:, client_action: "suspend")
-    end
-
-    if guardian.can_delete_user?(target_user)
-      build_action(actions, :delete_user, bundle:)
-      build_action(actions, :delete_and_block_user, bundle:)
-    end
-
-    bundle
+    build_bundle(
+      "#{id}-user-actions",
+      "reviewables.actions.user_actions.bundle_title",
+      bundle_actions,
+      source: "core",
+    )
   end
 
   # Build actions for the reviewable based on the current state and guardian permissions.
@@ -90,8 +78,12 @@ module ReviewableActionBuilder
   #
   # @return [void]
   def build_actions(actions, guardian, args)
+    @actions = actions
+    @guardian = guardian
+    @action_args = args
+
     if guardian.can_see_reviewable_ui_refresh?
-      build_new_separated_actions(actions, guardian, args)
+      build_new_separated_actions
     else
       build_legacy_combined_actions(actions, guardian, args)
     end
@@ -120,26 +112,42 @@ module ReviewableActionBuilder
   #
   # @TODO (reviewable-refresh) Remove this method once the new UI is fully implemented.
   #
-  # @param actions [Reviewable::Actions] Actions instance to add the bundle to.
-  # @param guardian [Guardian] Guardian instance to check permissions.
-  # @param args [Hash] Additional arguments for building actions.
-  #
   # @return [void]
-  def build_new_separated_actions(actions, guardian, args)
+  def build_new_separated_actions
     raise NotImplementedError, "Including class must implement #build_new_separated_actions"
   end
 
+  # Build a bundle of actions and add it to the provided actions list.
+  #
+  # @param id [String] ID for the bundle, used to derive I18n keys.
+  # @param label [String] I18n key for the bundle label.
+  # @param bundle_actions [Hash] Hash of action IDs and optional params to pass to build_action.
+  # @option bundle_actions [Symbol] :client_action Optional client-side action identifier (e.g. "edit").
+  # @option bundle_actions [Symbol] :confirm When true, uses "reviewables.actions.<id>.confirm" for confirm_message.
+  # @option bundle_actions [Symbol] :require_reject_reason When true, requires a rejection reason for the action.
+  # @param source [String] Optional source string for namespacing I18n keys. Will default to `type_source`.
+  #
+  # @return [Reviewable::Actions::Bundle] The created bundle.
+  def build_bundle(id, label, bundle_actions = {}, source: nil)
+    bundle = @actions.add_bundle(id, label:)
+    bundle_actions.each do |action_id, action_params|
+      build_action(@actions, action_id, bundle:, **action_params || {}, source:)
+    end
+    bundle
+  end
+
   # 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 button_class [String] Optional CSS class for buttons in clients that render it. Ignored in the refreshed UI.
   # @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.
+  # @param source [String] Optional source string for namespacing I18n keys. Will default to `type_source`.
   #
   # @return [Reviewable::Actions] The updated actions instance.
   def build_action(
@@ -150,10 +158,17 @@ module ReviewableActionBuilder
     bundle: nil,
     client_action: nil,
     confirm: false,
-    require_reject_reason: false
+    require_reject_reason: false,
+    source: nil
   )
     actions.add(id, bundle: bundle) do |action|
-      prefix = "reviewables.actions.#{id}"
+      source ||= type_source
+      if source == "core"
+        prefix = "reviewables.actions.#{id}"
+      else
+        prefix = "#{source.underscore}.reviewables.actions.#{id}"
+      end
+
       action.icon = icon if icon
       action.button_class = button_class if button_class
       action.label = "#{prefix}.title"
@@ -222,14 +237,8 @@ module ReviewableActionBuilder
   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
+    # TODO (reviewable-refresh): Implement convert to PM logic
+    create_result(:success, :rejected, [created_by_id], false)
   end
 
   private

app/models/reviewable_flagged_post.rb

@@ -149,9 +149,9 @@ class ReviewableFlaggedPost < Reviewable
   end
 
   # 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)
+  def build_new_separated_actions
+    build_post_actions_bundle
+    build_user_actions_bundle
   end
 
   def perform_ignore(performed_by, args)

app/models/reviewable_post.rb

@@ -79,9 +79,9 @@ class ReviewablePost < Reviewable
   end
 
   # 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)
+  def build_new_separated_actions
+    build_post_actions_bundle
+    build_user_actions_bundle
   end
 
   # TODO (reviewable-refresh): Remove combined actions below when fully migrated to new UI

app/models/reviewable_queued_post.rb

@@ -71,9 +71,9 @@ class ReviewableQueuedPost < Reviewable
     build_action(actions, :delete) if guardian.can_delete?(self)
   end
 
-  def build_new_separated_actions(actions, guardian, args)
+  def build_new_separated_actions
     # 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)
+    post_actions_bundle = build_post_actions_bundle
 
     unless approved?
       if topic&.closed?
@@ -89,7 +89,7 @@ class ReviewableQueuedPost < Reviewable
     end
 
     # User actions bundle
-    build_user_actions_bundle(actions, guardian) if pending?
+    build_user_actions_bundle if pending?
   end
 
   def build_editable_fields(fields, guardian, args)

app/models/reviewable_user.rb

@@ -22,8 +22,8 @@ class ReviewableUser < Reviewable
     delete_user_actions(actions, require_reject_reason: !is_a_suspect_user?)
   end
 
-  def build_new_separated_actions(actions, guardian, args)
-    build_legacy_combined_actions(actions, guardian, args)
+  def build_new_separated_actions
+    build_legacy_combined_actions(@actions, @guardian, @action_args)
   end
 
   def perform_approve_user(performed_by, args)

plugins/chat/app/models/chat/reviewable_message.rb

@@ -2,6 +2,8 @@
 
 module Chat
   class ReviewableMessage < Reviewable
+    include ReviewableActionBuilder
+
     validates :type, length: { maximum: 100 }
     validates :target_type, length: { maximum: 100 }
 
@@ -38,7 +40,8 @@ module Chat
       nil
     end
 
-    def build_actions(actions, guardian, args)
+    # TODO (reviewable-refresh): Remove this method when fully migrated to new UI
+    def build_legacy_combined_actions(actions, guardian, args)
       return unless pending?
 
       return build_action(actions, :ignore, icon: "up-right-from-square") if chat_message.blank?
@@ -86,6 +89,24 @@ module Chat
       end
     end
 
+    # TODO (reviewable-refresh): Merge this method into build_actions when fully migrated to new UI
+    def build_new_separated_actions
+      bundle_actions = { no_action_message: {} }
+      if chat_message.deleted_at?
+        bundle_actions[:restore_message] = {}
+      else
+        bundle_actions[:delete_message] = {}
+      end
+      build_bundle(
+        "#{id}-message-actions",
+        "chat.reviewables.actions.message_actions.bundle_title",
+        bundle_actions,
+      )
+
+      build_user_actions_bundle
+    end
+
+    # TODO (reviewable-refresh): Remove combined actions below when fully migrated to new UI
     def perform_agree_and_keep_message(performed_by, args)
       agree
     end
@@ -117,6 +138,25 @@ module Chat
     def perform_agree_and_keep_deleted(performed_by, args)
       agree
     end
+    # TODO (reviewable-refresh): Remove combined actions above when fully migrated to new UI
+
+    def perform_no_action_message(performed_by, args)
+      if chat_message.deleted_at?
+        create_result(:success, :approved, [created_by_id], true)
+      else
+        create_result(:success, :rejected, [created_by_id], true)
+      end
+    end
+
+    def perform_restore_message(_performed_by, args)
+      chat_message.recover!
+      create_result(:success, :rejected, [created_by_id], true)
+    end
+
+    def perform_delete_message(performed_by, args)
+      chat_message.trash!(performed_by)
+      create_result(:success, :approved, [created_by_id], true)
+    end
 
     private
 
@@ -145,26 +185,6 @@ module Chat
         result.update_flag_stats = { status: :ignored, user_ids: flagged_by_user_ids }
       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 = "chat.#{prefix}.title"
-        action.description = "chat.#{prefix}.description"
-        action.client_action = client_action
-        action.confirm_message = "#{prefix}.confirm" if confirm
-      end
-    end
   end
 end
 

plugins/chat/assets/javascripts/discourse/components/reviewable-refresh/chat-message.gjs

@@ -0,0 +1,7 @@
+import ReviewableChatMessage from "../reviewable-chat-message";
+
+<template>
+  <ReviewableChatMessage @reviewable={{@reviewable}}>
+    {{yield}}
+  </ReviewableChatMessage>
+</template>

plugins/chat/config/locales/server.en.yml

@@ -101,6 +101,17 @@ en:
     reviewables:
       message_already_handled: "Thanks, but we've already reviewed this message and determined it does not need to be flagged again."
       actions:
+        message_actions:
+          bundle_title: "What do you want to do with the message?"
+        no_action_message:
+          title: "No action"
+          description: "Leave the message as is."
+        delete_message:
+          title: "Delete message"
+          description: "Permanently delete the message."
+        restore_message:
+          title: "Restore message"
+          description: "Restore the message so that users can see it."
         agree:
           title: "Agree..."
         agree_and_keep_message:

spec/models/concerns/reviewable_action_builder_spec.rb

@@ -12,8 +12,14 @@ RSpec.describe ReviewableActionBuilder do
     end
     fab!(:post_actions) { Reviewable::Actions.new(reviewable_post, guardian) }
 
+    before do
+      reviewable_post.instance_variable_set(:@actions, post_actions)
+      reviewable_post.instance_variable_set(:@guardian, guardian)
+      reviewable_post.instance_variable_set(:@action_args, {})
+    end
+
     it "creates a user bundle with standard actions when allowed" do
-      bundle = reviewable_post.build_user_actions_bundle(post_actions, guardian)
+      bundle = reviewable_post.build_user_actions_bundle
 
       # bundle id and label
       expect(bundle.id).to eq("#{reviewable_post.id}-user-actions")
@@ -37,7 +43,7 @@ RSpec.describe ReviewableActionBuilder do
     it "includes only the no-op action when user is nil" do
       allow(reviewable_post).to receive(:target_created_by).and_return(nil)
 
-      bundle = reviewable_post.build_user_actions_bundle(post_actions, guardian)
+      bundle = reviewable_post.build_user_actions_bundle
       server_actions = bundle.actions.map(&:server_action)
 
       expect(server_actions).to include("no_action_user")