Skip to content

Commit

Permalink
DEV: Convert min_trust_level_to_tag_topics to groups (#25273)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Drenmi committed Jan 26, 2024
1 parent d178d75 commit 7e5d2a9
Show file tree
Hide file tree
Showing 25 changed files with 124 additions and 51 deletions.
2 changes: 2 additions & 0 deletions config/locales/server.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down Expand Up @@ -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:
Expand Down
8 changes: 8 additions & 0 deletions config/site_settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
3 changes: 1 addition & 2 deletions lib/guardian/tag_guardian.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ def can_create_tag?
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?
Expand Down
2 changes: 2 additions & 0 deletions lib/site_settings/deprecated_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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[
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion plugins/chat/spec/lib/chat/channel_archive_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class FakeArchiveError < StandardError
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 } }
Expand Down
12 changes: 7 additions & 5 deletions spec/integration/category_tag_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ def filter_allowed_tags(opts = {})
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
Expand Down Expand Up @@ -770,7 +770,7 @@ def filter_allowed_tags(opts = {})
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
Expand All @@ -782,8 +782,10 @@ def filter_allowed_tags(opts = {})
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])
Expand Down
2 changes: 1 addition & 1 deletion spec/jobs/export_user_archive_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
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) }
Expand Down
4 changes: 2 additions & 2 deletions spec/lib/discourse_tagging_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

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) }

Expand All @@ -18,7 +18,7 @@
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
Expand Down
14 changes: 11 additions & 3 deletions spec/lib/guardian_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3705,7 +3705,7 @@
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
Expand Down Expand Up @@ -3736,11 +3736,19 @@

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
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/new_post_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ def build_manager_with(raw)
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(
Expand Down
6 changes: 3 additions & 3 deletions spec/lib/post_creator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@
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
Expand All @@ -577,7 +577,7 @@
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
Expand All @@ -590,7 +590,7 @@
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")
Expand Down
14 changes: 7 additions & 7 deletions spec/lib/post_revisor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
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 } }

Expand Down Expand Up @@ -101,7 +101,7 @@

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)

Expand All @@ -123,7 +123,7 @@

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)
Expand All @@ -137,7 +137,7 @@

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)
Expand Down Expand Up @@ -165,7 +165,7 @@

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])
Expand Down Expand Up @@ -1233,7 +1233,7 @@
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
Expand Down Expand Up @@ -1491,7 +1491,7 @@
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
Expand Down
4 changes: 2 additions & 2 deletions spec/lib/search_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -1390,7 +1390,7 @@ def search
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
Expand Down
4 changes: 2 additions & 2 deletions spec/lib/topic_creator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@
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
Expand Down Expand Up @@ -214,7 +214,7 @@
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(
Expand Down
6 changes: 3 additions & 3 deletions spec/lib/topics_bulk_action_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@

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

Expand Down Expand Up @@ -482,7 +482,7 @@

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

Expand Down Expand Up @@ -552,7 +552,7 @@

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

Expand Down
1 change: 1 addition & 0 deletions spec/models/post_mover_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2225,6 +2225,7 @@ def create_topic_user(user, topic, opts = {})

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],
Expand Down
2 changes: 1 addition & 1 deletion spec/models/reviewable_queued_post_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@
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
Expand Down
2 changes: 1 addition & 1 deletion spec/models/tag_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def make_some_tags(count: 3, tag_a_topic: false)

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
Expand Down
Loading

0 comments on commit 7e5d2a9

Please sign in to comment.