DEV: Change accept_all_solutions_trust_level to group setting (#276)

This refactor makes for easier testing and makes things
more organised, the guardian extensions had no testing
whatsoever and I need some to make the TL -> group change.

Author: martin-brennan

app/lib/accepted_answer_cache.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+module DiscourseSolved
+  class AcceptedAnswerCache
+    @@allowed_accepted_cache = DistributedCache.new("allowed_accepted")
+
+    def self.reset_accepted_answer_cache
+      @@allowed_accepted_cache["allowed"] = begin
+        Set.new(
+          CategoryCustomField.where(
+            name: ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD,
+            value: "true",
+          ).pluck(:category_id),
+        )
+      end
+    end
+
+    def self.allowed
+      @@allowed_accepted_cache["allowed"]
+    end
+  end
+end

app/lib/guardian_extensions.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+module DiscourseSolved
+  module GuardianExtensions
+    def allow_accepted_answers?(category_id, tag_names = [])
+      return true if SiteSetting.allow_solved_on_all_topics
+
+      if SiteSetting.enable_solved_tags.present? && tag_names.present?
+        allowed_tags = SiteSetting.enable_solved_tags.split("|")
+        is_allowed = (tag_names & allowed_tags).present?
+
+        return true if is_allowed
+      end
+
+      return false if category_id.blank?
+      if !::DiscourseSolved::AcceptedAnswerCache.allowed
+        ::DiscourseSolved::AcceptedAnswerCache.reset_accepted_answer_cache
+      end
+      ::DiscourseSolved::AcceptedAnswerCache.allowed.include?(category_id)
+    end
+
+    def can_accept_answer?(topic, post)
+      return false if !authenticated?
+      return false if !topic || !post || post.whisper?
+      return false if !allow_accepted_answers?(topic.category_id, topic.tags.map(&:name))
+
+      return true if is_staff?
+      if current_user.in_any_groups?(SiteSetting.accept_all_solutions_allowed_groups_map)
+        return true
+      end
+      return true if can_perform_action_available_to_group_moderators?(topic)
+
+      topic.user_id == current_user.id && !topic.closed && SiteSetting.accept_solutions_topic_author
+    end
+  end
+end

config/locales/server.en.yml

@@ -5,6 +5,7 @@ en:
     solved_enabled: "Enable solved plugin, allow users to select solutions for topics"
     allow_solved_on_all_topics: "Allow users to select solutions on all topics (when unchecked, solutions can be enabled per category or tag)"
     accept_all_solutions_trust_level: "Minimum trust level required to accept solutions on any topic (even when not OP)"
+    accept_all_solutions_allowed_groups: "Groups that are allowed to accept solutions on any topic (even when not OP)"
     empty_box_on_unsolved: "Display an empty box next to unsolved topics"
     solved_quote_length: "Number of characters to quote when displaying the solution under the first post"
     solved_topics_auto_close_hours: "Auto close topic (n) hours after the last reply once the topic has been marked as solved. Set to 0 to disable auto closing."
@@ -16,6 +17,9 @@ en:
     enable_solved_tags: "Tags that will allow users to select solutions."
     prioritize_solved_topics_in_search: "Prioritize solved topics in search results."
 
+    keywords:
+      accept_all_solutions_allowed_groups: "accept_all_solutions_trust_level"
+
   reports:
     accepted_solutions:
       title: "Accepted solutions"

config/settings.yml

@@ -8,6 +8,15 @@ discourse_solved:
   accept_all_solutions_trust_level:
     default: 4
     client: true
+    enum: "TrustLevelSetting"
+    hidden: true
+  accept_all_solutions_allowed_groups:
+    default: "14" # auto group trust_level_4
+    type: group_list
+    client: false
+    allow_any: false
+    refresh: true
+    validator: "AtLeastOneGroupValidator"
   empty_box_on_unsolved:
     default: false
     client: true

db/post_migrate/20240116100023_fill_accept_all_solutions_allowed_groups_based_on_deprecated_setting.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+class FillAcceptAllSolutionsAllowedGroupsBasedOnDeprecatedSetting < ActiveRecord::Migration[7.0]
+  def up
+    old_setting_trust_level =
+      DB.query_single(
+        "SELECT value FROM site_settings WHERE name = 'accept_all_solutions_trust_level' LIMIT 1",
+      ).first
+
+    if old_setting_trust_level.present?
+      allowed_groups = "1#{old_setting_trust_level}"
+
+      DB.exec(
+        "INSERT INTO site_settings(name, value, data_type, created_at, updated_at)
+        VALUES('accept_all_solutions_allowed_groups', :setting, '20', NOW(), NOW())",
+        setting: allowed_groups,
+      )
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

plugin.rb

@@ -25,11 +25,15 @@
 
   %w[
     ../app/lib/first_accepted_post_solution_validator.rb
+    ../app/lib/accepted_answer_cache.rb
+    ../app/lib/guardian_extensions.rb
     ../app/serializers/concerns/topic_answer_mixin.rb
   ].each { |path| load File.expand_path(path, __FILE__) }
 
   skip_db = defined?(GlobalSetting.skip_db?) && GlobalSetting.skip_db?
 
+  reloadable_patch { |plugin| Guardian.prepend(DiscourseSolved::GuardianExtensions) }
+
   # we got to do a one time upgrade
   if !skip_db && defined?(UserAction::SOLVED)
     unless Discourse.redis.get("solved_already_upgraded")
@@ -501,52 +505,7 @@ class ::Category
     protected
 
     def reset_accepted_cache
-      ::Guardian.reset_accepted_answer_cache
-    end
-  end
-
-  class ::Guardian
-    @@allowed_accepted_cache = DistributedCache.new("allowed_accepted")
-
-    def self.reset_accepted_answer_cache
-      @@allowed_accepted_cache["allowed"] = begin
-        Set.new(
-          CategoryCustomField.where(
-            name: ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD,
-            value: "true",
-          ).pluck(:category_id),
-        )
-      end
-    end
-
-    def allow_accepted_answers?(category_id, tag_names = [])
-      return true if SiteSetting.allow_solved_on_all_topics
-
-      if SiteSetting.enable_solved_tags.present? && tag_names.present?
-        allowed_tags = SiteSetting.enable_solved_tags.split("|")
-        is_allowed = (tag_names & allowed_tags).present?
-
-        return true if is_allowed
-      end
-
-      return false if category_id.blank?
-      self.class.reset_accepted_answer_cache unless @@allowed_accepted_cache["allowed"]
-      @@allowed_accepted_cache["allowed"].include?(category_id)
-    end
-
-    def can_accept_answer?(topic, post)
-      return false if !authenticated?
-      return false if !topic || !post || post.whisper?
-      return false if !allow_accepted_answers?(topic.category_id, topic.tags.map(&:name))
-
-      return true if is_staff?
-      return true if current_user.trust_level >= SiteSetting.accept_all_solutions_trust_level
-
-      if respond_to? :can_perform_action_available_to_group_moderators?
-        return true if can_perform_action_available_to_group_moderators?(topic)
-      end
-
-      topic.user_id == current_user.id && !topic.closed && SiteSetting.accept_solutions_topic_author
+      ::DiscourseSolved::AcceptedAnswerCache.reset_accepted_answer_cache
     end
   end
 

spec/lib/guardian_extensions_spec.rb

@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+describe DiscourseSolved::GuardianExtensions do
+  fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
+  fab!(:other_user) { Fabricate(:user, refresh_auto_groups: true) }
+  fab!(:topic)
+  fab!(:post) { Fabricate(:post, topic: topic, user: other_user) }
+
+  let(:guardian) { user.guardian }
+
+  before { SiteSetting.allow_solved_on_all_topics = true }
+
+  describe ".can_accept_answer?" do
+    it "returns false for anon users" do
+      expect(Guardian.new.can_accept_answer?(topic, post)).to eq(false)
+    end
+
+    it "returns false if the topic is nil, the post is nil, or for whispers" do
+      expect(guardian.can_accept_answer?(nil, post)).to eq(false)
+      expect(guardian.can_accept_answer?(topic, nil)).to eq(false)
+
+      post.update!(post_type: Post.types[:whisper])
+      expect(guardian.can_accept_answer?(topic, post)).to eq(false)
+    end
+
+    it "returns false if accepted answers are not allowed" do
+      SiteSetting.allow_solved_on_all_topics = false
+      expect(guardian.can_accept_answer?(topic, post)).to eq(false)
+    end
+
+    it "returns true for admins" do
+      expect(
+        Guardian.new(Fabricate(:admin, refresh_auto_groups: true)).can_accept_answer?(topic, post),
+      ).to eq(true)
+    end
+
+    it "returns true if the user is in a group allowed to accept solutions" do
+      SiteSetting.accept_all_solutions_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
+      expect(guardian.can_accept_answer?(topic, post)).to eq(true)
+      SiteSetting.accept_all_solutions_allowed_groups = Group::AUTO_GROUPS[:trust_level_4]
+      expect(guardian.can_accept_answer?(topic, post)).to eq(false)
+    end
+
+    it "returns true if the user is a category group moderator for the topic" do
+      group = Fabricate(:group)
+      group.add(user)
+      category = Fabricate(:category, reviewable_by_group_id: group.id)
+      topic.update!(category: category)
+      SiteSetting.enable_category_group_moderation = true
+      expect(guardian.can_accept_answer?(topic, post)).to eq(true)
+    end
+
+    it "returns true if the user is the topic author for an open topic" do
+      SiteSetting.accept_solutions_topic_author = true
+      topic.update!(user: user)
+      expect(guardian.can_accept_answer?(topic, post)).to eq(true)
+    end
+  end
+end