Post work PR #3047

fix(#1898): keep Telegram link previews, and keep the outcome prompt's failures its own

Four follow-ups to the post-print outcome confirmation merged in #3047.

Telegram: link previews were switched off for every message rather than
only for the outcome prompt, so a print_complete template carrying
{finish_photo_url} lost its photo preview whenever the photo was too
large to attach. _send_telegram now takes link_preview, and only the
prompt turns it off, as the Slack unfurl change already did.

Archives: Reset left the new Unconfirmed filter on, so the list stayed
narrowed and the button seemed to do nothing.

Print start: when the external-print check hit a failed statement, it
rolled back the caller's whole transaction, which expired the printer
and the just-created archive; on an async session the next read of
either raises, and the start notification, energy reading and timelapse
baseline were skipped. The check's reads now run in a savepoint, and a
failed flag write reloads the archive and printer after its rollback.

Print complete: a failed outcome prompt left the notification session
needing a rollback, so the per-user print email sent on it next failed
too. The dispatch now rolls back on failure.

backend/app/main.py

@@ -3420,54 +3420,85 @@ async def _ask_outcome_for_external_print(db, printer_id: int, observed_name: st
     """
     logger = logging.getLogger(__name__)
     try:
-        from backend.app.api.routes.settings import get_setting, setting_is_true
+        # A savepoint rather than a rollback of the caller's transaction on
+        # failure: a failed statement leaves the transaction unusable, but a
+        # full rollback also expires every object the caller has loaded (the
+        # printer, the archive it just created), and on an async session the
+        # next attribute read then raises instead of reloading. Everything here
+        # is a read, so undoing the savepoint loses nothing.
+        async with db.begin_nested():
+            return await _decide_outcome_for_external_print(db, printer_id, observed_name, logger)
+    except Exception as e:
+        logger.warning("[CALLBACK] Could not decide the outcome prompt for printer %s: %s", printer_id, e)
+        return False
 
-        if not setting_is_true(await get_setting(db, "confirm_outcome_external_prints")):
-            return False
 
-        from backend.app.models.print_queue import PrintQueueItem
+async def _decide_outcome_for_external_print(db, printer_id: int, observed_name: str | None, logger) -> bool:
+    """The reads behind ``_ask_outcome_for_external_print``; see there."""
+    from backend.app.api.routes.settings import get_setting, setting_is_true
 
-        dispatched_here = await db.scalar(
-            select(PrintQueueItem)
-            .where(
-                PrintQueueItem.printer_id == printer_id,
-                PrintQueueItem.status == "printing",
-            )
-            .limit(1)
-        )
-        if dispatched_here is None:
-            return True
+    if not setting_is_true(await get_setting(db, "confirm_outcome_external_prints")):
+        return False
 
-        expected = await _queue_item_dispatched_name(db, dispatched_here)
-        observed = (observed_name or "").strip()
-        if not expected or not observed or _subtask_names_match(expected, observed):
-            logger.info(
-                "[CALLBACK] Not asking for the outcome on printer %s: queue item %s is still printing, so "
-                "Bambuddy dispatched this run and the item's own ask-for-outcome flag decides.",
-                printer_id,
-                dispatched_here.id,
-            )
-            return False
+    from backend.app.models.print_queue import PrintQueueItem
 
+    dispatched_here = await db.scalar(
+        select(PrintQueueItem)
+        .where(
+            PrintQueueItem.printer_id == printer_id,
+            PrintQueueItem.status == "printing",
+        )
+        .limit(1)
+    )
+    if dispatched_here is None:
+        return True
+
+    expected = await _queue_item_dispatched_name(db, dispatched_here)
+    observed = (observed_name or "").strip()
+    if not expected or not observed or _subtask_names_match(expected, observed):
         logger.info(
-            "[CALLBACK] Queue item %s is still marked printing on printer %s but was dispatched as %r, not "
-            "%r; treating this as an externally started print.",
-            dispatched_here.id,
+            "[CALLBACK] Not asking for the outcome on printer %s: queue item %s is still printing, so "
+            "Bambuddy dispatched this run and the item's own ask-for-outcome flag decides.",
             printer_id,
-            expected,
-            observed,
+            dispatched_here.id,
         )
-        return True
+        return False
+
+    logger.info(
+        "[CALLBACK] Queue item %s is still marked printing on printer %s but was dispatched as %r, not "
+        "%r; treating this as an externally started print.",
+        dispatched_here.id,
+        printer_id,
+        expected,
+        observed,
+    )
+    return True
+
+
+async def _dispatch_outcome_confirmation_safely(
+    db,
+    printer_id: int,
+    printer_name: str,
+    data: dict,
+    archive_id: int,
+    archive_data: dict | None = None,
+) -> None:
+    """``dispatch_outcome_confirmation`` for a caller that has more to do on ``db``.
+
+    The completion task sends the per-user print email on the same session
+    right after the prompt. A failed statement in the prompt leaves that
+    session needing a rollback, and without one the email step fails with
+    PendingRollbackError; the prompt is the optional part, so it must not be
+    the reason the email never goes out.
+    """
+    try:
+        await dispatch_outcome_confirmation(db, printer_id, printer_name, data, archive_id, archive_data)
     except Exception as e:
-        logger.warning("[CALLBACK] Could not decide the outcome prompt for printer %s: %s", printer_id, e)
-        # A failed statement deactivates the transaction, so without this the
-        # caller's own add()/commit() would raise PendingRollbackError and the
-        # print would go unarchived over a question that answers "no".
+        logging.getLogger(__name__).error("[NOTIFY-BG] Outcome-confirmation dispatch failed: %s", e, exc_info=True)
         try:
             await db.rollback()
         except Exception:
             pass
-        return False
 
 
 async def dispatch_outcome_confirmation(
@@ -4836,16 +4867,26 @@ async def on_print_start(printer_id: int, data: dict):
                 # start notification, the energy reading and the timelapse
                 # baseline below it with it, and a missing prompt is by far the
                 # cheaper failure.
+                archive_id = archive.id
                 try:
                     if await _ask_outcome_for_external_print(db, printer_id, subtask_name):
                         archive.confirm_requested = True
                         await db.commit()
                 except Exception as e:
-                    logger.warning("Could not flag archive %s for the outcome prompt: %s", archive.id, e)
+                    logger.warning("Could not flag archive %s for the outcome prompt: %s", archive_id, e)
+                    # The rollback expires every loaded object, and on an async
+                    # session reading one afterwards raises instead of
+                    # reloading, so the two this branch goes on to use are
+                    # fetched again. The archive itself was committed by
+                    # archive_print; only the flag is lost.
                     try:
                         await db.rollback()
-                    except Exception:
-                        pass
+                        archive = await db.get(PrintArchive, archive_id)
+                        printer = await db.get(Printer, printer_id)
+                    except Exception as reload_error:
+                        logger.warning(
+                            "Could not reload archive %s after the failed flag write: %s", archive_id, reload_error
+                        )
 
                 # Track this active print (use both original filename and downloaded filename)
                 _active_prints[(printer_id, downloaded_filename)] = archive.id
@@ -7887,12 +7928,9 @@ async def on_print_complete(printer_id: int, data: dict):
                 # background task so the finish photo fetched above rides
                 # along with the prompt.
                 if print_status == "completed" and archive_id:
-                    try:
-                        await dispatch_outcome_confirmation(
-                            db, printer_id, printer_name, data, archive_id, archive_data
-                        )
-                    except Exception as e:
-                        logger.error("[NOTIFY-BG] Outcome-confirmation dispatch failed: %s", e, exc_info=True)
+                    await _dispatch_outcome_confirmation_safely(
+                        db, printer_id, printer_name, data, archive_id, archive_data
+                    )
 
                 # Send user-specific email notification
                 if archive_data:

backend/app/services/notification_service.py

@@ -548,12 +548,14 @@ class NotificationService:
         message: str,
         image_data: bytes | None = None,
         buttons: list[dict] | None = None,
+        link_preview: bool = True,
     ) -> tuple[bool, str]:
         """Send notification via Telegram bot.
 
         ``buttons`` is one row of inline URL buttons (``{"text", "url"}``
         entries), used by the outcome-confirmation event (#1898) to put
-        one-tap Good/Reject under the message.
+        one-tap Good/Reject under the message. ``link_preview=False`` asks
+        Telegram not to fetch the first URL in the text for a preview card.
         """
         bot_token = config.get("bot_token", "").strip()
         chat_id = config.get("chat_id", "").strip()
@@ -606,13 +608,9 @@ class NotificationService:
                 "chat_id": chat_id,
                 "text": message,
                 "parse_mode": "Markdown",
-                # Telegram's servers GET the first URL in the text to build a
-                # preview card. The outcome prompt (#1898) carries single-use
-                # verdict links in its body, so that fetch would answer the
-                # question before the operator saw it. Bambuddy's messages are
-                # status text; a preview card adds nothing to any of them.
-                "disable_web_page_preview": True,
             }
+            if not link_preview:
+                payload["disable_web_page_preview"] = True
             if message_thread_id is not None:
                 payload["message_thread_id"] = message_thread_id
             if with_buttons:
@@ -1091,8 +1089,20 @@ class NotificationService:
                         {"text": "\U0001f44d Good", "url": one_tap_url(_tg_good)},
                         {"text": "\U0001f44e Reject", "url": one_tap_url(_tg_reject)},
                     ]
+                # Telegram's servers GET the first URL in the text to build a
+                # preview card. An outcome prompt whose edited body still
+                # carries {good_url} would have that fetch answer the question
+                # before the operator saw it, so the preview is off for this
+                # event. Only for this one, for the same reason as the Slack
+                # unfurl in _send_webhook: when the finish photo is too large to attach,
+                # the preview is how a {finish_photo_url} in a print_complete
+                # body still shows up as a photo in the chat.
                 return await self._send_telegram(
-                    config, f"*{title}*\n{message}", image_data=image_data, buttons=tg_buttons
+                    config,
+                    f"*{title}*\n{message}",
+                    image_data=image_data,
+                    buttons=tg_buttons,
+                    link_preview=event_type != "print_confirm_request",
                 )
             elif provider.provider_type == "email":
                 # finish_photo_url is pulled from the rendered template variables

backend/tests/integration/test_external_print_confirmation_1898.py

@@ -76,7 +76,7 @@ class _StubArchiveService:
         return archive
 
 
-async def _drive_print_start(test_engine, printer, *, download_ok: bool) -> None:
+async def _drive_print_start(test_engine, printer, *, download_ok: bool, extra_patches=()) -> dict:
     """Run ``on_print_start`` against the test database.
 
     ``download_ok`` picks the branch: False leaves the 3MF unreachable and the
@@ -116,9 +116,10 @@ async def _drive_print_start(test_engine, printer, *, download_ok: bool) -> None
         patch("backend.app.services.usage_tracker.on_print_start", new_callable=AsyncMock),
     ]
 
+    mocks: dict = {}
     with ExitStack() as stack:
-        for p in patches:
-            stack.enter_context(p)
+        for p in [*patches, *extra_patches]:
+            mocks[p.attribute] = stack.enter_context(p)
         notif = stack.enter_context(patch("backend.app.main.notification_service"))
         plug = stack.enter_context(patch("backend.app.main.smart_plug_manager"))
         ws = stack.enter_context(patch("backend.app.main.ws_manager"))
@@ -138,6 +139,7 @@ async def _drive_print_start(test_engine, printer, *, download_ok: bool) -> None
         from backend.app.main import on_print_start
 
         await on_print_start(printer.id, {"filename": DISPATCH, "subtask_name": SUBTASK})
+    return mocks
 
 
 async def _set_setting(db_session, key: str, value: str) -> None:
@@ -241,6 +243,79 @@ class TestExternalPrintGetsTheOutcomePrompt:
         assert (await _created_archive(db_session, printer.id)).confirm_requested is False
 
 
+class TestAFailureHereCostsOnlyThePrompt:
+    """The prompt is optional; the rest of print start is not.
+
+    After a failed statement, a rollback of the whole transaction expires every
+    object the session holds, and on an async session the next read of one
+    raises instead of reloading. The print start below the check reads both
+    the printer and the archive, so the check runs in a savepoint, and the flag
+    write reloads what it used after its own rollback.
+    """
+
+    @staticmethod
+    def _assert_print_start_finished(mocks, archive_id: int) -> None:
+        assert archive_id in _active_prints.values()
+        mocks["_record_energy_start"].assert_awaited()
+        assert mocks["_record_energy_start"].await_args.args[0].id == archive_id
+        mocks["_capture_timelapse_baseline_at_start"].assert_awaited()
+        assert mocks["_capture_timelapse_baseline_at_start"].await_args.kwargs["archive_id"] == archive_id
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    @pytest.mark.parametrize("download_ok", [True, False])
+    async def test_a_failing_query_in_the_check(self, test_engine, db_session, printer_factory, download_ok):
+        from sqlalchemy import text
+
+        from backend.app.api.routes import settings as settings_routes
+
+        real_get_setting = settings_routes.get_setting
+
+        async def _broken_for_this_key(db, key):
+            if key == "confirm_outcome_external_prints":
+                # A real failed statement, not just an exception: this is what
+                # leaves the transaction needing a rollback.
+                await db.execute(text("SELECT no_such_column FROM settings"))
+            return await real_get_setting(db, key)
+
+        printer = await printer_factory()
+        await _set_setting(db_session, "confirm_outcome_external_prints", "true")
+
+        mocks = await _drive_print_start(
+            test_engine,
+            printer,
+            download_ok=download_ok,
+            extra_patches=[patch.object(settings_routes, "get_setting", _broken_for_this_key)],
+        )
+
+        archive = await _created_archive(db_session, printer.id)
+        assert archive.confirm_requested is False
+        self._assert_print_start_finished(mocks, archive.id)
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_a_failing_flag_write(self, test_engine, db_session, printer_factory):
+        async def _yes_but_poison_the_commit(db, printer_id, observed_name=None):
+            # Two rows for one unique key: the commit that carries the flag fails.
+            db.add(Settings(key="x_1898_duplicate", value="a"))
+            db.add(Settings(key="x_1898_duplicate", value="b"))
+            return True
+
+        printer = await printer_factory()
+
+        mocks = await _drive_print_start(
+            test_engine,
+            printer,
+            download_ok=True,
+            extra_patches=[patch("backend.app.main._ask_outcome_for_external_print", _yes_but_poison_the_commit)],
+        )
+
+        archive = await _created_archive(db_session, printer.id)
+        # archive_print committed the row; only the flag is lost.
+        assert archive.confirm_requested is False
+        self._assert_print_start_finished(mocks, archive.id)
+
+
 class TestAQueuedPrintStillDecidesForItself:
     @pytest.mark.asyncio
     @pytest.mark.integration
@@ -536,6 +611,26 @@ class TestTheCompletionEmitsThePrompt:
         assert kwargs["reject_url"].startswith("http")
         assert kwargs["confirm_url"].startswith("http")
 
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_a_failed_prompt_leaves_the_session_usable_for_the_email(self, db_session):
+        """The completion task sends the per-user print email on the same
+        session right after the prompt, so a prompt that dies mid-flush must
+        not take the email with it."""
+        from backend.app.main import _dispatch_outcome_confirmation_safely
+
+        async def _dies_mid_flush(db, *args, **kwargs):
+            db.add(Settings(key="x_1898_duplicate", value="a"))
+            db.add(Settings(key="x_1898_duplicate", value="b"))
+            await db.flush()
+
+        with patch("backend.app.main.dispatch_outcome_confirmation", _dies_mid_flush):
+            await _dispatch_outcome_confirmation_safely(db_session, 1, "X1C", {}, 1, {})
+
+        # What _dispatch_user_print_email does next: read from the session.
+        # Without the rollback this raises PendingRollbackError.
+        await db_session.execute(select(Settings).limit(1))
+
     @pytest.mark.asyncio
     @pytest.mark.integration
     async def test_an_already_answered_archive_is_not_asked_again(self, db_session, printer_factory, archive_factory):

backend/tests/unit/test_confirm_link_unattended_fetch_1898.py

@@ -183,7 +183,7 @@ class TestTheOneTapMarkerRidesOnlyOnButtons:
         service = NotificationService()
         captured: dict = {}
 
-        async def _fake_telegram(config, message, image_data=None, buttons=None):
+        async def _fake_telegram(config, message, image_data=None, buttons=None, link_preview=True):
             captured["buttons"] = buttons
             captured["message"] = message
             return True, "ok"
@@ -299,6 +299,7 @@ class TestTelegramDoesNotAskForAPreview:
         ok, _ = await service._send_telegram(
             CONFIG,
             "*How did your print come out?*\nX1C: bracket.3mf\nGood: https://host/api/v1/archives/confirm/tok/good",
+            link_preview=False,
         )
 
         assert ok
@@ -316,8 +317,45 @@ class TestTelegramDoesNotAskForAPreview:
             {"text": "Good", "url": "https://host/api/v1/archives/confirm/tok/good"},
             {"text": "Reject", "url": "https://host/api/v1/archives/confirm/tok/reject"},
         ]
-        ok, _ = await service._send_telegram(CONFIG, "*T*\nbody", buttons=buttons)
+        ok, _ = await service._send_telegram(CONFIG, "*T*\nbody", buttons=buttons, link_preview=False)
 
         assert ok
         assert client.calls[0]["disable_web_page_preview"] is True
         assert client.calls[0]["reply_markup"] == {"inline_keyboard": [buttons]}
+
+    @pytest.mark.asyncio
+    async def test_every_other_message_keeps_its_preview(self):
+        """When the finish photo is too large to attach, the preview card is
+        how a {finish_photo_url} in a print_complete body still shows up as a
+        photo in the chat. Only the outcome prompt gives that up."""
+        service = NotificationService()
+        client = _Client()
+        service._http_client = client
+
+        ok, _ = await service._send_telegram(
+            CONFIG, "*Print complete*\nX1C: bracket.3mf\nhttps://farm.example.com/api/v1/archives/7/photos/finish_a.jpg"
+        )
+
+        assert ok
+        assert "disable_web_page_preview" not in client.calls[0]
+
+    @pytest.mark.asyncio
+    @pytest.mark.parametrize(
+        ("event_type", "expected"),
+        [("print_confirm_request", False), ("print_complete", True), (None, True)],
+    )
+    async def test_only_the_outcome_prompt_is_sent_without_a_preview(self, event_type, expected):
+        service = NotificationService()
+        captured: dict = {}
+
+        async def _fake_telegram(config, message, image_data=None, buttons=None, link_preview=True):
+            captured["link_preview"] = link_preview
+            return True, "ok"
+
+        service._send_telegram = _fake_telegram
+        provider = NotificationProvider(name="farm", provider_type="telegram", config=json.dumps(CONFIG))
+
+        ok, _ = await service._send_to_provider(provider, "Title", "body", event_type=event_type)
+
+        assert ok
+        assert captured["link_preview"] is expected

frontend/src/__tests__/pages/ArchivesPage.test.tsx

@@ -686,4 +686,24 @@ describe('ArchivesPage', () => {
       expect(await screen.findByTestId('confirm-outcome-dialog')).toBeInTheDocument();
     });
   });
+
+  describe('unconfirmed filter', () => {
+    afterEach(() => {
+      window.localStorage.removeItem('archiveFilterUnconfirmed');
+    });
+
+    it('is cleared by Reset like every other top filter', async () => {
+      render(<ArchivesPage />);
+      await screen.findByText('Benchy');
+
+      fireEvent.click(screen.getByTitle('Show only prints still waiting for their outcome verdict'));
+      // Neither fixture archive is waiting for a verdict, so the filter empties the list.
+      await waitFor(() => expect(screen.queryByText('Benchy')).not.toBeInTheDocument());
+
+      fireEvent.click(screen.getByRole('button', { name: 'Reset' }));
+
+      expect(await screen.findByText('Benchy')).toBeInTheDocument();
+      expect(screen.queryByRole('button', { name: 'Reset' })).not.toBeInTheDocument();
+    });
+  });
 });

frontend/src/pages/ArchivesPage.tsx

@@ -3707,6 +3707,7 @@ export function ArchivesPage() {
     setFilterMaterial(null);
     setFilterFavorites(false);
     setHideFailed(false);
+    setFilterUnconfirmed(false);
     setHideDuplicates(false);
     setFilterTag(null);
     setFilterFileType('all');

static/index.html

@@ -26,9 +26,9 @@
 
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-VsfgUmwa.js"></script>
+    <script type="module" crossorigin src="/assets/index-C3lPAZBn.js"></script>
     <link rel="modulepreload" crossorigin href="/assets/chunk-aKtaBQYM.js">
-    <link rel="stylesheet" crossorigin href="/assets/index-ldjSdDIZ.css">
+    <link rel="stylesheet" crossorigin href="/assets/index-TpYl7fWS.css">
   </head>
   <body>
     <div id="root"></div>