[PATCH RFC v2 03/25] review-tui: keep the rethread flag on an upgraded series row

Christian Brauner <[email protected]>
Newsgroups org.kernel.linux.tools
Message-ID <[email protected]>
The upgrade path resolves whether the target revision needs rethreading
and threads target_is_rethreaded through five call sites, then does not
pass it to add_series_to_db().  The UPSERT arm writes is_rethreaded from
the excluded row, so the parameter's False default lands on the row and
clears an existing 1.

A series row saying 0 for a revision that really was stitched together
from individually fetched patches sends retrieve_series_messages() down
the single-msgid path: 'e' shows one patch's thread instead of the
series, and the message count collapses to match.

Pass the flag the upgrade already computed, and stop depending on every
caller remembering to.  The flag describes the posting, which the
revisions catalog already records, so resolve it from there on the way in
and keep the argument for a first track, before there is a catalog row to
ask.  Make the UPSERT arm sticky rather than overwriting, as
add_revision() already is.

Signed-off-by: Christian Brauner (Amutable) <[email protected]>
---
 src/b4/review/tracking.py          | 20 +++++++++++++++++---
 src/b4/review_tui/_tracking_app.py |  6 ++++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/src/b4/review/tracking.py b/src/b4/review/tracking.py
index 52b9a897..710b4a0d 100644
--- a/src/b4/review/tracking.py
+++ b/src/b4/review/tracking.py
@@ -456,7 +456,16 @@ def add_series_to_db(
     an existing one in place.  Re-adds come from callers that never
     learned those fields -- the Patchwork tracker attaching its id,
     rescan_branches replaying a branch -- and each used to wipe whatever
-    the others had recorded.
+    the others had recorded.  ``is_rethreaded`` is sticky for the same
+    reason, matching the catalog's :func:`add_revision`.
+
+    ``is_rethreaded`` describes the posting, so the catalog's answer for
+    this revision wins over the argument on the way in.  The argument is
+    still what a first track supplies, before there is a catalog row to
+    ask.  Callers that hand-carry the flag from a catalog lookup are then
+    merely agreeing with the row rather than being the only thing standing
+    between it and a False default -- which is what made dropping it at
+    one call site quietly clear the column.
     """
     if added_at is None:
         added_at = datetime.datetime.now(datetime.timezone.utc).isoformat()
@@ -465,7 +474,9 @@ def add_series_to_db(
         INSERT INTO series
         (change_id, revision, subject, sender_name, sender_email, sent_at, added_at,
          message_id, num_patches, pw_series_id, fingerprint, is_rethreaded)
-        VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
+        VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,
+                MAX(?, COALESCE((SELECT is_rethreaded FROM revisions
+                                 WHERE change_id = ? AND revision = ?), 0)))
         ON CONFLICT (change_id, revision) DO UPDATE SET
             subject = excluded.subject,
             sender_name = excluded.sender_name,
@@ -476,7 +487,8 @@ def add_series_to_db(
             num_patches = excluded.num_patches,
             pw_series_id = COALESCE(excluded.pw_series_id, series.pw_series_id),
             fingerprint = COALESCE(excluded.fingerprint, series.fingerprint),
-            is_rethreaded = excluded.is_rethreaded
+            is_rethreaded = MAX(COALESCE(series.is_rethreaded, 0),
+                                excluded.is_rethreaded)
         RETURNING track_id
     """,
         (
@@ -492,6 +504,8 @@ def add_series_to_db(
             pw_series_id,
             fingerprint,
             int(is_rethreaded),
+            change_id,
+            revision,
         ),
     )
     track_id = cursor.fetchone()[0]
diff --git a/src/b4/review_tui/_tracking_app.py b/src/b4/review_tui/_tracking_app.py
index 9f0a0302..69da8f58 100644
--- a/src/b4/review_tui/_tracking_app.py
+++ b/src/b4/review_tui/_tracking_app.py
@@ -4462,6 +4462,12 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]):
                         sent_at,
                         target_msgid,
                         lser.expected or num_am,
+                        # The upgrade already resolved this; without it the
+                        # UPSERT writes the column's 0 default (and clears an
+                        # existing 1), which sends every later retrieval of
+                        # the now-tracked revision down the single-msgid path
+                        # instead of reassembling it from its member patches.
+                        is_rethreaded=bool(target_is_rethreaded),
                     )
                     b4.review.tracking.update_series_status(
                         conn, change_id, 'reviewing', revision=target_rev

-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.