mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-09-30 03:01:21 +02:00
fix(local-presets): optimistic remove on delete
The Slicer -> Local Profiles page kept showing a just-deleted row for
the ~hundreds of ms it took invalidateQueries to refetch. A quick
re-click on the same row opened a second delete-confirm modal that
resolved to a 404 from the backend.
Add an optimistic queryClient.setQueryData filter in deleteMutation's
onSuccess so the row disappears the instant the DELETE returns 200.
Existing invalidateQueries calls stay in place to reconcile any drift.
Found while reproducing #1713 (verifying maziggy's setup against the
reporter's). Unrelated to that investigation but caught here.
This commit is contained in:
@@ -12,6 +12,7 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
- **Admin-configurable session lifetime (#1706, reported by @AD3DStuff)** — The 24-hour session cap that ships with Bambuddy was an intentional security hardening (audit finding M-2 reduced it from 7 days), but the "Remember Me" checkbox only controlled storage location (localStorage vs sessionStorage), not session duration. iPhone PWA users and homelab admins on trusted networks were getting kicked out every 24 hours with no way to extend it. **New setting:** `session_max_hours` under Settings → Users with three presets (24h / 7 days / 30 days) plus a custom field, hard-capped at 30 days (720h). Default remains 24h so existing deployments and the M-2 audit baseline are untouched until an admin opts in. The Settings card surfaces a yellow warning whenever the value exceeds 24h: "Longer sessions reduce automatic logout protection. Recommended only for trusted single-user deployments." **Backend wiring:** new `resolve_session_max_minutes(db)` helper in `backend/app/core/auth.py` reads the setting, clamps to [1h, 720h], and falls back to 24h on missing / blank / unparseable values. The helper is called at all four token-issuance sites — plain `/auth/login`, 2FA TOTP/email completion, 2FA backup-code completion, and OIDC callback — so a long-session policy works uniformly regardless of how the user authenticates. DB errors in the resolver are deliberately NOT caught: login is already inside a transaction and a broken DB must abort the login rather than silently extend or shrink the session lifetime. Defense-in-depth `SESSION_MAX_HOURS_HARD_CEILING = 720` clamps any tampered DB row above the Pydantic ceiling. Already-issued tokens keep their original expiry — the new setting only affects future logins, so an admin lowering the value can't retroactively revoke active sessions and an admin raising it can't retroactively extend them. **What this does NOT change:** the "Remember Me" checkbox still controls only storage location (cleared on browser close vs persisted across restarts). The relabel from misleading-UX-perspective is left for a separate follow-up — that's a UX choice independent of the session-policy mechanism. API tokens (`MAX_TOKEN_LIFETIME_DAYS`), camera stream tokens (60min), WebSocket tokens (60min), and slicer download tokens (5min) keep their own TTLs and are unaffected. **Tests:** 15 new cases in `backend/tests/integration/test_session_policy.py` split across three classes. `TestResolveSessionMaxMinutes` pins the clamping resolver — missing row, empty string, unparseable value, zero/negative, 1h minimum, 7-day passthrough, 30-day passthrough, above-ceiling clamp. `TestLoginRespectsSessionPolicy` decodes the JWT `exp` claim end-to-end and asserts the token returned by `/auth/login` honours the configured ceiling for the default-24h, configured-7d, and above-ceiling-clamp cases. `TestSettingsAPIExposesSessionMaxHours` round-trips the field through `/settings/` (default = 24, valid update persists as int's string form, zero rejected with 422, above-ceiling rejected with 422). Existing 202-case auth + MFA suite still green. **i18n:** 8 new keys in `settings.sessionPolicy.*` namespace; full translations in all 10 non-en locales (de / es / fr / it / ja / ko / pt-BR / tr / zh-CN / zh-TW), no English fallback. Parity check 5149 leaves per locale. ESLint clean; `npm run build` clean; ruff clean.
|
||||
|
||||
### Fixed
|
||||
- **Local Presets page: deleted row stayed visible until refetch returned, allowing a second delete click → 404** — On the Slicer → Local Profiles page, clicking Delete → Confirm fired the `DELETE /api/v1/local-presets/{id}` request, then the `onSuccess` handler closed the confirmation modal and called `queryClient.invalidateQueries({ queryKey: ['localPresets'] })` without awaiting it. The global QueryClient default `staleTime: 1000 * 60` (App.tsx:78) doesn't block `invalidateQueries` from refetching, but the refetch is *async* — so for ~hundreds of ms the rendered table still showed the just-deleted row, and a quick re-click on the same row opened a fresh confirm dialog → second confirm → backend returns 404 (row already gone) → confusing error toast. Caught while reproducing #1713: log showed `DELETE /api/v1/local-presets/42 → 200` followed by two `→ 404` for the same id within 4 seconds. **Fix:** Add an optimistic `queryClient.setQueryData<LocalPreset[]>(['localPresets'], …)` in `frontend/src/components/LocalProfilesView.tsx::deleteMutation.onSuccess` that filters the deleted row out of the cached list synchronously, then leaves the existing `invalidateQueries` calls in place to reconcile any drift. Row disappears the instant the DELETE returns 200, no re-click window. The same import path's `importMutation` doesn't need the same treatment because additions can't trigger the symmetric "row I just acted on is still there" → 404 loop. ESLint clean; `npm run build` clean; existing `LocalProfilesView.test.tsx` suite still green (no new test added — the bug is a render-timing window the existing render-based vitests don't observe; the existing onSuccess assertions still pass with the new optimistic write).
|
||||
- **SpoolBuddy inventory search now matches spool ID, slicer filament name, and storage location (#1738, reported by @shaddowlink)** — The reporter found that typing a numeric spool ID into SpoolBuddy → Inventory's search box returned no results, even though the same query in Bambuddy's main Inventory page worked. Root cause: `frontend/src/pages/spoolbuddy/SpoolBuddyInventoryPage.tsx:147-155` reimplemented the search filter inline and only matched `material`, `subtype`, `brand`, `color_name`, and `note`. The main Inventory page delegates to the shared `filterSpoolsByQuery` helper in `frontend/src/utils/inventorySearch.ts:7`, which additionally matches `String(spool.id)`, `slicer_filament_name`, and `storage_location`. SpoolBuddy had diverged. **Fix:** replace the inline filter with a single call to `filterSpoolsByQuery(list, searchQuery.trim())`. Both inventory modes (internal via `getSpools`, Spoolman via `getSpoolmanInventorySpools`) return the same `InventorySpool` shape, so this covers both paths in one drop. SpoolBuddy now matches Bambuddy's search behaviour across all eight fields. **Tests:** new `SpoolBuddyInventorySearch.test.ts` with 4 cases pinning the parity — exact spool ID match, partial spool ID match, the five pre-fix fields still match, and the three newly-included fields (storage_location, slicer_filament_name, plus implicit id) match. Existing `inventorySearch.test.ts` ID matching test (#1336) still green. ESLint clean; `npm run build` clean. No backend change, no i18n, no new permission.
|
||||
|
||||
- **Sidebar entries for Files / Archives / Queue no longer hide from non-admin users with granular read access (#1755, reported by @knifesk)** — The reporter noticed the **File Manager** sidebar entry was hidden for a default Operators user even though the same user could load `/files` directly and the backend API accepted their requests. Root cause is broader than reported: `frontend/src/components/Layout.tsx::navPermissions` mapped `files → 'library:read'`, `archives → 'archives:read'`, `queue → 'queue:read'` — the LEGACY permission flags — but the default Operators group at `backend/app/core/permissions.py:368-380` is seeded with the GRANULAR variants only (`ARCHIVES_READ_OWN.value`, `QUEUE_READ_OWN.value`, `LIBRARY_READ_OWN.value`). The migration path at `backend/app/core/database.py:3034-3041` also flips legacy `*:read` → `*:read_own` on existing non-admin groups. So a non-admin user never holds the legacy permission, `hasPermission('library:read')` returns false, sidebar entry is suppressed — for all three resources, not just Files. Admins get `ALL_PERMISSIONS` which includes the legacy variant, so the sidebar always renders for them, which is why this regression went unnoticed until a real non-admin Operator account landed in #1755. **Fix:** `navPermissions` now accepts `Permission | Permission[]` and the three affected resources list all three tiers (`*:read`, `*:read_own`, `*:read_all`). The `isHidden` check switches on the array type — `some(hasPermission)` for arrays, current behavior for single values. Nothing else in the gate logic changed. `frontend/src/api/client.ts` Permission type extended with the missing granular variants (`archives:read_own`, `archives:read_all`, `queue:read_own`, `queue:read_all`, `library:read_own`, `library:read_all`) — these existed in the backend enum and were already being shipped to the frontend in `/auth/me`, but the TS type didn't declare them so any new code wanting to gate on the granular tier would TypeScript-error. **What this also fixes downstream:** any future feature that needs to gate UI on `*:read_own` / `*:read_all` can now do so without re-adding the same type entries. **Tests:** 5 new cases in `Layout.test.tsx::'Sidebar gate accepts granular read tiers (#1755)'` — Files visible with only `library:read_own`, Files visible with only `library:read_all`, Archives visible with only `archives:read_own`, Queue visible with only `queue:read_own`, and the negative case (`printers:read` only — none of Files / Archives / Queue render). 22/22 Layout vitests green; ESLint clean; `npm run build` clean. No backend change, no DB migration, no new i18n keys. No new permission — just unmasks UI for users who already had backend access.
|
||||
|
||||
@@ -15,7 +15,7 @@ import {
|
||||
AlertCircle,
|
||||
} from 'lucide-react';
|
||||
import { api } from '../api/client';
|
||||
import type { LocalPreset } from '../api/client';
|
||||
import type { LocalPreset, LocalPresetsResponse } from '../api/client';
|
||||
import { Card, CardContent } from './Card';
|
||||
import { Button } from './Button';
|
||||
import { useToast } from '../contexts/ToastContext';
|
||||
@@ -276,7 +276,21 @@ export function LocalProfilesView() {
|
||||
|
||||
const deleteMutation = useMutation({
|
||||
mutationFn: (id: number) => api.deleteLocalPreset(id),
|
||||
onSuccess: () => {
|
||||
onSuccess: (_, id) => {
|
||||
// Optimistically drop the row from the cached list so the rendered table
|
||||
// updates the instant the DELETE returns. Without this the row stays
|
||||
// visible until invalidateQueries' background refetch completes, and a
|
||||
// quick re-click on the same row opens a second delete-confirm modal
|
||||
// that resolves to a 404 (server already deleted it). The cache holds a
|
||||
// grouped response (filament / printer / process), not a flat list.
|
||||
queryClient.setQueryData<LocalPresetsResponse>(['localPresets'], (old) => {
|
||||
if (!old) return old;
|
||||
return {
|
||||
filament: old.filament.filter((p) => p.id !== id),
|
||||
printer: old.printer.filter((p) => p.id !== id),
|
||||
process: old.process.filter((p) => p.id !== id),
|
||||
};
|
||||
});
|
||||
queryClient.invalidateQueries({ queryKey: ['localPresets'] });
|
||||
// Match the import path: the SliceModal's `slicerPresets` query needs
|
||||
// to be invalidated too, otherwise the deleted preset keeps appearing
|
||||
|
||||
Reference in New Issue
Block a user