FIX: Accepting another answer does not commit (#360)

When an answer already exists, clicking "✅ Solution" on another post works, but does not commit.

This commit fixes that and also adds a test, and a transaction around accepting a solution (deleting the topic timer, previous user action, etc).

app/models/discourse_solved/solved_topic.rb

@@ -7,7 +7,7 @@ module DiscourseSolved
     belongs_to :topic, class_name: "Topic"
     belongs_to :answer_post, class_name: "Post", foreign_key: "answer_post_id"
     belongs_to :accepter, class_name: "User", foreign_key: "accepter_user_id"
-    belongs_to :topic_timer
+    belongs_to :topic_timer, dependent: :destroy
 
     validates :topic_id, presence: true
     validates :answer_post_id, presence: true

plugin.rb

@@ -33,11 +33,13 @@ after_initialize do
       DistributedMutex.synchronize("discourse_solved_toggle_answer_#{topic.id}") do
         solved = topic.solved
 
+        ActiveRecord::Base.transaction do
           if previous_accepted_post_id = solved&.answer_post_id
             UserAction.where(
               action_type: UserAction::SOLVED,
               target_post_id: previous_accepted_post_id,
             ).destroy_all
+            solved.destroy!
           else
             UserAction.log_action!(
               action_type: UserAction::SOLVED,
@@ -48,23 +50,21 @@ after_initialize do
             )
           end
 
-        solved ||=
+          solved =
             DiscourseSolved::SolvedTopic.new(topic:, answer_post: post, accepter: acting_user)
 
-        notification_data = {
-          message: "solved.accepted_notification",
-          display_username: acting_user.username,
-          topic_title: topic.title,
-          title: "solved.notification.title",
-        }.to_json
-
           unless acting_user.id == post.user_id
             Notification.create!(
               notification_type: Notification.types[:custom],
               user_id: post.user_id,
               topic_id: post.topic_id,
               post_number: post.post_number,
-            data: notification_data,
+              data: {
+                message: "solved.accepted_notification",
+                display_username: acting_user.username,
+                topic_title: topic.title,
+                title: "solved.notification.title",
+              }.to_json,
             )
           end
 
@@ -74,7 +74,12 @@ after_initialize do
               user_id: topic.user_id,
               topic_id: post.topic_id,
               post_number: post.post_number,
-            data: notification_data,
+              data: {
+                message: "solved.accepted_notification",
+                display_username: acting_user.username,
+                topic_title: topic.title,
+                title: "solved.notification.title",
+              }.to_json,
             )
           end
 
@@ -100,6 +105,7 @@ after_initialize do
           end
 
           solved.save!
+        end
 
         if WebHook.active_web_hooks(:accepted_solution).exists?
           payload = WebHook.generate_payload(:post, post)
@@ -127,7 +133,6 @@ after_initialize do
             topic_id: post.topic_id,
             post_number: post.post_number,
           )&.destroy!
-          solved.topic_timer.destroy! if solved.topic_timer
           solved.destroy!
         end
 

spec/integration/solved_spec.rb

@@ -540,6 +540,26 @@ RSpec.describe "Managing Posts solved status" do
     end
   end
 
+  describe "#accept_answer!" do
+    it "marks the post as the accepted answer correctly" do
+      user = Fabricate(:user, trust_level: 1)
+      topic = Fabricate(:topic, user:)
+      reply1 = Fabricate(:post, topic:, user:, post_number: 2)
+      reply2 = Fabricate(:post, topic:, user:, post_number: 3)
+
+      DiscourseSolved.accept_answer!(reply1, user)
+      topic.reload
+
+      expect(topic.solved.answer_post_id).to eq(reply1.id)
+      expect(topic.solved.topic_timer).to eq(topic.public_topic_timer)
+
+      DiscourseSolved.accept_answer!(reply2, user)
+      topic.reload
+
+      expect(topic.solved.answer_post_id).to eq(reply2.id)
+    end
+  end
+
   describe "user actions stream modifier" do
     it "correctly list solutions" do
       t1 = Fabricate(:topic)