From 66999ee3fbf89731491d7fbc12933b5b742519e9 Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Fri, 17 Sep 2021 11:12:47 -0300 Subject: [PATCH] FEATURE: Enable solved for topics with specific tags. (#164) This PR adds a site setting called `enable_solved_tags`. Solved will be enabled for topics containing these tags, just like we do for specific categories. --- .../concerns/topic_answer_mixin.rb | 2 +- config/locales/server.en.yml | 2 + config/settings.yml | 3 + plugin.rb | 30 +++++--- spec/components/post_revisor_spec.rb | 38 +++++++++- spec/requests/topics_controller_spec.rb | 72 ++++++++++++++----- 6 files changed, 117 insertions(+), 30 deletions(-) diff --git a/app/serializers/concerns/topic_answer_mixin.rb b/app/serializers/concerns/topic_answer_mixin.rb index 942704f..8c8fcc4 100644 --- a/app/serializers/concerns/topic_answer_mixin.rb +++ b/app/serializers/concerns/topic_answer_mixin.rb @@ -16,7 +16,7 @@ module TopicAnswerMixin def can_have_answer return true if SiteSetting.allow_solved_on_all_topics return false if object.closed || object.archived - scope.allow_accepted_answers_on_category?(object.category_id) + scope.allow_accepted_answer?(object.category_id, object.tags.map(&:name)) end def include_can_have_answer? diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 2eb4fe6..a00d2c0 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -13,6 +13,8 @@ en: disable_solved_education_message: "Disable education message for solved topics." accept_solutions_topic_author: "Allow the topic author to accept a solution." solved_add_schema_markup: "Add QAPage schema markup to HTML." + enable_solved_tags: "Allow users to select solutions on all topics (when unchecked, solutions can be enabled per category or tag)" + reports: accepted_solutions: title: "Accepted solutions" diff --git a/config/settings.yml b/config/settings.yml index 335cdf5..f939b35 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -34,4 +34,7 @@ plugins: - "never" - "always" - "answered only" + enable_solved_tags: + type: tag_list + default: "" diff --git a/plugin.rb b/plugin.rb index dd9fa14..ab2269d 100644 --- a/plugin.rb +++ b/plugin.rb @@ -282,7 +282,13 @@ SQL # a bit more prominent + cut down on pointless work return "" if SiteSetting.solved_add_schema_markup == "never" - return "" if !controller.guardian.allow_accepted_answers_on_category?(topic.category_id) + + allowed = controller + .guardian + .allow_accepted_answers?( + topic.category_id, topic.tags.pluck(:name) + ) + return "" if !allowed first_post = topic_view.posts&.first return "" if first_post&.post_number != 1 @@ -466,9 +472,17 @@ SQL end end - def allow_accepted_answers_on_category?(category_id) + 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 @@ -476,7 +490,7 @@ SQL def can_accept_answer?(topic, post) return false if !authenticated? return false if !topic || !post || post.whisper? - return false if !allow_accepted_answers_on_category?(topic.category_id) + 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 @@ -606,13 +620,13 @@ SQL end on(:before_post_publish_changes) do |post_changes, topic_changes, options| - category_id_changes = topic_changes.diff["category_id"] - next if category_id_changes.blank? + category_id_changes = topic_changes.diff['category_id'].to_a + tag_changes = topic_changes.diff['tags'].to_a - old_category_allows = Guardian.new.allow_accepted_answers_on_category?(category_id_changes[0]) - new_category_allows = Guardian.new.allow_accepted_answers_on_category?(category_id_changes[1]) + old_allowed = Guardian.new.allow_accepted_answers?(category_id_changes[0], tag_changes[0]) + new_allowed = Guardian.new.allow_accepted_answers?(category_id_changes[1], tag_changes[1]) - options[:refresh_stream] = true if old_category_allows != new_category_allows + options[:refresh_stream] = true if old_allowed != new_allowed end on(:after_populate_dev_records) do |records, type| diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb index 33ce5b3..d60a07c 100644 --- a/spec/components/post_revisor_spec.rb +++ b/spec/components/post_revisor_spec.rb @@ -5,6 +5,7 @@ require 'post_revisor' describe PostRevisor do fab!(:category) { Fabricate(:category_with_definition) } + fab!(:admin) { Fabricate(:admin) } fab!(:category_solved) do category = Fabricate(:category_with_definition) @@ -17,15 +18,48 @@ describe PostRevisor do post = Fabricate(:post, topic: topic) messages = MessageBus.track_publish("/topic/#{topic.id}") do - described_class.new(post).revise!(Fabricate(:admin), { category_id: category.id }) + described_class.new(post).revise!(admin, { category_id: category.id }) end expect(messages.first.data[:refresh_stream]).to eq(nil) messages = MessageBus.track_publish("/topic/#{topic.id}") do - described_class.new(post).revise!(Fabricate(:admin), { category_id: category_solved.id }) + described_class.new(post).revise!(admin, { category_id: category_solved.id }) end expect(messages.first.data[:refresh_stream]).to eq(true) end + + context 'Allowing solved via tags' do + before do + SiteSetting.solved_enabled = true + SiteSetting.tagging_enabled = true + end + + fab!(:tag1) { Fabricate(:tag) } + fab!(:tag2) { Fabricate(:tag) } + + fab!(:topic) { Fabricate(:topic) } + let(:post) { Fabricate(:post, topic: topic) } + + it 'sets the refresh option after adding an allowed tag' do + SiteSetting.enable_solved_tags = tag1.name + + messages = MessageBus.track_publish("/topic/#{topic.id}") do + described_class.new(post).revise!(admin, tags: [tag1.name]) + end + + expect(messages.first.data[:refresh_stream]).to eq(true) + end + + it 'sets the refresh option if the added tag matches any of the allowed tags' do + SiteSetting.enable_solved_tags = [tag1, tag2].map(&:name).join('|') + + messages = MessageBus.track_publish("/topic/#{topic.id}") do + described_class.new(post).revise!(admin, tags: [tag2.name]) + end + + expect(messages.first.data[:refresh_stream]).to eq(true) + end + end end diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 67e85eb..b101014 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -31,33 +31,67 @@ RSpec.describe TopicsController do } end - before do - SiteSetting.allow_solved_on_all_topics = true + context 'solved enabled on every topic' do + before do + SiteSetting.allow_solved_on_all_topics = true + end + + it 'should include correct schema information in header' do + get "/t/#{topic.slug}/#{topic.id}" + + expect(response.body).to include(schema_json(0)) + + p2.custom_fields["is_accepted_answer"] = true + p2.save_custom_fields + topic.custom_fields["accepted_answer_post_id"] = p2.id + topic.save_custom_fields + + get "/t/#{topic.slug}/#{topic.id}" + + expect(response.body).to include(schema_json(1)) + end + + it 'should include quoted content in schema information' do + post = topic.first_post + post.raw = "[quote]This is a quoted text.[/quote]" + post.save! + post.rebake! + + get "/t/#{topic.slug}/#{topic.id}" + + expect(response.body).to include('"text":"This is a quoted text."') + end end - it 'should include correct schema information in header' do - get "/t/#{topic.slug}/#{topic.id}" + context 'solved enabled for topics with specific tags' do + let(:tag) { Fabricate(:tag) } - expect(response.body).to include(schema_json(0)) + before { SiteSetting.enable_solved_tags = tag.name } - p2.custom_fields["is_accepted_answer"] = true - p2.save_custom_fields - topic.custom_fields["accepted_answer_post_id"] = p2.id - topic.save_custom_fields + it 'includes the correct schema information' do + DiscourseTagging.add_or_create_tags_by_name(topic, [tag.name]) + p2.custom_fields["is_accepted_answer"] = true + p2.save_custom_fields + topic.custom_fields["accepted_answer_post_id"] = p2.id + topic.save_custom_fields - get "/t/#{topic.slug}/#{topic.id}" + get "/t/#{topic.slug}/#{topic.id}" - expect(response.body).to include(schema_json(1)) - end + expect(response.body).to include(schema_json(1)) + end - it 'should include quoted content in schema information' do - post = topic.first_post - post.raw = "[quote]This is a quoted text.[/quote]" - post.save! - post.rebake! + it "doesn't include solved schema information when the topic has a different tag" do + another_tag = Fabricate(:tag) - get "/t/#{topic.slug}/#{topic.id}" + DiscourseTagging.add_or_create_tags_by_name(topic, [another_tag.name]) + p2.custom_fields["is_accepted_answer"] = true + p2.save_custom_fields + topic.custom_fields["accepted_answer_post_id"] = p2.id + topic.save_custom_fields - expect(response.body).to include('"text":"This is a quoted text."') + get "/t/#{topic.slug}/#{topic.id}" + + expect(response.body).not_to include(schema_json(1)) + end end end