Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DEV: Convert min_trust_level_to_tag_topics to groups #25271

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions config/locales/server.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2442,6 +2442,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 @@ -2584,6 +2585,7 @@ en:
create_tag_allowed_groups: "min_trust_to_create_tag"
send_email_messages_allowed_groups: "min_trust_to_send_email_messages"
skip_review_media_groups: "review_media_unless_trust_level"
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 @@ -3031,6 +3031,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('min_trust_level_to_tag_topics', :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 @@ -37,6 +37,7 @@ module SiteSettings::DeprecatedSettings
["min_trust_to_create_tag", "create_tag_allowed_groups", false, "3.3"],
["min_trust_to_send_email_messages", "send_email_messages_allowed_groups", false, "3.3"],
["review_media_unless_trust_level", "skip_review_media_groups", false, "3.3"],
["min_trust_level_to_tag_topics", "tag_topic_allowed_groups", false, "3.3"],
]

OVERRIDE_TL_GROUP_SETTINGS = %w[
Expand All @@ -58,6 +59,7 @@ module SiteSettings::DeprecatedSettings
min_trust_to_create_tag
min_trust_to_send_email_messages
review_media_unless_trust_level
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
10 changes: 6 additions & 4 deletions spec/integration/category_tag_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,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

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 @@ -3708,7 +3708,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 @@ -3739,11 +3739,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
4 changes: 2 additions & 2 deletions spec/lib/new_post_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

RSpec.describe NewPostManager do
fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
fab!(:topic)
fab!(:topic) { Fabricate(:topic, user: user) }

describe "default action" do
it "creates the post by default" do
Expand Down Expand Up @@ -415,7 +415,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 @@ -1378,7 +1378,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
6 changes: 4 additions & 2 deletions spec/lib/topic_creator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,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 @@ -136,6 +136,8 @@
)
end

before { Discourse.system_user.change_trust_level!(TrustLevel[4]) }

it "adds watched words as tags" do
topic =
TopicCreator.create(
Expand Down Expand Up @@ -216,7 +218,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 @@ -419,7 +419,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]
Group.refresh_automatic_groups!
end
Expand Down Expand Up @@ -488,7 +488,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]
Group.refresh_automatic_groups!
end
Expand Down Expand Up @@ -559,7 +559,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]
Group.refresh_automatic_groups!
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
2 changes: 1 addition & 1 deletion spec/models/tag_user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,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

def regular
Expand Down
Loading
Loading