From 7dd150bc954f9499472e0153dc5ba0be2a39c8f3 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Tue, 12 Dec 2023 15:20:37 +1100 Subject: [PATCH] DEV: Convert min_trust_to_edit_wiki_post to groups (#24766) This change converts the min_trust_to_edit_wiki_post site setting to edit_wiki_post_allowed_groups. See: https://meta.discourse.org/t/283408 Hides the old setting Adds the new site setting Add a deprecation warning Updates to use the new setting Adds a migration to fill in the new setting if the old setting was changed Adds an entry to the site_setting.keywords section Updates tests to account for the new change After a couple of months, we will remove the email_in_min_trust setting entirely. Internal ref: /t/117248 --- config/locales/server.en.yml | 2 ++ config/site_settings.yml | 7 ++++++ ...wed_groups_based_on_deprecated_settings.rb | 24 +++++++++++++++++++ lib/guardian/post_guardian.rb | 2 +- lib/guardian/topic_guardian.rb | 2 +- lib/site_settings/deprecated_settings.rb | 1 + plugins/discourse-presence/plugin.rb | 4 +--- .../spec/integration/presence_spec.rb | 6 ++--- spec/lib/guardian_spec.rb | 18 +++++++------- spec/requests/topics_controller_spec.rb | 3 ++- .../serializers/topic_view_serializer_spec.rb | 5 ++-- 11 files changed, 54 insertions(+), 20 deletions(-) create mode 100644 db/migrate/20231207011238_fill_edit_wiki_post_allowed_groups_based_on_deprecated_settings.rb diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index be052baf1e9..19d75e88d6a 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1961,6 +1961,7 @@ en: allow_flagging_staff: "If enabled, users can flag posts from staff accounts." min_trust_to_edit_wiki_post: "The minimum trust level required to edit post marked as wiki." + edit_wiki_post_allowed_groups: "Groups that are allowed to edit posts marked as wiki." min_trust_to_edit_post: "The minimum trust level required to edit posts." @@ -2551,6 +2552,7 @@ en: approve_unless_allowed_groups: "approve_unless_trust_level" approve_new_topics_unless_allowed_groups: "approve_new_topics_unless_trust_level" email_in_allowed_groups: "email_in_min_trust" + edit_wiki_post_allowed_groups: "minmin_trust_to_edit_wiki_post" placeholder: discourse_connect_provider_secrets: diff --git a/config/site_settings.yml b/config/site_settings.yml index b9b59b83670..e7f909f305d 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1675,6 +1675,13 @@ trust: min_trust_to_edit_wiki_post: default: 1 enum: "TrustLevelSetting" + hidden: true + edit_wiki_post_allowed_groups: + default: 11 + type: group_list + allow_any: false + refresh: true + validator: "AtLeastOneGroupValidator" min_trust_to_edit_post: default: 0 enum: "TrustLevelSetting" diff --git a/db/migrate/20231207011238_fill_edit_wiki_post_allowed_groups_based_on_deprecated_settings.rb b/db/migrate/20231207011238_fill_edit_wiki_post_allowed_groups_based_on_deprecated_settings.rb new file mode 100644 index 00000000000..0a90e0c85ed --- /dev/null +++ b/db/migrate/20231207011238_fill_edit_wiki_post_allowed_groups_based_on_deprecated_settings.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class FillEditWikiPostAllowedGroupsBasedOnDeprecatedSettings < ActiveRecord::Migration[7.0] + def up + old_setting_trust_level = + DB.query_single( + "SELECT value FROM site_settings WHERE name = 'min_trust_to_edit_wiki_post' LIMIT 1", + ).first + + if old_setting_trust_level.present? + edit_wiki_post_allowed_groups = "1#{old_setting_trust_level}" + + DB.exec( + "INSERT INTO site_settings(name, value, data_type, created_at, updated_at) + VALUES('edit_wiki_post_allowed_groups', :setting, '20', NOW(), NOW())", + setting: edit_wiki_post_allowed_groups, + ) + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index ec11c01db46..0584e0b1c3a 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -159,7 +159,7 @@ module PostGuardian return true end - if post.wiki && (@user.trust_level >= SiteSetting.min_trust_to_edit_wiki_post.to_i) + if post.wiki && @user.in_any_groups?(SiteSetting.edit_wiki_post_allowed_groups_map) return can_create_post?(post.topic) end diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index 35e9d4fa860..2c8df78f902 100644 --- a/lib/guardian/topic_guardian.rb +++ b/lib/guardian/topic_guardian.rb @@ -330,7 +330,7 @@ module TopicGuardian return true if can_edit_topic?(topic) if topic&.first_post&.wiki && - (@user.trust_level >= SiteSetting.min_trust_to_edit_wiki_post.to_i) + @user.in_any_groups?(SiteSetting.edit_wiki_post_allowed_groups_map) return can_create_post?(topic) end diff --git a/lib/site_settings/deprecated_settings.rb b/lib/site_settings/deprecated_settings.rb index a81c2314a07..b77170f1871 100644 --- a/lib/site_settings/deprecated_settings.rb +++ b/lib/site_settings/deprecated_settings.rb @@ -19,6 +19,7 @@ module SiteSettings::DeprecatedSettings "3.3", ], ["email_in_min_trust", "email_in_allowed_groups", false, "3.3"], + ["min_trust_to_edit_wiki_post", "edit_wiki_post_allowed_groups", false, "3.3"], ] def setup_deprecated_methods diff --git a/plugins/discourse-presence/plugin.rb b/plugins/discourse-presence/plugin.rb index 98c15ca3836..f61fc12bd76 100644 --- a/plugins/discourse-presence/plugin.rb +++ b/plugins/discourse-presence/plugin.rb @@ -52,9 +52,7 @@ after_initialize do config.allowed_user_ids += topic.allowed_users.pluck(:id) config.allowed_group_ids += topic.allowed_groups.pluck(:id) elsif post.wiki - config.allowed_group_ids << Group::AUTO_GROUPS[ - :"trust_level_#{SiteSetting.min_trust_to_edit_wiki_post}" - ] + config.allowed_group_ids += SiteSetting.edit_wiki_post_allowed_groups_map end if !topic.private_message? && SiteSetting.edit_all_post_groups.present? diff --git a/plugins/discourse-presence/spec/integration/presence_spec.rb b/plugins/discourse-presence/spec/integration/presence_spec.rb index ac4afb93288..66b7f9776f0 100644 --- a/plugins/discourse-presence/spec/integration/presence_spec.rb +++ b/plugins/discourse-presence/spec/integration/presence_spec.rb @@ -149,13 +149,13 @@ RSpec.describe "discourse-presence" do expect(c.config.allowed_user_ids).to contain_exactly(user.id) end - it "follows the wiki edit trust level site setting" do + it "follows the wiki edit allowed groups site setting" do p = Fabricate(:post, topic: public_topic, user: user, wiki: true) - SiteSetting.min_trust_to_edit_wiki_post = TrustLevel.levels[:basic] + SiteSetting.edit_wiki_post_allowed_groups = Group::AUTO_GROUPS[:trust_level_3] c = PresenceChannel.new("/discourse-presence/edit/#{p.id}") expect(c.config.public).to eq(false) - expect(c.config.allowed_group_ids).to include(Group::AUTO_GROUPS[:trust_level_1]) + expect(c.config.allowed_group_ids).to include(Group::AUTO_GROUPS[:trust_level_3]) expect(c.config.allowed_user_ids).to contain_exactly(user.id) end diff --git a/spec/lib/guardian_spec.rb b/spec/lib/guardian_spec.rb index a3b471463e8..881b9a82736 100644 --- a/spec/lib/guardian_spec.rb +++ b/spec/lib/guardian_spec.rb @@ -1682,26 +1682,26 @@ RSpec.describe Guardian do expect(Guardian.new(post.user).can_edit?(post)).to be_truthy end - it "returns false when another user has too low trust level to edit wiki post" do - SiteSetting.min_trust_to_edit_wiki_post = 2 + it "returns false when another user is not member of edit wiki post group" do + SiteSetting.edit_wiki_post_allowed_groups = Group::AUTO_GROUPS[:trust_level_2] post.wiki = true - coding_horror.trust_level = 1 + Group.user_trust_level_change!(coding_horror.id, 1) expect(Guardian.new(coding_horror).can_edit?(post)).to be_falsey end - it "returns true when another user has adequate trust level to edit wiki post" do - SiteSetting.min_trust_to_edit_wiki_post = 2 + it "returns true when another user is member of edit wiki post group" do + SiteSetting.edit_wiki_post_allowed_groups = Group::AUTO_GROUPS[:trust_level_2] post.wiki = true - coding_horror.trust_level = 2 + Group.user_trust_level_change!(coding_horror.id, 2) expect(Guardian.new(coding_horror).can_edit?(post)).to be_truthy end - it "returns true for post author even when he has too low trust level to edit wiki post" do - SiteSetting.min_trust_to_edit_wiki_post = 2 + it "returns true for post author even when author is not member of edit wiki post group" do + SiteSetting.edit_wiki_post_allowed_groups = Group::AUTO_GROUPS[:trust_level_2] post.wiki = true - post.user.trust_level = 1 + Group.user_trust_level_change!(post.user, 1) expect(Guardian.new(post.user).can_edit?(post)).to be_truthy end diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 3f4b674d2fc..364de87fd80 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -1877,7 +1877,7 @@ RSpec.describe TopicsController do end it "can add a tag to wiki topic" do - SiteSetting.min_trust_to_edit_wiki_post = 2 + SiteSetting.edit_wiki_post_allowed_groups = Group::AUTO_GROUPS[:trust_level_2] topic.first_post.update!(wiki: true) sign_in(user_2) @@ -1887,6 +1887,7 @@ RSpec.describe TopicsController do expect(response.status).to eq(403) user_2.update!(trust_level: 2) + Group.refresh_automatic_groups! expect do put "/t/#{topic.id}/tags.json", params: { tags: [tag.name] } end.to change { topic.reload.first_post.revisions.count diff --git a/spec/serializers/topic_view_serializer_spec.rb b/spec/serializers/topic_view_serializer_spec.rb index 4282871aa26..560ef202c82 100644 --- a/spec/serializers/topic_view_serializer_spec.rb +++ b/spec/serializers/topic_view_serializer_spec.rb @@ -445,7 +445,7 @@ RSpec.describe TopicViewSerializer do context "with can_edit_tags" do before do SiteSetting.tagging_enabled = true - SiteSetting.min_trust_to_edit_wiki_post = 2 + SiteSetting.edit_wiki_post_allowed_groups = Group::AUTO_GROUPS[:trust_level_2] end it "returns true when user can edit a wiki topic" do @@ -456,8 +456,9 @@ RSpec.describe TopicViewSerializer do expect(json[:details][:can_edit_tags]).to be_nil user.update!(trust_level: 2) + Group.refresh_automatic_groups! - json = serialize_topic(topic, user) + json = serialize_topic(topic, user.reload) expect(json[:details][:can_edit_tags]).to eq(true) end end