From 48116186afa085d750d9f86a1128952875c84c69 Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Thu, 14 Dec 2023 09:56:42 +0800 Subject: [PATCH] DEV: Convert tl4_delete_posts_and_topics to groups (#24866) 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 tl4_delete_posts_and_topics site setting to delete_all_posts_and_topics_allowed_groups. This one is a bit different from previous ones, as it's a boolean flag, and the default should be no group. Pay special attention to the migration during review. --- .../javascripts/discourse/app/models/topic.js | 6 ++-- config/locales/server.en.yml | 2 ++ config/site_settings.yml | 7 +++++ ...wed_groups_based_on_deprecated_settings.rb | 28 +++++++++++++++++++ lib/guardian/post_guardian.rb | 4 +-- lib/guardian/topic_guardian.rb | 6 ++-- lib/site_settings/deprecated_settings.rb | 1 + spec/lib/guardian/topic_guardian_spec.rb | 10 +++---- spec/lib/guardian_spec.rb | 8 +++--- 9 files changed, 56 insertions(+), 16 deletions(-) create mode 100644 db/migrate/20231213103248_fill_delete_all_posts_and_topics_allowed_groups_based_on_deprecated_settings.rb diff --git a/app/assets/javascripts/discourse/app/models/topic.js b/app/assets/javascripts/discourse/app/models/topic.js index 6e656ed2df8..f826ad576b6 100644 --- a/app/assets/javascripts/discourse/app/models/topic.js +++ b/app/assets/javascripts/discourse/app/models/topic.js @@ -487,8 +487,10 @@ const Topic = RestModel.extend({ (group) => group.name === this.category?.reviewable_by_group_name ) && !( - this.siteSettings.tl4_delete_posts_and_topics && - deleted_by.trust_level >= 4 + this.siteSettings.delete_all_posts_and_topics_allowed_groups && + deleted_by.isInAnyGroups( + this.siteSettings.delete_all_posts_and_topics_allowed_groups + ) )) ) { DiscourseURL.redirectTo("/"); diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 664f1ea7581..33fd40dd3af 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1955,6 +1955,7 @@ en: tl3_requires_likes_received: "The minimum number of likes that must be received in the last (tl3 time period) days to qualify for promotion to trust level 3." tl3_links_no_follow: "Do not remove rel=nofollow from links posted by trust level 3 users." tl4_delete_posts_and_topics: "Allow TL4 users to delete posts and topics created by other users. TL4 users will also be able to see deleted topics and posts." + delete_all_posts_and_topics_allowed_groups: "Groups allowed to delete posts and topics created by other users. These groups will also be able to see deleted topics and posts." edit_all_topic_groups: "Allow users in this group to edit other users' topic titles, tags, and categories" edit_all_post_groups: "Allow users in this group to edit other users' posts" @@ -2562,6 +2563,7 @@ en: create_topic_allowed_groups: "min_trust_to_create_topic" edit_post_allowed_groups: "min_trust_to_edit_post" flag_post_allowed_groups: "min_trust_to_flag_posts" + tl4_delete_posts_and_topics: "delete_all_posts_and_topics_allowed_groups" placeholder: discourse_connect_provider_secrets: diff --git a/config/site_settings.yml b/config/site_settings.yml index fd6a3746f1f..a45ad9687e9 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1803,6 +1803,13 @@ trust: tl4_delete_posts_and_topics: default: false client: true + hidden: true + delete_all_posts_and_topics_allowed_groups: + default: "" + type: group_list + allow_any: false + refresh: true + client: true edit_all_topic_groups: default: "13" type: group_list diff --git a/db/migrate/20231213103248_fill_delete_all_posts_and_topics_allowed_groups_based_on_deprecated_settings.rb b/db/migrate/20231213103248_fill_delete_all_posts_and_topics_allowed_groups_based_on_deprecated_settings.rb new file mode 100644 index 00000000000..787d7832b7d --- /dev/null +++ b/db/migrate/20231213103248_fill_delete_all_posts_and_topics_allowed_groups_based_on_deprecated_settings.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class FillDeleteAllPostsAndTopicsAllowedGroupsBasedOnDeprecatedSettings < ActiveRecord::Migration[ + 7.0 +] + def up + currently_enabled = + DB.query_single( + "SELECT value FROM site_settings WHERE name = 'tl4_delete_posts_and_topics' AND value = 't' LIMIT 1", + ).first + + if currently_enabled == "t" + # Matches Group::AUTO_GROUPS to the trust levels. + tl4 = "14" + + # Data_type 20 is group_list. + DB.exec( + "INSERT INTO site_settings(name, value, data_type, created_at, updated_at) + VALUES('delete_all_posts_and_topics_allowed_groups', :setting, '20', NOW(), NOW())", + setting: tl4, + ) + end + end + + def down + raise ActiveRecord::IrreversibleMigrationError + end +end diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index abc7242c440..bb6d0e16f98 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -207,7 +207,7 @@ module PostGuardian return true if is_staff? || is_category_group_moderator?(post.topic&.category) - return true if SiteSetting.tl4_delete_posts_and_topics && user.has_trust_level?(TrustLevel[4]) + return true if user.in_any_groups?(SiteSetting.delete_all_posts_and_topics_allowed_groups_map) # Can't delete posts in archived topics unless you are staff return false if post.topic&.archived? @@ -354,7 +354,7 @@ module PostGuardian def can_see_deleted_posts?(category = nil) is_staff? || is_category_group_moderator?(category) || - (SiteSetting.tl4_delete_posts_and_topics && @user.has_trust_level?(TrustLevel[4])) + @user.in_any_groups?(SiteSetting.delete_all_posts_and_topics_allowed_groups_map) end def can_view_raw_email?(post) diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index 2512789ebc5..f1eddd4e2e7 100644 --- a/lib/guardian/topic_guardian.rb +++ b/lib/guardian/topic_guardian.rb @@ -149,7 +149,7 @@ module TopicGuardian def can_recover_topic?(topic) if is_staff? || (topic&.category && is_category_group_moderator?(topic.category)) || - (SiteSetting.tl4_delete_posts_and_topics && user&.has_trust_level?(TrustLevel[4])) + user&.in_any_groups?(SiteSetting.delete_all_posts_and_topics_allowed_groups_map) !!(topic && topic.deleted_at) else topic && can_recover_post?(topic.ordered_posts.first) @@ -164,7 +164,7 @@ module TopicGuardian is_my_own?(topic) && topic.posts_count <= 1 && topic.created_at && topic.created_at > 24.hours.ago ) || is_category_group_moderator?(topic.category) || - (SiteSetting.tl4_delete_posts_and_topics && user.has_trust_level?(TrustLevel[4])) + user&.in_any_groups?(SiteSetting.delete_all_posts_and_topics_allowed_groups_map) ) && !topic.is_category_topic? && !Discourse.static_doc_topic_ids.include?(topic.id) end @@ -220,7 +220,7 @@ module TopicGuardian def can_see_deleted_topics?(category) is_staff? || is_category_group_moderator?(category) || - (SiteSetting.tl4_delete_posts_and_topics && user&.has_trust_level?(TrustLevel[4])) + user&.in_any_groups?(SiteSetting.delete_all_posts_and_topics_allowed_groups_map) end # Accepts an array of `Topic#id` and returns an array of `Topic#id` which the user can see. diff --git a/lib/site_settings/deprecated_settings.rb b/lib/site_settings/deprecated_settings.rb index 3abcad28db6..888f53f5c97 100644 --- a/lib/site_settings/deprecated_settings.rb +++ b/lib/site_settings/deprecated_settings.rb @@ -24,6 +24,7 @@ module SiteSettings::DeprecatedSettings ["min_trust_to_create_topic", "create_topic_allowed_groups", false, "3.3"], ["min_trust_to_edit_post", "edit_post_allowed_groups", false, "3.3"], ["min_trust_to_flag_posts", "flag_post_allowed_groups", false, "3.3"], + ["tl4_delete_posts_and_topics", "delete_all_posts_and_topics_allowed_groups", false, "3.3"], ] def setup_deprecated_methods diff --git a/spec/lib/guardian/topic_guardian_spec.rb b/spec/lib/guardian/topic_guardian_spec.rb index c463e8c9f8e..af2af985ebb 100644 --- a/spec/lib/guardian/topic_guardian_spec.rb +++ b/spec/lib/guardian/topic_guardian_spec.rb @@ -4,7 +4,7 @@ RSpec.describe TopicGuardian do fab!(:user) fab!(:admin) fab!(:tl3_user) { Fabricate(:trust_level_3) } - fab!(:tl4_user) { Fabricate(:trust_level_4) } + fab!(:tl4_user) { Fabricate(:trust_level_4, refresh_auto_groups: true) } fab!(:moderator) fab!(:category) fab!(:group) @@ -95,12 +95,12 @@ RSpec.describe TopicGuardian do it "returns true when tl4 can delete posts and topics" do expect(Guardian.new(tl4_user).can_see_deleted_topics?(topic.category)).to eq(false) - SiteSetting.tl4_delete_posts_and_topics = true + SiteSetting.delete_all_posts_and_topics_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] expect(Guardian.new(tl4_user).can_see_deleted_topics?(topic.category)).to eq(true) end it "returns false for anonymous user" do - SiteSetting.tl4_delete_posts_and_topics = true + SiteSetting.delete_all_posts_and_topics_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] expect(Guardian.new.can_see_deleted_topics?(topic.category)).to be_falsey end end @@ -122,12 +122,12 @@ RSpec.describe TopicGuardian do it "returns true when tl4 can delete posts and topics" do expect(Guardian.new(tl4_user).can_recover_topic?(Topic.with_deleted.last)).to eq(false) - SiteSetting.tl4_delete_posts_and_topics = true + SiteSetting.delete_all_posts_and_topics_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] expect(Guardian.new(tl4_user).can_recover_topic?(Topic.with_deleted.last)).to eq(true) end it "returns false for anonymous user" do - SiteSetting.tl4_delete_posts_and_topics = true + SiteSetting.delete_all_posts_and_topics_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] expect(Guardian.new.can_recover_topic?(Topic.with_deleted.last)).to eq(false) end end diff --git a/spec/lib/guardian_spec.rb b/spec/lib/guardian_spec.rb index ad284de4877..23c6fce0497 100644 --- a/spec/lib/guardian_spec.rb +++ b/spec/lib/guardian_spec.rb @@ -1439,7 +1439,7 @@ RSpec.describe Guardian do it "returns true when tl4 can delete posts and topics" do PostDestroyer.new(moderator, topic.first_post).destroy expect(Guardian.new(trust_level_4).can_recover_topic?(topic)).to be_falsey - SiteSetting.tl4_delete_posts_and_topics = true + SiteSetting.delete_all_posts_and_topics_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] expect(Guardian.new(trust_level_4).can_recover_topic?(topic.reload)).to be_truthy end @@ -2297,7 +2297,7 @@ RSpec.describe Guardian do it "returns true when tl4 can delete posts and topics" do expect(Guardian.new(trust_level_4).can_delete?(topic)).to be_falsey - SiteSetting.tl4_delete_posts_and_topics = true + SiteSetting.delete_all_posts_and_topics_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] expect(Guardian.new(trust_level_4).can_delete?(topic)).to be_truthy end @@ -2349,7 +2349,7 @@ RSpec.describe Guardian do it "returns true when tl4 can delete posts and topics" do expect(Guardian.new(trust_level_4).can_delete?(post)).to be_falsey - SiteSetting.tl4_delete_posts_and_topics = true + SiteSetting.delete_all_posts_and_topics_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] expect(Guardian.new(trust_level_4).can_delete?(post)).to be_truthy end @@ -2622,7 +2622,7 @@ RSpec.describe Guardian do it "returns true when tl4 can delete posts and topics" do expect(Guardian.new(trust_level_4).can_see_deleted_posts?(post)).to be_falsey - SiteSetting.tl4_delete_posts_and_topics = true + SiteSetting.delete_all_posts_and_topics_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] expect(Guardian.new(trust_level_4).can_see_deleted_posts?(post)).to be_truthy end end