FEATURE: Separated 'trusted users can edit others' setting for trust level 3 & 4 (#21493)

config/locales/server.en.yml

@@ -1923,7 +1923,8 @@ 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."
-    trusted_users_can_edit_others: "Allow users with high trust levels to edit content from other users"
+    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"
 
     min_trust_to_create_topic: "The minimum trust level required to create a new topic."
     allow_flagging_staff: "If enabled, users can flag posts from staff accounts."

config/site_settings.yml

@@ -1698,8 +1698,12 @@ trust:
   tl4_delete_posts_and_topics:
     default: false
     client: true
-  trusted_users_can_edit_others:
-    default: true
+  edit_all_topic_groups:
+    default: "13"
+    type: group_list
+  edit_all_post_groups:
+    default: "14"
+    type: group_list
 
 security:
   detailed_404: false

db/migrate/20230509214723_separate_trusted_users_can_edit_others_site_setting.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+class SeparateTrustedUsersCanEditOthersSiteSetting < ActiveRecord::Migration[7.0]
+  def up
+    if select_value(
+         "SELECT 1 FROM site_settings WHERE name = 'trusted_users_can_edit_others' AND value = 'f'",
+       )
+      execute <<~SQL
+        INSERT INTO site_settings (name, data_type, value, created_at, updated_at) VALUES ('edit_all_topic_groups', 20, '', now(), now());
+        INSERT INTO site_settings (name, data_type, value, created_at, updated_at) VALUES ('edit_all_post_groups', 20, '', now(), now());
+      SQL
+    end
+  end
+
+  def down
+    execute <<~SQL
+      DELETE FROM site_settings WHERE name = 'edit_all_topic_groups';
+      DELETE FROM site_settings WHERE name = 'edit_all_post_groups';
+    SQL
+  end
+end

lib/guardian/post_guardian.rb

@@ -129,11 +129,7 @@ module PostGuardian
     # Must be staff to edit a locked post
     return false if post.locked? && !is_staff?
 
-    if (
-         is_staff? ||
-           (SiteSetting.trusted_users_can_edit_others? && @user.has_trust_level?(TrustLevel[4])) ||
-           is_category_group_moderator?(post.topic&.category)
-       )
+    if (is_staff? || is_in_edit_post_groups? || is_category_group_moderator?(post.topic&.category))
       return can_create_post?(post.topic)
     end
 
@@ -173,6 +169,11 @@ module PostGuardian
     false
   end
 
+  def is_in_edit_post_groups?
+    SiteSetting.edit_all_post_groups.present? &&
+      user.in_any_groups?(SiteSetting.edit_all_post_groups.to_s.split("|").map(&:to_i))
+  end
+
   def can_edit_hidden_post?(post)
     return false if post.nil?
     post.hidden_at.nil? ||

lib/guardian/topic_guardian.rb

@@ -119,18 +119,16 @@ module TopicGuardian
       return true
     end
 
-    # TL4 users can edit archived topics, but can not edit private messages
     if (
-         SiteSetting.trusted_users_can_edit_others? && topic.archived && !topic.private_message? &&
-           user.has_trust_level?(TrustLevel[4]) && can_create_post?(topic)
+         is_in_edit_post_groups? && topic.archived && !topic.private_message? &&
+           can_create_post?(topic)
        )
       return true
     end
 
-    # TL3 users can not edit archived topics and private messages
     if (
-         SiteSetting.trusted_users_can_edit_others? && !topic.archived && !topic.private_message? &&
-           user.has_trust_level?(TrustLevel[3]) && can_create_post?(topic)
+         is_in_edit_topic_groups? && !topic.archived && !topic.private_message? &&
+           can_create_post?(topic)
        )
       return true
     end
@@ -141,6 +139,11 @@ module TopicGuardian
       (!first_post&.hidden? || can_edit_hidden_post?(first_post))
   end
 
+  def is_in_edit_topic_groups?
+    SiteSetting.edit_all_topic_groups.present? &&
+      user.in_any_groups?(SiteSetting.edit_all_topic_groups.to_s.split("|").map(&:to_i))
+  end
+
   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]))

plugins/discourse-presence/plugin.rb

@@ -57,8 +57,8 @@ after_initialize do
         ]
       end
 
-      if !topic.private_message? && SiteSetting.trusted_users_can_edit_others?
-        config.allowed_group_ids << Group::AUTO_GROUPS[:trust_level_4]
+      if !topic.private_message? && SiteSetting.edit_all_post_groups.present?
+        config.allowed_group_ids += SiteSetting.edit_all_post_groups.split("|").map(&:to_i)
       end
 
       if SiteSetting.enable_category_group_moderation? &&

plugins/discourse-presence/spec/integration/presence_spec.rb

@@ -66,7 +66,7 @@ RSpec.describe "discourse-presence" do
     end
 
     it "handles category moderators for edit" do
-      SiteSetting.trusted_users_can_edit_others = false
+      SiteSetting.edit_all_post_groups = nil
       p = Fabricate(:post, topic: private_topic, user: private_topic.user)
 
       c = PresenceChannel.new("/discourse-presence/edit/#{p.id}")
@@ -79,6 +79,20 @@ RSpec.describe "discourse-presence" do
       expect(c.config.allowed_group_ids).to contain_exactly(Group::AUTO_GROUPS[:staff], group.id)
     end
 
+    it "adds edit_all_post_groups to the presence channel" do
+      p = Fabricate(:post, topic: public_topic, user: user)
+      g = Fabricate(:group)
+
+      SiteSetting.edit_all_post_groups = "#{Group::AUTO_GROUPS[:trust_level_1]}|#{g.id}"
+
+      c = PresenceChannel.new("/discourse-presence/edit/#{p.id}")
+      expect(c.config.allowed_group_ids).to contain_exactly(
+        Group::AUTO_GROUPS[:staff],
+        Group::AUTO_GROUPS[:trust_level_1],
+        g.id,
+      )
+    end
+
     it "handles permissions for a public topic" do
       c = PresenceChannel.new("/discourse-presence/reply/#{public_topic.id}")
       expect(c.config.public).to eq(false)
@@ -135,27 +149,13 @@ RSpec.describe "discourse-presence" do
       expect(c.config.allowed_user_ids).to contain_exactly(user.id)
     end
 
-    it "allows only author and staff when editing a public post with tl4 editing disabled" do
-      SiteSetting.trusted_users_can_edit_others = false
-
-      p = Fabricate(:post, topic: public_topic, user: user)
-      c = PresenceChannel.new("/discourse-presence/edit/#{p.id}")
-      expect(c.config.public).to eq(false)
-      expect(c.config.allowed_group_ids).to contain_exactly(Group::AUTO_GROUPS[:staff])
-      expect(c.config.allowed_user_ids).to contain_exactly(user.id)
-    end
-
     it "follows the wiki edit trust level site setting" do
       p = Fabricate(:post, topic: public_topic, user: user, wiki: true)
       SiteSetting.min_trust_to_edit_wiki_post = TrustLevel.levels[:basic]
-      SiteSetting.trusted_users_can_edit_others = false
 
       c = PresenceChannel.new("/discourse-presence/edit/#{p.id}")
       expect(c.config.public).to eq(false)
-      expect(c.config.allowed_group_ids).to contain_exactly(
-        Group::AUTO_GROUPS[:staff],
-        Group::AUTO_GROUPS[:trust_level_1],
-      )
+      expect(c.config.allowed_group_ids).to include(Group::AUTO_GROUPS[:trust_level_1])
       expect(c.config.allowed_user_ids).to contain_exactly(user.id)
     end
 

spec/lib/guardian/post_guardian_spec.rb

@@ -56,4 +56,24 @@ RSpec.describe PostGuardian do
       end
     end
   end
+
+  describe "#is_in_edit_post_groups?" do
+    it "returns true if the user is in edit_all_post_groups" do
+      SiteSetting.edit_all_post_groups = group.id.to_s
+
+      expect(Guardian.new(user).is_in_edit_post_groups?).to eq(true)
+    end
+
+    it "returns false if the user is not in edit_all_post_groups" do
+      SiteSetting.edit_all_post_groups = Group::AUTO_GROUPS[:trust_level_4]
+
+      expect(Guardian.new(Fabricate(:trust_level_3)).is_in_edit_post_groups?).to eq(false)
+    end
+
+    it "returns false if the edit_all_post_groups is empty" do
+      SiteSetting.edit_all_post_groups = nil
+
+      expect(Guardian.new(user).is_in_edit_post_groups?).to eq(false)
+    end
+  end
 end

spec/lib/guardian/topic_guardian_spec.rb

@@ -173,6 +173,27 @@ RSpec.describe TopicGuardian do
     end
   end
 
+  describe "#is_in_edit_topic_groups?" do
+    it "returns true if the user is in edit_all_topic_groups" do
+      group.add(user)
+      SiteSetting.edit_all_topic_groups = group.id.to_s
+
+      expect(Guardian.new(user).is_in_edit_topic_groups?).to eq(true)
+    end
+
+    it "returns false if the user is not in edit_all_topic_groups" do
+      SiteSetting.edit_all_topic_groups = Group::AUTO_GROUPS[:trust_level_4]
+
+      expect(Guardian.new(tl3_user).is_in_edit_topic_groups?).to eq(false)
+    end
+
+    it "returns false if the edit_all_topic_groups is empty" do
+      SiteSetting.edit_all_topic_groups = nil
+
+      expect(Guardian.new(user).is_in_edit_topic_groups?).to eq(false)
+    end
+  end
+
   describe "#can_review_topic?" do
     it "returns false for TL4 users" do
       topic = Fabricate(:topic)

spec/lib/guardian_spec.rb

@@ -1626,11 +1626,6 @@ RSpec.describe Guardian do
         expect(Guardian.new(trust_level_4).can_edit?(post)).to be_truthy
       end
 
-      it "returns false as a TL4 user if trusted_users_can_edit_others is false" do
-        SiteSetting.trusted_users_can_edit_others = false
-        expect(Guardian.new(trust_level_4).can_edit?(post)).to eq(false)
-      end
-
       it "returns false when trying to edit a topic with no trust" do
         SiteSetting.min_trust_to_edit_post = 2
         post.user.trust_level = 1
@@ -1876,11 +1871,6 @@ RSpec.describe Guardian do
           expect(Guardian.new(trust_level_3).can_edit?(topic)).to eq(true)
         end
 
-        it "is false at TL3, if `trusted_users_can_edit_others` is false" do
-          SiteSetting.trusted_users_can_edit_others = false
-          expect(Guardian.new(trust_level_3).can_edit?(topic)).to eq(false)
-        end
-
         it "returns false when the category is read only" do
           topic.category.set_permissions(everyone: :readonly)
           topic.category.save
@@ -1930,9 +1920,14 @@ RSpec.describe Guardian do
           expect(Guardian.new(trust_level_4).can_edit?(archived_topic)).to be_truthy
         end
 
-        it "is false at TL4, if `trusted_users_can_edit_others` is false" do
-          SiteSetting.trusted_users_can_edit_others = false
-          expect(Guardian.new(trust_level_4).can_edit?(archived_topic)).to eq(false)
+        it "returns true if the user is in edit_all_post_groups" do
+          SiteSetting.edit_all_post_groups = "14"
+          expect(Guardian.new(trust_level_4).can_edit?(archived_topic)).to eq(true)
+        end
+
+        it "returns false if the user is not in edit_all_post_groups" do
+          SiteSetting.edit_all_post_groups = "14"
+          expect(Guardian.new(trust_level_3).can_edit?(archived_topic)).to eq(false)
         end
 
         it "returns false at trust level 3" do

spec/requests/topics_controller_spec.rb

@@ -1819,8 +1819,9 @@ RSpec.describe TopicsController do
         end
 
         describe "when first post is locked" do
-          it "blocks non-staff from editing even if 'trusted_users_can_edit_others' is true" do
-            SiteSetting.trusted_users_can_edit_others = true
+          it "blocks user from editing even if they are in 'edit_all_topic_groups' and 'edit_all_post_groups'" do
+            SiteSetting.edit_all_topic_groups = Group::AUTO_GROUPS[:trust_level_3]
+            SiteSetting.edit_all_post_groups = Group::AUTO_GROUPS[:trust_level_4]
             user.update!(trust_level: 3)
             topic.first_post.update!(locked_by_id: admin.id)