DEV: Convert min_trust_to_edit_post to groups (#24840)

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_edit_post site setting to edit_post_allowed_groups.

The old implementation will co-exist for a short period while I update any references in plugins and themes.

app/models/concerns/limited_edit.rb

@@ -4,13 +4,22 @@ module LimitedEdit
   extend ActiveSupport::Concern
 
   def edit_time_limit_expired?(user)
-    return true if user.trust_level < SiteSetting.min_trust_to_edit_post
+    return true if !trusted_with_edits?(user)
     time_limit = user_time_limit(user)
     created_at && (time_limit > 0) && (created_at < time_limit.minutes.ago)
   end
 
   private
 
+  # TODO: This duplicates some functionality from PostGuardian. This should
+  #       probably be checked there instead, because this has nothing to do
+  #       with time limits.
+  #
+  def trusted_with_edits?(user)
+    user.trust_level >= SiteSetting.min_trust_to_edit_post ||
+      user.in_any_groups?(SiteSetting.edit_post_allowed_groups_map)
+  end
+
   def user_time_limit(user)
     if user.trust_level < 2
       SiteSetting.post_edit_time_limit.to_i

config/locales/server.en.yml

@@ -1965,6 +1965,7 @@ en:
     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."
+    edit_post_allowed_groups: "Groups that are allowed to edit posts."
 
     min_trust_to_allow_self_wiki: "The minimum trust level required to make user's own post wiki."
 
@@ -2557,6 +2558,7 @@ en:
       edit_wiki_post_allowed_groups: "minmin_trust_to_edit_wiki_post"
       uploaded_avatars_allowed_groups: "allow_uploaded_avatars"
       create_topic_allowed_groups: "min_trust_to_create_topic"
+      edit_post_allowed_groups: "min_trust_to_edit_post"
 
     placeholder:
       discourse_connect_provider_secrets:

config/site_settings.yml

@@ -1699,6 +1699,13 @@ trust:
   min_trust_to_edit_post:
     default: 0
     enum: "TrustLevelSetting"
+    hidden: true
+  edit_post_allowed_groups:
+    default: "10"
+    type: group_list
+    allow_any: false
+    refresh: true
+    validator: "AtLeastOneGroupValidator"
   min_trust_to_allow_self_wiki:
     default: 3
     enum: "TrustLevelSetting"

db/migrate/20231212044856_fill_edit_post_allowed_groups_based_on_deprecated_settings.rb

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

lib/guardian/post_guardian.rb

@@ -163,7 +163,7 @@ module PostGuardian
       return can_create_post?(post.topic)
     end
 
-    return false if @user.trust_level < SiteSetting.min_trust_to_edit_post
+    return false if !trusted_with_edits?
 
     if is_my_own?(post)
       return false if @user.silenced?
@@ -371,6 +371,11 @@ module PostGuardian
 
   private
 
+  def trusted_with_edits?
+    @user.trust_level >= SiteSetting.min_trust_to_edit_post ||
+      @user.in_any_groups?(SiteSetting.edit_post_allowed_groups_map)
+  end
+
   def can_create_post_in_topic?(topic)
     if !SiteSetting.enable_system_message_replies? && topic.try(:subtype) == "system_message"
       return false

lib/site_settings/deprecated_settings.rb

@@ -22,6 +22,7 @@ module SiteSettings::DeprecatedSettings
     ["min_trust_to_edit_wiki_post", "edit_wiki_post_allowed_groups", false, "3.3"],
     ["allow_uploaded_avatars", "uploaded_avatars_allowed_groups", false, "3.3"],
     ["min_trust_to_create_topic", "create_topic_allowed_groups", false, "3.3"],
+    ["min_trust_to_edit_post", "edit_post_allowed_groups", false, "3.3"],
   ]
 
   def setup_deprecated_methods

spec/lib/guardian_spec.rb

@@ -14,11 +14,11 @@ RSpec.describe Guardian do
   fab!(:automatic_group) { Fabricate(:group, automatic: true) }
   fab!(:plain_category) { Fabricate(:category) }
 
-  fab!(:trust_level_0) { Fabricate(:user, trust_level: 0) }
-  fab!(:trust_level_1) { Fabricate(:user, trust_level: 1) }
-  fab!(:trust_level_2) { Fabricate(:user, trust_level: 2) }
-  fab!(:trust_level_3) { Fabricate(:user, trust_level: 3) }
-  fab!(:trust_level_4) { Fabricate(:user, trust_level: 4) }
+  fab!(:trust_level_0) { Fabricate(:user, trust_level: 0, refresh_auto_groups: true) }
+  fab!(:trust_level_1) { Fabricate(:user, trust_level: 1, refresh_auto_groups: true) }
+  fab!(:trust_level_2) { Fabricate(:user, trust_level: 2, refresh_auto_groups: true) }
+  fab!(:trust_level_3) { Fabricate(:user, trust_level: 3, refresh_auto_groups: true) }
+  fab!(:trust_level_4) { Fabricate(:user, trust_level: 4, refresh_auto_groups: true) }
   fab!(:another_admin) { Fabricate(:admin) }
   fab!(:coding_horror)
 
@@ -1678,6 +1678,7 @@ RSpec.describe Guardian do
 
       it "returns false when trying to edit a topic with no trust" do
         SiteSetting.min_trust_to_edit_post = 2
+        SiteSetting.edit_post_allowed_groups = 12
         post.user.trust_level = 1
 
         expect(Guardian.new(topic.user).can_edit?(topic)).to be_falsey
@@ -1685,6 +1686,7 @@ RSpec.describe Guardian do
 
       it "returns false when trying to edit a post with no trust" do
         SiteSetting.min_trust_to_edit_post = 2
+        SiteSetting.edit_post_allowed_groups = 12
         post.user.trust_level = 1
 
         expect(Guardian.new(post.user).can_edit?(post)).to be_falsey
@@ -1731,7 +1733,6 @@ RSpec.describe Guardian do
           SiteSetting.shared_drafts_category = category.id
           SiteSetting.shared_drafts_allowed_groups = Group::AUTO_GROUPS[:trust_level_2]
           Fabricate(:shared_draft, topic: topic)
-          Group.refresh_automatic_groups!
         end
 
         it "returns true if a shared draft exists" do
@@ -1769,7 +1770,8 @@ RSpec.describe Guardian do
 
       describe "post edit time limits" do
         context "when post is older than post_edit_time_limit" do
-          let(:topic) { Fabricate(:topic) }
+          let(:user) { Fabricate(:user, refresh_auto_groups: true) }
+          let(:topic) { Fabricate(:topic, user: user) }
           let(:old_post) do
             Fabricate(:post, topic: topic, user: topic.user, created_at: 6.minutes.ago)
           end

spec/lib/validators/post_validator_spec.rb

@@ -318,7 +318,7 @@ RSpec.describe PostValidator do
   end
 
   describe "force_edit_last_validator" do
-    fab!(:user)
+    fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
     fab!(:other_user) { Fabricate(:user) }
     fab!(:topic)
 

spec/requests/posts_controller_spec.rb

@@ -82,9 +82,9 @@ end
 RSpec.describe PostsController do
   fab!(:admin)
   fab!(:moderator)
-  fab!(:user)
-  fab!(:user_trust_level_0) { Fabricate(:trust_level_0) }
-  fab!(:user_trust_level_1) { Fabricate(:trust_level_1) }
+  fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
+  fab!(:user_trust_level_0) { Fabricate(:trust_level_0, refresh_auto_groups: true) }
+  fab!(:user_trust_level_1) { Fabricate(:trust_level_1, refresh_auto_groups: true) }
   fab!(:category)
   fab!(:topic)
   fab!(:post_by_user) { Fabricate(:post, user: user) }
@@ -827,7 +827,6 @@ RSpec.describe PostsController do
     before do
       SiteSetting.min_first_post_typing_time = 0
       SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
-      Group.refresh_automatic_groups!
     end
 
     context "with api" do
@@ -1078,7 +1077,7 @@ RSpec.describe PostsController do
     end
 
     describe "when logged in" do
-      fab!(:user)
+      fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
 
       before { sign_in(user) }
 
@@ -1232,7 +1231,6 @@ RSpec.describe PostsController do
       end
 
       it "can send a message to a group" do
-        Group.refresh_automatic_groups!
         group = Group.create(name: "test_group", messageable_level: Group::ALIAS_LEVELS[:nobody])
         user1 = user
         group.add(user1)
@@ -1268,7 +1266,6 @@ RSpec.describe PostsController do
       end
 
       it "can send a message to a group with caps" do
-        Group.refresh_automatic_groups!
         group = Group.create(name: "Test_group", messageable_level: Group::ALIAS_LEVELS[:nobody])
         user1 = user
         group.add(user1)
@@ -1489,11 +1486,6 @@ RSpec.describe PostsController do
           xit "should add custom fields to topic that is permitted for a non-staff user via the deprecated `meta_data` param" do
             sign_in(user)
 
-            Discourse.expects(:deprecate).with(
-              "the :meta_data param is deprecated, use the :topic_custom_fields param instead",
-              anything,
-            )
-
             post "/posts.json",
                  params: {
                    raw: "this is the test content",
@@ -1586,7 +1578,6 @@ RSpec.describe PostsController do
         user_4 = Fabricate(:user, username: "Iyi_Iyi")
         user_4.update_attribute(:username, "İyi_İyi")
         user_4.update_attribute(:username_lower, "İyi_İyi".downcase)
-        Group.refresh_automatic_groups!
 
         post "/posts.json",
              params: {
@@ -1652,8 +1643,7 @@ RSpec.describe PostsController do
 
         it "it triggers flag_linked_posts_as_spam when the post creator returns spam" do
           SiteSetting.newuser_spam_host_threshold = 1
-          sign_in(Fabricate(:user, trust_level: 0))
-          Group.refresh_automatic_groups!
+          sign_in(Fabricate(:user, trust_level: 0, refresh_auto_groups: true))
 
           post "/posts.json",
                params: {
@@ -1844,9 +1834,7 @@ RSpec.describe PostsController do
     end
 
     describe "warnings" do
-      fab!(:user_2) { Fabricate(:user) }
-
-      before { Group.refresh_automatic_groups! }
+      fab!(:user_2) { Fabricate(:user, refresh_auto_groups: true) }
 
       context "as a staff user" do
         before { sign_in(admin) }
@@ -1948,7 +1936,7 @@ RSpec.describe PostsController do
       end
 
       context "with TL4 users" do
-        fab!(:trust_level_4)
+        fab!(:trust_level_4) { Fabricate(:trust_level_4, refresh_auto_groups: true) }
 
         before { sign_in(trust_level_4) }
 
@@ -2341,8 +2329,6 @@ RSpec.describe PostsController do
     include_examples "action requires login", :get, "/posts/system/deleted.json"
 
     describe "when logged in" do
-      before { Group.refresh_automatic_groups! }
-
       it "raises an error if the user doesn't have permission to see the deleted posts" do
         sign_in(user)
         get "/posts/system/deleted.json"

spec/requests/topics_controller_spec.rb

@@ -1630,7 +1630,7 @@ RSpec.describe TopicsController do
         end
       end
 
-      describe "with permission" do
+      context "with permission" do
         fab!(:post_hook) { Fabricate(:post_web_hook) }
         fab!(:topic_hook) { Fabricate(:topic_web_hook) }
 
@@ -1887,8 +1887,7 @@ RSpec.describe TopicsController do
             end.not_to change { topic.reload.first_post.revisions.count }
 
             expect(response.status).to eq(403)
-            user_2.update!(trust_level: 2)
-            Group.refresh_automatic_groups!
+            user_2.groups << Group.find_by(name: "trust_level_2")
 
             expect do put "/t/#{topic.id}/tags.json", params: { tags: [tag.name] } end.to change {
               topic.reload.first_post.revisions.count
@@ -3600,8 +3599,6 @@ RSpec.describe TopicsController do
       end
 
       context "with private message" do
-        before_all { Group.refresh_automatic_groups! }
-
         fab!(:group) do
           Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:everyone]).tap do |g|
             g.add(user_2)
@@ -4529,8 +4526,6 @@ RSpec.describe TopicsController do
       fab!(:topic) { Fabricate(:topic, user: user) }
       fab!(:post) { Fabricate(:post, user: user, topic: topic) }
 
-      before { Group.refresh_automatic_groups! }
-
       it "raises an error when the user doesn't have permission to convert topic" do
         sign_in(user)
         put "/t/#{topic.id}/convert-topic/private.json"
@@ -4557,8 +4552,6 @@ RSpec.describe TopicsController do
       fab!(:topic) { Fabricate(:private_message_topic, user: user) }
       fab!(:post) { Fabricate(:post, user: post_author1, topic: topic) }
 
-      before { Group.refresh_automatic_groups! }
-
       it "raises an error when the user doesn't have permission to convert topic" do
         sign_in(user)
         put "/t/#{topic.id}/convert-topic/public.json"
@@ -4961,10 +4954,7 @@ RSpec.describe TopicsController do
 
         fab!(:moderator_pm) { Fabricate(:private_message_topic, user: moderator) }
 
-        before do
-          SiteSetting.max_allowed_message_recipients = 2
-          Group.refresh_automatic_groups!
-        end
+        before { SiteSetting.max_allowed_message_recipients = 2 }
 
         it "doesn't allow normal users to invite" do
           post "/t/#{pm.id}/invite.json", params: { user: user_2.username }
@@ -5292,8 +5282,6 @@ RSpec.describe TopicsController do
   end
 
   describe "#private_message_reset_new" do
-    before_all { Group.refresh_automatic_groups! }
-
     fab!(:group) do
       Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:everyone]).tap { |g| g.add(user_2) }
     end
@@ -5412,8 +5400,6 @@ RSpec.describe TopicsController do
   end
 
   describe "#archive_message" do
-    before_all { Group.refresh_automatic_groups! }
-
     fab!(:group) do
       Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:everyone]).tap { |g| g.add(user) }
     end

spec/system/table_builder_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 describe "Table Builder", type: :system do
-  fab!(:user)
+  fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
   let(:composer) { PageObjects::Components::Composer.new }
   let(:insert_table_modal) { PageObjects::Modals::InsertTable.new }
   fab!(:topic) { Fabricate(:topic, user: user) }