FEATURE: custom flag can require additional message (#27908)

Allow admin to create custom flag which requires an additional message.

I decided to rename the old `custom_flag` into `require_message` as it is more descriptive.

app/assets/javascripts/admin/addon/components/admin-flags-form.gjs

@@ -1,14 +1,15 @@
 import Component from "@glimmer/component";
 import { tracked } from "@glimmer/tracking";
-import { Input } from "@ember/component";
+import { fn, hash } from "@ember/helper";
-import { hash } from "@ember/helper";
 import { TextArea } from "@ember/legacy-built-in-components";
+import { on } from "@ember/modifier";
 import { action } from "@ember/object";
 import { LinkTo } from "@ember/routing";
 import { service } from "@ember/service";
 import { isEmpty } from "@ember/utils";
 import { not } from "truth-helpers";
 import DButton from "discourse/components/d-button";
+import withEventValue from "discourse/helpers/with-event-value";
 import { ajax } from "discourse/lib/ajax";
 import { popupAjaxError } from "discourse/lib/ajax-error";
 import dIcon from "discourse-common/helpers/d-icon";
@@ -23,6 +24,7 @@ export default class AdminFlagsForm extends Component {
   @service site;
 
   @tracked enabled = true;
+  @tracked requireMessage = false;
   @tracked name;
   @tracked description;
   @tracked appliesTo;
@@ -33,6 +35,7 @@ export default class AdminFlagsForm extends Component {
       this.name = this.args.flag.name;
       this.description = this.args.flag.description;
       this.appliesTo = this.args.flag.applies_to;
+      this.requireMessage = this.args.flag.require_message;
       this.enabled = this.args.flag.enabled;
     }
   }
@@ -73,6 +76,16 @@ export default class AdminFlagsForm extends Component {
     this.isUpdate ? this.update() : this.create();
   }
 
+  @action
+  onToggleRequireMessage(e) {
+    this.requireMessage = e.target.checked;
+  }
+
+  @action
+  onToggleEnabled(e) {
+    this.enabled = e.target.checked;
+  }
+
   @bind
   create() {
     return ajax(`/admin/config/flags`, {
@@ -98,6 +111,7 @@ export default class AdminFlagsForm extends Component {
         this.args.flag.name = response.flag.name;
         this.args.flag.description = response.flag.description;
         this.args.flag.applies_to = response.flag.applies_to;
+        this.args.flag.require_message = response.flag.require_message;
         this.args.flag.enabled = response.flag.enabled;
         this.router.transitionTo("adminConfig.flags");
       })
@@ -112,6 +126,7 @@ export default class AdminFlagsForm extends Component {
       name: this.name,
       description: this.description,
       applies_to: this.appliesTo,
+      require_message: this.requireMessage,
       enabled: this.enabled,
     };
   }
@@ -132,12 +147,13 @@ export default class AdminFlagsForm extends Component {
             <label for="name">
               {{i18n "admin.config_areas.flags.form.name"}}
             </label>
-            <Input
+            <input
               name="name"
-              @type="text"
+              type="text"
-              @value={{this.name}}
+              value={{this.name}}
               maxlength="200"
               class="admin-flag-form__name"
+              {{on "input" (withEventValue (fn (mut this.name)))}}
             />
           </div>
 
@@ -164,9 +180,31 @@ export default class AdminFlagsForm extends Component {
             />
           </div>
 
+          <div class="control-group">
+            <label class="checkbox-label admin-flag-form__require-reason">
+              <input
+                {{on "input" this.onToggleRequireMessage}}
+                type="checkbox"
+                checked={{this.requireMessage}}
+              />
+              <div>
+                {{i18n "admin.config_areas.flags.form.require_message"}}
+                <div class="admin-flag-form__require-message-description">
+                  {{i18n
+                    "admin.config_areas.flags.form.require_message_description"
+                  }}
+                </div>
+              </div>
+            </label>
+          </div>
+
           <div class="control-group">
             <label class="checkbox-label admin-flag-form__enabled">
-              <Input @type="checkbox" @checked={{this.enabled}} />
+              <input
+                {{on "input" this.onToggleEnabled}}
+                type="checkbox"
+                checked={{this.enabled}}
+              />
               {{i18n "admin.config_areas.flags.form.enabled"}}
             </label>
           </div>

app/assets/javascripts/discourse/app/components/flag-action-type.js

@@ -35,7 +35,7 @@ export default Component.extend({
     return flag === selectedFlag;
   },
 
-  showMessageInput: and("flag.is_custom_flag", "selected"),
+  showMessageInput: and("flag.require_message", "selected"),
   showConfirmation: and("flag.isIllegal", "selected"),
   showDescription: not("showMessageInput"),
   isNotifyUser: equal("flag.name_key", "notify_user"),

app/assets/javascripts/discourse/app/components/modal/flag.js

@@ -73,7 +73,7 @@ export default class Flag extends Component {
   }
 
   get submitLabel() {
-    if (this.selected?.is_custom_flag) {
+    if (this.selected?.require_message) {
       return this.args.model.flagTarget.customSubmitLabel();
     }
 
@@ -97,7 +97,7 @@ export default class Flag extends Component {
       return false;
     }
 
-    if (!this.selected.is_custom_flag) {
+    if (!this.selected.require_message) {
       return true;
     }
 
@@ -119,7 +119,7 @@ export default class Flag extends Component {
   get canTakeAction() {
     return (
       !this.args.model.flagTarget.targetsTopic() &&
-      !this.selected?.is_custom_flag &&
+      !this.selected?.require_message &&
       this.currentUser.staff
     );
   }
@@ -184,7 +184,7 @@ export default class Flag extends Component {
 
   @action
   createFlag(opts = {}) {
-    if (this.selected.is_custom_flag) {
+    if (this.selected.require_message) {
       opts.message = this.message;
     }
     this.args.model.flagTarget.create(this, opts);

app/assets/javascripts/discourse/app/models/post-action-type.js

@@ -1,9 +1,8 @@
-import { equal, not } from "@ember/object/computed";
+import { equal } from "@ember/object/computed";
 import RestModel from "discourse/models/rest";
 
 export const MAX_MESSAGE_LENGTH = 500;
 
 export default class PostActionType extends RestModel {
-  @not("is_custom_flag") notCustomFlag;
   @equal("name_key", "illegal") isIllegal;
 }

app/assets/javascripts/discourse/tests/fixtures/site-fixtures.js

@@ -568,7 +568,7 @@ export default {
           is_flag: false,
           icon: null,
           id: 1,
-          is_custom_flag: false,
+          require_message: false,
         },
         {
           name_key: "like",
@@ -578,7 +578,7 @@ export default {
           is_flag: false,
           icon: "heart",
           id: 2,
-          is_custom_flag: false,
+          require_message: false,
         },
         {
           name_key: "off_topic",
@@ -589,7 +589,7 @@ export default {
           is_flag: true,
           icon: null,
           id: 3,
-          is_custom_flag: false,
+          require_message: false,
           enabled: true,
           applies_to: ["Post", "Chat::Message"]
         },
@@ -603,7 +603,7 @@ export default {
           is_flag: true,
           icon: null,
           id: 4,
-          is_custom_flag: false,
+          require_message: false,
           enabled: true,
           applies_to: ["Post", "Topic", "Chat::Message"]
         },
@@ -615,7 +615,7 @@ export default {
           is_flag: false,
           icon: null,
           id: 5,
-          is_custom_flag: false,
+          require_message: false,
           enabled: true
         },
         {
@@ -627,7 +627,7 @@ export default {
           is_flag: true,
           icon: null,
           id: 8,
-          is_custom_flag: false,
+          require_message: false,
           enabled: true,
           applies_to: ["Post", "Topic", "Chat::Message"]
         },
@@ -641,7 +641,7 @@ export default {
           is_flag: true,
           icon: null,
           id: 6,
-          is_custom_flag: true,
+          require_message: true,
           enabled: true,
           applies_to: ["Post", "Topic", "Chat::Message"]
         },
@@ -654,7 +654,7 @@ export default {
           is_flag: true,
           icon: null,
           id: 7,
-          is_custom_flag: true,
+          require_message: true,
           enabled: true,
           applies_to: ["Post", "Topic", "Chat::Message"]
         },
@@ -668,7 +668,7 @@ export default {
           is_flag: true,
           icon: null,
           id: 4,
-          is_custom_flag: false,
+          require_message: false,
           enabled: true,
           applies_to: ["Post", "Topic", "Chat::Message"]
         },
@@ -680,7 +680,7 @@ export default {
           is_flag: true,
           icon: null,
           id: 8,
-          is_custom_flag: false,
+          require_message: false,
           enabled: true,
           applies_to: ["Post", "Topic", "Chat::Message"]
         },
@@ -692,7 +692,7 @@ export default {
           is_flag: true,
           icon: null,
           id: 7,
-          is_custom_flag: true,
+          require_message: true,
           enabled: true,
           applies_to: ["Post", "Topic", "Chat::Message"]
         },

app/assets/javascripts/discourse/tests/helpers/site.js

@@ -278,7 +278,7 @@ PreloadStore.store("site", {
       is_flag: false,
       icon: null,
       id: 1,
-      is_custom_flag: false,
+      require_message: false,
     },
     {
       name_key: "like",
@@ -288,7 +288,7 @@ PreloadStore.store("site", {
       is_flag: false,
       icon: "heart",
       id: 2,
-      is_custom_flag: false,
+      require_message: false,
     },
     {
       name_key: "off_topic",
@@ -299,7 +299,7 @@ PreloadStore.store("site", {
       is_flag: true,
       icon: null,
       id: 3,
-      is_custom_flag: false,
+      require_message: false,
     },
     {
       name_key: "inappropriate",
@@ -310,7 +310,7 @@ PreloadStore.store("site", {
       is_flag: true,
       icon: null,
       id: 4,
-      is_custom_flag: false,
+      require_message: false,
     },
     {
       name_key: "vote",
@@ -320,7 +320,7 @@ PreloadStore.store("site", {
       is_flag: false,
       icon: null,
       id: 5,
-      is_custom_flag: false,
+      require_message: false,
     },
     {
       name_key: "spam",
@@ -331,7 +331,7 @@ PreloadStore.store("site", {
       is_flag: true,
       icon: null,
       id: 8,
-      is_custom_flag: false,
+      require_message: false,
     },
     {
       name_key: "notify_user",
@@ -342,7 +342,7 @@ PreloadStore.store("site", {
       is_flag: true,
       icon: null,
       id: 6,
-      is_custom_flag: true,
+      require_message: true,
     },
     {
       name_key: "notify_moderators",
@@ -353,7 +353,7 @@ PreloadStore.store("site", {
       is_flag: true,
       icon: null,
       id: 7,
-      is_custom_flag: true,
+      require_message: true,
     },
   ],
   archetypes: [{ id: "regular", name: "Regular Topic", options: [] }],

app/assets/stylesheets/common/admin/flags.scss

@@ -56,6 +56,15 @@
   &__applies-to.select-kit.multi-select {
     width: 60%;
   }
+  &__require-message-description {
+    clear: both;
+    flex-basis: 100%;
+    font-size: var(--font-down-2);
+    margin-top: 0.25em;
+  }
+  label {
+    flex-wrap: wrap;
+  }
 }
 
 .admin-flags__header {

app/models/concerns/reports/moderators_activity.rb

@@ -76,7 +76,7 @@ module Reports::ModeratorsActivity
           disagreed_by_id,
           deferred_by_id
           FROM post_actions
-          WHERE post_action_type_id IN (#{PostActionType.flag_types_without_custom.values.join(",")})
+          WHERE post_action_type_id IN (#{PostActionType.flag_types_without_additional_message.values.join(",")})
           AND created_at >= '#{report.start_date}'
           AND created_at <= '#{report.end_date}'
           ),

app/models/flag.rb

@@ -1,6 +1,9 @@
 # frozen_string_literal: true
 
 class Flag < ActiveRecord::Base
+  # TODO(2025-01-15): krisk remove
+  self.ignored_columns = ["custom_type"]
+
   DEFAULT_VALID_APPLIES_TO = %w[Post Topic]
   MAX_SYSTEM_FLAG_ID = 1000
   MAX_NAME_LENGTH = 200
@@ -62,7 +65,7 @@ end
 #  description      :text
 #  notify_type      :boolean          default(FALSE), not null
 #  auto_action_type :boolean          default(FALSE), not null
-#  custom_type      :boolean          default(FALSE), not null
+#  require_message  :boolean          default(FALSE), not null
 #  applies_to       :string           not null, is an Array
 #  position         :integer          not null
 #  enabled          :boolean          default(TRUE), not null

app/models/post.rb

@@ -556,7 +556,7 @@ class Post < ActiveRecord::Base
 
   def flags
     post_actions.where(
-      post_action_type_id: PostActionType.flag_types_without_custom.values,
+      post_action_type_id: PostActionType.flag_types_without_additional_message.values,
       deleted_at: nil,
     )
   end

app/models/post_action_type.rb

@@ -59,9 +59,9 @@ class PostActionType < ActiveRecord::Base
       @public_type_ids ||= public_types.values
     end
 
-    def flag_types_without_custom
+    def flag_types_without_additional_message
-      return flag_settings.without_custom_types if overridden_by_plugin_or_skipped_db?
+      return flag_settings.without_additional_message_types if overridden_by_plugin_or_skipped_db?
-      flag_enum(all_flags.reject(&:custom_type))
+      flag_enum(all_flags.reject(&:require_message))
     end
 
     def flag_types
@@ -106,9 +106,9 @@ class PostActionType < ActiveRecord::Base
       flag_enum(all_flags.filter(&:enabled))
     end
 
-    def custom_types
+    def additional_message_types
-      return flag_settings.custom_types if overridden_by_plugin_or_skipped_db?
+      return flag_settings.with_additional_message if overridden_by_plugin_or_skipped_db?
-      flag_enum(all_flags.select(&:custom_type))
+      flag_enum(all_flags.select(&:require_message))
     end
 
     def names

app/models/user.rb

@@ -1225,7 +1225,7 @@ class User < ActiveRecord::Base
   def flags_given_count
     PostAction.where(
       user_id: id,
-      post_action_type_id: PostActionType.flag_types_without_custom.values,
+      post_action_type_id: PostActionType.flag_types_without_additional_message.values,
     ).count
   end
 
@@ -1236,7 +1236,10 @@ class User < ActiveRecord::Base
   def flags_received_count
     posts
       .includes(:post_actions)
-      .where("post_actions.post_action_type_id" => PostActionType.flag_types_without_custom.values)
+      .where(
+        "post_actions.post_action_type_id" =>
+          PostActionType.flag_types_without_additional_message.values,
+      )
       .count
   end
 

app/serializers/flag_serializer.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 class FlagSerializer < ApplicationSerializer
-  attributes :id, :name, :name_key, :description, :applies_to, :position, :enabled
+  attributes :id, :name, :name_key, :description, :applies_to, :position, :require_message, :enabled
 end

app/serializers/post_action_type_serializer.rb

@@ -8,7 +8,7 @@ class PostActionTypeSerializer < ApplicationSerializer
     :description,
     :short_description,
     :is_flag,
-    :is_custom_flag,
+    :require_message,
     :enabled,
     :applies_to,
     :is_used,
@@ -16,8 +16,8 @@ class PostActionTypeSerializer < ApplicationSerializer
 
   include ConfigurableUrls
 
-  def is_custom_flag
+  def require_message
-    !!PostActionType.custom_types[object.id]
+    !!PostActionType.additional_message_types[object.id]
   end
 
   def is_flag

app/services/flags/create_flag.rb

@@ -15,6 +15,7 @@ class Flags::CreateFlag
   class Contract
     attribute :name, :string
     attribute :description, :string
+    attribute :require_message, :boolean
     attribute :enabled, :boolean
     attribute :applies_to
     validates :name, presence: true
@@ -26,11 +27,12 @@ class Flags::CreateFlag
 
   private
 
-  def instantiate_flag(name:, description:, applies_to:, enabled:)
+  def instantiate_flag(name:, description:, applies_to:, require_message:, enabled:)
     Flag.new(
       name: name,
       description: description,
       applies_to: applies_to,
+      require_message: require_message,
       enabled: enabled,
       notify_type: true,
     )
@@ -51,6 +53,7 @@ class Flags::CreateFlag
         name: flag.name,
         description: flag.description,
         applies_to: flag.applies_to,
+        require_message: flag.require_message,
         enabled: flag.enabled,
       },
     )

app/services/flags/update_flag.rb

@@ -17,6 +17,7 @@ class Flags::UpdateFlag
   class Contract
     attribute :name, :string
     attribute :description, :string
+    attribute :require_message, :boolean
     attribute :enabled, :boolean
     attribute :applies_to
     validates :name, presence: true
@@ -44,8 +45,14 @@ class Flags::UpdateFlag
     guardian.can_edit_flag?(flag)
   end
 
-  def update(flag:, name:, description:, applies_to:, enabled:)
+  def update(flag:, name:, description:, applies_to:, require_message:, enabled:)
-    flag.update!(name: name, description: description, applies_to: applies_to, enabled: enabled)
+    flag.update!(
+      name: name,
+      description: description,
+      applies_to: applies_to,
+      require_message: require_message,
+      enabled: enabled,
+    )
   end
 
   def log(guardian:, flag:)
@@ -55,6 +62,7 @@ class Flags::UpdateFlag
         name: flag.name,
         description: flag.description,
         applies_to: flag.applies_to,
+        require_message: flag.require_message,
         enabled: flag.enabled,
       },
     )

config/locales/client.en.yml

@@ -5531,6 +5531,8 @@ en:
             non_editable: "You cannot edit this flag because it is a system flag or has already been used in the review system, however you can still disable it."
             delete_flag: "Delete flag"
             non_deletable: "You cannot delete this flag because it is a system flag or has already been used in the review system, however you can still disable it."
+            require_message: "Prompt users to provide additional reasons"
+            require_message_description: "When this flag is selected, a text field will be displayed for the user to provide additional detail about why they are flagging the content"
           more_options:
             title: "More options"
             move_up: "Move up"

db/fixtures/003_flags.rb

@@ -5,7 +5,7 @@ Flag.seed do |s|
   s.name = "notify_user"
   s.notify_type = false
   s.auto_action_type = false
-  s.custom_type = true
+  s.require_message = true
   s.applies_to = %w[Post Chat::Message]
 end
 Flag.seed do |s|
@@ -13,7 +13,7 @@ Flag.seed do |s|
   s.name = "off_topic"
   s.notify_type = true
   s.auto_action_type = true
-  s.custom_type = false
+  s.require_message = false
   s.applies_to = %w[Post Chat::Message]
 end
 Flag.seed do |s|
@@ -21,7 +21,7 @@ Flag.seed do |s|
   s.name = "inappropriate"
   s.notify_type = true
   s.auto_action_type = true
-  s.custom_type = false
+  s.require_message = false
   s.applies_to = %w[Post Topic Chat::Message]
 end
 Flag.seed do |s|
@@ -29,7 +29,7 @@ Flag.seed do |s|
   s.name = "spam"
   s.notify_type = true
   s.auto_action_type = true
-  s.custom_type = false
+  s.require_message = false
   s.applies_to = %w[Post Topic Chat::Message]
 end
 Flag.seed do |s|
@@ -37,7 +37,7 @@ Flag.seed do |s|
   s.name = "illegal"
   s.notify_type = true
   s.auto_action_type = false
-  s.custom_type = true
+  s.require_message = true
   s.applies_to = %w[Post Topic Chat::Message]
 end
 Flag.seed do |s|
@@ -45,7 +45,7 @@ Flag.seed do |s|
   s.name = "notify_moderators"
   s.notify_type = true
   s.auto_action_type = false
-  s.custom_type = true
+  s.require_message = true
   s.applies_to = %w[Post Topic Chat::Message]
 end
 Flag.unscoped.seed do |s|
@@ -53,7 +53,7 @@ Flag.unscoped.seed do |s|
   s.name = "needs_approval"
   s.notify_type = false
   s.auto_action_type = false
-  s.custom_type = false
+  s.require_message = false
   s.score_type = true
   s.applies_to = %w[]
 end

db/migrate/20240714231226_duplicate_flags_custom_type_to_require_message.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class DuplicateFlagsCustomTypeToRequireMessage < ActiveRecord::Migration[7.0]
+  def up
+    add_column :flags, :require_message, :boolean, default: false, null: false
+
+    Migration::ColumnDropper.mark_readonly("flags", "custom_type")
+
+    DB.exec <<~SQL
+      UPDATE flags
+      SET require_message = custom_type
+    SQL
+  end
+
+  def down
+    Migration::ColumnDropper.drop_readonly("flags", "custom_type")
+    remove_column :flags, :require_message
+  end
+end

db/post_migrate/20240714231516_drop_custom_type_from_flags.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class DropCustomTypeFromFlags < ActiveRecord::Migration[7.0]
+  DROPPED_COLUMNS ||= { flags: %i[custom_type] }
+
+  def up
+    DROPPED_COLUMNS.each { |table, columns| Migration::ColumnDropper.execute_drop(table, columns) }
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/badge_queries.rb

@@ -73,7 +73,7 @@ module BadgeQueries
       SELECT pa.user_id, min(pa.id) id
       FROM post_actions pa
       JOIN badge_posts p on p.id = pa.post_id
-      WHERE post_action_type_id IN (#{PostActionType.flag_types_without_custom.values.join(",")}) AND
+      WHERE post_action_type_id IN (#{PostActionType.flag_types_without_additional_message.values.join(",")}) AND
         (:backfill OR pa.post_id IN (:post_ids) )
       GROUP BY pa.user_id
     ) x

lib/flag_settings.rb

@@ -2,11 +2,11 @@
 
 class FlagSettings
   attr_reader(
-    :without_custom_types,
+    :without_additional_message_types,
     :notify_types,
     :topic_flag_types,
     :auto_action_types,
-    :custom_types,
+    :additional_message_types,
     :names,
   )
 
@@ -15,8 +15,8 @@ class FlagSettings
     @topic_flag_types = Enum.new
     @notify_types = Enum.new
     @auto_action_types = Enum.new
-    @custom_types = Enum.new
+    @additional_message_types = Enum.new
-    @without_custom_types = Enum.new
+    @without_additional_message_types = Enum.new
     @names = Enum.new
   end
 
@@ -26,7 +26,7 @@ class FlagSettings
     topic_type: nil,
     notify_type: nil,
     auto_action_type: nil,
-    custom_type: nil,
+    require_message: nil,
     name: nil
   )
     @all_flag_types[name_key] = id
@@ -35,10 +35,10 @@ class FlagSettings
     @auto_action_types[name_key] = id if !!auto_action_type
     @names[id] = name if name
 
-    if !!custom_type
+    if !!require_message
-      @custom_types[name_key] = id
+      @additional_message_types[name_key] = id
     else
-      @without_custom_types[name_key] = id
+      @without_additional_message_types[name_key] = id
     end
   end
 

lib/guardian/post_guardian.rb

@@ -38,7 +38,8 @@ module PostGuardian
 
     taken = opts[:taken_actions].try(:keys).to_a
     is_flag =
-      PostActionType.notify_flag_types[action_key] || PostActionType.custom_types[action_key]
+      PostActionType.notify_flag_types[action_key] ||
+        PostActionType.additional_message_types[action_key]
     already_taken_this_action = taken.any? && taken.include?(PostActionType.types[action_key])
     already_did_flagging = taken.any? && (taken & PostActionType.notify_flag_types.values).any?
 

lib/post_action_creator.rb

@@ -115,7 +115,9 @@ class PostActionCreator
 
     # create meta topic / post if needed
     if @message.present? &&
-         %i[notify_moderators notify_user spam illegal].include?(@post_action_name)
+         (PostActionType.additional_message_types.keys | %i[spam illegal]).include?(
+           @post_action_name,
+         )
       creator = create_message_creator
       # We need to check if the creator exists because it's possible `create_message_creator` returns nil
       # in the event that a `post_action_notify_user_handler` evaluated to false, haulting the post creation.
@@ -325,6 +327,7 @@ class PostActionCreator
         "post_action_types.#{@post_action_name}.email_title",
         title: @post.topic.title,
         locale: SiteSetting.default_locale,
+        default: I18n.t("post_action_types.illegal.email_title"),
       )
 
     body =
@@ -333,6 +336,7 @@ class PostActionCreator
         message: @message,
         link: "#{Discourse.base_url}#{@post.url}",
         locale: SiteSetting.default_locale,
+        default: I18n.t("post_action_types.illegal.email_body"),
       )
 
     create_args = {
@@ -342,18 +346,9 @@ class PostActionCreator
       raw: body,
     }
 
-    if %i[notify_moderators spam illegal].include?(@post_action_name)
+    if @post_action_name == :notify_user
-      create_args[:subtype] = TopicSubtype.notify_moderators
-      create_args[:target_group_names] = [Group[:moderators].name]
-
-      if SiteSetting.enable_category_group_moderation? &&
-           @post.topic&.category&.reviewable_by_group_id?
-        create_args[:target_group_names] << @post.topic.category.reviewable_by_group.name
-      end
-    else
       create_args[:subtype] = TopicSubtype.notify_user
 
-      if @post_action_name == :notify_user
       create_args[:target_usernames] = @post.user.username
 
       # Evaluate DiscoursePluginRegistry.post_action_notify_user_handlers.
@@ -363,10 +358,13 @@ class PostActionCreator
           handler.call(@created_by, @post, @message)
         end
       return if handler_values.any? { |value| value == false }
-      elsif @post_action_name != :notify_moderators
+    else
-        # this is a hack to allow a PM with no recipients, we should think through
+      create_args[:subtype] = TopicSubtype.notify_moderators
-        # a cleaner technique, a PM with myself is valid for flagging
+      create_args[:target_group_names] = [Group[:moderators].name]
-        "x"
+
+      if SiteSetting.enable_category_group_moderation? &&
+           @post.topic&.category&.reviewable_by_group_id?
+        create_args[:target_group_names] << @post.topic.category.reviewable_by_group.name
       end
     end
 

lib/post_revisor.rb

@@ -542,7 +542,7 @@ class PostRevisor
     flaggers = []
     @post
       .post_actions
-      .where(post_action_type_id: PostActionType.flag_types_without_custom.values)
+      .where(post_action_type_id: PostActionType.flag_types_without_additional_message.values)
       .each do |action|
         flaggers << action.user if action.user
         action.remove_act!(Discourse.system_user)

spec/lib/flag_settings_spec.rb

@@ -21,7 +21,7 @@ RSpec.describe FlagSettings do
       settings.add(4, :inappropriate, topic_type: true)
       expect(settings.flag_types).to include(:inappropriate)
       expect(settings.topic_flag_types).to include(:inappropriate)
-      expect(settings.without_custom_types).to include(:inappropriate)
+      expect(settings.without_additional_message_types).to include(:inappropriate)
     end
 
     it "will add a notify type" do
@@ -36,11 +36,11 @@ RSpec.describe FlagSettings do
       expect(settings.auto_action_types).to include(:notify_moderators)
     end
 
-    it "will add a custom type" do
+    it "will add a type which requires message" do
-      settings.add(7, :notify_user, custom_type: true)
+      settings.add(7, :notify_user, require_message: true)
       expect(settings.flag_types).to include(:notify_user)
-      expect(settings.custom_types).to include(:notify_user)
+      expect(settings.additional_message_types).to include(:notify_user)
-      expect(settings.without_custom_types).to be_empty
+      expect(settings.without_additional_message_types).to be_empty
     end
   end
 end

spec/models/post_action_spec.rb

@@ -767,7 +767,7 @@ RSpec.describe PostAction do
 
   # flags are already being tested
   all_types_except_flags =
-    PostActionType.types.except(*PostActionType.flag_types_without_custom.keys)
+    PostActionType.types.except(*PostActionType.flag_types_without_additional_message.keys)
   all_types_except_flags.values.each do |action|
     it "prevents user to act twice at the same time" do
       expect(PostActionCreator.new(eviltrout, post, action).perform).to be_success
@@ -783,6 +783,25 @@ RSpec.describe PostAction do
       expect(result.reviewable_score.meta_topic_id).to be_nil
     end
 
+    it "does not create a message for custom flag when message is not required" do
+      flag_without_message =
+        Fabricate(:flag, name: "flag without message", notify_type: true, require_message: false)
+
+      result =
+        PostActionCreator.new(
+          Discourse.system_user,
+          post,
+          PostActionType.types[:flag_without_message],
+          message: "WAT",
+        ).perform
+
+      expect(result).to be_success
+      expect(result.post_action.related_post_id).to be_nil
+      expect(result.reviewable_score.meta_topic_id).to be_nil
+
+      flag_without_message.destroy!
+    end
+
     %i[notify_moderators notify_user spam].each do |post_action_type|
       it "creates a message for #{post_action_type}" do
         result =
@@ -797,6 +816,25 @@ RSpec.describe PostAction do
       end
     end
 
+    it "creates a message for custom flags when message is required" do
+      flag_with_message =
+        Fabricate(:flag, name: "flag with message", notify_type: true, require_message: true)
+
+      result =
+        PostActionCreator.new(
+          Discourse.system_user,
+          post,
+          PostActionType.types[:flag_with_message],
+          message: "WAT",
+        ).perform
+
+      expect(result).to be_success
+      expect(result.post_action.related_post_id).to be_present
+      expect(result.reviewable_score.meta_topic_id).to be_present
+
+      flag_with_message.destroy!
+    end
+
     it "should raise the right errors when it fails to create a post" do
       user = Fabricate(:user)
       UserSilencer.new(user, Discourse.system_user).silence

spec/requests/api/schemas/json/site_response.json

@@ -351,7 +351,7 @@
             "is_flag": {
               "type": "boolean"
             },
-            "is_custom_flag": {
+            "require_message": {
               "type": "boolean"
             },
             "enabled": {
@@ -371,7 +371,7 @@
             "description",
             "short_description",
             "is_flag",
-            "is_custom_flag",
+            "require_message",
             "enabled",
             "applies_to",
             "is_used"
@@ -403,7 +403,7 @@
             "is_flag": {
               "type": "boolean"
             },
-            "is_custom_flag": {
+            "require_message": {
               "type": "boolean"
             },
             "enabled": {
@@ -423,7 +423,7 @@
             "description",
             "short_description",
             "is_flag",
-            "is_custom_flag",
+            "require_message",
             "enabled",
             "applies_to",
             "is_used"

spec/services/flags/create_flag_spec.rb

@@ -7,6 +7,7 @@ RSpec.describe(Flags::CreateFlag) do
       name: name,
       description: description,
       applies_to: applies_to,
+      require_message: require_message,
       enabled: enabled,
     )
   end
@@ -15,6 +16,7 @@ RSpec.describe(Flags::CreateFlag) do
   let(:description) { "custom flag description" }
   let(:applies_to) { ["Topic"] }
   let(:enabled) { true }
+  let(:require_message) { true }
 
   context "when user is not allowed to perform the action" do
     fab!(:current_user) { Fabricate(:user) }
@@ -72,6 +74,7 @@ RSpec.describe(Flags::CreateFlag) do
       expect(flag.name).to eq("custom flag name")
       expect(flag.description).to eq("custom flag description")
       expect(flag.applies_to).to eq(["Topic"])
+      expect(flag.require_message).to be true
       expect(flag.enabled).to be true
     end
 
@@ -80,7 +83,7 @@ RSpec.describe(Flags::CreateFlag) do
       expect(UserHistory.last).to have_attributes(
         custom_type: "create_flag",
         details:
-          "name: custom flag name\ndescription: custom flag description\napplies_to: [\"Topic\"]\nenabled: true",
+          "name: custom flag name\ndescription: custom flag description\napplies_to: [\"Topic\"]\nrequire_message: true\nenabled: true",
       )
     end
   end

spec/services/flags/update_flag_spec.rb

@@ -10,6 +10,7 @@ RSpec.describe(Flags::UpdateFlag) do
       name: name,
       description: description,
       applies_to: applies_to,
+      require_message: require_message,
       enabled: enabled,
     )
   end
@@ -19,6 +20,7 @@ RSpec.describe(Flags::UpdateFlag) do
   let(:name) { "edited custom flag name" }
   let(:description) { "edited custom flag description" }
   let(:applies_to) { ["Topic"] }
+  let(:require_message) { true }
   let(:enabled) { false }
 
   context "when user is not allowed to perform the action" do
@@ -74,6 +76,7 @@ RSpec.describe(Flags::UpdateFlag) do
       expect(flag.reload.name).to eq("edited custom flag name")
       expect(flag.description).to eq("edited custom flag description")
       expect(flag.applies_to).to eq(["Topic"])
+      expect(flag.require_message).to be true
       expect(flag.enabled).to be false
     end
 
@@ -82,7 +85,7 @@ RSpec.describe(Flags::UpdateFlag) do
       expect(UserHistory.last).to have_attributes(
         custom_type: "update_flag",
         details:
-          "name: edited custom flag name\ndescription: edited custom flag description\napplies_to: [\"Topic\"]\nenabled: false",
+          "name: edited custom flag name\ndescription: edited custom flag description\napplies_to: [\"Topic\"]\nrequire_message: true\nenabled: false",
       )
     end
   end