From 9db72efb381ee340ed5ef62486b2a372468b2804 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba <70247653+Grubba27@users.noreply.github.com> Date: Wed, 8 May 2024 20:17:45 +0200 Subject: [PATCH] FEATURE: change status on unsolve & fix assign changes (#289) * FEATURE: change status on unsolve & fix assign changes When a topic is unsolved, it should have an option, defined in the settings, to change its status to that state. Fix assign changes when a topic was solved, previously it was changing the assignee. * DEV: Change names in tests and remove comments * DEV: Update change status on solve implementation Update tests to verify that the change status on solve feature is working as expected. Change the implementation to loop throught the topic assignments and update the status. * DEV: address review feedback --- config/locales/server.en.yml | 1 + config/settings.yml | 3 +++ plugin.rb | 43 +++++++++++++++++++++---------- spec/integration/solved_spec.rb | 45 +++++++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 14 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index ff72237..cd0fe0c 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -13,6 +13,7 @@ en: notify_on_staff_accept_solved: "Send notification to the topic creator when a post is marked as solution by a staff." 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" + assignment_status_on_unsolve: "When a topic is unsolved 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 5e67f0b..674398c 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -37,6 +37,9 @@ discourse_solved: assignment_status_on_solve: type: string default: "" + assignment_status_on_unsolve: + type: string + default: "" disable_solved_education_message: default: false accept_solutions_topic_author: diff --git a/plugin.rb b/plugin.rb index 701c303..d0c7de3 100644 --- a/plugin.rb +++ b/plugin.rb @@ -303,7 +303,7 @@ after_initialize do if SiteSetting.prioritize_solved_topics_in_search condition = <<~SQL EXISTS ( - SELECT 1 + SELECT 1 FROM topic_custom_fields WHERE topic_id = topics.id AND name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' @@ -336,7 +336,7 @@ after_initialize do topics.id IN ( SELECT topic_id FROM topic_custom_fields - WHERE name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' + WHERE name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' AND value IS NOT NULL ) SQL @@ -349,7 +349,7 @@ after_initialize do topics.id NOT IN ( SELECT topic_id FROM topic_custom_fields - WHERE name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' + WHERE name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' AND value IS NOT NULL ) SQL @@ -361,15 +361,15 @@ after_initialize do topics.id IN ( SELECT t.id FROM topics t - JOIN category_custom_fields cc + JOIN category_custom_fields cc ON t.category_id = cc.category_id - AND cc.name = '#{::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD}' + AND cc.name = '#{::DiscourseSolved::ENABLE_ACCEPTED_ANSWERS_CUSTOM_FIELD}' AND cc.value = 'true' - ) - OR + ) + OR topics.id IN ( - SELECT topic_id - FROM topic_tags + SELECT topic_id + FROM topic_tags WHERE tag_id IN (?) ) SQL @@ -404,7 +404,7 @@ after_initialize do ->(r) do sql = <<~SQL NOT EXISTS ( - SELECT 1 + SELECT 1 FROM topic_custom_fields WHERE topic_id = topics.id AND name = '#{::DiscourseSolved::ACCEPTED_ANSWER_POST_ID_CUSTOM_FIELD}' @@ -591,10 +591,25 @@ after_initialize do 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, - ) + assignments = Assignment.includes(:target).where(topic: post.topic) + assignments.each do |assignment| + assigned_user = User.find_by(id: assignment.assigned_to_id) + Assigner.new(assignment.target, assigned_user).assign( + assigned_user, + status: SiteSetting.assignment_status_on_solve, + ) + end + end + on(:unaccepted_solution) do |post| + next if SiteSetting.assignment_status_on_unsolve.blank? + assignments = Assignment.includes(:target).where(topic: post.topic) + assignments.each do |assignment| + assigned_user = User.find_by(id: assignment.assigned_to_id) + Assigner.new(assignment.target, assigned_user).assign( + assigned_user, + status: SiteSetting.assignment_status_on_unsolve, + ) + end end end end diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index 3dea06c..aac659f 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -439,6 +439,7 @@ RSpec.describe "Managing Posts solved status" do SiteSetting.assign_allowed_on_groups = "#{group.id}" SiteSetting.assigns_public = true SiteSetting.assignment_status_on_solve = "Done" + SiteSetting.assignment_status_on_unsolve = "New" SiteSetting.ignore_solved_topics_in_assigned_reminder = false group.add(p1.acting_user) group.add(user) @@ -458,6 +459,50 @@ RSpec.describe "Managing Posts solved status" do expect(p1.topic.assignment.reload.status).to eq("Done") end + it "update all assignments to this status when a post is unaccepted" do + assigner = Assigner.new(p1.topic, user) + result = assigner.assign(user) + expect(result[:success]).to eq(true) + + DiscourseSolved.accept_answer!(p1, user) + + expect(p1.reload.topic.assignment.reload.status).to eq("Done") + + DiscourseSolved.unaccept_answer!(p1) + + expect(p1.reload.custom_fields["is_accepted_answer"]).to eq(nil) + expect(p1.reload.topic.assignment.reload.status).to eq("New") + end + + it "does not update the assignee when a post is accepted" do + user_1 = Fabricate(:user) + user_2 = Fabricate(:user) + user_3 = Fabricate(:user) + group.add(user_1) + group.add(user_2) + group.add(user_3) + + topic_question = Fabricate(:topic, user: user_1) + post_question = Fabricate(:post, topic: topic_question, user: user_1) + + user_2_response = Fabricate(:post, topic: topic_question, user: user_2) + assigner = Assigner.new(topic_question, user_2) + result = assigner.assign(user_2) + expect(result[:success]).to eq(true) + + post_response = Fabricate(:post, topic: topic_question, user: user_3) + Assigner.new(post_response, user_3).assign(user_3) + + DiscourseSolved.accept_answer!(post_response, user_1) + + expect(topic_question.assignment.assigned_to_id).to eq(user_2.id) + expect(post_response.assignment.assigned_to_id).to eq(user_3.id) + DiscourseSolved.unaccept_answer!(post_response) + + expect(topic_question.assignment.assigned_to_id).to eq(user_2.id) + expect(post_response.assignment.assigned_to_id).to eq(user_3.id) + end + 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")