DEV: Convert min_trust_level_to_tag_topics to groups ()

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_tag_topics site setting to tag_topic_allowed_groups.
This commit is contained in:
Ted Johansson 2024-01-26 13:25:03 +08:00 committed by GitHub
parent d178d75e1e
commit 7e5d2a95ee
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
25 changed files with 124 additions and 51 deletions

@ -2448,6 +2448,7 @@ en:
tag_style: "Visual style for tag badges."
pm_tags_allowed_for_groups: "Allow members of included group(s) to tag any personal message"
min_trust_level_to_tag_topics: "Minimum trust level required to tag topics"
tag_topic_allowed_groups: "Groups that are allowed to tag topics."
suppress_overlapping_tags_in_list: "If tags match exact words in topic titles, don't show the tag"
remove_muted_tags_from_latest: "Don't show topics tagged only with muted tags in the latest topic list."
force_lowercase_tags: "Force all new tags to be entirely lowercase."
@ -2594,6 +2595,7 @@ en:
embedded_media_allowed_groups: "min_trust_to_post_embedded_media"
post_links_allowed_groups: "min_trust_to_post_links"
user_api_key_allowed_groups: "min_trust_level_for_user_api_key"
tag_topic_allowed_groups: "min_trust_level_to_tag_topics"
placeholder:
discourse_connect_provider_secrets:

@ -3063,6 +3063,14 @@ tags:
default: "0"
enum: "TrustLevelAndStaffSetting"
client: true
hidden: true
tag_topic_allowed_groups:
default: "10"
type: group_list
allow_any: false
refresh: true
client: true
validator: "AtLeastOneGroupValidator"
max_tag_search_results:
client: true
default: 5

@ -0,0 +1,27 @@
# frozen_string_literal: true
class FillTagTopicAllowedGroupsBasedOnDeprecatedSettings < ActiveRecord::Migration[7.0]
def up
configured_trust_level =
DB.query_single(
"SELECT value FROM site_settings WHERE name = 'min_trust_level_to_tag_topics' LIMIT 1",
).first
# Default for old setting is TL0, we only need to do anything if it's been changed in the DB.
if configured_trust_level.present?
# Matches Group::AUTO_GROUPS to the trust levels.
corresponding_group = "1#{configured_trust_level}"
# Data_type 20 is group_list.
DB.exec(
"INSERT INTO site_settings(name, value, data_type, created_at, updated_at)
VALUES('tag_topic_allowed_groups', :setting, '20', NOW(), NOW())",
setting: corresponding_group,
)
end
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

@ -15,8 +15,7 @@ module TagGuardian
end
def can_tag_topics?
SiteSetting.tagging_enabled &&
@user.has_trust_level_or_staff?(SiteSetting.min_trust_level_to_tag_topics)
SiteSetting.tagging_enabled && @user.in_any_groups?(SiteSetting.tag_topic_allowed_groups_map)
end
def can_tag_pms?

@ -40,6 +40,7 @@ module SiteSettings::DeprecatedSettings
["min_trust_to_post_embedded_media", "embedded_media_post_allowed_groups", false, "3.3"],
["min_trust_to_post_links", "post_links_allowed_groups", false, "3.3"],
["min_trust_level_for_user_api_key", "user_api_key_allowed_groups", false, "3.3"],
["min_trust_level_to_tag_topics", "tag_topic_allowed_groups", false, "3.3"],
]
OVERRIDE_TL_GROUP_SETTINGS = %w[
@ -64,6 +65,7 @@ module SiteSettings::DeprecatedSettings
min_trust_to_post_embedded_media
min_trust_to_post_links
min_trust_level_for_user_api_key
min_trust_level_to_tag_topics
]
def group_to_tl(old_setting, new_setting)

@ -7,7 +7,7 @@ describe Chat::ChannelArchiveService do
end
fab!(:channel) { Fabricate(:category_channel) }
fab!(:user) { Fabricate(:user, admin: true) }
fab!(:user) { Fabricate(:user, admin: true, refresh_auto_groups: true) }
fab!(:category)
let(:topic_params) { { topic_title: "This will be a new topic", category_id: category.id } }

@ -13,12 +13,12 @@ RSpec.describe "category tag restrictions" do
let(:tag_with_colon) { Fabricate(:tag, name: "with:colon") }
fab!(:user)
fab!(:admin)
fab!(:admin) { Fabricate(:admin, refresh_auto_groups: true) }
before do
SiteSetting.tagging_enabled = true
SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
end
context "with tags restricted to one category" do
@ -770,7 +770,7 @@ RSpec.describe "tag topic counts per category" do
before do
SiteSetting.tagging_enabled = true
SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
end
it "counts when a topic is created with tags" do
@ -782,8 +782,10 @@ RSpec.describe "tag topic counts per category" do
end
it "counts when tag is added to an existing topic" do
topic = Fabricate(:topic, category: category)
post = Fabricate(:post, user: topic.user, topic: topic)
user = Fabricate(:user, refresh_auto_groups: true)
topic = Fabricate(:topic, user: user, category: category)
post = Fabricate(:post, user: user, topic: topic)
expect(CategoryTagStat.where(category: category).count).to eq(0)
expect {
PostRevisor.new(post).revise!(topic.user, raw: post.raw, tags: [tag1.name, tag2.name])

@ -14,7 +14,7 @@ RSpec.describe Jobs::ExportUserArchive do
end
let(:component) { raise "component not set" }
fab!(:admin)
fab!(:admin) { Fabricate(:admin, refresh_auto_groups: true) }
fab!(:category) { Fabricate(:category_with_definition, name: "User Archive Category") }
fab!(:subcategory) { Fabricate(:category_with_definition, parent_category_id: category.id) }
fab!(:topic) { Fabricate(:topic, category: category) }

@ -7,7 +7,7 @@ require "discourse_tagging"
RSpec.describe DiscourseTagging do
fab!(:admin) { Fabricate(:admin, refresh_auto_groups: true) }
fab!(:user)
fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
let(:admin_guardian) { Guardian.new(admin) }
let(:guardian) { Guardian.new(user) }
@ -18,7 +18,7 @@ RSpec.describe DiscourseTagging do
before do
SiteSetting.tagging_enabled = true
SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
end
describe "visible_tags" do

@ -3705,7 +3705,7 @@ RSpec.describe Guardian do
context "when tagging is enabled" do
before do
SiteSetting.tagging_enabled = true
SiteSetting.min_trust_level_to_tag_topics = 1
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_1]
end
context "when minimum trust level to create tags is 3" do
@ -3736,11 +3736,19 @@ RSpec.describe Guardian do
describe "can_tag_topics" do
it "returns false if trust level is too low" do
expect(Guardian.new(Fabricate(:user, trust_level: 0)).can_tag_topics?).to be_falsey
expect(
Guardian.new(
Fabricate(:user, trust_level: 0, refresh_auto_groups: true),
).can_tag_topics?,
).to be_falsey
end
it "returns true if trust level is high enough" do
expect(Guardian.new(Fabricate(:user, trust_level: 1)).can_tag_topics?).to be_truthy
expect(
Guardian.new(
Fabricate(:user, trust_level: 1, refresh_auto_groups: true),
).can_tag_topics?,
).to be_truthy
end
it "returns true for staff" do

@ -409,7 +409,7 @@ RSpec.describe NewPostManager do
it "calls custom enqueuing handlers" do
SiteSetting.tagging_enabled = true
SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
manager =
NewPostManager.new(

@ -556,7 +556,7 @@ RSpec.describe PostCreator do
context "when can create tags" do
before do
SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
end
it "can create all tags if none exist" do
@ -577,7 +577,7 @@ RSpec.describe PostCreator do
context "when cannot create tags" do
before do
SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_4]
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
end
it "only uses existing tags" do
@ -590,7 +590,7 @@ RSpec.describe PostCreator do
context "when automatically tagging first posts" do
before do
SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
Fabricate(:tag, name: "greetings")
Fabricate(:tag, name: "hey")
Fabricate(:tag, name: "about-art")

@ -7,7 +7,7 @@ RSpec.describe PostRevisor do
fab!(:newuser) { Fabricate(:newuser, last_seen_at: Date.today) }
fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
fab!(:coding_horror)
fab!(:admin)
fab!(:admin) { Fabricate(:admin, refresh_auto_groups: true) }
fab!(:moderator)
let(:post_args) { { user: newuser, topic: topic } }
@ -101,7 +101,7 @@ RSpec.describe PostRevisor do
it "does not revise category if incorrect amount of tags" do
SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
new_category = Fabricate(:category, minimum_required_tags: 1)
@ -123,7 +123,7 @@ RSpec.describe PostRevisor do
it "returns an error if the topic does not have minimum amount of tags that the new category requires" do
SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
old_category = Fabricate(:category, minimum_required_tags: 0)
new_category = Fabricate(:category, minimum_required_tags: 1)
@ -137,7 +137,7 @@ RSpec.describe PostRevisor do
it "returns an error if the topic has tags not allowed in the new category" do
SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
tag1 = Fabricate(:tag)
tag2 = Fabricate(:tag)
@ -165,7 +165,7 @@ RSpec.describe PostRevisor do
it "returns an error if the topic is missing tags required from a tag group in the new category" do
SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
tag1 = Fabricate(:tag)
tag_group = Fabricate(:tag_group, tags: [tag1])
@ -1233,7 +1233,7 @@ RSpec.describe PostRevisor do
context "when can create tags" do
before do
SiteSetting.create_tag_allowed_groups = "1|3|#{Group::AUTO_GROUPS[:trust_level_0]}"
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = "1|3|#{Group::AUTO_GROUPS[:trust_level_0]}"
end
it "can create all tags if none exist" do
@ -1491,7 +1491,7 @@ RSpec.describe PostRevisor do
context "when cannot create tags" do
before do
SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_4]
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
end
it "only uses existing tags" do

@ -1,7 +1,7 @@
# frozen_string_literal: true
RSpec.describe Search do
fab!(:admin)
fab!(:admin) { Fabricate(:admin, refresh_auto_groups: true) }
fab!(:topic)
before do
@ -1390,7 +1390,7 @@ RSpec.describe Search do
SiteSetting.tagging_enabled = true
DiscourseTagging.tag_topic_by_names(
post.topic,
Guardian.new(Fabricate.build(:admin)),
Guardian.new(Fabricate(:admin, refresh_auto_groups: true)),
[tag.name, uppercase_tag.name],
)
post.topic.save

@ -106,7 +106,7 @@ RSpec.describe TopicCreator do
before do
SiteSetting.tagging_enabled = true
SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
end
context "with regular tags" do
@ -214,7 +214,7 @@ RSpec.describe TopicCreator do
end
it "lets new user create a topic if they don't have sufficient trust level to tag topics" do
SiteSetting.min_trust_level_to_tag_topics = 1
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_1]
new_user = Fabricate(:newuser, refresh_auto_groups: true)
topic =
TopicCreator.create(

@ -414,7 +414,7 @@ RSpec.describe TopicsBulkAction do
before do
SiteSetting.tagging_enabled = true
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
topic.tags = [tag1, tag2]
end
@ -482,7 +482,7 @@ RSpec.describe TopicsBulkAction do
before do
SiteSetting.tagging_enabled = true
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
topic.tags = [tag1, tag2]
end
@ -552,7 +552,7 @@ RSpec.describe TopicsBulkAction do
before do
SiteSetting.tagging_enabled = true
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
topic.tags = [tag1, tag2]
end

@ -2225,6 +2225,7 @@ RSpec.describe PostMover do
it "can add tags to new message when staff group is included in pm_tags_allowed_for_groups" do
SiteSetting.pm_tags_allowed_for_groups = "1|2|3"
SiteSetting.tag_topic_allowed_groups = "1|2|3"
personal_message.move_posts(
admin,
[p2.id, p5.id],

@ -239,7 +239,7 @@ RSpec.describe ReviewableQueuedPost, type: :model do
before do
SiteSetting.tagging_enabled = true
SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
end
context "when editing" do

@ -17,7 +17,7 @@ RSpec.describe Tag do
before do
SiteSetting.tagging_enabled = true
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
end
describe "Associations" do

@ -5,7 +5,7 @@ RSpec.describe TagUser do
before do
SiteSetting.tagging_enabled = true
SiteSetting.create_tag_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
SiteSetting.min_trust_level_to_tag_topics = 0
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
end
def regular
@ -186,7 +186,7 @@ RSpec.describe TagUser do
end
describe "integration" do
fab!(:user)
fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
fab!(:watched_tag) { Fabricate(:tag) }
let(:muted_tag) { Fabricate(:tag) }
fab!(:tracked_tag) { Fabricate(:tag) }
@ -311,7 +311,7 @@ RSpec.describe TagUser do
end
it "correctly handles staff tags" do
staff = Fabricate(:admin)
staff = Fabricate(:admin, refresh_auto_groups: true)
topic = create_post.topic
create_staff_only_tags(["foo"])

@ -8,7 +8,7 @@ RSpec.describe TopicEmbed do
it { is_expected.to validate_presence_of :embed_url }
describe ".import" do
fab!(:user)
fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
let(:title) { "How to turn a fish from good to evil in 30 seconds" }
let(:url) { "http://eviltrout.com/123" }
let(:contents) do

@ -1,7 +1,7 @@
# frozen_string_literal: true
RSpec.describe TopicTrackingState do
fab!(:user)
fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
fab!(:whisperers_group) { Fabricate(:group) }
fab!(:private_message_post)
let(:private_message_topic) { private_message_post.topic }
@ -663,14 +663,11 @@ RSpec.describe TopicTrackingState do
describe "tag support" do
before do
SiteSetting.tagging_enabled = true
SiteSetting.create_tag_allowed_groups = "10"
post.topic.notifier.watch_topic!(post.topic.user_id)
DiscourseTagging.tag_topic_by_names(
post.topic,
Guardian.new(Discourse.system_user),
%w[bananas apples],
)
DiscourseTagging.tag_topic_by_names(post.topic, Guardian.new(user), %w[bananas apples])
end
it "includes tags based on the `tagging_enabled` site setting" do

@ -1372,7 +1372,7 @@ RSpec.describe PostsController do
it "cannot create a post with a tag without tagging permission" do
SiteSetting.tagging_enabled = true
SiteSetting.min_trust_level_to_tag_topics = 4
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_4]
tag = Fabricate(:tag)
post "/posts.json",

@ -2258,7 +2258,12 @@ RSpec.describe UsersController do
fab!(:upload)
fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
before { sign_in(user) }
before do
User.set_callback(:create, :after, :ensure_in_trust_level_group)
sign_in(user)
end
after { User.skip_callback(:create, :after, :ensure_in_trust_level_group) }
it "allows the update" do
SiteSetting.tagging_enabled = true

@ -2057,6 +2057,7 @@ RSpec.describe PostAlerter do
before do
SiteSetting.tagging_enabled = true
SiteSetting.tag_topic_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
Jobs.run_immediately!
TagUser.change(user.id, watched_tag.id, TagUser.notification_levels[:watching_first_post])
TopicUser.change(
@ -2075,7 +2076,10 @@ RSpec.describe PostAlerter do
).to eq(0)
expect {
PostRevisor.new(post).revise!(Fabricate(:user), tags: [other_tag.name, watched_tag.name])
PostRevisor.new(post).revise!(
Fabricate(:user, refresh_auto_groups: true),
tags: [other_tag.name, watched_tag.name],
)
}.to change { Notification.where(user_id: user.id).count }.by(1)
expect(
user
@ -2085,7 +2089,10 @@ RSpec.describe PostAlerter do
).to eq(1)
expect {
PostRevisor.new(post).revise!(Fabricate(:user), tags: [watched_tag.name, other_tag.name])
PostRevisor.new(post).revise!(
Fabricate(:user, refresh_auto_groups: true),
tags: [watched_tag.name, other_tag.name],
)
}.not_to change { Notification.count }
expect(
user
@ -2149,11 +2156,26 @@ RSpec.describe PostAlerter do
end
it "only notifies staff watching added tag" do
expect(PostRevisor.new(post).revise!(Fabricate(:admin), tags: [other_tag.name])).to be true
expect(
PostRevisor.new(post).revise!(
Fabricate(:admin, refresh_auto_groups: true),
tags: [other_tag.name],
),
).to be true
expect(Notification.where(user_id: staged.id).count).to eq(0)
expect(PostRevisor.new(post).revise!(Fabricate(:admin), tags: [other_tag2.name])).to be true
expect(
PostRevisor.new(post).revise!(
Fabricate(:admin, refresh_auto_groups: true),
tags: [other_tag2.name],
),
).to be true
expect(Notification.where(user_id: admin.id).count).to eq(0)
expect(PostRevisor.new(post).revise!(Fabricate(:admin), tags: [other_tag3.name])).to be true
expect(
PostRevisor.new(post).revise!(
Fabricate(:admin, refresh_auto_groups: true),
tags: [other_tag3.name],
),
).to be true
expect(Notification.where(user_id: admin.id).count).to eq(1)
end
end