[PATCH b4 v2 10/44] review-tui: say which way a take did not reach 'accepted'

[Christian Brauner <brauner@kernel.org>]

The take helpers return None for three different things: accept
unchecked in the dialog, a take that stopped short, and patches that
landed with only the record of them missing. All three were reported
as "series not marked accepted". The last is the worst: it reports a
take that did not happen over one that did.

Tell them apart. The dialog answers the first. The unrecorded take
gets a status of its own, kept out of the tracking database and
Patchwork like None. What is left is a take that stopped short.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 src/b4/review_tui/_tracking_app.py | 33 ++++++++++++++++++++++++++-------
 src/tests/test_tui_tracking.py     |  6 ++++--
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/src/b4/review_tui/_tracking_app.py b/src/b4/review_tui/_tracking_app.py
index 8d91b37..83e33c7 100644
--- a/src/b4/review_tui/_tracking_app.py
+++ b/src/b4/review_tui/_tracking_app.py
@@ -2645,11 +2645,25 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]):
                     'Thank & archive skipped: series only partially applied',
                     severity='warning',
                 )
-            else:
+            elif new_status == 'unrecorded':
+                self.notify(
+                    'Thank & archive skipped: the take was not recorded',
+                    severity='warning',
+                )
+            elif not take_screen.accept_series:
                 self.notify(
                     'Thank & archive skipped: series not marked accepted',
                     severity='warning',
                 )
+            else:
+                # _do_take_* returns None both for "accept was unchecked" and
+                # for "the take never finished"; only the latter is left here,
+                # and claiming a status the series never reached would send the
+                # maintainer looking for a take that did not happen.
+                self.notify(
+                    'Thank & archive skipped: take did not complete',
+                    severity='warning',
+                )
 
     @staticmethod
     def _record_take_metadata(
@@ -2664,7 +2678,8 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]):
 
         Computes patch coverage and returns the resulting series status:
         'accepted' if all patches are now taken, 'partial' if some remain,
-        or None if *accepted* is False (no status change requested).
+        'unrecorded' if the patches went in but the tracking data could not
+        be read, or None if *accepted* is False (no status change requested).
 
         Args:
             topdir: Repository top-level directory.
@@ -2677,8 +2692,10 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]):
         try:
             cover_text, tracking = b4.review.load_tracking(topdir, review_branch)
         except SystemExit:
-            logger.warning('Could not load tracking data for recording take metadata')
-            return None
+            # The patches are applied; only the record of it is missing, which
+            # is a different thing from a take that never got this far.
+            logger.critical('Could not load tracking data for recording the take')
+            return 'unrecorded'
 
         series = tracking.get('series', {})
         take_info = {
@@ -2993,14 +3010,16 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]):
     ) -> None:
         """Common post-take steps: record branch, update DB, update Patchwork.
 
-        *new_status* is the status computed by _record_take_metadata: 'accepted',
-        'partial', or None (when the user did not request a status change).
+        *new_status* is the status computed by _record_take_metadata:
+        'accepted', 'partial', 'unrecorded' (nothing was written to the
+        tracking commit, so there is no coverage to propagate), or None (when
+        the user did not request a status change).
         """
         common_dir = b4.git_get_common_dir(topdir)
         if common_dir:
             b4.review.tracking.record_take_branch(common_dir, target_branch)
 
-        if new_status and self._identifier and change_id:
+        if new_status in ('accepted', 'partial') and self._identifier and change_id:
             revision = series.get('revision')
             existing_target = None
             try:
diff --git a/src/tests/test_tui_tracking.py b/src/tests/test_tui_tracking.py
index 3e69621..6a3f2f8 100644
--- a/src/tests/test_tui_tracking.py
+++ b/src/tests/test_tui_tracking.py
@@ -4484,10 +4484,12 @@ class TestTakeThankArchiveChain:
         assert not thanks
         assert any('partially applied' in n for n in notices)
 
-    def test_incomplete_take_does_not_chain(self) -> None:
+    def test_aborted_take_says_so(self) -> None:
+        """A take that never finished is reported as such, not as a series
+        the maintainer declined to accept."""
         thanks, notices = self._run_take_final(None, True)
         assert not thanks
-        assert any('not marked accepted' in n for n in notices)
+        assert any('take did not complete' in n for n in notices)
 
     def test_unchecked_box_never_chains(self) -> None:
         thanks, notices = self._run_take_final('accepted', False)

-- 
2.53.0