From 1d337347485879f93271bb6ef8bff82cf8add4e4 Mon Sep 17 00:00:00 2001
From: Hampton Lintorn-Catlin <hampton@squareup.com>
Date: Tue, 9 Jun 2026 14:34:42 -0400
Subject: [PATCH] Fix Comment#first_comment_in_thread? UUID string comparison
 (COPLAN-30)

Comment IDs are random UUIDs, not insertion-ordered, so `id < ?` did a
string comparison: a reply whose UUID sorted below the opener's was
wrongly treated as the thread opener, firing a duplicate 'new comment
thread' notification to the plan author.

Switch to `comments.count == 1` (after_create_commit guarantees the row
is persisted), matching the workaround already used in
track_comment_created, which now reuses the fixed helper.

Adds a regression spec that fails against the old implementation.

Amp-Thread-ID: https://ampcode.com/threads/T-019eada7-5685-759f-bfc2-d3f52592cfa5
Co-authored-by: Amp <amp@ampcode.com>
---
 engine/app/models/coplan/comment.rb | 14 +++++---------
 spec/models/comment_spec.rb         | 15 +++++++++++++++
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/engine/app/models/coplan/comment.rb b/engine/app/models/coplan/comment.rb
index 156546e4..6686ea3a 100644
--- a/engine/app/models/coplan/comment.rb
+++ b/engine/app/models/coplan/comment.rb
@@ -34,7 +34,10 @@ def author
     private
 
     def first_comment_in_thread?
-      !comment_thread.comments.where("id < ?", id).exists?
+      # IDs are random UUIDs, not insertion-ordered, so we can't compare them
+      # with `id < ?`. after_create_commit guarantees the row is persisted, so
+      # a total count of 1 reliably means this comment opened the thread.
+      comment_thread.comments.count == 1
     end
 
     def notify_plan_author
@@ -42,13 +45,6 @@ def notify_plan_author
     end
 
     def track_comment_created
-      # NOTE: We deliberately don't reuse `first_comment_in_thread?` here —
-      # that helper compares UUIDs with `id < ?`, which is not insertion-
-      # ordered. After-create-commit guarantees the row is persisted, so a
-      # total count of 1 is the reliable signal for "this comment opened
-      # the thread."
-      is_first = comment_thread.comments.count == 1
-
       CoPlan::Analytics.track(
         "comment_created",
         user: author,
@@ -56,7 +52,7 @@ def track_comment_created
         comment_thread_id: comment_thread_id,
         comment_id: id,
         author_type: author_type,
-        is_first_in_thread: is_first,
+        is_first_in_thread: first_comment_in_thread?,
         body_length: body_markdown.to_s.length
       )
     end
diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb
index 0221a31f..eeba0b1d 100644
--- a/spec/models/comment_spec.rb
+++ b/spec/models/comment_spec.rb
@@ -40,5 +40,20 @@
         create(:comment, comment_thread: thread_record)
       }.not_to have_enqueued_job(CoPlan::NotificationJob)
     end
+
+    # Regression for COPLAN-30: `first_comment_in_thread?` used to compare
+    # UUIDs with `id < ?`, which is not insertion-ordered. A reply whose UUID
+    # sorts before the opener's was wrongly treated as the thread opener,
+    # firing a duplicate "new comment thread" notification for the plan author.
+    it "does not enqueue NotificationJob for a reply whose UUID sorts before the opener" do
+      high_id = "ffffffff-ffff-ffff-ffff-ffffffffffff"
+      low_id  = "00000000-0000-0000-0000-000000000001"
+
+      create(:comment, comment_thread: thread_record, id: high_id)
+
+      expect {
+        create(:comment, comment_thread: thread_record, id: low_id)
+      }.not_to have_enqueued_job(CoPlan::NotificationJob)
+    end
   end
 end