FIX: custom flag name should be unique (#28869)

Validation to ensure that the custom flag name is unique.

app/controllers/admin/config/flags_controller.rb

@@ -33,6 +33,7 @@ class Admin::Config::FlagsController < Admin::AdminController
       end
       on_failure { render(json: failed_json, status: 422) }
       on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess }
+      on_failed_policy(:unique_name) { render_json_error(I18n.t("flags.errors.unique_name")) }
       on_failed_contract do |contract|
         render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400)
       end
@@ -50,6 +51,7 @@ class Admin::Config::FlagsController < Admin::AdminController
       on_failed_policy(:not_system) { render_json_error(I18n.t("flags.errors.system")) }
       on_failed_policy(:not_used) { render_json_error(I18n.t("flags.errors.used")) }
       on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess }
+      on_failed_policy(:unique_name) { render_json_error(I18n.t("flags.errors.unique_name")) }
       on_failed_contract do |contract|
         render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400)
       end

app/services/flags/create_flag.rb

@@ -5,6 +5,7 @@ class Flags::CreateFlag
 
   contract
   policy :invalid_access
+  policy :unique_name
   model :flag, :instantiate_flag
 
   transaction do
@@ -27,6 +28,10 @@ class Flags::CreateFlag
 
   private
 
+  def unique_name(name:)
+    !Flag.custom.where(name: name).exists?
+  end
+
   def instantiate_flag(name:, description:, applies_to:, require_message:, enabled:)
     Flag.new(
       name: name,

app/services/flags/update_flag.rb

@@ -8,6 +8,7 @@ class Flags::UpdateFlag
   policy :not_system
   policy :not_used
   policy :invalid_access
+  policy :unique_name
 
   transaction do
     step :update
@@ -29,6 +30,10 @@ class Flags::UpdateFlag
 
   private
 
+  def unique_name(id:, name:)
+    !Flag.custom.where(name: name).where.not(id: id).exists?
+  end
+
   def fetch_flag(id:)
     Flag.find(id)
   end

config/locales/server.en.yml

@@ -1270,6 +1270,7 @@ en:
       wrong_move: "Flag cannot be moved"
       system: "System flag cannot be updated or deleted."
       used: "Flag cannot be updated or deleted because has already been used."
+      unique_name: "Flag name must be unique"
   reports:
     default:
       labels:

spec/services/flags/create_flag_spec.rb

@@ -24,6 +24,15 @@ RSpec.describe(Flags::CreateFlag) do
     it { is_expected.to fail_a_policy(:invalid_access) }
   end
 
+  context "when title is not unique" do
+    fab!(:current_user) { Fabricate(:admin) }
+    fab!(:flag) { Fabricate(:flag, name: "custom flag name") }
+
+    it { is_expected.to fail_a_policy(:unique_name) }
+
+    after { Flag.destroy_by(name: "custom flag name") }
+  end
+
   context "when applies to is invalid" do
     fab!(:current_user) { Fabricate(:admin) }
     let(:applies_to) { ["User"] }

spec/services/flags/update_flag_spec.rb

@@ -43,6 +43,15 @@ RSpec.describe(Flags::UpdateFlag) do
     it { is_expected.to fail_a_contract }
   end
 
+  context "when title is not unique" do
+    fab!(:current_user) { Fabricate(:admin) }
+    fab!(:flag_2) { Fabricate(:flag, name: "edited custom flag name") }
+
+    it { is_expected.to fail_a_policy(:unique_name) }
+
+    after { Flag.destroy_by(name: "edited custom flag name") }
+  end
+
   context "when title is too long" do
     fab!(:current_user) { Fabricate(:admin) }
     let(:name) { "a" * 201 }