fix(ui): restore missing per-user Notifications nav item (#1901)

The sidebar-ordering refactor in #1673 accidentally dropped the
`notifications` entry from `defaultNavItems` and its
`notifications:user_email` permission mapping, but kept the advanced-auth
visibility gate that references that id. With no nav entry the id never
enters the render set, so the /notifications page (route, page, and API
all intact) became reachable only by typing the URL — users could no
longer opt in/out of their own print email notifications from the menu.

Restore both the defaultNavItems entry and the permission gate, matching
the permission the user-email-preferences API actually requires
(notifications:user_email, held by both default groups). Add comments so
the entry isn't dropped again in a future sidebar refactor.
This commit is contained in:
maziggy
2026-07-06 07:38:44 +02:00
parent f3450e60fd
commit a82eeff483
5 changed files with 17 additions and 5 deletions
+1
View File
@@ -5,6 +5,7 @@ All notable changes to Bambuddy will be documented in this file.
## [0.2.5b2] - Unreleased
### Fixed
- **Per-user Notifications page unreachable from the sidebar (#1901, reporter @JmanB52D)** — The Notifications entry (where each user opts in/out of their own print email notifications) disappeared from the left navigation. The page (`/notifications`) and its API were both intact — only the sidebar link was gone, so the screen was reachable only by typing the URL. Root cause: the sidebar-ordering refactor in #1673 accidentally deleted the `notifications` item from `defaultNavItems` (and its `notifications:user_email` permission mapping) while extracting the ordering helpers, but left the advanced-auth visibility gate that references that id — so the gate had nothing to gate and the item could never render. Restored both the `defaultNavItems` entry and the permission gate; the item now shows for any user holding `notifications:user_email` (both default groups, Administrators and Operators, do) when advanced auth and user email notifications are enabled, exactly as before #1673.
- **Virtual Printer FTP uploads silently truncated under uvloop — a corrupt `.gcode.3mf` was archived, queued, and forwarded to the real printer with a `226 Transfer complete` (#1896, reporter @dj-oyu)** — On a native venv install (not Docker), slicing in Bambu Studio and sending to a queue-mode VP produced a truncated upload: Bambuddy logged `226 Transfer complete`, archived the file, added it to the queue, and later pushed the corrupt file to the physical printer (A1 mini), which then failed to parse/start the job. Every truncated file ended at an exact multiple of 4096 bytes with a valid `PK\x03\x04` local header but no ZIP End-Of-Central-Directory record. **Root cause — isolated deterministically by the reporter.** uvloop's SSL layer discards already-received but still-buffered data when the client closes the data connection **without a TLS `close_notify`** (a "ragged EOF") while the reader is **flow-control-paused** (slow consumer). `cmd_STOR` writes each 64 KiB chunk to disk synchronously inside the read loop; on slow storage (the reporter's data dir was on a microSD on an ARM64 SBC) the reader falls behind, the transport pauses, and the tail of the upload is lost — `read()` then returns a clean empty EOF, so the loop exits normally with **no exception and no write error**, and the server acks 226 for a file it truncated itself. The reporter's isolation matrix reproduces it on a minimal uvloop 0.22.1 TLS server (slow reader → 2,248,704 of 2,500,001 bytes, 3/3 runs) but never on CPython's default asyncio loop or on uvloop with a fast reader — which is why Docker/x86-with-SSD deployments almost never hit it (the reader keeps up, flow control never pauses, the loss window never opens). Bambuddy's Dockerfile already runs `--loop asyncio`; **every native launch path did not** and so auto-selected uvloop via `uvicorn[standard]`. **Fix — two independent layers.** (1) *Remove the trigger.* Added `--loop asyncio` to every native launch path so uploads actually arrive intact, matching the Dockerfile: `deploy/bambuddy.service`, `install/install.sh` (systemd unit + macOS launchd plist), `spoolbuddy/install/install.sh` (the bundled Bambuddy backend service), `installers/windows/service/install-service.bat` (NSSM), `README.md`, and the wiki install docs (run command, systemd, launchd) — each with an inline "do not remove, see #1896" note. The reporter verified `--loop asyncio` fully resolves it (before: 8/8 real Bambu Studio uploads truncated; after: 2/2 intact, valid ZIPs, `testzip` clean). (2) *Defense in depth, loop-independent.* `cmd_STOR` now validates that a received `.3mf` opens as a ZIP (reads the central directory — O(dir), no decompression) **before** replying 226. A truncated/corrupt 3MF is treated exactly like a failed transfer: the file is dropped and the slicer gets `426 Transfer failed: uploaded 3MF is incomplete or corrupt`, and the `on_file_received` callback that archives/queues/forwards the job never runs — so a broken upload surfaces as an immediate, actionable slicer-side send error instead of a confusing printer-side parse failure later. Validation is scoped to `.3mf` uploads; other filetypes keep the prior pass-through behaviour. This layer protects anyone who still runs uvloop for any reason (custom launch command, future `uvicorn[standard]` default). **Tests.** `test_vp_ftp_stor.py`: the happy-path test now feeds a real multi-chunk ZIP and asserts 226 (not 426); new `test_stor_rejects_truncated_3mf` drops the EOCD-bearing tail and asserts 426 + file removed + `on_file_received` never called; new `test_stor_skips_zip_validation_for_non_3mf` asserts a plain `.gcode` still gets 226 (no false-positive). 6/6 in the file green, ruff clean. **Scope.** Backend (one validation block in `cmd_STOR` + `zipfile` import) plus launch-config across repo installers, the Windows/SpoolBuddy installers, README, and the install wiki. No DB migration, no new permission, no i18n key, no frontend change. Users on a native install: after upgrade, re-run the installer (or add `--loop asyncio` to your existing service command) to stop the truncation at the source; the ZIP-validation guard takes effect on the next Bambuddy restart regardless. **Workaround for older versions:** launch uvicorn with `--loop asyncio`.
- **API keys could not manage Projects — every project mutation returned `403 "API keys cannot be used for administrative operations"` regardless of the key's granted permissions (#1893, reporter @abbasegbeyemi)** — `POST /projects/{id}/add-archives`, project create/update/delete, and every other project mutation route was unreachable for any API key. **Root cause.** `PROJECTS_CREATE` / `PROJECTS_UPDATE` / `PROJECTS_DELETE` were in `_APIKEY_DENIED_PERMISSIONS` in `core/auth.py` with no corresponding entry in `_APIKEY_SCOPE_BY_PERMISSION` and no `can_manage_projects` flag on `api_keys` at all — so under the GHSA-r2qv allowlist model they resolved to scope `None` and raised the generic administrative-operations 403. This is the exact regression class already fixed for archives (#1888) and library (#1832): the projects block sat directly between the comment blocks documenting those two carve-outs but was never itself carved out. **Fix.** New per-key scope `can_manage_projects` (column on `api_keys`, DEFAULT TRUE for keys created via the UI going forward; existing rows backfill to FALSE so the upgrade path never silently widens scope — these permissions were explicitly denied for every key before, so nothing relies on them). Unlike archives/library, the project routes gate on plain `RequirePermissionIfAuthEnabled(Permission.PROJECTS_*)` — there is no OWN/ALL ownership split for projects — so all three CRUD permissions map directly to the one scope. Project **membership** edits (`add_archives_to_project` etc.) gate on `PROJECTS_UPDATE`, so they're covered by the same toggle; `PROJECTS_READ` is unchanged (already under `can_read_status`, so API keys could always read projects). Users opt a key in from Settings → API Keys ("Manage Projects" toggle, with a "Projects" badge on the key list). The bundled SpoolBuddy kiosk key (created via the CLI) is set to `can_manage_projects=False` to stay minimally scoped. **Migration** is dialect-agnostic (`BOOLEAN` is valid on both SQLite and Postgres); verified end-to-end on a throwaway fresh SQLite and Postgres 17 that the column adds, legacy rows backfill to FALSE, and a new row defaults to TRUE. **Tests.** `test_auth_apikey_rbac.py` extended: the `_check_apikey_permissions` scope matrix now covers all three project permissions (true→allow, false→403, no cross-scope leakage), and `PROJECTS_CREATE` / `_UPDATE` / `_DELETE` added to the operational-allowed drift guard + threaded through the structural allowlist/flag-parity checks — 63 cases green. **Scope.** Backend (model + migration + allowlist + schema + route + CLI) plus the Settings API-key UI (toggle + badge + type) and 11-locale i18n for the new label/description/badge. No change to the project routes themselves — they already gated on the right permissions; only the API-key classification of those permissions was wrong.
- **Auto-drying stopped a manually started AMS drying cycle after exactly 30 minutes, cutting long PETG/PA dries short (#1892, reporter @Spionkiller01)** — With ambient/queue auto-drying enabled, starting a drying cycle *manually on the printer* (or a cycle that survived a Bambuddy restart) got a stop-drying MQTT command ~30 minutes in, every time — killing an intended 8-12 h cycle. The reporter had Bambu Lab support analyse the printer logs, which confirmed an external tool issued the stop; that tool was Bambuddy. **Root cause — two defects compounding in `_check_auto_drying()` (`backend/app/services/print_scheduler.py`).** (1) The already-drying branch carried the comment *"Drying we didn't start (manual or from before restart) — track but don't stop"* but the very next lines applied the humidity-based auto-stop to it anyway; a manually started dry was treated identically to a Bambuddy-initiated one. (2) The humidity re-check is fundamentally unreliable: relative humidity drops steeply in heated air, so the AMS sensor reads ~15-20% within minutes of the dryer starting even while the filament is still saturated (the reporter's log shows 18%). So `humidity <= threshold` is effectively *always true* once drying runs, and the only thing delaying the stop was the `_min_drying_seconds = 1800` floor — which is why the kill landed at exactly the 30-minute mark. This second defect also silently truncated Bambuddy's **own** preset-duration dries (e.g. a PETG 8 h cycle) to ~30 min, not just manual ones. **Fix.** Removed the humidity-based early-stop entirely — a running drying cycle is now left to run to its configured duration, which the firmware stops when the duration elapses. This is simpler and more correct than exempting only manual dries (the reporter's suggested `_manual_drying` set), because the humidity re-check can't distinguish "filament is dry" from "air is hot" for *any* cycle, so it never did its intended job — it just always fired at the floor. Scheduling-driven stops are unaffected and still work through `_stop_drying()`: a print taking priority, or queue-mode no longer needing the dry, still stops it. The now-unused `_min_drying_seconds` attribute was removed. Bambuddy still *starts* auto-drying on the same humidity-over-threshold trigger; only the mid-cycle humidity re-stop is gone. **Tests.** `test_scheduler_auto_drying.py` updated: `TestMinimumDryingTime` now pins the #1892 contract (a running dry is never stopped by a humidity re-check — before or long after the old floor, including when humidity reads low), and `TestBlockForDryingBugFix` asserts an already-running dry in block mode is left alone (block mode still gates *new* starts on printers with pending items). 51/51 in the file green, ruff clean. **Scope.** Backend-only, one branch simplified in `_check_auto_drying` plus the attribute removal. No DB migration, no new permission, no i18n key, no frontend change. Users on 0.2.5b1 and earlier: the fix takes effect on the next Bambuddy restart. **Workaround for older versions:** disable ambient/queue auto-drying in Settings before starting a manual drying cycle.
@@ -303,7 +303,7 @@ describe('SettingsPage', () => {
expect(localStorage.setItem).toHaveBeenCalledWith(
SIDEBAR_ORDER_KEY,
JSON.stringify(['ext-7', 'printers', 'inventory', 'archives', 'queue', 'projects', 'files', 'makerworld', 'profiles', 'maintenance', 'stats', 'settings']),
JSON.stringify(['ext-7', 'printers', 'inventory', 'archives', 'queue', 'projects', 'files', 'makerworld', 'profiles', 'maintenance', 'stats', 'notifications', 'settings']),
);
});
@@ -345,7 +345,7 @@ describe('SettingsPage', () => {
expect(localStorage.setItem).toHaveBeenCalledWith(SIDEBAR_HIDDEN_SYSTEM_ITEMS_KEY, JSON.stringify([]));
expect(localStorage.setItem).toHaveBeenCalledWith(
SIDEBAR_ORDER_KEY,
JSON.stringify(['printers', 'inventory', 'archives', 'queue', 'projects', 'files', 'makerworld', 'profiles', 'maintenance', 'stats', 'settings', 'ext-7']),
JSON.stringify(['printers', 'inventory', 'archives', 'queue', 'projects', 'files', 'makerworld', 'profiles', 'maintenance', 'stats', 'notifications', 'settings', 'ext-7']),
);
const settingsRow = screen.getAllByText('Settings')
@@ -416,6 +416,7 @@ describe('SettingsPage', () => {
'profiles',
'maintenance',
'stats',
'notifications',
'settings',
],
hiddenSystemItemIds: ['stats'],
+11 -1
View File
@@ -1,6 +1,6 @@
import { useState, useEffect, useCallback, useRef, useMemo } from 'react';
import { NavLink, Outlet, useNavigate, useLocation } from 'react-router-dom';
import { Printer, Archive, ListOrdered, BarChart3, Cloud, Settings, Sun, Moon, Monitor, ChevronLeft, ChevronRight, Keyboard, Github, ArrowUpCircle, Wrench, FolderKanban, FolderOpen, X, Menu, Info, Plug, Bug, LogOut, Key, Loader2, Disc3, ShieldAlert, Globe, type LucideIcon } from 'lucide-react';
import { Printer, Archive, ListOrdered, BarChart3, Cloud, Settings, Sun, Moon, Monitor, ChevronLeft, ChevronRight, Keyboard, Github, ArrowUpCircle, Wrench, FolderKanban, FolderOpen, X, Menu, Info, Plug, Bug, LogOut, Key, Loader2, Disc3, ShieldAlert, Globe, Bell, type LucideIcon } from 'lucide-react';
import { useTranslation } from 'react-i18next';
import { useTheme } from '../contexts/ThemeContext';
import { KeyboardShortcutsModal } from './KeyboardShortcutsModal';
@@ -48,6 +48,11 @@ export const defaultNavItems: NavItem[] = [
{ id: 'profiles', to: '/profiles', icon: Cloud, labelKey: 'nav.profiles' },
{ id: 'maintenance', to: '/maintenance', icon: Wrench, labelKey: 'nav.maintenance' },
{ id: 'stats', to: '/stats', icon: BarChart3, labelKey: 'nav.stats' },
// User-account feature: gated in isHidden() on advanced auth + user_notifications
// + the notifications:user_email permission. Kept adjacent to Settings
// intentionally. Do not drop this entry — without it the /notifications page
// is orphaned (route + page still exist but no nav link) (#1901).
{ id: 'notifications', to: '/notifications', icon: Bell, labelKey: 'nav.notifications' },
{ id: 'settings', to: '/settings', icon: Settings, labelKey: 'nav.settings' },
];
@@ -296,6 +301,11 @@ export function Layout() {
files: ['library:read', 'library:read_own', 'library:read_all'],
makerworld: 'makerworld:view',
settings: 'settings:read',
// The user-email-preferences API requires notifications:user_email, so
// gate the nav item on the same permission (both default groups —
// Administrators and Operators — hold it). The advanced-auth /
// user_notifications enablement gate is applied separately below.
notifications: 'notifications:user_email',
};
const isHidden = (id: string) => {
File diff suppressed because one or more lines are too long
+1 -1
View File
@@ -26,7 +26,7 @@
<!-- Splash screens for iOS -->
<link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
<script type="module" crossorigin src="/assets/index-Bwc9H1Fy.js"></script>
<script type="module" crossorigin src="/assets/index-DqJZ0C8s.js"></script>
<link rel="stylesheet" crossorigin href="/assets/index-BxVhuRti.css">
</head>
<body>