From fc2259d1c8646b99eadc81e8715f7c431829e339 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Tue, 6 Aug 2024 10:50:12 +1000 Subject: [PATCH] FIX: limit the number of custom flags to 50 (#28221) Admin can create up to 50 custom flags. It is limited for performance reasons. When the limit is reached "Add button" is disabled and backend is protected by guardian. --- .../addon/components/admin-config-areas/flags.gjs | 11 +++++++++++ .../addon/components/admin-config-header.gjs | 1 + app/models/flag.rb | 1 + config/site_settings.yml | 4 ++++ lib/guardian/flag_guardian.rb | 2 +- spec/lib/guardian/flag_guardian_spec.rb | 13 +++++++++++++ spec/system/admin_flags_spec.rb | 15 +++++++++++++-- spec/system/page_objects/pages/admin_flags.rb | 8 ++++++++ 8 files changed, 52 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/admin/addon/components/admin-config-areas/flags.gjs b/app/assets/javascripts/admin/addon/components/admin-config-areas/flags.gjs index c6cdbc220ec..1030f05192a 100644 --- a/app/assets/javascripts/admin/addon/components/admin-config-areas/flags.gjs +++ b/app/assets/javascripts/admin/addon/components/admin-config-areas/flags.gjs @@ -4,6 +4,7 @@ import { action } from "@ember/object"; import { service } from "@ember/service"; import { ajax } from "discourse/lib/ajax"; import { popupAjaxError } from "discourse/lib/ajax-error"; +import { SYSTEM_FLAG_IDS } from "discourse/lib/constants"; import i18n from "discourse-common/helpers/i18n"; import { bind } from "discourse-common/utils/decorators"; import AdminConfigHeader from "admin/components/admin-config-header"; @@ -11,8 +12,17 @@ import AdminFlagItem from "admin/components/admin-flag-item"; export default class AdminConfigAreasFlags extends Component { @service site; + @service siteSettings; @tracked flags = this.site.flagTypes; + get addFlagButtonDisabled() { + return ( + this.flags.filter( + (flag) => !Object.values(SYSTEM_FLAG_IDS).includes(flag.id) + ).length >= this.siteSettings.custom_flags_limit + ); + } + @bind isFirstFlag(flag) { return this.flags.indexOf(flag) === 1; @@ -68,6 +78,7 @@ export default class AdminConfigAreasFlags extends Component { @primaryActionCssClass="admin-flags__header-add-flag" @primaryActionIcon="plus" @primaryActionLabel="admin.config_areas.flags.add" + @primaryActionDisabled={{this.addFlagButtonDisabled}} /> diff --git a/app/assets/javascripts/admin/addon/components/admin-config-header.gjs b/app/assets/javascripts/admin/addon/components/admin-config-header.gjs index 458789349ca..10578fe2c84 100644 --- a/app/assets/javascripts/admin/addon/components/admin-config-header.gjs +++ b/app/assets/javascripts/admin/addon/components/admin-config-header.gjs @@ -20,6 +20,7 @@ export default class AdminFlagItem extends Component { "btn-icon-text" @primaryActionCssClass }} + @disabled={{@primaryActionDisabled}} > {{dIcon @primaryActionIcon}} {{i18n @primaryActionLabel}} diff --git a/app/models/flag.rb b/app/models/flag.rb index de09d0043b1..cebf7e01033 100644 --- a/app/models/flag.rb +++ b/app/models/flag.rb @@ -10,6 +10,7 @@ class Flag < ActiveRecord::Base MAX_DESCRIPTION_LENGTH = 1000 scope :enabled, -> { where(enabled: true) } scope :system, -> { where("id < 1000") } + scope :custom, -> { where("id >= 1000") } before_save :set_position before_save :set_name_key diff --git a/config/site_settings.yml b/config/site_settings.yml index aa4807127b6..b4262ef9363 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -2434,6 +2434,10 @@ developer: type: group_list hidden: true allow_any: false + custom_flags_limit: + default: 50 + hidden: true + client: true navigation: navigation_menu: diff --git a/lib/guardian/flag_guardian.rb b/lib/guardian/flag_guardian.rb index 8d32d651c8e..abd925443bb 100644 --- a/lib/guardian/flag_guardian.rb +++ b/lib/guardian/flag_guardian.rb @@ -6,7 +6,7 @@ module FlagGuardian end def can_create_flag? - @user.admin? + @user.admin? && Flag.custom.count < SiteSetting.custom_flags_limit end def can_toggle_flag? diff --git a/spec/lib/guardian/flag_guardian_spec.rb b/spec/lib/guardian/flag_guardian_spec.rb index f221084f6fc..77680d4fcdc 100644 --- a/spec/lib/guardian/flag_guardian_spec.rb +++ b/spec/lib/guardian/flag_guardian_spec.rb @@ -6,6 +6,19 @@ RSpec.describe FlagGuardian do fab!(:moderator) after(:each) { Flag.reset_flag_settings! } + describe "#can_create_flag?" do + it "returns true for admin and when custom flags limit is not reached" do + SiteSetting.custom_flags_limit = 1 + + expect(Guardian.new(admin).can_create_flag?).to eq(true) + expect(Guardian.new(user).can_create_flag?).to eq(false) + + Fabricate(:flag) + + expect(Guardian.new(admin).can_create_flag?).to eq(false) + expect(Guardian.new(user).can_create_flag?).to eq(false) + end + end describe "#can_edit_flag?" do it "returns true for admin and false for moderator and regular user" do diff --git a/spec/system/admin_flags_spec.rb b/spec/system/admin_flags_spec.rb index 8eb380d1c8a..46b9ea5182a 100644 --- a/spec/system/admin_flags_spec.rb +++ b/spec/system/admin_flags_spec.rb @@ -9,7 +9,10 @@ describe "Admin Flags Page", type: :system do let(:admin_flag_form_page) { PageObjects::Pages::AdminFlagForm.new } let(:flag_modal) { PageObjects::Modals::Flag.new } - before { sign_in(admin) } + before do + sign_in(admin) + SiteSetting.custom_flags_limit = 1 + end it "allows admin to disable, change order, create, update and delete flags" do # disable @@ -69,7 +72,11 @@ describe "Admin Flags Page", type: :system do "Something Else", ) - admin_flags_page.visit.click_add_flag + admin_flags_page.visit + + expect(admin_flags_page).to have_add_flag_button_enabled + + admin_flags_page.click_add_flag admin_flag_form_page .fill_in_name("Vulgar") .fill_in_description("New flag description") @@ -87,6 +94,8 @@ describe "Admin Flags Page", type: :system do "Vulgar", ) + expect(admin_flags_page).to have_add_flag_button_disabled + topic_page.visit_topic(post.topic).open_flag_topic_modal expect(flag_modal).to have_choices( @@ -126,6 +135,8 @@ describe "Admin Flags Page", type: :system do expect(admin_flags_page).to have_no_flag("tasteless") + expect(admin_flags_page).to have_add_flag_button_enabled + topic_page.visit_topic(post.topic).open_flag_topic_modal expect(flag_modal).to have_choices( diff --git a/spec/system/page_objects/pages/admin_flags.rb b/spec/system/page_objects/pages/admin_flags.rb index e26a9da2bcc..a7e01d6ff41 100644 --- a/spec/system/page_objects/pages/admin_flags.rb +++ b/spec/system/page_objects/pages/admin_flags.rb @@ -47,6 +47,14 @@ module PageObjects all(".admin-flag-item__name").map(&:text) == flags end + def has_add_flag_button_enabled? + has_css?(".admin-flags__header-add-flag:not([disabled])") + end + + def has_add_flag_button_disabled? + has_no_css?(".admin-flags__header-add-flag[disabled]") + end + def has_flag?(flag) has_css?(".admin-flag-item.#{flag}") end