DEV: Convert min_trust_to_post_links to groups (#25298)

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_to_post_links  site setting to post_links_allowed_groups.

This isn't used by any of our plugins or themes, so very little fallout.
This commit is contained in:
Ted Johansson 2024-01-18 14:08:40 +08:00 committed by GitHub
parent 20c5f7aef8
commit fb087b7ff6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 89 additions and 43 deletions

View File

@ -1984,6 +1984,7 @@ en:
min_trust_to_flag_posts: "The minimum trust level required to flag posts"
flag_post_allowed_groups: "Groups that are allowed to flag posts."
min_trust_to_post_links: "The minimum trust level required to include links in posts"
post_links_allowed_groups: "Groups that are allowed to include links in posts."
min_trust_to_post_embedded_media: "The minimum trust level required to embed media items in a post"
min_trust_level_to_allow_profile_background: "The minimum trust level required to upload a profile background"
min_trust_level_to_allow_user_card_background: "The minimum trust level required to upload a user card background"
@ -2586,6 +2587,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"
post_links_allowed_groups: "min_trust_to_post_links"
placeholder:
discourse_connect_provider_secrets:

View File

@ -1756,6 +1756,13 @@ trust:
min_trust_to_post_links:
default: 0
enum: "TrustLevelSetting"
hidden: true
post_links_allowed_groups:
default: "10"
type: group_list
allow_any: false
refresh: true
validator: "AtLeastOneGroupValidator"
min_trust_to_post_embedded_media:
default: 0
enum: "TrustLevelSetting"

View File

@ -0,0 +1,27 @@
# frozen_string_literal: true
class FillPostLinksAllowedGroupsBasedOnDeprecatedSettings < ActiveRecord::Migration[7.0]
def up
configured_trust_level =
DB.query_single(
"SELECT value FROM site_settings WHERE name = 'min_trust_to_post_links' 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('post_links_allowed_groups', :setting, '20', NOW(), NOW())",
setting: corresponding_group,
)
end
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -3,7 +3,7 @@
# mixin for all guardian methods dealing with post permissions
module PostGuardian
def unrestricted_link_posting?
authenticated? && @user.has_trust_level?(TrustLevel[SiteSetting.min_trust_to_post_links])
authenticated? && @user.in_any_groups?(SiteSetting.post_links_allowed_groups_map)
end
def link_posting_access

View File

@ -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_to_post_links", "post_links_allowed_groups", false, "3.3"],
]
OVERRIDE_TL_GROUP_SETTINGS = %w[
@ -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_to_post_links
]
def group_to_tl(old_setting, new_setting)

View File

@ -1,7 +1,7 @@
# frozen_string_literal: true
Fabricator(:post) do
user
user { Fabricate(:user, refresh_auto_groups: true) }
topic { |attrs| Fabricate(:topic, user: attrs[:user]) }
raw "Hello world"
post_type Post.types[:regular]
@ -124,7 +124,7 @@ Fabricator(:post_with_uploads_and_links, from: :post) { raw <<~MD }
MD
Fabricator(:post_with_external_links, from: :post) do
user
user { Fabricate(:user, refresh_auto_groups: true) }
topic
raw <<~MD
Here's a link to twitter: http://twitter.com

View File

@ -184,9 +184,11 @@ RSpec.describe CookedPostProcessor do
- #{url_with_query_param}
RAW
let(:staff_post) { Fabricate(:post, user: Fabricate(:admin), raw: <<~RAW) }
let(:staff_post) do
Fabricate(:post, user: Fabricate(:admin, refresh_auto_groups: true), raw: <<~RAW)
This is a #{url_with_path} topic
RAW
end
before do
urls.each do |url|

View File

@ -50,15 +50,15 @@ RSpec.describe Guardian do
end
it "is none for a user of a low trust level" do
user.trust_level = 0
SiteSetting.min_trust_to_post_links = 1
user.change_trust_level!(TrustLevel[0])
SiteSetting.post_links_allowed_groups = Group::AUTO_GROUPS[:trust_level_1]
expect(Guardian.new(user).link_posting_access).to eq("none")
end
it "is limited for a user of a low trust level with a allowlist" do
SiteSetting.allowed_link_domains = "example.com"
user.trust_level = 0
SiteSetting.min_trust_to_post_links = 1
user.change_trust_level!(TrustLevel[0])
SiteSetting.post_links_allowed_groups = Group::AUTO_GROUPS[:trust_level_1]
expect(Guardian.new(user).link_posting_access).to eq("limited")
end
end
@ -75,10 +75,10 @@ RSpec.describe Guardian do
end
it "supports customization by site setting" do
user.trust_level = 0
SiteSetting.min_trust_to_post_links = 0
user.change_trust_level!(TrustLevel[0])
SiteSetting.post_links_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
expect(Guardian.new(user).can_post_link?(host: host)).to eq(true)
SiteSetting.min_trust_to_post_links = 1
SiteSetting.post_links_allowed_groups = Group::AUTO_GROUPS[:trust_level_1]
expect(Guardian.new(user).can_post_link?(host: host)).to eq(false)
end
@ -86,8 +86,8 @@ RSpec.describe Guardian do
before { SiteSetting.allowed_link_domains = host }
it "allows a new user to post the link to the host" do
user.trust_level = 0
SiteSetting.min_trust_to_post_links = 1
user.change_trust_level!(TrustLevel[0])
SiteSetting.post_links_allowed_groups = Group::AUTO_GROUPS[:trust_level_1]
expect(Guardian.new(user).can_post_link?(host: host)).to eq(true)
expect(Guardian.new(user).can_post_link?(host: "another-host.com")).to eq(false)
end

View File

@ -1000,7 +1000,7 @@ RSpec.describe PostDestroyer do
let!(:second_post) { Fabricate(:post, topic: topic) }
fab!(:other_topic) { Fabricate(:topic) }
let!(:other_post) { Fabricate(:post, topic: other_topic) }
fab!(:user)
fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
let!(:base_url) { URI.parse(Discourse.base_url) }
let!(:guardian) { Guardian.new }
let!(:url) do

View File

@ -854,7 +854,7 @@ RSpec.describe PostRevisor do
end
describe "admin editing a new user's post" do
fab!(:changed_by) { Fabricate(:admin) }
fab!(:changed_by) { Fabricate(:admin, refresh_auto_groups: true) }
before do
SiteSetting.newuser_max_embedded_media = 0

View File

@ -2,7 +2,7 @@
RSpec.describe PostMover do
fab!(:admin)
fab!(:evil_trout)
fab!(:evil_trout) { Fabricate(:evil_trout, refresh_auto_groups: true) }
describe "#move_types" do
context "when verifying enum sequence" do
@ -532,8 +532,8 @@ RSpec.describe PostMover do
end
fab!(:user1) { Fabricate(:user) }
fab!(:user2) { Fabricate(:user) }
fab!(:user3) { Fabricate(:user) }
fab!(:user2) { Fabricate(:user, refresh_auto_groups: true) }
fab!(:user3) { Fabricate(:user, refresh_auto_groups: true) }
fab!(:admin1) { Fabricate(:admin) }
fab!(:admin2) { Fabricate(:admin) }

View File

@ -275,7 +275,7 @@ RSpec.describe Post do
end
describe "maximum media embeds" do
fab!(:newuser) { Fabricate(:user, trust_level: TrustLevel[0]) }
fab!(:newuser) { Fabricate(:user, trust_level: TrustLevel[0], refresh_auto_groups: true) }
let(:post_no_images) { Fabricate.build(:post, post_args.merge(user: newuser)) }
let(:post_one_image) { post_with_body("![sherlock](http://bbc.co.uk/sherlock.jpg)", newuser) }
let(:post_two_images) do
@ -421,7 +421,7 @@ RSpec.describe Post do
end
describe "maximum attachments" do
fab!(:newuser) { Fabricate(:user, trust_level: TrustLevel[0]) }
fab!(:newuser) { Fabricate(:user, trust_level: TrustLevel[0], refresh_auto_groups: true) }
let(:post_no_attachments) { Fabricate.build(:post, post_args.merge(user: newuser)) }
let(:post_one_attachment) do
post_with_body(
@ -474,7 +474,7 @@ RSpec.describe Post do
end
describe "links" do
fab!(:newuser) { Fabricate(:user, trust_level: TrustLevel[0]) }
fab!(:newuser) { Fabricate(:user, trust_level: TrustLevel[0], refresh_auto_groups: true) }
let(:no_links) { post_with_body("hello world my name is evil trout", newuser) }
let(:one_link) { post_with_body("[jlawr](http://www.imdb.com/name/nm2225369)", newuser) }
let(:two_links) do
@ -557,7 +557,7 @@ RSpec.describe Post do
end
describe "maximums" do
fab!(:newuser) { Fabricate(:user, trust_level: TrustLevel[0]) }
fab!(:newuser) { Fabricate(:user, trust_level: TrustLevel[0], refresh_auto_groups: true) }
let(:post_one_link) do
post_with_body("[sherlock](http://www.bbc.co.uk/programmes/b018ttws)", newuser)
end
@ -607,29 +607,29 @@ RSpec.describe Post do
expect(post_two_links).to be_valid
end
context "with min_trust_to_post_links" do
context "when posting links is limited to certain TL groups" do
it "considers oneboxes links" do
SiteSetting.min_trust_to_post_links = 3
post_onebox.user.trust_level = TrustLevel[2]
SiteSetting.post_links_allowed_groups = Group::AUTO_GROUPS[:trust_level_3]
post_onebox.user.change_trust_level!(TrustLevel[2])
expect(post_onebox).not_to be_valid
end
it "considers links within code" do
SiteSetting.min_trust_to_post_links = 3
post_onebox.user.trust_level = TrustLevel[2]
SiteSetting.post_links_allowed_groups = Group::AUTO_GROUPS[:trust_level_3]
post_onebox.user.change_trust_level!(TrustLevel[2])
expect(post_code_link).not_to be_valid
end
it "doesn't allow allow links if `min_trust_to_post_links` is not met" do
SiteSetting.min_trust_to_post_links = 2
post_two_links.user.trust_level = TrustLevel[1]
it "doesn't allow allow links if user is not in allowed groups" do
SiteSetting.post_links_allowed_groups = Group::AUTO_GROUPS[:trust_level_2]
post_two_links.user.change_trust_level!(TrustLevel[1])
expect(post_one_link).not_to be_valid
end
it "will skip the check for allowlisted domains" do
SiteSetting.allowed_link_domains = "www.bbc.co.uk"
SiteSetting.min_trust_to_post_links = 2
post_two_links.user.trust_level = TrustLevel[1]
SiteSetting.post_links_allowed_groups = "12"
post_two_links.user.change_trust_level!(TrustLevel[1])
expect(post_one_link).to be_valid
end
end
@ -686,7 +686,7 @@ RSpec.describe Post do
end
context "with max mentions" do
fab!(:newuser) { Fabricate(:user, trust_level: TrustLevel[0]) }
fab!(:newuser) { Fabricate(:user, trust_level: TrustLevel[0], refresh_auto_groups: true) }
let(:post_with_one_mention) { post_with_body("@Jake is the person I'm mentioning", newuser) }
let(:post_with_two_mentions) do
post_with_body("@Jake @Finn are the people I'm mentioning", newuser)
@ -1127,7 +1127,13 @@ RSpec.describe Post do
describe "cooking" do
let(:post) do
Fabricate.build(:post, post_args.merge(raw: "please read my blog http://blog.example.com"))
Fabricate.build(
:post,
post_args.merge(
raw: "please read my blog http://blog.example.com",
user: Fabricate(:user, refresh_auto_groups: true),
),
)
end
it "should unconditionally follow links for staff" do

View File

@ -11,7 +11,7 @@ RSpec.describe TopicLinkClick do
describe "topic_links" do
before do
@topic = Fabricate(:topic)
@topic = Fabricate(:topic, user: Fabricate(:user, refresh_auto_groups: true))
@post = Fabricate(:post_with_external_links, user: @topic.user, topic: @topic)
TopicLink.extract_from(@post)
@topic_link = @topic.topic_links.first
@ -247,7 +247,7 @@ RSpec.describe TopicLinkClick do
context "with a query param and google analytics" do
before do
@topic = Fabricate(:topic)
@topic = Fabricate(:topic, user: Fabricate(:user, refresh_auto_groups: true))
@post =
Fabricate(
:post,

View File

@ -2,8 +2,8 @@
RSpec.describe TopicLink do
let(:test_uri) { URI.parse(Discourse.base_url) }
fab!(:topic) { Fabricate(:topic, title: "unique topic name") }
fab!(:user) { topic.user }
fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
fab!(:topic) { Fabricate(:topic, user: user, title: "unique topic name") }
fab!(:post)
it { is_expected.to validate_presence_of :url }
@ -443,7 +443,7 @@ RSpec.describe TopicLink do
context "with data" do
let(:post) do
topic = Fabricate(:topic)
topic = Fabricate(:topic, user: Fabricate(:user, refresh_auto_groups: true))
Fabricate(:post_with_external_links, user: topic.user, topic: topic)
end
@ -536,7 +536,7 @@ RSpec.describe TopicLink do
end
describe ".duplicate_lookup" do
fab!(:user) { Fabricate(:user, username: "junkrat") }
fab!(:user) { Fabricate(:user, username: "junkrat", refresh_auto_groups: true) }
let(:post_with_internal_link) do
Fabricate(:post, user: user, raw: "Check out this topic #{post.topic.url}/122131")

View File

@ -1181,7 +1181,7 @@ RSpec.describe Admin::UsersController do
end
describe "#destroy" do
fab!(:delete_me) { Fabricate(:user) }
fab!(:delete_me) { Fabricate(:user, refresh_auto_groups: true) }
shared_examples "user deletion possible" do
it "returns a 403 if the user doesn't exist" do

View File

@ -29,7 +29,7 @@ RSpec.describe UserSummarySerializer do
end
it "returns correct links data ranking" do
topic = Fabricate(:topic)
topic = Fabricate(:topic, user: Fabricate(:user, refresh_auto_groups: true))
post = Fabricate(:post_with_external_links, user: topic.user, topic: topic)
TopicLink.extract_from(post)
TopicLink

View File

@ -1,7 +1,7 @@
# frozen_string_literal: true
RSpec.describe UserDestroyer do
fab!(:user) { Fabricate(:user_with_secondary_email) }
fab!(:user) { Fabricate(:user_with_secondary_email, refresh_auto_groups: true) }
fab!(:admin) { Fabricate(:admin, refresh_auto_groups: true) }
describe ".new" do