diff --git a/CHANGELOG.md b/CHANGELOG.md index 81c960a80..9d530e766 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ All notable changes to Bambuddy will be documented in this file. **Print Anyway diagnostic log (Arn0uDz follow-up).** `_block_on_filament_deficit` in `print_scheduler.py:1983` now logs at INFO when it honours `item.skip_filament_check`, so a future "Print Anyway didn't work" report (the third commenter on #1762 hit this shape) has an actionable line in the standard support bundle without needing debug logging enabled. The route-side log at `print_queue.py:1278` is unchanged. Without logs from the original report we can't isolate the user's failure mode (the wire path on both ends still looks correct on inspection), so this is the minimal trace required to investigate the next occurrence — bundled in the same drop because Block 1 makes the original symptom disappear for users who had backup ON anyway. **Tests.** 8 new backend cases in `test_filament_deficit.py::TestFilamentDeficitBackupAware` pin every dimension: pool covers the assigned-slot shortfall → no deficit (the reporter scenario, with matching `slicer_filament` preset + matching colour); pool insufficient → deficit emitted with the correct slot id; peer slot holds a DIFFERENT preset → no pool, deficit fires; backup OFF → strict regression with the pre-#1762 per-slot accounting (using identical inputs to the "pool covers" case but flipping the toggle); dual-extruder printer with a peer on the OPPOSITE side → deficit fires because the firmware can't cross; STRICT-rule — two spools with material+colour match but NO preset must NEVER pair; COLOUR-strict — same preset + DIFFERENT colours must NOT pool (the reporter screenshot scenario, three PETG HF in different colours); COLOUR normalisation — 6-char and 8-char hex of the same RGB pool correctly (`000000` matches `000000FF`). 13 frontend cases in `PrintersPageBackupGroups.test.ts` pin `computeBackupGroups`: empty for missing input; ignores empty slots; pairs via preset; no-preset spools NEVER pair even on attribute-tuple match; different presets never cross; SAME preset + DIFFERENT colours don't pair; colour-hex 6-char and 8-char normalisation; lone slots returned alongside pairs in the same list; dual-extruder scopes per-side both ways; HT AMS pairs with regular AMS via `getGlobalTrayId`; preserves display name + tray colour for the modal swatch; DEFENSIVE dedup of duplicate `ams.id` entries (first wins). 10 modal render cases in `AmsBackupModal.test.tsx`: closed → null; ring renders for pairs and OMITS lone slots; **Esc keypress closes the modal**; Esc is a no-op after the modal closes (listener actually unmounts); toggle reflects ON state + fires onToggle(false) on click; toggle disabled when state unknown (A1 family); toggle disabled when permission missing; no-pairs empty state when no pair can form; R/L badges render when extruder map carries two distinct values; R/L badges absent when the map collapses to one extruder. Existing 8 `test_filament_deficit.py` + 60 `PrintersPage.test.tsx` cases stay green — the no-backup path is a strict no-op vs the pre-#1762 logic. **i18n.** 12 new keys × 11 locales for the modal + the active-print pill (`printers.amsBackup.modalTitle / modalHelp / modalNoSlots / modalNoPairs / stateOn / stateOff / stateUnknown / extruderRightShort / extruderLeftShort`, plus `printers.activeJobSlot.title / ariaLabel`) translated in de / en / es / fr / it / ja / ko / pt-BR / tr / zh-CN / zh-TW. Parity check 5253 leaves per locale, no English fallback; "AMS Filament Backup" is a Bambu product/firmware name and is allowlisted as a cognate where the European locales keep it verbatim. **Scope.** No DB migration, no new permission. The global Filament Backup badge stays where #1766 put it (Filaments section header on the printer card) — firmware reality is one bit on `print.cfg`, and moving the toggle per-AMS would misrepresent that. The badge click no longer toggles directly; it opens the modal, where the same `setAmsFilamentBackup` mutation is wired to the toggle. No schema change to `FilamentDeficit` — same shape, same wire payload, same 409 response under `code: insufficient_filament`. No change to the `disable_filament_warnings` setting (#720) — when on, the deficit check is still a no-op regardless of backup state. **Behaviour shift worth flagging.** Pre-PR, prints could be blocked with "insufficient filament" even when the firmware would actually have switched to a same-`(preset, colour)` peer mid-print. Post-PR, those prints dispatch. Users with backup misconfigured at the firmware level (e.g. FTS routing wrong) may see prints dispatch that previously got blocked at the deficit check; the printer would then fail mid-run rather than at queue-start time. The trade-off is correct — the warning shouldn't fire when backup will save you — but worth surfacing for anyone debugging post-upgrade. +- **Inline finish-photo embed in failure-event emails + `user_print_*` template disambiguation (#1792, reported by @elit3ge)** — Two related changes to the notification stack. **(1) Template-driven inline finish-photo in email.** Pushover / Telegram / Discord / ntfy users already get the finish-photo JPEG attached to terminal-print notifications (`print_complete` / `print_failed` / `print_stopped` event types), thanks to the capture path shipped in 0.2.5b1 (#1397) that extracts the last timelapse frame at print end and loads up to 2.5 MB into `archive_data["image_data"]`. Email was the one provider that dropped those bytes on the floor — text-only body, no visual context for the reporter's "Reason: unknown" failure mails. `notification_service._send_email` (`backend/app/services/notification_service.py:413`) now accepts `finish_photo_url` alongside `image_data` and the dispatcher (`_send_to_provider` at `:745`) threads the URL from the rendered template variables dict. **Inline embed is opt-in via the existing `{finish_photo_url}` template variable** — first draft of this fix unconditionally inlined the photo whenever bytes were present, which @maziggy correctly flagged as bypassing the template system ("standard is to have variables for all available items in a template"). The contract now: if the user puts `{finish_photo_url}` in their email template body, the URL substring in the rendered body triggers the multipart/related shape — HTML part replaces the escaped URL in-place with `` (so the image appears WHERE the variable was, not stapled to the bottom), plain-text part keeps the URL as a clickable link, MIMEImage attached inline with `Content-ID: ` per RFC 2392. If the template doesn't reference the variable, single-part text-only — no surprise image. Default templates are unchanged; reporter (and any user who wants this) edits their `print_complete` / `print_failed` / `print_stopped` body once to add the variable. XSS hygiene: rendered body is `html.escape`d before the URL→`` swap, newlines become `
`. Pushover/Telegram/Discord/ntfy senders untouched — their pre-existing "auto-attach whenever `image_data` is set" behaviour stays because their bodies aren't HTML-templatable for inline images anyway. **(2) `user_print_*` template names get an " Email" suffix.** Same reporter surfaced a separate confusion: the Message Templates list showed "Print Completed" and "User Print Completed" side-by-side with no cue they're different dispatch paths — the first is a provider-level broadcast to whatever notification channels the admin configured (ntfy/pushover/telegram/discord/email/webhook/homeassistant), the second is a per-user SMTP-only email to the user who submitted the job (requires advanced auth + `user_notifications_enabled` toggle + user has email + per-user pref opt-in). The `EVENT_NAMES` display map in `backend/app/api/routes/notification_templates.py:51` already used the disambiguated "User Print Completed Email" label, but the seed wrote the short name to the DB, so the UI rendered the ambiguous one. Fresh installs now get the suffixed name straight from `DEFAULT_TEMPLATES` (`backend/app/models/notification_template.py:198+`). Existing installs get the rename via a new `_migrate_rename_user_print_template_names` (`backend/app/core/database.py:3081+`) that runs on startup and updates rows for the four `user_print_*` event types WHERE the name still matches the old default — admin-edited names are preserved. Standard SQL UPDATE works on both SQLite and Postgres without dialect branching. **Tests:** 6 new `TestEmailProvider` cases in `backend/tests/unit/services/test_notification_service.py` pinning the template-driven contract (no-image-no-URL → text-only, image-without-template-reference → STILL text-only, URL-in-body + bytes → multipart/related with cid, URL-arg-missing → text-only defence-in-depth, body-escape hygiene, URL→`` in-place swap). 5 new migration cases in `backend/tests/unit/test_user_print_template_rename_migration.py` covering default-rename, user-edited preservation, provider-template don't-touch, second-run idempotency, empty-table fresh-install no-op. 11/11 + 140/140 adjacent notification tests green. Ruff clean. **Verified end-to-end** against a real SMTP provider with a real 48 KB finish-photo JPEG — Gmail rendered the inline image where the URL marker was in the body. + - **Dedicated "AI Failure Detection" notification event (#1794, reported by @maziggy from a user report)** — Obico failure detection now fires its own notification event (`on_ai_failure_detection`) instead of riding the multiplexed `on_printer_error` toggle. Reporter (P1S, Discord provider) had Obico enabled with `obico_action=notify`, detection was firing correctly per the logs, every other Discord notification was working — but spaghetti detections never reached Discord. **Root cause.** `obico_actions._notify` at `obico_actions.py:75` was calling `notification_service.on_printer_error(..., error_type="ai_failure_detection")`. The notification service's provider filter at `notification_service.py:722-725` requires the SUBSCRIBED-event boolean column to be True; the `on_printer_error` column defaults to False; the reporter's Discord provider was created without explicitly enabling Printer Error. The user couldn't have found the right toggle even if they'd known to look — the UI labels it "Printer Error" with no hint that flipping it also subscribes to AI detection. The same toggle multiplexed three distinct events (HMS hardware errors at `main.py:1248` + Obico spaghetti + a `error_type="ai_failure_detection"` discriminator passed in the variables payload), so a user who wanted spaghetti alerts but not chamber-fan-stalled HMS pages had no way to express that. **Fix.** New `on_ai_failure_detection` Boolean column on `notification_providers` (defaults False — matches the conservative default of every other opt-in event); new `notification_service.on_ai_failure_detection(printer_id, printer_name, task_name, confidence, action, db, image_data)` method following the exact shape of `on_printer_error` (mirrors variable handling, template fan-out, provider filter, fail-open under quiet-hours / digest); new `ai_failure_detection` template entry seeded by `seed_notification_templates` with variables `{printer}`, `{task_name}`, `{confidence}`, `{action}`. The seeder only adds templates whose `event_type` is missing, so existing installations get the new template on next start without clobbering customised ones. `obico_actions._notify` swapped to the new method. **Migration.** Branched SQLite (`DEFAULT 0`) vs Postgres (`DEFAULT false`) per the existing stock-alert migration shape at `database.py:2750` — Postgres rejects `DEFAULT 0` for BOOLEAN columns. Existing providers receive the column with the conservative False default; they continue NOT receiving Obico notifications UNTIL they explicitly toggle the new "AI Failure Detection" event ON. This is the intended UX: previously the toggle was on `Printer Error`, which the reporter had OFF, so today they get nothing; after this change they still get nothing until they opt in via the dedicated toggle, but now they can find the toggle without trial-and-error. **Frontend.** New toggle row in `NotificationProviderCard.tsx` (between Printer Error and Low Filament) with a description line "Notify when Obico AI detects a possible print failure" so users discover the link to Obico without having to read source. New summary badge ("AI Failure Detection" in fuchsia) in the collapsed card view so admins can see at a glance which providers route AI alerts. New toggle in `AddNotificationModal.tsx` Printer Status section with matching state hook (`onAiFailureDetection`) wired through the create + update payload. ntfy per-event priority block also picks up the new event when enabled, matching how Printer Error and the stock-alert events behave there. **Schema.** `NotificationProvider` model + `NotificationProviderBase`/`NotificationProviderUpdate` schemas + `_provider_to_dict` route serialiser + create route + PATCH route (the latter uses `model_dump(exclude_unset=True)` so it picks up the new field automatically). Frontend `NotificationProvider` type + the update-payload variant. **i18n.** Two new keys — `notifications.aiFailureDetection` (label) and `notifications.aiFailureDetectionDescription` (help text) — translated in all 11 locales (de / en / es / fr / it / ja / ko / pt-BR / tr / zh-CN / zh-TW). Parity check 5242 leaves per locale, no English fallback. **Tests.** 4 new backend cases in `test_notification_service.py::TestAIFailureDetectionNotifications` (dispatch uses the new event field — NOT the legacy multiplexed one; provider with only `on_printer_error=True` is NOT notified — the regression guard for the reporter's symptom; variables include task_name + 2-decimal-formatted confidence + action; empty task_name falls back to "current job"). 3 new backend cases in `test_obico_actions.py` (`execute_action(action='notify')` calls `on_ai_failure_detection` and explicitly does NOT call `on_printer_error`; the `pause` action still pauses + notifies; notification-service exceptions are swallowed so a transient Discord blip can't kill the Obico detection loop). 5 new frontend cases — 4 in `NotificationProviderCardAiFailureDetection.test.tsx` (badge renders when ON; absent when OFF; toggle appears in expanded settings; toggling PATCHes the correct field and explicitly NOT `on_printer_error`) and 3 in `AddNotificationModal.test.tsx` (toggle renders in Printer Status section; save persists the new field without touching `on_printer_error`; ntfy priority block includes the event when enabled). Existing 87 `test_notification_service.py` + 52 Obico tests + 65 `NotificationProviderCard*` / `AddNotificationModal*` tests still green. Backend `pytest -n 30` clean; ruff clean; `npm run build` clean; ESLint clean. **Scope.** No change to HMS hardware-error notifications — `main.py::on_printer_error` callers still fire the `on_printer_error` event with `error_type` shapes like `"AMS Error"` / `"Heating Error"`, unchanged. The `on_printer_error` column stays on the table (default False, used for HMS only). Users who had it ON for HMS errors keep getting HMS notifications; what they LOSE is silent AI-failure dispatch on the same toggle, which most users with HMS-on never received anyway because `error_type="ai_failure_detection"` was the same value `obico_actions._notify` hardcoded. The full Obico action surface (`notify` / `pause` / `pause_and_off`) is unchanged on the dispatch side — `execute_action` still pauses + cuts plug power for `pause_and_off`; the only thing that moved is which notification-service method runs the fan-out. - **Page-wide drag-and-drop upload on the File Manager (#1510, requested by @maikolscripts)** — File Manager gains the same drag-and-drop upload surface that the Archives page has had: drop any file anywhere on the page and the upload modal opens pre-populated with the dropped files, no need to click the **Upload Files** button first. The hardcoded `"Upload 3MF"` flow was the only path before this change. Unlike the Archives variant — which filters dropped files to `.3mf` only — the File Manager drop zone accepts whatever the upload modal itself accepts (3MF, STL, ZIP, images), so the page-wide surface is never more restrictive than the button it shortcuts. Permission-gated on `library:upload` so a viewer-tier user can't accidentally trigger the overlay. **Shared hook.** `frontend/src/hooks/usePageFileDrop.ts` is the new home for the drag-handler set — `isDraggingOver` state, `dragHandlers` to spread on the wrapper, optional `extensions` filter, optional `onRejected` callback for "you dropped something we won't accept" toasts, `disabled` flag for permission gating. Archives and File Manager both consume it; future drop-zones can opt in without re-implementing the cancel-safe logic. **`FileUploadModal.initialFiles` prop.** Modal accepts a `File[]` to pre-seed itself on first mount via a `seededInitialRef` guard so the same files don't re-add on subsequent renders. Existing manual-open paths (Upload Files button) pass nothing and behave unchanged. **i18n.** New key `fileManager.releaseToUpload` translated in all 11 locales (en: Release to upload, de: Loslassen zum Hochladen, es: Suelte para subir, fr: Relâcher pour téléverser, it: Rilascia per caricare, ja: 離してアップロード, ko: 놓아서 업로드, pt-BR: Solte para enviar, tr: Yüklemek için bırakın, zh-CN: 释放以上传, zh-TW: 釋放以上傳); existing `fileManager.dropFilesHere` reused. Parity 5240 leaves × 11 green, no English fallback. **Tests.** 13 new cases in `src/__tests__/hooks/usePageFileDrop.test.tsx` covering: overlay on dragenter, non-file payload ignored, child-element dragLeave keeps overlay (relatedTarget inside wrapper), outside-element dragLeave hides it, null relatedTarget hides it (cursor left window), document drop / dragend / Escape all reset (the three cancel paths the prior inline implementation missed — see the Fixed entry), drop with mixed file types filters by extension, onRejected fires when extension filter drops everything, disabled is a no-op, overlay clears on successful drop. Existing 85 cases across ArchivesPage / FileManagerPage / FileManagerExternalFolder vitest still green. ESLint clean; `npm run build` clean. diff --git a/backend/app/core/database.py b/backend/app/core/database.py index ca34fef02..a3540eea4 100644 --- a/backend/app/core/database.py +++ b/backend/app/core/database.py @@ -3079,6 +3079,40 @@ async def run_migrations(conn): "ALTER TABLE notification_providers ADD COLUMN on_ai_failure_detection BOOLEAN DEFAULT false", ) + # Migration: Disambiguate the four ``user_print_*`` notification template + # names by appending " Email" (#1792). See ``_migrate_rename_user_print_template_names``. + await _migrate_rename_user_print_template_names(conn) + + +_USER_PRINT_TEMPLATE_RENAMES: tuple[tuple[str, str, str], ...] = ( + ("user_print_start", "User Print Started", "User Print Started Email"), + ("user_print_complete", "User Print Completed", "User Print Completed Email"), + ("user_print_failed", "User Print Failed", "User Print Failed Email"), + ("user_print_stopped", "User Print Stopped", "User Print Stopped Email"), +) + + +async def _migrate_rename_user_print_template_names(conn) -> None: + """Append " Email" to the four ``user_print_*`` notification template names (#1792). + + The provider-level "Print Completed" and the per-user "User Print Completed" + rows were visually indistinguishable in the Message Templates list because + the seed name lacked the suffix that the EVENT_NAMES display map in + routes/notification_templates.py already uses ("User Print Completed Email"). + + Renames only rows where ``name`` is still the old default — admins who + renamed the template themselves keep their custom name. Standard SQL + UPDATE works on both SQLite and Postgres. + """ + from sqlalchemy import text + + async with conn.begin_nested(): + for event_type, old_name, new_name in _USER_PRINT_TEMPLATE_RENAMES: + await conn.execute( + text("UPDATE notification_templates SET name = :new WHERE event_type = :et AND name = :old"), + {"new": new_name, "et": event_type, "old": old_name}, + ) + async def seed_notification_templates(): """Seed default notification templates if they don't exist.""" diff --git a/backend/app/models/notification_template.py b/backend/app/models/notification_template.py index 450514681..8ff87f193 100644 --- a/backend/app/models/notification_template.py +++ b/backend/app/models/notification_template.py @@ -195,28 +195,32 @@ DEFAULT_TEMPLATES = [ "title_template": "Stock Break Risk: {material}", "body_template": "{material} ({brand}) will run out before replenishment arrives.\nStock: {stock_g}g | Rate: {rate_g_day}g/day | Lead time: {lead_time_days}d\nOnly {days_left}d of stock remaining — order immediately.", }, - # User email notification templates (sent to the print job owner) + # User email notification templates (sent to the print job owner). + # Names include " Email" so they aren't confused with the provider-level + # `print_*` templates above, which share the same body shape but are + # broadcast to admin-configured providers (ntfy/pushover/telegram/discord/ + # etc.) rather than mailed to a specific user. { "event_type": "user_print_start", - "name": "User Print Started", + "name": "User Print Started Email", "title_template": "Your Print Has Started", "body_template": "Hello {username},\n\nYour print job has started on {printer}.\n\nFile: {filename}\n\nYou will be notified when it completes.", }, { "event_type": "user_print_complete", - "name": "User Print Completed", + "name": "User Print Completed Email", "title_template": "Your Print Is Complete", "body_template": "Hello {username},\n\nYour print job has completed on {printer}.\n\nFile: {filename}", }, { "event_type": "user_print_failed", - "name": "User Print Failed", + "name": "User Print Failed Email", "title_template": "Your Print Has Failed", "body_template": "Hello {username},\n\nYour print job has failed on {printer}.\n\nFile: {filename}", }, { "event_type": "user_print_stopped", - "name": "User Print Stopped", + "name": "User Print Stopped Email", "title_template": "Your Print Has Been Stopped", "body_template": "Hello {username},\n\nYour print job was stopped on {printer}.\n\nFile: {filename}", }, diff --git a/backend/app/services/notification_service.py b/backend/app/services/notification_service.py index 7a2e00808..932c2c50c 100644 --- a/backend/app/services/notification_service.py +++ b/backend/app/services/notification_service.py @@ -1,11 +1,13 @@ """Notification service for sending push notifications via various providers.""" import asyncio +import html import json import logging import re import smtplib from datetime import datetime, timedelta, timezone +from email.mime.image import MIMEImage from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText from typing import Any @@ -410,8 +412,27 @@ class NotificationService: else: return False, f"HTTP {response.status_code}: {response.text[:200]}" - async def _send_email(self, config: dict, subject: str, body: str) -> tuple[bool, str]: - """Send notification via email (SMTP).""" + async def _send_email( + self, + config: dict, + subject: str, + body: str, + image_data: bytes | None = None, + finish_photo_url: str | None = None, + ) -> tuple[bool, str]: + """Send notification via email (SMTP). + + Inline finish-photo embed is opt-in via the template: when the rendered + ``body`` contains the substituted ``{finish_photo_url}`` value AND the + finish-photo bytes are present, the message is built as + ``multipart/related`` wrapping a ``multipart/alternative`` (plain + HTML) + plus an inline ``MIMEImage`` with ``Content-ID: ``. + The HTML part replaces the URL with ````; the plain- + text part keeps the URL as a clickable link. When the template doesn't + reference ``{finish_photo_url}`` (or image bytes aren't available), the + original single-part text shape is used — no attachment, no surprise + inline image (#1792). + """ smtp_server = config.get("smtp_server", "").strip() smtp_port = int(config.get("smtp_port", 587)) username = config.get("username", "").strip() @@ -429,12 +450,48 @@ class NotificationService: if auth_enabled and not all([username, password]): return False, "Username and password are required when authentication is enabled" + # Template-driven: only inline-embed when the user's template explicitly + # referenced {finish_photo_url} (so the URL appears in the rendered body) + # AND the photo bytes are available. Falls back to text-only otherwise. + inline_photo = bool(image_data and finish_photo_url and finish_photo_url in body) + try: - msg = MIMEMultipart() - msg["From"] = from_email - msg["To"] = to_email - msg["Subject"] = f"[Bambuddy] {subject}" - msg.attach(MIMEText(body, "plain")) + if inline_photo: + # multipart/related → (multipart/alternative → text, html) + inline image + msg = MIMEMultipart("related") + msg["From"] = from_email + msg["To"] = to_email + msg["Subject"] = f"[Bambuddy] {subject}" + + alt = MIMEMultipart("alternative") + alt.attach(MIMEText(body, "plain")) + # Build HTML body: escape the rendered body, then swap the + # escaped URL substring for an inline referencing the + # MIMEImage we attach below. Done AFTER escape so the cid: URL + # we inject isn't re-escaped. + escaped_body = html.escape(body).replace("\n", "
\n") + escaped_url = html.escape(finish_photo_url) + img_tag = ( + '' + ) + html_body = f"

{escaped_body.replace(escaped_url, img_tag)}

" + alt.attach(MIMEText(html_body, "html")) + msg.attach(alt) + + img = MIMEImage(image_data, _subtype="jpeg") + # Angle-bracketed Content-ID per RFC 2392, referenced from HTML + # without the brackets via ``cid:bambuddy-finish-photo``. + img.add_header("Content-ID", "") + img.add_header("Content-Disposition", "inline", filename="finish-photo.jpg") + msg.attach(img) + else: + msg = MIMEMultipart() + msg["From"] = from_email + msg["To"] = to_email + msg["Subject"] = f"[Bambuddy] {subject}" + msg.attach(MIMEText(body, "plain")) if security == "ssl": # Direct SSL connection (typically port 465) @@ -682,7 +739,13 @@ class NotificationService: elif provider.provider_type == "telegram": return await self._send_telegram(config, f"*{title}*\n{message}", image_data=image_data) elif provider.provider_type == "email": - return await self._send_email(config, title, message) + # finish_photo_url is pulled from the rendered template variables + # so _send_email can detect whether the template referenced the + # URL and inline-embed the photo only in that case. + finish_photo_url = (variables or {}).get("finish_photo_url") + return await self._send_email( + config, title, message, image_data=image_data, finish_photo_url=finish_photo_url + ) elif provider.provider_type == "discord": return await self._send_discord(config, title, message, image_data=image_data) elif provider.provider_type == "webhook": diff --git a/backend/tests/unit/services/test_notification_service.py b/backend/tests/unit/services/test_notification_service.py index b52372099..a65db277e 100644 --- a/backend/tests/unit/services/test_notification_service.py +++ b/backend/tests/unit/services/test_notification_service.py @@ -2346,3 +2346,199 @@ class TestNtfyOutbound: assert ok is False assert "Cloudflare" in detail + + +class TestEmailProvider: + """Tests for SMTP email provider, including #1792 finish-photo inline embed. + + Embed is opt-in via the template: only when the user's template referenced + ``{finish_photo_url}`` (so the URL appears in the rendered body) AND the + photo bytes are available does ``_send_email`` build the multipart/related + shape. Otherwise it stays single-part text — no surprise inline image. + """ + + PHOTO_URL = "https://printer.local/api/v1/archives/42/photos/finish.jpg" + + @pytest.fixture + def service(self): + return NotificationService() + + @pytest.fixture + def smtp_config(self): + return { + "smtp_server": "smtp.example.com", + "smtp_port": "587", + "username": "alice", + "password": "secret", + "from_email": "bambuddy@example.com", + "to_email": "alice@example.com", + "security": "starttls", + "auth_enabled": "true", + } + + @staticmethod + def _fake_smtp_class(captured: dict): + class FakeSMTP: + def __init__(self, host, port): + captured["host"] = host + captured["port"] = port + + def starttls(self): + captured["starttls"] = True + + def login(self, u, p): + captured["login"] = (u, p) + + def sendmail(self, frm, to, body): + captured["from"] = frm + captured["to"] = to + captured["raw"] = body + + def quit(self): + captured["quit"] = True + + return FakeSMTP + + @pytest.mark.asyncio + async def test_email_without_image_or_url_stays_text_only(self, service, smtp_config): + """No image_data and no URL in body → original single-part text shape.""" + captured: dict = {} + with patch("backend.app.services.notification_service.smtplib.SMTP", self._fake_smtp_class(captured)): + ok, _ = await service._send_email(smtp_config, "Print Failed", "Reason: unknown") + + assert ok is True + assert "image/jpeg" not in captured["raw"] + assert "multipart/related" not in captured["raw"] + assert "cid:bambuddy-finish-photo" not in captured["raw"] + assert "Reason: unknown" in captured["raw"] + + @pytest.mark.asyncio + async def test_email_image_without_template_reference_stays_text_only(self, service, smtp_config): + """image_data present but template didn't include {finish_photo_url} → no embed. + + Pins the template-driven contract: a user whose body is just + "Print failed. Reason: unknown" does NOT get a surprise inline image + stapled to the bottom, even though the photo bytes are available + upstream from the archive. + """ + captured: dict = {} + with patch("backend.app.services.notification_service.smtplib.SMTP", self._fake_smtp_class(captured)): + ok, _ = await service._send_email( + smtp_config, + "Print Failed", + "Reason: unknown", + image_data=b"\xff\xd8\xff\xe0jpeg", + finish_photo_url=self.PHOTO_URL, + ) + + assert ok is True + raw = captured["raw"] + assert "image/jpeg" not in raw + assert "multipart/related" not in raw + assert "cid:bambuddy-finish-photo" not in raw + + @pytest.mark.asyncio + async def test_email_inlines_when_template_uses_finish_photo_url(self, service, smtp_config): + """URL in body + image_data present → multipart/related + cid embed; HTML swaps URL for .""" + captured: dict = {} + body = f"Print failed. Reason: unknown\n\nSnapshot: {self.PHOTO_URL}" + + with patch("backend.app.services.notification_service.smtplib.SMTP", self._fake_smtp_class(captured)): + ok, _ = await service._send_email( + smtp_config, + "Print Failed", + body, + image_data=b"\xff\xd8\xff\xe0fake-jpeg-bytes", + finish_photo_url=self.PHOTO_URL, + ) + + assert ok is True + raw = captured["raw"] + # multipart/related shape with both alt parts and an image part + assert "multipart/related" in raw + assert "multipart/alternative" in raw + assert "text/plain" in raw + assert "text/html" in raw + assert "image/jpeg" in raw + # HTML references the exact cid the Content-ID header registers + assert "Content-ID: " in raw + assert 'src="cid:bambuddy-finish-photo"' in raw + # Inline disposition so renders embedded, not as download attachment + assert 'Content-Disposition: inline; filename="finish-photo.jpg"' in raw + # Plain-text body keeps the URL so non-HTML clients still get a clickable link + assert self.PHOTO_URL in raw + + @pytest.mark.asyncio + async def test_email_image_data_without_url_arg_stays_text_only(self, service, smtp_config): + """image_data passed but finish_photo_url=None → defence-in-depth, no embed. + + Even if a future caller forgets to thread the URL through but does pass + the bytes, the conservative default is no embed (avoids attaching an + unreferenced image to an unrelated event type). + """ + captured: dict = {} + with patch("backend.app.services.notification_service.smtplib.SMTP", self._fake_smtp_class(captured)): + ok, _ = await service._send_email( + smtp_config, + "Print Failed", + f"Snapshot: {self.PHOTO_URL}", + image_data=b"\xff\xd8\xff\xe0jpeg", + finish_photo_url=None, + ) + + assert ok is True + assert "image/jpeg" not in captured["raw"] + assert "multipart/related" not in captured["raw"] + + @pytest.mark.asyncio + async def test_email_html_body_escapes_user_content(self, service, smtp_config): + """Template-rendered body must not be injected raw into the HTML part.""" + captured: dict = {} + body = f"Filename: \nLine 2\nSnapshot: {self.PHOTO_URL}" + + with patch("backend.app.services.notification_service.smtplib.SMTP", self._fake_smtp_class(captured)): + ok, _ = await service._send_email( + smtp_config, + "Print Failed", + body, + image_data=b"\xff\xd8\xff\xe0jpeg", + finish_photo_url=self.PHOTO_URL, + ) + + assert ok is True + raw = captured["raw"] + # Raw HTML must NOT round-trip into the HTML part — verify escaped form is present. + assert "<script>alert(1)</script>" in raw + # Newlines in the body become
in HTML + assert "Line 2" in raw + assert "
" in raw + + @pytest.mark.asyncio + async def test_email_html_swaps_url_for_img_tag(self, service, smtp_config): + """In the HTML part, the URL substring is replaced with the tag. + + Plain text keeps the URL; HTML clients see the inline image where the + URL was. The URL must NOT appear inside an wrapping the image + — we replace the URL outright with the img tag (renderers don't need + the URL twice in the HTML part when the image is already inline). + """ + captured: dict = {} + body = f"See: {self.PHOTO_URL} for the snapshot." + + with patch("backend.app.services.notification_service.smtplib.SMTP", self._fake_smtp_class(captured)): + ok, _ = await service._send_email( + smtp_config, + "Print Failed", + body, + image_data=b"\xff\xd8\xff\xe0jpeg", + finish_photo_url=self.PHOTO_URL, + ) + + assert ok is True + raw = captured["raw"] + # The tag appears in the HTML part + assert 'src="cid:bambuddy-finish-photo"' in raw + # The escaped URL is the marker we replaced — the HTML part should not + # contain BOTH the escaped URL AND the cid img (we swapped, not duplicated). + # The plain-text part still has the URL; check it's there at least once. + assert self.PHOTO_URL in raw diff --git a/backend/tests/unit/test_user_print_template_rename_migration.py b/backend/tests/unit/test_user_print_template_rename_migration.py new file mode 100644 index 000000000..337655a79 --- /dev/null +++ b/backend/tests/unit/test_user_print_template_rename_migration.py @@ -0,0 +1,146 @@ +"""Regression test for the user_print_* notification template rename migration (#1792). + +The four ``user_print_*`` notification templates seeded with names like +"User Print Completed" looked indistinguishable from the provider-level +"Print Completed" template in the Message Templates list (the EVENT_NAMES +display map in routes/notification_templates.py already used the disambiguated +"User Print Completed Email" label, but the seed wrote the short name to the +DB, so the UI rendered the ambiguous one). + +The migration appends " Email" to those four template names IF AND ONLY IF +the row still has the old default name — admins who renamed the template +themselves keep their custom name. This test verifies both branches. +""" + +from __future__ import annotations + +import pytest +from sqlalchemy import text +from sqlalchemy.ext.asyncio import create_async_engine + +from backend.app.core.database import _migrate_rename_user_print_template_names + + +@pytest.fixture +async def engine(): + """In-memory SQLite with just the notification_templates table. + + The migration is a single UPDATE on one table, so the fixture only needs + that table — avoids the brittleness of registering every model in the + project just to satisfy run_migrations's broader DDL surface. + """ + from backend.app.models.notification_template import NotificationTemplate + + engine = create_async_engine("sqlite+aiosqlite:///:memory:", echo=False) + async with engine.begin() as conn: + await conn.run_sync(NotificationTemplate.__table__.create) + try: + yield engine + finally: + await engine.dispose() + + +_OLD_DEFAULTS = { + "user_print_start": "User Print Started", + "user_print_complete": "User Print Completed", + "user_print_failed": "User Print Failed", + "user_print_stopped": "User Print Stopped", +} +_NEW_DEFAULTS = { + "user_print_start": "User Print Started Email", + "user_print_complete": "User Print Completed Email", + "user_print_failed": "User Print Failed Email", + "user_print_stopped": "User Print Stopped Email", +} + + +async def _insert_template(conn, event_type: str, name: str) -> None: + await conn.execute( + text( + "INSERT INTO notification_templates " + "(event_type, name, title_template, body_template, is_default) " + "VALUES (:et, :n, 't', 'b', 1)" + ), + {"et": event_type, "n": name}, + ) + + +async def _name_for(conn, event_type: str) -> str: + return ( + await conn.execute( + text("SELECT name FROM notification_templates WHERE event_type = :et"), + {"et": event_type}, + ) + ).scalar_one() + + +async def test_migration_renames_default_named_user_print_rows(engine): + """Rows with the old default name get the new disambiguated name.""" + async with engine.begin() as conn: + for event_type, old_name in _OLD_DEFAULTS.items(): + await _insert_template(conn, event_type, old_name) + + async with engine.begin() as conn: + await _migrate_rename_user_print_template_names(conn) + + async with engine.begin() as conn: + for event_type, new_name in _NEW_DEFAULTS.items(): + assert await _name_for(conn, event_type) == new_name + + +async def test_migration_preserves_user_edited_names(engine): + """An admin who renamed a template keeps their custom name across the migration.""" + async with engine.begin() as conn: + await _insert_template(conn, "user_print_complete", "My Custom Renamed Template") + await _insert_template(conn, "user_print_failed", "User Print Failed") # still default + + async with engine.begin() as conn: + await _migrate_rename_user_print_template_names(conn) + + async with engine.begin() as conn: + # Custom name preserved + assert await _name_for(conn, "user_print_complete") == "My Custom Renamed Template" + # Default name renamed + assert await _name_for(conn, "user_print_failed") == "User Print Failed Email" + + +async def test_migration_does_not_touch_provider_templates(engine): + """The non-user provider templates with similar names must not be renamed.""" + async with engine.begin() as conn: + await _insert_template(conn, "print_complete", "Print Completed") + await _insert_template(conn, "print_failed", "Print Failed") + + async with engine.begin() as conn: + await _migrate_rename_user_print_template_names(conn) + + async with engine.begin() as conn: + assert await _name_for(conn, "print_complete") == "Print Completed" + assert await _name_for(conn, "print_failed") == "Print Failed" + + +async def test_migration_is_idempotent(engine): + """Running the migration twice must not double-suffix already-renamed rows.""" + async with engine.begin() as conn: + for event_type, old_name in _OLD_DEFAULTS.items(): + await _insert_template(conn, event_type, old_name) + + async with engine.begin() as conn: + await _migrate_rename_user_print_template_names(conn) + async with engine.begin() as conn: + await _migrate_rename_user_print_template_names(conn) + + async with engine.begin() as conn: + for event_type, new_name in _NEW_DEFAULTS.items(): + current = await _name_for(conn, event_type) + assert current == new_name + assert "Email Email" not in current + + +async def test_migration_handles_empty_table(engine): + """Migration on an empty table must be a safe no-op (fresh install path).""" + async with engine.begin() as conn: + await _migrate_rename_user_print_template_names(conn) + + async with engine.begin() as conn: + count = (await conn.execute(text("SELECT COUNT(*) FROM notification_templates"))).scalar_one() + assert count == 0