diff --git a/.discourse-compatibility b/.discourse-compatibility index bf0ed96..cd1e7e8 100644 --- a/.discourse-compatibility +++ b/.discourse-compatibility @@ -1,3 +1,4 @@ +< 3.3.0.beta2-dev: a18ce6d712fafed286bcc99543dd173110c6dfb8 < 3.3.0.beta1-dev: 526a44644a7b3f0c2a3ba4fc16e72f364e9fce6d < 3.2.0.beta2-dev: 9fbf43e2f077e86f0a1ff769af6036d4e78bfff1 3.1.999: b5d487d6a5bfe2571d936eec5911d02a5f3fcc32 diff --git a/plugin.rb b/plugin.rb index 053564c..9ee53c1 100644 --- a/plugin.rb +++ b/plugin.rb @@ -9,11 +9,9 @@ enabled_site_setting :solved_enabled -if respond_to?(:register_svg_icon) - register_svg_icon "far fa-check-square" - register_svg_icon "check-square" - register_svg_icon "far fa-square" -end +register_svg_icon "far fa-check-square" +register_svg_icon "check-square" +register_svg_icon "far fa-square" register_asset "stylesheets/solutions.scss" register_asset "stylesheets/mobile/solutions.scss", :mobile @@ -59,24 +57,20 @@ def self.accept_answer!(post, acting_user, topic: nil) p2.custom_fields.delete(IS_ACCEPTED_ANSWER_CUSTOM_FIELD) p2.save! - if defined?(UserAction::SOLVED) - UserAction.where(action_type: UserAction::SOLVED, target_post_id: p2.id).destroy_all - end + UserAction.where(action_type: UserAction::SOLVED, target_post_id: p2.id).destroy_all end end post.custom_fields[IS_ACCEPTED_ANSWER_CUSTOM_FIELD] = "true" topic.custom_fields[ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD] = post.id - if defined?(UserAction::SOLVED) - UserAction.log_action!( - action_type: UserAction::SOLVED, - user_id: post.user_id, - acting_user_id: acting_user.id, - target_post_id: post.id, - target_topic_id: post.topic_id, - ) - end + UserAction.log_action!( + action_type: UserAction::SOLVED, + user_id: post.user_id, + acting_user_id: acting_user.id, + target_post_id: post.id, + target_topic_id: post.topic_id, + ) notification_data = { message: "solved.accepted_notification", @@ -156,9 +150,7 @@ def self.unaccept_answer!(post, topic: nil) post.save! # TODO remove_action! does not allow for this type of interface - if defined?(UserAction::SOLVED) - UserAction.where(action_type: UserAction::SOLVED, target_post_id: post.id).destroy_all - end + UserAction.where(action_type: UserAction::SOLVED, target_post_id: post.id).destroy_all # yank notification notification = @@ -191,12 +183,10 @@ def self.skip_db? ::TopicViewSerializer.prepend(DiscourseSolved::TopicViewSerializerExtension) ::Category.prepend(DiscourseSolved::CategoryExtension) ::PostSerializer.prepend(DiscourseSolved::PostSerializerExtension) - ::UserSummary.prepend(DiscourseSolved::UserSummaryExtension) if defined?(::UserAction::SOLVED) - if respond_to?(:register_topic_list_preload_user_ids) - ::Topic.attr_accessor(:accepted_answer_user_id) - ::TopicPostersSummary.alias_method(:old_user_ids, :user_ids) - ::TopicPostersSummary.prepend(DiscourseSolved::TopicPostersSummaryExtension) - end + ::UserSummary.prepend(DiscourseSolved::UserSummaryExtension) + ::Topic.attr_accessor(:accepted_answer_user_id) + ::TopicPostersSummary.alias_method(:old_user_ids, :user_ids) + ::TopicPostersSummary.prepend(DiscourseSolved::TopicPostersSummaryExtension) [ ::TopicListItemSerializer, ::SearchTopicListItemSerializer, @@ -207,7 +197,7 @@ def self.skip_db? end # we got to do a one time upgrade - if !::DiscourseSolved.skip_db? && defined?(UserAction::SOLVED) + if !::DiscourseSolved.skip_db? unless Discourse.redis.get("solved_already_upgraded") unless UserAction.where(action_type: UserAction::SOLVED).exists? Rails.logger.info("Upgrading storage for solved") @@ -272,65 +262,60 @@ def self.skip_db? DiscourseSolved::BeforeHeadClose.new(controller).html end - if Report.respond_to?(:add_report) - Report.add_report("accepted_solutions") do |report| - report.data = [] + Report.add_report("accepted_solutions") do |report| + report.data = [] - accepted_solutions = - TopicCustomField.where(name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD) + accepted_solutions = + TopicCustomField.where(name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD) - category_id, include_subcategories = report.add_category_filter - if category_id - if include_subcategories - accepted_solutions = - accepted_solutions.joins(:topic).where( - "topics.category_id IN (?)", - Category.subcategory_ids(category_id), - ) - else - accepted_solutions = - accepted_solutions.joins(:topic).where("topics.category_id = ?", category_id) - end + category_id, include_subcategories = report.add_category_filter + if category_id + if include_subcategories + accepted_solutions = + accepted_solutions.joins(:topic).where( + "topics.category_id IN (?)", + Category.subcategory_ids(category_id), + ) + else + accepted_solutions = + accepted_solutions.joins(:topic).where("topics.category_id = ?", category_id) end + end + accepted_solutions + .where("topic_custom_fields.created_at >= ?", report.start_date) + .where("topic_custom_fields.created_at <= ?", report.end_date) + .group("DATE(topic_custom_fields.created_at)") + .order("DATE(topic_custom_fields.created_at)") + .count + .each { |date, count| report.data << { x: date, y: count } } + report.total = accepted_solutions.count + report.prev30Days = accepted_solutions - .where("topic_custom_fields.created_at >= ?", report.start_date) - .where("topic_custom_fields.created_at <= ?", report.end_date) - .group("DATE(topic_custom_fields.created_at)") - .order("DATE(topic_custom_fields.created_at)") + .where("topic_custom_fields.created_at >= ?", report.start_date - 30.days) + .where("topic_custom_fields.created_at <= ?", report.start_date) .count - .each { |date, count| report.data << { x: date, y: count } } - report.total = accepted_solutions.count - report.prev30Days = - accepted_solutions - .where("topic_custom_fields.created_at >= ?", report.start_date - 30.days) - .where("topic_custom_fields.created_at <= ?", report.start_date) - .count - end end - if respond_to?(:register_modifier) - register_modifier(:search_rank_sort_priorities) do |priorities, search| - if SiteSetting.prioritize_solved_topics_in_search - condition = <<~SQL - EXISTS - ( - SELECT 1 FROM topic_custom_fields f - WHERE topics.id = f.topic_id - AND f.name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' - ) + register_modifier(:search_rank_sort_priorities) do |priorities, _search| + if SiteSetting.prioritize_solved_topics_in_search + condition = <<~SQL + EXISTS ( + SELECT 1 + FROM topic_custom_fields + WHERE topic_id = topics.id + AND name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' + AND value IS NOT NULL + ) SQL - priorities.push([condition, 1.1]) - else - priorities - end + priorities.push([condition, 1.1]) + else + priorities end end - if defined?(::UserAction::SOLVED) - add_to_serializer(:user_summary, :solved_count) { object.solved_count } - end + add_to_serializer(:user_summary, :solved_count) { object.solved_count } add_to_serializer(:post, :can_accept_answer) do scope.can_accept_answer?(topic, object) && object.post_number > 1 && !accepted_answer end @@ -344,107 +329,91 @@ def self.skip_db? topic&.custom_fields&.[](::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD).present? end - #TODO Remove when plugin is 1.0 - if Search.respond_to? :advanced_filter - Search.advanced_filter(/status:solved/) do |posts| - posts.where( - "topics.id IN ( - SELECT tc.topic_id - FROM topic_custom_fields tc - WHERE tc.name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' AND - tc.value IS NOT NULL - )", + solved_callback = ->(scope) do + sql = <<~SQL + topics.id IN ( + SELECT topic_id + FROM topic_custom_fields + WHERE name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' + AND value IS NOT NULL ) - end + SQL - Search.advanced_filter(/status:unsolved/) do |posts| - if SiteSetting.allow_solved_on_all_topics - posts.where( - "topics.id NOT IN ( - SELECT tc.topic_id - FROM topic_custom_fields tc - WHERE tc.name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' AND - tc.value IS NOT NULL - )", - ) - else - tag_ids = Tag.where(name: SiteSetting.enable_solved_tags.split("|")).pluck(:id) - - posts.where( - "topics.id NOT IN ( - SELECT tc.topic_id - FROM topic_custom_fields tc - WHERE tc.name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' AND - tc.value IS NOT NULL - ) AND (topics.id IN ( - SELECT top.id - FROM topics top - INNER JOIN category_custom_fields cc - ON top.category_id = cc.category_id - WHERE cc.name = '#{::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD}' AND - cc.value = 'true' - ) OR topics.id IN ( - SELECT top.id - FROM topics top - INNER JOIN topic_tags tt - ON top.id = tt.topic_id - WHERE tt.tag_id IN (?) - ))", - tag_ids, + scope.where(sql) + end + + unsolved_callback = ->(scope) do + scope = scope.where <<~SQL + topics.id NOT IN ( + SELECT topic_id + FROM topic_custom_fields + WHERE name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' + AND value IS NOT NULL + ) + SQL + + if !SiteSetting.allow_solved_on_all_topics + tag_ids = Tag.where(name: SiteSetting.enable_solved_tags.split("|")).pluck(:id) + + scope = scope.where <<~SQL, tag_ids + topics.id IN ( + SELECT t.id + FROM topics t + JOIN category_custom_fields cc + ON t.category_id = cc.category_id + AND cc.name = '#{::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD}' + AND cc.value = 'true' + ) + OR + topics.id IN ( + SELECT topic_id + FROM topic_tags + WHERE tag_id IN (?) ) - end + SQL end + + scope end - if Discourse.has_needed_version?(Discourse::VERSION::STRING, "1.8.0.beta6") - TopicQuery.add_custom_filter(:solved) do |results, topic_query| - if topic_query.options[:solved] == "yes" - results = - results.where( - "topics.id IN ( - SELECT tc.topic_id - FROM topic_custom_fields tc - WHERE tc.name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' AND - tc.value IS NOT NULL - )", - ) - elsif topic_query.options[:solved] == "no" - results = - results.where( - "topics.id NOT IN ( - SELECT tc.topic_id - FROM topic_custom_fields tc - WHERE tc.name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' AND - tc.value IS NOT NULL - )", - ) - end + register_custom_filter_by_status("solved", &solved_callback) + register_custom_filter_by_status("unsolved", &unsolved_callback) + + register_search_advanced_filter(/status:solved/, &solved_callback) + register_search_advanced_filter(/status:unsolved/, &unsolved_callback) + + TopicQuery.add_custom_filter(:solved) do |results, topic_query| + if topic_query.options[:solved] == "yes" + solved_callback.call(results) + elsif topic_query.options[:solved] == "no" + unsolved_callback.call(results) + else results end end - if TopicList.respond_to? :preloaded_custom_fields - TopicList.preloaded_custom_fields << ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD - end - if Site.respond_to? :preloaded_category_custom_fields - Site.preloaded_category_custom_fields << ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD - end - if Search.respond_to? :preloaded_topic_custom_fields - Search.preloaded_topic_custom_fields << ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD - end + TopicList.preloaded_custom_fields << ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD + Site.preloaded_category_custom_fields << ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD + Search.preloaded_topic_custom_fields << ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD + CategoryList.preloaded_topic_custom_fields << ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD - if CategoryList.respond_to?(:preloaded_topic_custom_fields) - CategoryList.preloaded_topic_custom_fields << ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD - end + on(:filter_auto_bump_topics) do |_category, filters| + filters.push( + ->(r) do + sql = <<~SQL + NOT EXISTS ( + SELECT 1 + FROM topic_custom_fields + WHERE topic_id = topics.id + AND name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' + AND value IS NOT NULL + ) + SQL - on(:filter_auto_bump_topics) { |_category, filters| filters.push(->(r) { r.where(<<~SQL) }) } - NOT EXISTS( - SELECT 1 FROM topic_custom_fields - WHERE topic_id = topics.id - AND name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' - AND value IS NOT NULL - ) - SQL + r.where(sql) + end, + ) + end on(:before_post_publish_changes) do |post_changes, topic_changes, options| category_id_changes = topic_changes.diff["category_id"].to_a @@ -521,7 +490,7 @@ def self.skip_db? AND di.period_type = :period_type AND di.solutions <> x.solutions " - add_directory_column("solutions", query: query) if respond_to?(:add_directory_column) + add_directory_column("solutions", query: query) add_to_class(:composer_messages_finder, :check_topic_is_solved) do return if !SiteSetting.solved_enabled || SiteSetting.disable_solved_education_message @@ -538,88 +507,82 @@ def self.skip_db? } end - if defined?(UserAction::SOLVED) - add_to_serializer(:user_card, :accepted_answers) do - UserAction.where(user_id: object.id).where(action_type: UserAction::SOLVED).count - end + add_to_serializer(:user_card, :accepted_answers) do + UserAction.where(user_id: object.id).where(action_type: UserAction::SOLVED).count end - if respond_to?(:register_topic_list_preload_user_ids) - register_topic_list_preload_user_ids do |topics, user_ids, topic_list| - answer_post_ids = - TopicCustomField - .select("value::INTEGER") - .where(name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD) - .where(topic_id: topics.map(&:id)) - answer_user_ids = Post.where(id: answer_post_ids).pluck(:topic_id, :user_id).to_h - topics.each { |topic| topic.accepted_answer_user_id = answer_user_ids[topic.id] } - user_ids.concat(answer_user_ids.values) - end + register_topic_list_preload_user_ids do |topics, user_ids, topic_list| + answer_post_ids = + TopicCustomField + .select("value::INTEGER") + .where(name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD) + .where(topic_id: topics.map(&:id)) + answer_user_ids = Post.where(id: answer_post_ids).pluck(:topic_id, :user_id).to_h + topics.each { |topic| topic.accepted_answer_user_id = answer_user_ids[topic.id] } + user_ids.concat(answer_user_ids.values) end if defined?(DiscourseAutomation) - if respond_to?(:add_triggerable_to_scriptable) - on(:accepted_solution) do |post| - # testing directly automation is prone to issues - # we prefer to abstract logic in service object and test this - next if Rails.env.test? - - name = "first_accepted_solution" - DiscourseAutomation::Automation - .where(trigger: name, enabled: true) - .find_each do |automation| - maximum_trust_level = automation.trigger_field("maximum_trust_level")&.dig("value") - if FirstAcceptedPostSolutionValidator.check(post, trust_level: maximum_trust_level) - automation.trigger!( - "kind" => name, - "accepted_post_id" => post.id, - "usernames" => [post.user.username], - "placeholders" => { - "post_url" => Discourse.base_url + post.url, - }, - ) - end + on(:accepted_solution) do |post| + # testing directly automation is prone to issues + # we prefer to abstract logic in service object and test this + next if Rails.env.test? + + name = "first_accepted_solution" + DiscourseAutomation::Automation + .where(trigger: name, enabled: true) + .find_each do |automation| + maximum_trust_level = automation.trigger_field("maximum_trust_level")&.dig("value") + if FirstAcceptedPostSolutionValidator.check(post, trust_level: maximum_trust_level) + automation.trigger!( + "kind" => name, + "accepted_post_id" => post.id, + "usernames" => [post.user.username], + "placeholders" => { + "post_url" => Discourse.base_url + post.url, + }, + ) end - end + end + end - add_triggerable_to_scriptable(:first_accepted_solution, :send_pms) - - DiscourseAutomation::Triggerable.add(:first_accepted_solution) do - placeholder :post_url - - field :maximum_trust_level, - component: :choices, - extra: { - content: [ - { - id: 1, - name: - "discourse_automation.triggerables.first_accepted_solution.max_trust_level.tl1", - }, - { - id: 2, - name: - "discourse_automation.triggerables.first_accepted_solution.max_trust_level.tl2", - }, - { - id: 3, - name: - "discourse_automation.triggerables.first_accepted_solution.max_trust_level.tl3", - }, - { - id: 4, - name: - "discourse_automation.triggerables.first_accepted_solution.max_trust_level.tl4", - }, - { - id: "any", - name: - "discourse_automation.triggerables.first_accepted_solution.max_trust_level.any", - }, - ], - }, - required: true - end + add_triggerable_to_scriptable(:first_accepted_solution, :send_pms) + + DiscourseAutomation::Triggerable.add(:first_accepted_solution) do + placeholder :post_url + + field :maximum_trust_level, + component: :choices, + extra: { + content: [ + { + id: 1, + name: + "discourse_automation.triggerables.first_accepted_solution.max_trust_level.tl1", + }, + { + id: 2, + name: + "discourse_automation.triggerables.first_accepted_solution.max_trust_level.tl2", + }, + { + id: 3, + name: + "discourse_automation.triggerables.first_accepted_solution.max_trust_level.tl3", + }, + { + id: 4, + name: + "discourse_automation.triggerables.first_accepted_solution.max_trust_level.tl4", + }, + { + id: "any", + name: + "discourse_automation.triggerables.first_accepted_solution.max_trust_level.any", + }, + ], + }, + required: true end end end diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index 43c20fd..898edfb 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -9,6 +9,68 @@ before { SiteSetting.allow_solved_on_all_topics = true } + describe "customer filters" do + before do + SiteSetting.allow_solved_on_all_topics = false + SiteSetting.enable_solved_tags = solvable_tag.name + end + + fab!(:solvable_category) do + category = Fabricate(:category) + + CategoryCustomField.create( + category_id: category.id, + name: ::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD, + value: "true", + ) + + category + end + + fab!(:solvable_tag) { Fabricate(:tag) } + + fab!(:solved_in_category) do + Fabricate( + :custom_topic, + category: solvable_category, + custom_topic_name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD, + value: "42", + ) + end + + fab!(:solved_in_tag) do + Fabricate( + :custom_topic, + tags: [solvable_tag], + custom_topic_name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD, + value: "42", + ) + end + + fab!(:unsolved_in_category) { Fabricate(:topic, category: solvable_category) } + fab!(:unsolved_in_tag) { Fabricate(:topic, tags: [solvable_tag]) } + + fab!(:unsolved_topic) { Fabricate(:topic) } + + it "can filter by solved status" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_from_query_string("status:solved") + .pluck(:id), + ).to contain_exactly(solved_in_category.id, solved_in_tag.id) + end + + it "can filter by unsolved status" do + expect( + TopicsFilter + .new(guardian: Guardian.new) + .filter_from_query_string("status:unsolved") + .pluck(:id), + ).to contain_exactly(unsolved_in_category.id, unsolved_in_tag.id) + end + end + describe "search" do before { SearchIndexer.enable }