Browse Source

fix(#730): back-fill archive.created_by_id on reprint when NULL

  Reprint from Archive kept showing `created_by_id = NULL` even after the
  Direct Print / File Manager / Library attribution fixes in 0.2.4b1.

  Root cause: reprint reuses the source archive row (via
  register_expected_print → _expected_prints lookup) to avoid duplicate
  archives. When the source was auto-created from a printer-initiated
  print, its created_by_id was NULL — and reprint never touched it.
  Print Log correctly attributed the reprinter (set_current_print_user
  → _print_user_info at print-complete), but the Statistics per-user
  filter reads archive.created_by_id and stayed unassigned forever.

  Fix in main.py's print-complete handler: when the archive's
  created_by_id is NULL and a print-session user is known, back-fill
  from _print_user_info. Never overwrites existing attribution — the
  original uploader keeps ownership; only NULLs are filled.

  Already-completed archives stay NULL (no retroactive rewrite). Next
  print after deploy credits the current user on any NULL archive.
maziggy 1 month ago
parent
commit
6538f723a4
2 changed files with 12 additions and 0 deletions
  1. 1 0
      CHANGELOG.md
  2. 11 0
      backend/app/main.py

+ 1 - 0
CHANGELOG.md

@@ -15,6 +15,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **Settings page: permission-gated instead of admin-only** — the Settings sidebar entry has always been visible to any user holding `settings:read`, but the route guard required admin role, so a non-admin with `settings:read` would see the entry, click it, and get silently redirected back to the dashboard. The route guard now matches the sidebar: any user with `settings:read` can open the page, and the individual tabs / cards continue to enforce their own per-feature permissions (`users:read`, `groups:update`, `oidc:*`, etc. — many of them admin-only, some not). Group editor routes moved to permission-based guards too (`groups:create` for `/groups/new`, `groups:update` for `/groups/:id/edit`), so permission delegation works end-to-end. Admins retain full access since admins implicitly hold every permission.
 
 ### Fixed
+- **Reprint-from-Archive left `created_by_id` as `NULL`** ([#730](https://github.com/maziggy/bambuddy/issues/730) follow-up) — 0.2.4b1 fixed user attribution for Direct Print / File Manager / Library prints, but the reprint path was still unattributed on the *archive* row. Reprint intentionally reuses the source archive (to avoid duplicate rows — see `register_expected_print`), so an archive auto-created from a printer-initiated print with no known user stayed `created_by_id=NULL` forever, even after multiple reprints by authenticated Bambuddy users. Print Log got the reprinter's username correctly (via `_print_user_info`), but the Statistics per-user filter — which reads `archive.created_by_id` — kept showing the archive as unassigned. Fix in `main.py`'s print-complete handler: when the archive has no `created_by_id` and a print-session user is set (which reprint always sets via `set_current_print_user`), back-fill the archive's attribution. Never overwrites an existing attribution — the original uploader keeps ownership; NULL archives are the only ones touched. Thanks to @3823u44238 for the detailed retest that caught this.
 - **Settings: failed-save toast looped forever when the user lacked `settings:update`** — the Settings page runs a debounced auto-save effect that fires `PATCH /settings` whenever `localSettings` diverges from the last server snapshot. When a delegated user with `settings:read` but not `settings:update` toggled a control, the effect fired `PATCH`, got `403`, and kept re-firing every ~500 ms producing an endless stream of identical "Failed to save" toasts. Gated at three points so the mutation is never attempted without permission: (1) the `updateSetting` callback — every onChange path — shows one `settings.toast.noPermissionUpdate` toast and short-circuits before diverging `localSettings`; (2) the debounced-save effect safety-nets the same check in case any call site bypassed `updateSetting`; (3) the language `<select>` was a fire-and-forget direct `api.updateSettings` call that always flashed a success toast regardless of outcome — it now goes through `updateMutation` with the same permission guard. New `settings.toast.noPermissionUpdate` key added across all 8 locales with full translations (not English-fallback).
 - **Groups: edits to custom-group permissions appeared lost on reopen** ([#1083](https://github.com/maziggy/bambuddy/issues/1083)) — creating a custom group and reopening the editor showed the correct permissions, but after editing that group's permissions and saving, reopening the editor within ~1 minute displayed the pre-edit snapshot as if the save had failed. The backend `PATCH /api/v1/groups/{id}` was persisting correctly (now covered by four new integration tests in `test_groups_api.py`, including a direct DB read after update); the issue was purely in the frontend React Query cache — `GroupEditPage.onSuccess` invalidated `['groups']` (the list) but left the `['group', id]` detail cache stale, and with the app-wide 60 s `staleTime` the next mount served the cached pre-update body instead of refetching. `onSuccess` now primes the `['group', id]` detail cache with the `PATCH` response body so the next mount hits fresh data immediately without a round-trip. Create-path invalidates `['group']` for symmetry. Regression test in `GroupEditPage.test.tsx` verifies the detail cache contains the updated permissions after save.
 - **Setup: re-enabling auth could 422 on a password the form no longer needs** — after disabling authentication and re-enabling it (common when switching between local auth and LDAP, or recovering from a bad config), the setup form still sends `admin_password` in the body even though the backend route ignores it when an admin user already exists. The `SetupRequest` Pydantic schema enforced password complexity (uppercase + lowercase + digit + special char) unconditionally, so any existing password that predated the complexity rule — or a legitimate LDAP-mode placeholder — triggered `422 Value error, Password must contain at least one special character` before the route body could decide to ignore the field. Complexity validation has moved out of the schema and into the route body, scoped to the branch that actually creates a new local admin. Re-enabling auth with an existing admin (or any LDAP user) now accepts whatever the form sends; fresh first-time setup still rejects weak passwords with a clear 400. Two regression tests added in `test_auth_api.py`: weak password rejected at setup when creating the first admin, weak/placeholder password accepted when an admin already exists.

+ 11 - 0
backend/app/main.py

@@ -3019,6 +3019,17 @@ async def on_print_complete(printer_id: int, data: dict):
 
             archive = await db.get(PrintArchive, archive_id)
             if archive:
+                # Back-fill created_by_id on reprint (#730): reprint reuses the
+                # source archive row rather than creating a new one, so an
+                # archive that was auto-created from a printer-initiated
+                # print (created_by_id=NULL) would otherwise stay unattributed
+                # forever. When we have a print-session user AND the archive
+                # has no attribution yet, credit the current user. Never
+                # overwrite an existing attribution — the original uploader
+                # keeps ownership.
+                _print_user_id = _print_user_info.get("user_id") if _print_user_info else None
+                if archive.created_by_id is None and _print_user_id is not None:
+                    archive.created_by_id = _print_user_id
                 p_info = printer_manager.get_printer(printer_id)
                 await write_log_entry(
                     db,