From c6f879415bc7d0697a04344b2b689faafcbd4be3 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba Date: Tue, 23 Apr 2024 18:01:39 -0300 Subject: [PATCH 01/12] FEATURE: Prevents assign notification & change status on solved Relates to this [topic](https://meta.discourse.org/t/assign-plugin-for-informatica/256974/94) Add an event listener to `accepted_solution` event Add `assigns_reminder_assigned_topics_query` modifier to not notify if `prevent_assign_notification` setting is on. Add settings to prevent assign notification and change status on solved --- app/lib/plugin_initializer.rb | 31 +++++++++++++++++++++++++++++++ config/locales/server.en.yml | 3 +++ config/settings.yml | 7 +++++++ plugin.rb | 9 +++++++++ 4 files changed, 50 insertions(+) create mode 100644 app/lib/plugin_initializer.rb diff --git a/app/lib/plugin_initializer.rb b/app/lib/plugin_initializer.rb new file mode 100644 index 00000000..e118016d --- /dev/null +++ b/app/lib/plugin_initializer.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module DiscourseSolved + class PluginInitializer + attr_reader :plugin + + def initialize(plugin) + @plugin = plugin + end + + def apply_plugin_api + # this method should be overridden by subclasses + raise NotImplementedError + end + end + + class AssignsRemainderForTopicsQuery < PluginInitializer + def apply_plugin_api + plugin.register_modifier(:assigns_reminder_assigned_topics_query) do |query| + next query if !SiteSetting.prevent_assign_notification + query.where( + "topics.id NOT IN ( + SELECT topic_id + FROM topic_custom_fields + WHERE name = 'accepted_answer_post_id' + )", + ) + end + end + end +end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index faef8537..54e46306 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -11,6 +11,9 @@ en: 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." show_filter_by_solved_status: "Show a dropdown to filter a topic list by solved status." notify_on_staff_accept_solved: "Send notification to the topic creator when a post is marked as solution by a staff." + prevent_assign_notification: "Prevent notification from discourse-assign for solved topics." + assign_to_status_on_solved: "Enable assigning a status when a topic is solved. Requires discourse-assign plugin." + status_to_assign_on_solved: "The status to assign when a topic is solved." 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." diff --git a/config/settings.yml b/config/settings.yml index 21e9ba1e..6187de6b 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -32,6 +32,13 @@ discourse_solved: client: true notify_on_staff_accept_solved: default: false + prevent_assign_notification: + default: false + assign_to_status_on_solved: + default: false + status_to_assign_on_solved: + type: string + default: "Done" disable_solved_education_message: default: false accept_solutions_topic_author: diff --git a/plugin.rb b/plugin.rb index 053564c7..e16e2adc 100644 --- a/plugin.rb +++ b/plugin.rb @@ -622,4 +622,13 @@ def self.skip_db? end end end + + if defined?(DiscourseAssign) && SiteSetting.assign_to_status_on_solved + on(:accepted_solution) do |post| + Assigner.new(post.topic, post.acting_user).assign( + post.acting_user, + status: SiteSetting.status_to_assign_on_solved, + ) + end + end end From 0487888fb5287dbd34bf41a0bfeee22d4762df84 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba Date: Wed, 24 Apr 2024 11:06:32 -0300 Subject: [PATCH 02/12] DEV: Address review comments Update SiteSettings names. --- app/lib/plugin_initializer.rb | 2 +- config/locales/server.en.yml | 5 ++--- config/settings.yml | 8 +++----- plugin.rb | 4 ++-- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/app/lib/plugin_initializer.rb b/app/lib/plugin_initializer.rb index e118016d..1d49f4a3 100644 --- a/app/lib/plugin_initializer.rb +++ b/app/lib/plugin_initializer.rb @@ -17,7 +17,7 @@ def apply_plugin_api class AssignsRemainderForTopicsQuery < PluginInitializer def apply_plugin_api plugin.register_modifier(:assigns_reminder_assigned_topics_query) do |query| - next query if !SiteSetting.prevent_assign_notification + next query if !SiteSetting.ignore_solved_topics_in_assigned_reminder query.where( "topics.id NOT IN ( SELECT topic_id diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 54e46306..ff722377 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -11,9 +11,8 @@ en: 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." show_filter_by_solved_status: "Show a dropdown to filter a topic list by solved status." notify_on_staff_accept_solved: "Send notification to the topic creator when a post is marked as solution by a staff." - prevent_assign_notification: "Prevent notification from discourse-assign for solved topics." - assign_to_status_on_solved: "Enable assigning a status when a topic is solved. Requires discourse-assign plugin." - status_to_assign_on_solved: "The status to assign when a topic is solved." + ignore_solved_topics_in_assigned_reminder: "Prevent assigned reminder from including solved topics. only relevant when discourse-assign." + assignment_status_on_solve: "When a topic is solved update all assignments to this status" 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." diff --git a/config/settings.yml b/config/settings.yml index 6187de6b..5e67f0bb 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -32,13 +32,11 @@ discourse_solved: client: true notify_on_staff_accept_solved: default: false - prevent_assign_notification: + ignore_solved_topics_in_assigned_reminder: default: false - assign_to_status_on_solved: - default: false - status_to_assign_on_solved: + assignment_status_on_solve: type: string - default: "Done" + default: "" disable_solved_education_message: default: false accept_solutions_topic_author: diff --git a/plugin.rb b/plugin.rb index e16e2adc..d6b190d4 100644 --- a/plugin.rb +++ b/plugin.rb @@ -623,11 +623,11 @@ def self.skip_db? end end - if defined?(DiscourseAssign) && SiteSetting.assign_to_status_on_solved + if defined?(DiscourseAssign) && SiteSetting.assignment_status_on_solve.present? on(:accepted_solution) do |post| Assigner.new(post.topic, post.acting_user).assign( post.acting_user, - status: SiteSetting.status_to_assign_on_solved, + status: SiteSetting.assignment_status_on_solve, ) end end From e63af67145df937f8983f450d5f460478ced2428 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba Date: Wed, 24 Apr 2024 15:31:29 -0300 Subject: [PATCH 03/12] DEV(WIP): Add tests for integration with discourse-assign Add test for integration with discourse-assign plugin checks if the assignment status is moved to `Done` --- about.json | 7 ++++++ spec/integration/solved_spec.rb | 40 +++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 about.json diff --git a/about.json b/about.json new file mode 100644 index 00000000..0ab5dbcd --- /dev/null +++ b/about.json @@ -0,0 +1,7 @@ +{ + "tests": { + "requiredPlugins": [ + "https://github.com/discourse/discourse-assign" + ] + } +} diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index 43c20fd5..a622ca99 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -366,4 +366,44 @@ expect(response.status).to eq(200) end end + + context "with discourse-assign installed" , if: defined?(DiscourseAssign) do + let(:admin) { Fabricate(:admin) } + before do + SiteSetting.solved_enabled = true + SiteSetting.assign_enabled = true + SiteSetting.enable_assign_status = true + SiteSetting.assigns_public = true + SiteSetting.assignment_status_on_solve = "Done" + sign_in(admin) + end + + it "update all assignments to this status when a post is accepted" do + Jobs.run_immediately! + assigner = Assigner.new( + p1.topic, + admin + ) + result = assigner.assign(admin) + expect(result[:success]).to eq(true) + + expect(p1.topic.assignment.status).to eq("New") + + + # post "/solution/accept.json", params: { id: p1.id } + # expect(response.status).to eq(200) + + DiscourseSolved.accept_answer!(p1, admin) + + expect(p1.reload.custom_fields["is_accepted_answer"]).to eq("true") + + # still does not trigger + # DiscourseEvent.trigger(:accepted_solution, p1) + # It runs inside of a DistributedMutex.synchronize + + expect(p1.topic.assignment.reload.status).to eq("Done") + + end + + end end From c1d37ac7a90d648d474f732a3464e6166959f864 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba Date: Wed, 24 Apr 2024 15:35:56 -0300 Subject: [PATCH 04/12] DEV: lint solved_spec.rb --- spec/integration/solved_spec.rb | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index a622ca99..126e70bf 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -367,7 +367,7 @@ end end - context "with discourse-assign installed" , if: defined?(DiscourseAssign) do + context "with discourse-assign installed", if: defined?(DiscourseAssign) do let(:admin) { Fabricate(:admin) } before do SiteSetting.solved_enabled = true @@ -380,16 +380,12 @@ it "update all assignments to this status when a post is accepted" do Jobs.run_immediately! - assigner = Assigner.new( - p1.topic, - admin - ) + assigner = Assigner.new(p1.topic, admin) result = assigner.assign(admin) expect(result[:success]).to eq(true) expect(p1.topic.assignment.status).to eq("New") - # post "/solution/accept.json", params: { id: p1.id } # expect(response.status).to eq(200) @@ -402,8 +398,6 @@ # It runs inside of a DistributedMutex.synchronize expect(p1.topic.assignment.reload.status).to eq("Done") - end - end end From e102d82ec5888ecf7c590286528cea9d58895ae9 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba Date: Wed, 24 Apr 2024 16:30:27 -0300 Subject: [PATCH 05/12] DEV: Update test where it updates all assignments Change `on(:accepted_solution)` is defined Update test to use acting_user instead of admin --- plugin.rb | 3 ++- spec/integration/solved_spec.rb | 25 +++++++++++-------------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/plugin.rb b/plugin.rb index d6b190d4..a6ad3859 100644 --- a/plugin.rb +++ b/plugin.rb @@ -623,8 +623,9 @@ def self.skip_db? end end - if defined?(DiscourseAssign) && SiteSetting.assignment_status_on_solve.present? + if defined?(DiscourseAssign) on(:accepted_solution) do |post| + next if SiteSetting.assignment_status_on_solve.blank? Assigner.new(post.topic, post.acting_user).assign( post.acting_user, status: SiteSetting.assignment_status_on_solve, diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index 126e70bf..6b2db177 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -369,34 +369,31 @@ context "with discourse-assign installed", if: defined?(DiscourseAssign) do let(:admin) { Fabricate(:admin) } + fab!(:group) + before do SiteSetting.solved_enabled = true SiteSetting.assign_enabled = true SiteSetting.enable_assign_status = true + SiteSetting.assign_allowed_on_groups = "#{group.id}" SiteSetting.assigns_public = true SiteSetting.assignment_status_on_solve = "Done" - sign_in(admin) + + group.add(p1.acting_user) + group.add(user) + + sign_in(user) end it "update all assignments to this status when a post is accepted" do - Jobs.run_immediately! - assigner = Assigner.new(p1.topic, admin) - result = assigner.assign(admin) + assigner = Assigner.new(p1.topic, user) + result = assigner.assign(user) expect(result[:success]).to eq(true) expect(p1.topic.assignment.status).to eq("New") - - # post "/solution/accept.json", params: { id: p1.id } - # expect(response.status).to eq(200) - - DiscourseSolved.accept_answer!(p1, admin) + DiscourseSolved.accept_answer!(p1, user) expect(p1.reload.custom_fields["is_accepted_answer"]).to eq("true") - - # still does not trigger - # DiscourseEvent.trigger(:accepted_solution, p1) - # It runs inside of a DistributedMutex.synchronize - expect(p1.topic.assignment.reload.status).to eq("Done") end end From 3e0a596daac4c34bbfe99256e9d5c9e212af341f Mon Sep 17 00:00:00 2001 From: Gabriel Grubba Date: Thu, 25 Apr 2024 18:25:01 -0300 Subject: [PATCH 06/12] DEV: lint & add tests for assigns_reminder_assigned_topics_query Linted and added tests for `assigns_reminder_assigned_topics_query` modifier. --- app/lib/plugin_initializer.rb | 13 ++++++------- plugin.rb | 2 ++ spec/integration/solved_spec.rb | 26 ++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/app/lib/plugin_initializer.rb b/app/lib/plugin_initializer.rb index 1d49f4a3..f3aa8f85 100644 --- a/app/lib/plugin_initializer.rb +++ b/app/lib/plugin_initializer.rb @@ -14,16 +14,15 @@ def apply_plugin_api end end - class AssignsRemainderForTopicsQuery < PluginInitializer + class AssignsReminderForTopicsQuery < PluginInitializer def apply_plugin_api plugin.register_modifier(:assigns_reminder_assigned_topics_query) do |query| next query if !SiteSetting.ignore_solved_topics_in_assigned_reminder - query.where( - "topics.id NOT IN ( - SELECT topic_id - FROM topic_custom_fields - WHERE name = 'accepted_answer_post_id' - )", + query.where.not( + id: + TopicCustomField.where( + name: ::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD, + ).pluck(:topic_id), ) end end diff --git a/plugin.rb b/plugin.rb index a6ad3859..f16205d5 100644 --- a/plugin.rb +++ b/plugin.rb @@ -47,6 +47,8 @@ class Engine < ::Rails::Engine require_relative "app/lib/web_hook_extension" require_relative "app/serializers/concerns/topic_answer_mixin" + require_relative "app/lib/plugin_initializer" + DiscourseSolved::AssignsReminderForTopicsQuery.new(self).apply_plugin_api module ::DiscourseSolved def self.accept_answer!(post, acting_user, topic: nil) topic ||= post.topic diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index 6b2db177..bc830c72 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -397,4 +397,30 @@ expect(p1.topic.assignment.reload.status).to eq("Done") end end + + context "with using assigns_reminder_assigned_topics_query modifier" do + class DummyClass + def test + DiscoursePluginRegistry.apply_modifier(:assigns_reminder_assigned_topics_query, Topic.all) + end + end + + before { SiteSetting.ignore_solved_topics_in_assigned_reminder = true } + + it "should not include solved topics in the query" do + topic = Fabricate(:topic) + topic2 = Fabricate(:topic) + topic3 = Fabricate(:topic) + post = Fabricate(:post, topic: topic) + + DiscourseSolved.accept_answer!(post, Discourse.system_user) + + topics = DummyClass.new.test.to_a + + expect(topics).not_to include(topic) + + expect(topics).to include(topic2) + expect(topics).to include(topic3) + end + end end From 6603d8465b8c4763d67b44eeee6c2ad0f3b031d9 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba Date: Fri, 26 Apr 2024 11:04:39 -0300 Subject: [PATCH 07/12] DEV: Update tests based on review feedback change plugin_initializer location update spec with new tests to test integration with discourse-assign --- .../assigned_reminder_exclude_solved.rb} | 0 plugin.rb | 2 +- spec/integration/solved_spec.rb | 33 ++++++++----------- 3 files changed, 15 insertions(+), 20 deletions(-) rename app/lib/{plugin_initializer.rb => plugin_initializers/assigned_reminder_exclude_solved.rb} (100%) diff --git a/app/lib/plugin_initializer.rb b/app/lib/plugin_initializers/assigned_reminder_exclude_solved.rb similarity index 100% rename from app/lib/plugin_initializer.rb rename to app/lib/plugin_initializers/assigned_reminder_exclude_solved.rb diff --git a/plugin.rb b/plugin.rb index f16205d5..24b99bb4 100644 --- a/plugin.rb +++ b/plugin.rb @@ -47,7 +47,7 @@ class Engine < ::Rails::Engine require_relative "app/lib/web_hook_extension" require_relative "app/serializers/concerns/topic_answer_mixin" - require_relative "app/lib/plugin_initializer" + require_relative "app/lib/plugin_initializers/assigned_reminder_exclude_solved" DiscourseSolved::AssignsReminderForTopicsQuery.new(self).apply_plugin_api module ::DiscourseSolved def self.accept_answer!(post, acting_user, topic: nil) diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index bc830c72..b0addf4a 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -370,7 +370,6 @@ context "with discourse-assign installed", if: defined?(DiscourseAssign) do let(:admin) { Fabricate(:admin) } fab!(:group) - before do SiteSetting.solved_enabled = true SiteSetting.assign_enabled = true @@ -396,31 +395,27 @@ expect(p1.reload.custom_fields["is_accepted_answer"]).to eq("true") expect(p1.topic.assignment.reload.status).to eq("Done") end - end - context "with using assigns_reminder_assigned_topics_query modifier" do - class DummyClass - def test - DiscoursePluginRegistry.apply_modifier(:assigns_reminder_assigned_topics_query, Topic.all) - end - end + it "should not include solved topics in the query" do + other_topic = Fabricate(:topic, title: "Topic that should be there") + post = Fabricate(:post, topic: other_topic, user: user) - before { SiteSetting.ignore_solved_topics_in_assigned_reminder = true } + assigner1 = Assigner.new(p1.topic, user) + result1 = assigner1.assign(user) + expect(result1[:success]).to eq(true) - it "should not include solved topics in the query" do - topic = Fabricate(:topic) - topic2 = Fabricate(:topic) - topic3 = Fabricate(:topic) - post = Fabricate(:post, topic: topic) + assigner2 = Assigner.new(post.topic, user) + result2 = assigner2.assign(user) + expect(result2[:success]).to eq(true) - DiscourseSolved.accept_answer!(post, Discourse.system_user) + DiscourseSolved.accept_answer!(p1, Discourse.system_user) - topics = DummyClass.new.test.to_a + reminder = PendingAssignsReminder.new + reminder.remind(user) + topics = reminder.send(:assigned_topics, user, order: :asc) expect(topics).not_to include(topic) - - expect(topics).to include(topic2) - expect(topics).to include(topic3) + expect(topics).to include(other_topic) end end end From 9d0e6024949da470be39195f969904fae913fe0c Mon Sep 17 00:00:00 2001 From: Gabriel Grubba Date: Fri, 26 Apr 2024 11:14:48 -0300 Subject: [PATCH 08/12] DEV: Add describe to spec for discourse-assign integration tests --- spec/integration/solved_spec.rb | 51 +++++++++++++++++---------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index b0addf4a..4df282dc 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -383,39 +383,40 @@ sign_in(user) end + describe "when a post is accepted" do + it "update all assignments to this status when a post is accepted" do + assigner = Assigner.new(p1.topic, user) + result = assigner.assign(user) + expect(result[:success]).to eq(true) - it "update all assignments to this status when a post is accepted" do - assigner = Assigner.new(p1.topic, user) - result = assigner.assign(user) - expect(result[:success]).to eq(true) - - expect(p1.topic.assignment.status).to eq("New") - DiscourseSolved.accept_answer!(p1, user) + expect(p1.topic.assignment.status).to eq("New") + DiscourseSolved.accept_answer!(p1, user) - expect(p1.reload.custom_fields["is_accepted_answer"]).to eq("true") - expect(p1.topic.assignment.reload.status).to eq("Done") - end + expect(p1.reload.custom_fields["is_accepted_answer"]).to eq("true") + expect(p1.topic.assignment.reload.status).to eq("Done") + end - it "should not include solved topics in the query" do - other_topic = Fabricate(:topic, title: "Topic that should be there") - post = Fabricate(:post, topic: other_topic, user: user) + it "should not include solved topics in the query" do + other_topic = Fabricate(:topic, title: "Topic that should be there") + post = Fabricate(:post, topic: other_topic, user: user) - assigner1 = Assigner.new(p1.topic, user) - result1 = assigner1.assign(user) - expect(result1[:success]).to eq(true) + assigner1 = Assigner.new(p1.topic, user) + result1 = assigner1.assign(user) + expect(result1[:success]).to eq(true) - assigner2 = Assigner.new(post.topic, user) - result2 = assigner2.assign(user) - expect(result2[:success]).to eq(true) + assigner2 = Assigner.new(post.topic, user) + result2 = assigner2.assign(user) + expect(result2[:success]).to eq(true) - DiscourseSolved.accept_answer!(p1, Discourse.system_user) + DiscourseSolved.accept_answer!(p1, Discourse.system_user) - reminder = PendingAssignsReminder.new - reminder.remind(user) - topics = reminder.send(:assigned_topics, user, order: :asc) + reminder = PendingAssignsReminder.new + reminder.remind(user) + topics = reminder.send(:assigned_topics, user, order: :asc) - expect(topics).not_to include(topic) - expect(topics).to include(other_topic) + expect(topics).not_to include(topic) + expect(topics).to include(other_topic) + end end end end From f3988435bf931b7174ac82b8df6a139e335fecd4 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba Date: Fri, 26 Apr 2024 11:16:53 -0300 Subject: [PATCH 09/12] DEV: update describe name for discourse-assing spec integration --- spec/integration/solved_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index 4df282dc..1ee0c6c3 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -383,7 +383,7 @@ sign_in(user) end - describe "when a post is accepted" do + describe "updating assignment status on solve when assignment_status_on_solve is set" do it "update all assignments to this status when a post is accepted" do assigner = Assigner.new(p1.topic, user) result = assigner.assign(user) From ab17e398851431b616f36132e0e5151811c45d6e Mon Sep 17 00:00:00 2001 From: Gabriel Grubba Date: Fri, 26 Apr 2024 12:56:31 -0300 Subject: [PATCH 10/12] DEV: Add more tests to spec for discourse-assign integration --- spec/integration/solved_spec.rb | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index 1ee0c6c3..503df50b 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -377,7 +377,7 @@ SiteSetting.assign_allowed_on_groups = "#{group.id}" SiteSetting.assigns_public = true SiteSetting.assignment_status_on_solve = "Done" - + SiteSetting.ignore_solved_topics_in_assigned_reminder = false group.add(p1.acting_user) group.add(user) @@ -396,25 +396,40 @@ expect(p1.topic.assignment.reload.status).to eq("Done") end - it "should not include solved topics in the query" do + describe "assigned topic reminder" + it "excludes solved topics when ignore_solved_topics_in_assigned_reminder is false" do other_topic = Fabricate(:topic, title: "Topic that should be there") post = Fabricate(:post, topic: other_topic, user: user) - assigner1 = Assigner.new(p1.topic, user) - result1 = assigner1.assign(user) - expect(result1[:success]).to eq(true) + other_topic2 = Fabricate(:topic, title: "Topic that should be there2") + post2 = Fabricate(:post, topic: other_topic2, user: user) + + assigner = Assigner.new(p1.topic, user) + result = assigner.assign(user) + expect(result[:success]).to eq(true) assigner2 = Assigner.new(post.topic, user) result2 = assigner2.assign(user) expect(result2[:success]).to eq(true) - DiscourseSolved.accept_answer!(p1, Discourse.system_user) + assigner3 = Assigner.new(post2.topic, user) + result3 = assigner3.assign(user) + expect(result3[:success]).to eq(true) + reminder = PendingAssignsReminder.new - reminder.remind(user) topics = reminder.send(:assigned_topics, user, order: :asc) + expect(topics.to_a.length).to eq(3) + + DiscourseSolved.accept_answer!(post2, Discourse.system_user) + topics = reminder.send(:assigned_topics, user, order: :asc) + expect(topics.to_a.length).to eq(3) + expect(topics).to include(other_topic2) - expect(topics).not_to include(topic) + SiteSetting.ignore_solved_topics_in_assigned_reminder = true + topics = reminder.send(:assigned_topics, user, order: :asc) + expect(topics.to_a.length).to eq(2) + expect(topics).not_to include(other_topic2) expect(topics).to include(other_topic) end end From 1a3a77d80aa6f0e24d0231693700e110765bfdb2 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba Date: Fri, 26 Apr 2024 12:59:35 -0300 Subject: [PATCH 11/12] DEV: Lint solved_spec --- spec/integration/solved_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index 503df50b..ccb1d765 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -416,7 +416,6 @@ result3 = assigner3.assign(user) expect(result3[:success]).to eq(true) - reminder = PendingAssignsReminder.new topics = reminder.send(:assigned_topics, user, order: :asc) expect(topics.to_a.length).to eq(3) From 8584bf6053d60d2e5052b2332075ac67d2706eb7 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba Date: Fri, 26 Apr 2024 13:10:41 -0300 Subject: [PATCH 12/12] DEV: Lint and update spec to not have `p1` topic inside --- spec/integration/solved_spec.rb | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index ccb1d765..43116104 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -404,30 +404,21 @@ other_topic2 = Fabricate(:topic, title: "Topic that should be there2") post2 = Fabricate(:post, topic: other_topic2, user: user) - assigner = Assigner.new(p1.topic, user) - result = assigner.assign(user) - expect(result[:success]).to eq(true) - - assigner2 = Assigner.new(post.topic, user) - result2 = assigner2.assign(user) - expect(result2[:success]).to eq(true) - - assigner3 = Assigner.new(post2.topic, user) - result3 = assigner3.assign(user) - expect(result3[:success]).to eq(true) + Assigner.new(post.topic, user).assign(user) + Assigner.new(post2.topic, user).assign(user) reminder = PendingAssignsReminder.new topics = reminder.send(:assigned_topics, user, order: :asc) - expect(topics.to_a.length).to eq(3) + expect(topics.to_a.length).to eq(2) DiscourseSolved.accept_answer!(post2, Discourse.system_user) topics = reminder.send(:assigned_topics, user, order: :asc) - expect(topics.to_a.length).to eq(3) + expect(topics.to_a.length).to eq(2) expect(topics).to include(other_topic2) SiteSetting.ignore_solved_topics_in_assigned_reminder = true topics = reminder.send(:assigned_topics, user, order: :asc) - expect(topics.to_a.length).to eq(2) + expect(topics.to_a.length).to eq(1) expect(topics).not_to include(other_topic2) expect(topics).to include(other_topic) end