From 6ab1a19e93e6a8181848958af974943c2e688135 Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Mon, 18 Dec 2023 12:07:36 +0800 Subject: [PATCH] DEV: Convert min_trust_level_to_allow_invite to groups (#24893) 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_allow_invite site setting to invite_allowed_groups. Nothing much of note. This is used in one place and there's no fallout. --- config/locales/server.en.yml | 2 ++ config/site_settings.yml | 7 +++++ ...wed_groups_based_on_deprecated_settings.rb | 27 +++++++++++++++++++ lib/guardian.rb | 2 +- lib/site_settings/deprecated_settings.rb | 1 + spec/lib/guardian_spec.rb | 18 ++++++------- spec/mailers/invite_mailer_spec.rb | 2 +- spec/models/topic_spec.rb | 8 +++--- spec/requests/invites_controller_spec.rb | 2 +- spec/requests/topics_controller_spec.rb | 2 +- spec/requests/users_controller_spec.rb | 14 +++++----- 11 files changed, 60 insertions(+), 25 deletions(-) create mode 100644 db/migrate/20231214023728_fill_invite_allowed_groups_based_on_deprecated_settings.rb diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 073762db19d..7c9fd47b702 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1981,6 +1981,7 @@ en: min_trust_level_to_allow_user_card_background: "The minimum trust level required to upload a user card background" user_card_background_allowed_groups: "Groups that are allowed to upload a user card background." min_trust_level_to_allow_invite: "The minimum trust level required to invite users" + invite_allowed_groups: "Groups that are allowed to invite users." min_trust_level_to_allow_ignore: "The minimum trust level required to ignore users" allowed_link_domains: "Domains that users may link to even if they don't have the appropriate trust level to post links" @@ -2567,6 +2568,7 @@ en: flag_post_allowed_groups: "min_trust_to_flag_posts" delete_all_posts_and_topics_allowed_groups: "tl4_delete_posts_and_topics" user_card_background_allowed_groups: "min_trust_level_to_allow_user_card_background" + invite_allowed_groups: "min_trust_level_to_allow_invite" placeholder: discourse_connect_provider_secrets: diff --git a/config/site_settings.yml b/config/site_settings.yml index 3f525053b04..bb74c331b10 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1749,6 +1749,13 @@ trust: min_trust_level_to_allow_invite: default: 2 enum: "TrustLevelSetting" + hidden: true + invite_allowed_groups: + default: "12" + type: group_list + allow_any: false + refresh: true + validator: "AtLeastOneGroupValidator" min_trust_level_to_allow_ignore: default: 2 enum: "TrustLevelSetting" diff --git a/db/migrate/20231214023728_fill_invite_allowed_groups_based_on_deprecated_settings.rb b/db/migrate/20231214023728_fill_invite_allowed_groups_based_on_deprecated_settings.rb new file mode 100644 index 00000000000..69a318dc905 --- /dev/null +++ b/db/migrate/20231214023728_fill_invite_allowed_groups_based_on_deprecated_settings.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class FillInviteAllowedGroupsBasedOnDeprecatedSettings < ActiveRecord::Migration[7.0] + def up + configured_trust_level = + DB.query_single( + "SELECT value FROM site_settings WHERE name = 'min_trust_level_to_allow_invite' LIMIT 1", + ).first + + # Default for old setting is TL2, 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('invite_allowed_groups', :setting, '20', NOW(), NOW())", + setting: corresponding_group, + ) + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/guardian.rb b/lib/guardian.rb index d5e5b8243ac..795a1e6fe68 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -418,7 +418,7 @@ class Guardian def can_invite_to_forum?(groups = nil) authenticated? && (is_staff? || SiteSetting.max_invites_per_day.to_i.positive?) && - (is_staff? || @user.has_trust_level?(SiteSetting.min_trust_level_to_allow_invite.to_i)) && + (is_staff? || @user.in_any_groups?(SiteSetting.invite_allowed_groups_map)) && (is_admin? || groups.blank? || groups.all? { |g| can_edit_group?(g) }) end diff --git a/lib/site_settings/deprecated_settings.rb b/lib/site_settings/deprecated_settings.rb index 8d0dd39a00c..303fa60af15 100644 --- a/lib/site_settings/deprecated_settings.rb +++ b/lib/site_settings/deprecated_settings.rb @@ -31,6 +31,7 @@ module SiteSettings::DeprecatedSettings false, "3.3", ], + ["min_trust_level_to_allow_invite", "invite_allowed_groups", false, "3.3"], ] def setup_deprecated_methods diff --git a/spec/lib/guardian_spec.rb b/spec/lib/guardian_spec.rb index 23c6fce0497..dc867774615 100644 --- a/spec/lib/guardian_spec.rb +++ b/spec/lib/guardian_spec.rb @@ -607,13 +607,13 @@ RSpec.describe Guardian do fab!(:moderator) it "returns true if user has sufficient trust level" do - SiteSetting.min_trust_level_to_allow_invite = 2 + SiteSetting.invite_allowed_groups = Group::AUTO_GROUPS[:trust_level_2] expect(Guardian.new(trust_level_2).can_invite_to_forum?).to be_truthy expect(Guardian.new(moderator).can_invite_to_forum?).to be_truthy end it "returns false if user trust level does not have sufficient trust level" do - SiteSetting.min_trust_level_to_allow_invite = 2 + SiteSetting.invite_allowed_groups = Group::AUTO_GROUPS[:trust_level_2] expect(Guardian.new(trust_level_1).can_invite_to_forum?).to be_falsey end @@ -639,7 +639,7 @@ RSpec.describe Guardian do let(:groups) { [group, another_group] } before do - user.update!(trust_level: TrustLevel[2]) + user.change_trust_level!(TrustLevel[2]) group.add_owner(user) end @@ -660,8 +660,8 @@ RSpec.describe Guardian do describe "can_invite_to?" do describe "regular topics" do before do - SiteSetting.min_trust_level_to_allow_invite = 2 - user.update!(trust_level: SiteSetting.min_trust_level_to_allow_invite) + SiteSetting.invite_allowed_groups = Group::AUTO_GROUPS[:trust_level_2] + user.update!(trust_level: 2) end fab!(:category) { Fabricate(:category, read_restricted: true) } fab!(:topic) @@ -693,7 +693,7 @@ RSpec.describe Guardian do end it "returns true for a group owner" do - group_owner.update!(trust_level: SiteSetting.min_trust_level_to_allow_invite) + group_owner.update!(trust_level: 2) expect(Guardian.new(group_owner).can_invite_to?(group_private_topic)).to be_truthy end @@ -718,7 +718,7 @@ RSpec.describe Guardian do end it "should return true for a group owner" do - group_owner.update!(trust_level: SiteSetting.min_trust_level_to_allow_invite) + group_owner.update!(trust_level: 2) expect(Guardian.new(group_owner).can_invite_to?(topic)).to eq(true) end @@ -748,8 +748,8 @@ RSpec.describe Guardian do fab!(:pm) { Fabricate(:private_message_topic, user: user) } before do - user.change_trust_level!(SiteSetting.min_trust_level_to_allow_invite) - moderator.change_trust_level!(SiteSetting.min_trust_level_to_allow_invite) + user.change_trust_level!(TrustLevel[2]) + moderator.change_trust_level!(TrustLevel[2]) end context "when private messages are disabled" do diff --git a/spec/mailers/invite_mailer_spec.rb b/spec/mailers/invite_mailer_spec.rb index f50e857f2db..e963455b46d 100644 --- a/spec/mailers/invite_mailer_spec.rb +++ b/spec/mailers/invite_mailer_spec.rb @@ -78,7 +78,7 @@ RSpec.describe InviteMailer do end context "when inviting to topic" do - let(:trust_level_2) { build(:user, trust_level: 2) } + fab!(:trust_level_2) { Fabricate(:user, trust_level: 2, refresh_auto_groups: true) } let(:topic) do Fabricate( :topic, diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index b9ca3151357..cd933de1b33 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -12,9 +12,7 @@ RSpec.describe Topic do fab!(:evil_trout) fab!(:admin) fab!(:group) - fab!(:trust_level_2) do - Fabricate(:user, trust_level: SiteSetting.min_trust_level_to_allow_invite) - end + fab!(:trust_level_2) { Fabricate(:user, trust_level: 2, refresh_auto_groups: true) } it_behaves_like "it has custom fields" @@ -1001,7 +999,7 @@ RSpec.describe Topic do end describe "when user does not have sufficient trust level" do - before { user.update!(trust_level: TrustLevel[1]) } + before { user.change_trust_level!(TrustLevel[1]) } it "should not create an invite" do expect do expect(topic.invite(user, "test@email.com")).to eq(nil) end.to_not change { @@ -1105,7 +1103,7 @@ RSpec.describe Topic do end describe "when user can invite via email" do - before { user.update!(trust_level: SiteSetting.min_trust_level_to_allow_invite) } + before { user.change_trust_level!(TrustLevel[2]) } it "should create an invite" do Jobs.run_immediately! diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb index c31648e8795..79f6f3d623c 100644 --- a/spec/requests/invites_controller_spec.rb +++ b/spec/requests/invites_controller_spec.rb @@ -2,7 +2,7 @@ RSpec.describe InvitesController do fab!(:admin) - fab!(:user) { Fabricate(:user, trust_level: SiteSetting.min_trust_level_to_allow_invite) } + fab!(:user) { Fabricate(:user, trust_level: TrustLevel[2], refresh_auto_groups: true) } describe "#show" do fab!(:invite) diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 8b1876a7e6b..36d41d0f025 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -4926,7 +4926,7 @@ RSpec.describe TopicsController do fab!(:user_topic) { Fabricate(:topic, user: user) } it "should return the right response" do - user.update!(trust_level: SiteSetting.min_trust_level_to_allow_invite) + user.update!(trust_level: TrustLevel[2]) post "/t/#{user_topic.id}/invite.json", params: { email: "someguy@email.com" } diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 56750b21427..8247ce3fca3 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -1979,7 +1979,7 @@ RSpec.describe UsersController do end it "returns success" do - user = Fabricate(:user, trust_level: 2) + user = Fabricate(:user, trust_level: 2, refresh_auto_groups: true) Fabricate(:invite, invited_by: user) sign_in(user) @@ -1991,7 +1991,7 @@ RSpec.describe UsersController do end it "filters by all if viewing self" do - inviter = Fabricate(:user, trust_level: 2) + inviter = Fabricate(:user, trust_level: 2, refresh_auto_groups: true) sign_in(inviter) Fabricate(:invite, email: "billybob@example.com", invited_by: inviter) @@ -2018,8 +2018,8 @@ RSpec.describe UsersController do end it "doesn't filter by email if another regular user" do - inviter = Fabricate(:user, trust_level: 2) - sign_in(Fabricate(:user, trust_level: 2)) + inviter = Fabricate(:user, trust_level: 2, refresh_auto_groups: true) + sign_in(Fabricate(:user, trust_level: 2, refresh_auto_groups: true)) Fabricate(:invite, email: "billybob@example.com", invited_by: inviter) redeemed_invite = Fabricate(:invite, email: "jimtom@example.com", invited_by: inviter) @@ -2074,7 +2074,7 @@ RSpec.describe UsersController do context "with redeemed invites" do it "returns invited_users" do - inviter = Fabricate(:user, trust_level: 2) + inviter = Fabricate(:user, trust_level: 2, refresh_auto_groups: true) sign_in(inviter) invite = Fabricate(:invite, invited_by: inviter) _invited_user = Fabricate(:invited_user, invite: invite, user: invitee) @@ -2093,7 +2093,7 @@ RSpec.describe UsersController do context "with pending invites" do context "with permission to see pending invites" do it "returns invites" do - inviter = Fabricate(:user, trust_level: 2) + inviter = Fabricate(:user, trust_level: 2, refresh_auto_groups: true) invite = Fabricate(:invite, invited_by: inviter) sign_in(inviter) @@ -2122,7 +2122,7 @@ RSpec.describe UsersController do context "with permission to see invite links" do it "returns own invites" do - inviter = sign_in(Fabricate(:user, trust_level: 2)) + inviter = sign_in(Fabricate(:user, trust_level: 2, refresh_auto_groups: true)) invite = Fabricate( :invite,