From a5f093530748506c2c7738021524edccf0db1a3e Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Fri, 5 Jan 2024 10:19:43 +0800 Subject: [PATCH] DEV: Convert min_trust_level_to_create_tag to groups (#24899) We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the min_trust_level_to_create_tag site setting to create_tag_allowed_groups. This PR maintains backwards compatibility until we can update plugins and themes using this. --- config/locales/server.en.yml | 2 ++ config/site_settings.yml | 6 ++++ ...wed_groups_based_on_deprecated_settings.rb | 35 +++++++++++++++++++ lib/guardian/tag_guardian.rb | 5 ++- lib/site_settings/deprecated_settings.rb | 2 ++ spec/integration/category_tag_spec.rb | 4 +-- spec/lib/discourse_tagging_spec.rb | 4 +-- spec/lib/guardian_spec.rb | 21 +++++++---- spec/lib/new_post_manager_spec.rb | 2 +- spec/lib/post_creator_spec.rb | 6 ++-- spec/lib/post_revisor_spec.rb | 14 ++++---- spec/lib/topic_creator_spec.rb | 2 +- spec/lib/topics_bulk_action_spec.rb | 8 ++--- spec/models/post_mover_spec.rb | 2 +- spec/models/reviewable_queued_post_spec.rb | 2 +- spec/models/tag_user_spec.rb | 2 +- spec/requests/topics_controller_spec.rb | 6 ++-- 17 files changed, 90 insertions(+), 33 deletions(-) create mode 100644 db/migrate/20231214061615_fill_create_tag_allowed_groups_based_on_deprecated_settings.rb diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index c3c880bbab0..2481a3a4305 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2427,6 +2427,7 @@ en: tagging_enabled: "Enable tags on topics?" min_trust_to_create_tag: "The minimum trust level required to create a tag." + create_tag_allowed_groups: "Groups that are allowed to create tags." max_tags_per_topic: "The maximum tags that can be applied to a topic." enable_max_tags_per_email_subject: "Use max_tags_per_email_subject when generating the subject of an email" max_tags_per_email_subject: "The maximum tags that can be in the subject of an email" @@ -2577,6 +2578,7 @@ en: invite_allowed_groups: "min_trust_level_to_allow_invite" ignore_allowed_groups: "min_trust_level_to_allow_ignore" self_wiki_allowed_groups: "min_trust_to_allow_self_wiki" + create_tag_allowed_groups: "min_trust_to_create_tag" placeholder: discourse_connect_provider_secrets: diff --git a/config/site_settings.yml b/config/site_settings.yml index 70373a058f6..9f13943173b 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -2994,6 +2994,12 @@ tags: min_trust_to_create_tag: default: "3" enum: "TrustLevelAndStaffSetting" + create_tag_allowed_groups: + default: "1|3|13" + type: group_list + allow_any: false + refresh: true + validator: "AtLeastOneGroupValidator" min_trust_level_to_tag_topics: default: "0" enum: "TrustLevelAndStaffSetting" diff --git a/db/migrate/20231214061615_fill_create_tag_allowed_groups_based_on_deprecated_settings.rb b/db/migrate/20231214061615_fill_create_tag_allowed_groups_based_on_deprecated_settings.rb new file mode 100644 index 00000000000..15a90971a9e --- /dev/null +++ b/db/migrate/20231214061615_fill_create_tag_allowed_groups_based_on_deprecated_settings.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class FillCreateTagAllowedGroupsBasedOnDeprecatedSettings < ActiveRecord::Migration[7.0] + def up + configured_trust_level = + DB.query_single( + "SELECT value FROM site_settings WHERE name = 'min_trust_to_create_tag' LIMIT 1", + ).first + + # Default for old setting is TL3, we only need to do anything if it's been changed in the DB. + if configured_trust_level.present? + corresponding_group = + case configured_trust_level + when "admin" + "1" + when "staff" + "1|3" + # Matches Group::AUTO_GROUPS to the trust levels. + else + "1|3|1#{configured_trust_level}" + end + + # Data_type 20 is group_list. + DB.exec( + "INSERT INTO site_settings(name, value, data_type, created_at, updated_at) + VALUES('create_tag_allowed_groups', :setting, '20', NOW(), NOW())", + setting: corresponding_group, + ) + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/guardian/tag_guardian.rb b/lib/guardian/tag_guardian.rb index f6fd968e984..66fa58a14c1 100644 --- a/lib/guardian/tag_guardian.rb +++ b/lib/guardian/tag_guardian.rb @@ -8,7 +8,10 @@ module TagGuardian def can_create_tag? SiteSetting.tagging_enabled && - @user.has_trust_level_or_staff?(SiteSetting.min_trust_to_create_tag) + ( + @user.has_trust_level_or_staff?(SiteSetting.min_trust_to_create_tag) || + @user.in_any_groups?(SiteSetting.create_tag_allowed_groups_map) + ) end def can_tag_topics? diff --git a/lib/site_settings/deprecated_settings.rb b/lib/site_settings/deprecated_settings.rb index e2aa58cc78b..ba3bcd7e124 100644 --- a/lib/site_settings/deprecated_settings.rb +++ b/lib/site_settings/deprecated_settings.rb @@ -34,6 +34,7 @@ module SiteSettings::DeprecatedSettings ["min_trust_level_to_allow_invite", "invite_allowed_groups", false, "3.3"], ["min_trust_level_to_allow_ignore", "ignore_allowed_groups", false, "3.3"], ["min_trust_to_allow_self_wiki", "self_wiki_allowed_groups", false, "3.3"], + ["min_trust_to_create_tag", "create_tag_allowed_groups", false, "3.3"], ] OVERRIDE_TL_GROUP_SETTINGS = %w[ @@ -52,6 +53,7 @@ module SiteSettings::DeprecatedSettings min_trust_level_to_allow_user_card_background min_trust_level_to_allow_invite min_trust_level_to_allow_ignore + min_trust_to_create_tag ] def group_to_tl(old_setting, new_setting) diff --git a/spec/integration/category_tag_spec.rb b/spec/integration/category_tag_spec.rb index 4b7bf23e9e0..ebfd67253b5 100644 --- a/spec/integration/category_tag_spec.rb +++ b/spec/integration/category_tag_spec.rb @@ -17,7 +17,7 @@ RSpec.describe "category tag restrictions" do before do SiteSetting.tagging_enabled = true - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] SiteSetting.min_trust_level_to_tag_topics = 0 end @@ -769,7 +769,7 @@ RSpec.describe "tag topic counts per category" do before do SiteSetting.tagging_enabled = true - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] SiteSetting.min_trust_level_to_tag_topics = 0 end diff --git a/spec/lib/discourse_tagging_spec.rb b/spec/lib/discourse_tagging_spec.rb index 42b0a8ee348..36859c69b32 100644 --- a/spec/lib/discourse_tagging_spec.rb +++ b/spec/lib/discourse_tagging_spec.rb @@ -6,7 +6,7 @@ require "discourse_tagging" # More tests are found in the category_tag_spec integration specs RSpec.describe DiscourseTagging do - fab!(:admin) + fab!(:admin) { Fabricate(:admin, refresh_auto_groups: true) } fab!(:user) let(:admin_guardian) { Guardian.new(admin) } let(:guardian) { Guardian.new(user) } @@ -17,7 +17,7 @@ RSpec.describe DiscourseTagging do before do SiteSetting.tagging_enabled = true - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] SiteSetting.min_trust_level_to_tag_topics = 0 end diff --git a/spec/lib/guardian_spec.rb b/spec/lib/guardian_spec.rb index 3b0d898f767..c28bf6b0e3d 100644 --- a/spec/lib/guardian_spec.rb +++ b/spec/lib/guardian_spec.rb @@ -3711,8 +3711,10 @@ RSpec.describe Guardian do SiteSetting.min_trust_level_to_tag_topics = 1 end - context "when min_trust_to_create_tag is 3" do - before { SiteSetting.min_trust_to_create_tag = 3 } + context "when minimum trust level to create tags is 3" do + before do + SiteSetting.create_tag_allowed_groups = "1|3|#{Group::AUTO_GROUPS[:trust_level_3]}" + end describe "#can_see_tag?" do it "is always true" do @@ -3751,8 +3753,12 @@ RSpec.describe Guardian do end end - context 'when min_trust_to_create_tag is "staff"' do - before { SiteSetting.min_trust_to_create_tag = "staff" } + context "when staff and admin groups are allowed to create tags" do + before do + SiteSetting.min_trust_to_create_tag = "staff" + SiteSetting.create_tag_allowed_groups = + "#{Group::AUTO_GROUPS[:staff]}|#{Group::AUTO_GROUPS[:admins]}" + end it "returns false if not staff" do expect(Guardian.new(trust_level_4).can_create_tag?).to eq(false) @@ -3764,8 +3770,11 @@ RSpec.describe Guardian do end end - context 'when min_trust_to_create_tag is "admin"' do - before { SiteSetting.min_trust_to_create_tag = "admin" } + context "when only admin group is allowed to create tags" do + before do + SiteSetting.min_trust_to_create_tag = "admin" + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:admins] + end it "returns false if not admin" do expect(Guardian.new(trust_level_4).can_create_tag?).to eq(false) diff --git a/spec/lib/new_post_manager_spec.rb b/spec/lib/new_post_manager_spec.rb index 796afa11881..6b0df409cd7 100644 --- a/spec/lib/new_post_manager_spec.rb +++ b/spec/lib/new_post_manager_spec.rb @@ -411,7 +411,7 @@ RSpec.describe NewPostManager do it "calls custom enqueuing handlers" do SiteSetting.tagging_enabled = true - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] SiteSetting.min_trust_level_to_tag_topics = 0 manager = diff --git a/spec/lib/post_creator_spec.rb b/spec/lib/post_creator_spec.rb index de055714aff..ebe840a625c 100644 --- a/spec/lib/post_creator_spec.rb +++ b/spec/lib/post_creator_spec.rb @@ -555,7 +555,7 @@ RSpec.describe PostCreator do context "when can create tags" do before do - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] SiteSetting.min_trust_level_to_tag_topics = 0 end @@ -576,7 +576,7 @@ RSpec.describe PostCreator do context "when cannot create tags" do before do - SiteSetting.min_trust_to_create_tag = 4 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] SiteSetting.min_trust_level_to_tag_topics = 0 end @@ -589,7 +589,7 @@ RSpec.describe PostCreator do context "when automatically tagging first posts" do before do - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] SiteSetting.min_trust_level_to_tag_topics = 0 Fabricate(:tag, name: "greetings") Fabricate(:tag, name: "hey") diff --git a/spec/lib/post_revisor_spec.rb b/spec/lib/post_revisor_spec.rb index 252f19e422d..52e66d7da4f 100644 --- a/spec/lib/post_revisor_spec.rb +++ b/spec/lib/post_revisor_spec.rb @@ -5,7 +5,7 @@ require "post_revisor" RSpec.describe PostRevisor do fab!(:topic) fab!(:newuser) { Fabricate(:newuser, last_seen_at: Date.today) } - fab!(:user) + fab!(:user) { Fabricate(:user, refresh_auto_groups: true) } fab!(:coding_horror) fab!(:admin) fab!(:moderator) @@ -100,7 +100,7 @@ RSpec.describe PostRevisor do end it "does not revise category if incorrect amount of tags" do - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] SiteSetting.min_trust_level_to_tag_topics = 0 new_category = Fabricate(:category, minimum_required_tags: 1) @@ -122,7 +122,7 @@ RSpec.describe PostRevisor do end it "returns an error if the topic does not have minimum amount of tags that the new category requires" do - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] SiteSetting.min_trust_level_to_tag_topics = 0 old_category = Fabricate(:category, minimum_required_tags: 0) @@ -136,7 +136,7 @@ RSpec.describe PostRevisor do end it "returns an error if the topic has tags not allowed in the new category" do - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] SiteSetting.min_trust_level_to_tag_topics = 0 tag1 = Fabricate(:tag) @@ -164,7 +164,7 @@ RSpec.describe PostRevisor do end it "returns an error if the topic is missing tags required from a tag group in the new category" do - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] SiteSetting.min_trust_level_to_tag_topics = 0 tag1 = Fabricate(:tag) @@ -1232,7 +1232,7 @@ RSpec.describe PostRevisor do context "when can create tags" do before do - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = "1|3|#{Group::AUTO_GROUPS[:trust_level_0]}" SiteSetting.min_trust_level_to_tag_topics = 0 end @@ -1490,7 +1490,7 @@ RSpec.describe PostRevisor do context "when cannot create tags" do before do - SiteSetting.min_trust_to_create_tag = 4 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] SiteSetting.min_trust_level_to_tag_topics = 0 end diff --git a/spec/lib/topic_creator_spec.rb b/spec/lib/topic_creator_spec.rb index aa993e64750..e7673d8c27d 100644 --- a/spec/lib/topic_creator_spec.rb +++ b/spec/lib/topic_creator_spec.rb @@ -107,7 +107,7 @@ RSpec.describe TopicCreator do before do SiteSetting.tagging_enabled = true - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] SiteSetting.min_trust_level_to_tag_topics = 0 end diff --git a/spec/lib/topics_bulk_action_spec.rb b/spec/lib/topics_bulk_action_spec.rb index 339456691cc..a1cd6cb9ae6 100644 --- a/spec/lib/topics_bulk_action_spec.rb +++ b/spec/lib/topics_bulk_action_spec.rb @@ -425,7 +425,7 @@ RSpec.describe TopicsBulkAction do end it "can change the tags, and can create new tags" do - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] tba = TopicsBulkAction.new( topic.user, @@ -440,7 +440,7 @@ RSpec.describe TopicsBulkAction do end it "can change the tags but not create new ones" do - SiteSetting.min_trust_to_create_tag = 4 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] tba = TopicsBulkAction.new( topic.user, @@ -494,7 +494,7 @@ RSpec.describe TopicsBulkAction do end it "can append new or existing tags" do - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] tba = TopicsBulkAction.new( topic.user, @@ -517,7 +517,7 @@ RSpec.describe TopicsBulkAction do end context "when the user can't create new topics" do - before { SiteSetting.min_trust_to_create_tag = 4 } + before { SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] } it "can append existing tags but doesn't append new tags" do tba = diff --git a/spec/models/post_mover_spec.rb b/spec/models/post_mover_spec.rb index 910036a212f..fca9f76d052 100644 --- a/spec/models/post_mover_spec.rb +++ b/spec/models/post_mover_spec.rb @@ -22,7 +22,7 @@ RSpec.describe PostMover do context "with topics" do before { freeze_time } - fab!(:user) { Fabricate(:user, admin: true) } + fab!(:user) { Fabricate(:admin, refresh_auto_groups: true) } fab!(:another_user) { evil_trout } fab!(:category) { Fabricate(:category, user: user) } fab!(:topic) { Fabricate(:topic, user: user, created_at: 4.hours.ago) } diff --git a/spec/models/reviewable_queued_post_spec.rb b/spec/models/reviewable_queued_post_spec.rb index 022cdd24db2..76f249bd5ef 100644 --- a/spec/models/reviewable_queued_post_spec.rb +++ b/spec/models/reviewable_queued_post_spec.rb @@ -238,7 +238,7 @@ RSpec.describe ReviewableQueuedPost, type: :model do before do SiteSetting.tagging_enabled = true - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] SiteSetting.min_trust_level_to_tag_topics = 0 end diff --git a/spec/models/tag_user_spec.rb b/spec/models/tag_user_spec.rb index cb7be70d38d..8e8772b73b2 100644 --- a/spec/models/tag_user_spec.rb +++ b/spec/models/tag_user_spec.rb @@ -4,7 +4,7 @@ RSpec.describe TagUser do before do SiteSetting.tagging_enabled = true - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] SiteSetting.min_trust_level_to_tag_topics = 0 end diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 3ba7317f505..a27a4c1bd82 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -17,7 +17,7 @@ RSpec.describe TopicsController do fab!(:post_author5) { Fabricate(:user) } fab!(:post_author6) { Fabricate(:user) } fab!(:moderator) - fab!(:admin) + fab!(:admin) { Fabricate(:admin, refresh_auto_groups: true) } fab!(:trust_level_0) { Fabricate(:trust_level_0, refresh_auto_groups: true) } fab!(:trust_level_1) { Fabricate(:trust_level_1, refresh_auto_groups: true) } fab!(:trust_level_4) { Fabricate(:trust_level_4, refresh_auto_groups: true) } @@ -1854,7 +1854,7 @@ RSpec.describe TopicsController do end it "can create a tag" do - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] expect do put "/t/#{topic.slug}/#{topic.id}.json", params: { tags: ["newtag"] } end.to change { topic.reload.first_post.revisions.count }.by(1) @@ -1864,7 +1864,7 @@ RSpec.describe TopicsController do end it "can change the category and create a new tag" do - SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0] expect do put "/t/#{topic.slug}/#{topic.id}.json", params: {