From cbd69f21e774de3d577f9fb5bfc43ad6c62e9532 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 17 Jun 2024 12:41:13 +0200 Subject: [PATCH] FIX: when deleting users with solved posts (#297) it would raise an error because we delete the topic before and we need the topic record to update various meta data. This ensures we load the "deleted" version in such cases. Internal ref - t/130797 --- plugin.rb | 3 ++ spec/integration/solved_spec.rb | 67 +++++++++++++++++++++------------ 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/plugin.rb b/plugin.rb index d0c7de3..dc8f32b 100644 --- a/plugin.rb +++ b/plugin.rb @@ -137,6 +137,9 @@ after_initialize do def self.unaccept_answer!(post, topic: nil) topic ||= post.topic + topic ||= Topic.unscoped.find_by(id: post.topic_id) + + return if topic.nil? DistributedMutex.synchronize("discourse_solved_toggle_answer_#{topic.id}") do post.custom_fields.delete(IS_ACCEPTED_ANSWER_CUSTOM_FIELD) diff --git a/spec/integration/solved_spec.rb b/spec/integration/solved_spec.rb index aac659f..3ed6e69 100644 --- a/spec/integration/solved_spec.rb +++ b/spec/integration/solved_spec.rb @@ -446,6 +446,7 @@ RSpec.describe "Managing Posts solved status" do sign_in(user) end + 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) @@ -483,11 +484,11 @@ RSpec.describe "Managing Posts solved status" do 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) + Fabricate(:post, topic: topic_question, user: user_1) + Fabricate(:post, topic: topic_question, user: user_2) + + result = Assigner.new(topic_question, user_2).assign(user_2) expect(result[:success]).to eq(true) post_response = Fabricate(:post, topic: topic_question, user: user_3) @@ -503,32 +504,50 @@ RSpec.describe "Managing Posts solved status" do 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") - post = Fabricate(:post, topic: other_topic, user: user) + describe "assigned topic reminder" do + 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) - other_topic2 = Fabricate(:topic, title: "Topic that should be there2") - post2 = Fabricate(:post, topic: other_topic2, user: user) + other_topic2 = Fabricate(:topic, title: "Topic that should be there2") + post2 = Fabricate(:post, topic: other_topic2, user: user) - Assigner.new(post.topic, user).assign(user) - Assigner.new(post2.topic, user).assign(user) + 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(2) + reminder = PendingAssignsReminder.new + topics = reminder.send(:assigned_topics, user, order: :asc) + 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(2) - expect(topics).to include(other_topic2) + DiscourseSolved.accept_answer!(post2, Discourse.system_user) + topics = reminder.send(:assigned_topics, user, order: :asc) + 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(1) - expect(topics).not_to include(other_topic2) - expect(topics).to include(other_topic) + SiteSetting.ignore_solved_topics_in_assigned_reminder = true + topics = reminder.send(:assigned_topics, user, order: :asc) + expect(topics.to_a.length).to eq(1) + expect(topics).not_to include(other_topic2) + expect(topics).to include(other_topic) + end end end end + + describe "#unaccept_answer!" do + it "works even when the topic has been deleted" do + user = Fabricate(:user, trust_level: 1) + topic = Fabricate(:topic, user:) + reply = Fabricate(:post, topic:, user:, post_number: 2) + + DiscourseSolved.accept_answer!(reply, user) + + topic.trash!(Discourse.system_user) + reply.reload + + expect(reply.topic).to eq(nil) + + expect { DiscourseSolved.unaccept_answer!(reply) }.not_to raise_error + end + end end