From 444dac8a9a8f30c45a64f088569a99c83663c519 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Tue, 23 Jan 2024 11:45:32 +1000 Subject: [PATCH] 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. --- app/lib/accepted_answer_cache.rb | 22 +++++++ app/lib/guardian_extensions.rb | 36 +++++++++++ config/locales/server.en.yml | 4 ++ config/settings.yml | 9 +++ ...owed_groups_based_on_deprecated_setting.rb | 24 ++++++++ plugin.rb | 51 ++-------------- spec/lib/guardian_extensions_spec.rb | 61 +++++++++++++++++++ 7 files changed, 161 insertions(+), 46 deletions(-) create mode 100644 app/lib/accepted_answer_cache.rb create mode 100644 app/lib/guardian_extensions.rb create mode 100644 db/post_migrate/20240116100023_fill_accept_all_solutions_allowed_groups_based_on_deprecated_setting.rb create mode 100644 spec/lib/guardian_extensions_spec.rb diff --git a/app/lib/accepted_answer_cache.rb b/app/lib/accepted_answer_cache.rb new file mode 100644 index 0000000..a25b2c2 --- /dev/null +++ b/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 diff --git a/app/lib/guardian_extensions.rb b/app/lib/guardian_extensions.rb new file mode 100644 index 0000000..2d6193b --- /dev/null +++ b/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 diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 823c4e2..faef853 100644 --- a/config/locales/server.en.yml +++ b/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" diff --git a/config/settings.yml b/config/settings.yml index 0b93c3e..21e9ba1 100644 --- a/config/settings.yml +++ b/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 diff --git a/db/post_migrate/20240116100023_fill_accept_all_solutions_allowed_groups_based_on_deprecated_setting.rb b/db/post_migrate/20240116100023_fill_accept_all_solutions_allowed_groups_based_on_deprecated_setting.rb new file mode 100644 index 0000000..b75ff8f --- /dev/null +++ b/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 diff --git a/plugin.rb b/plugin.rb index e0d03d7..87fa045 100644 --- a/plugin.rb +++ b/plugin.rb @@ -25,11 +25,15 @@ after_initialize do %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 @@ SQL 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 diff --git a/spec/lib/guardian_extensions_spec.rb b/spec/lib/guardian_extensions_spec.rb new file mode 100644 index 0000000..a956f79 --- /dev/null +++ b/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