From 33fd6801e5222fb265613fde61e64141fc10a7a9 Mon Sep 17 00:00:00 2001 From: Keegan George Date: Fri, 6 Jun 2025 10:56:36 -0700 Subject: [PATCH] DEV: Add back validator for Spam setting (#1415) ## :mag: Overview This update re-introduces the validator used on the `ai_spam_detection_enabled` setting. It was initially added here: https://github.com/discourse/discourse-ai/pull/1374 to prevent Spam from being enabled without creating an `AiModerationSetting` value in the database. However, due to issues with backups/migrations we temporarily removed it here: https://github.com/discourse/discourse-ai/pull/1393. Now with some internal fixes, we can re-introduce it. We also update the validator so that it only validates when trying to turn on rather than when turning off too. --- config/settings.yml | 1 + lib/configuration/spam_detection_validator.rb | 3 +- spec/configuration/llm_enumerator_spec.rb | 5 ---- .../spam_detection_validator_spec.rb | 30 +++++++++++++++++++ .../requests/admin/ai_spam_controller_spec.rb | 7 +++-- 5 files changed, 38 insertions(+), 8 deletions(-) create mode 100644 spec/configuration/spam_detection_validator_spec.rb diff --git a/config/settings.yml b/config/settings.yml index f9f6b9a9..ab3675df 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -412,6 +412,7 @@ discourse_ai: ai_spam_detection_enabled: default: false + validator: "DiscourseAi::Configuration::SpamDetectionValidator" ai_spam_detection_user_id: default: "" hidden: true diff --git a/lib/configuration/spam_detection_validator.rb b/lib/configuration/spam_detection_validator.rb index 4201dcc4..6201cf3d 100644 --- a/lib/configuration/spam_detection_validator.rb +++ b/lib/configuration/spam_detection_validator.rb @@ -8,7 +8,8 @@ module DiscourseAi end def valid_value?(val) - return true if Rails.env.test? + # only validate when enabling spam detection + return true if val == "f" || val == "false" return true if AiModerationSetting.spam false diff --git a/spec/configuration/llm_enumerator_spec.rb b/spec/configuration/llm_enumerator_spec.rb index e7651b10..7737da45 100644 --- a/spec/configuration/llm_enumerator_spec.rb +++ b/spec/configuration/llm_enumerator_spec.rb @@ -62,10 +62,5 @@ RSpec.describe DiscourseAi::Configuration::LlmEnumerator do { fake_model.id => [{ type: :automation, name: "some automation", id: automation.id }] }, ) end - - it "doesn't error on spam when spam detection is enabled but moderation setting is missing" do - SiteSetting.ai_spam_detection_enabled = true - expect { described_class.global_usage }.not_to raise_error - end end end diff --git a/spec/configuration/spam_detection_validator_spec.rb b/spec/configuration/spam_detection_validator_spec.rb new file mode 100644 index 00000000..bd359254 --- /dev/null +++ b/spec/configuration/spam_detection_validator_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +RSpec.describe DiscourseAi::Configuration::SpamDetectionValidator do + let(:validator) { described_class.new } + + it "always returns true if setting the value to false" do + expect(validator.valid_value?("f")).to eq(true) + end + + context "when a moderation setting exists" do + fab!(:llm_model) + before { AiModerationSetting.create!(setting_type: "spam", llm_model_id: llm_model.id) } + + it "returns true if a moderation setting for spam exists" do + expect(validator.valid_value?("t")).to eq(true) + end + end + + context "when no moderation setting exists" do + it "returns false if a moderation setting for spam does not exist" do + expect(validator.valid_value?("t")).to eq(false) + end + + it "returns an error message when no moderation setting exists" do + expect(validator.error_message).to eq( + I18n.t("discourse_ai.spam_detection.configuration_missing"), + ) + end + end +end diff --git a/spec/requests/admin/ai_spam_controller_spec.rb b/spec/requests/admin/ai_spam_controller_spec.rb index 3ed484e5..ccf05af7 100644 --- a/spec/requests/admin/ai_spam_controller_spec.rb +++ b/spec/requests/admin/ai_spam_controller_spec.rb @@ -209,7 +209,10 @@ RSpec.describe DiscourseAi::Admin::AiSpamController do describe "#show" do context "when logged in as admin" do - before { sign_in(admin) } + before do + sign_in(admin) + AiModerationSetting.create!(setting_type: :spam, llm_model_id: llm_model.id) + end it "correctly filters seeded llms" do SiteSetting.ai_spam_detection_enabled = true @@ -248,7 +251,7 @@ RSpec.describe DiscourseAi::Admin::AiSpamController do it "return proper settings when spam detection is enabled" do SiteSetting.ai_spam_detection_enabled = true - AiModerationSetting.create( + AiModerationSetting.update!( { setting_type: :spam, llm_model_id: llm_model.id,