Skip to content

Commit

Permalink
DEV: Convert min_trust_level_to_tag_topics to groups
Browse files Browse the repository at this point in the history
  • Loading branch information
Drenmi committed Jan 16, 2024
1 parent fd7c6f1 commit ad22b57
Show file tree
Hide file tree
Showing 25 changed files with 126 additions and 49 deletions.
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('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 @@ -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
6 changes: 4 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,9 @@ 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]

Group.user_trust_level_change!(Discourse.system_user.id, Discourse.system_user.trust_level)

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

0 comments on commit ad22b57

Please sign in to comment.