From 12dddada0a2fe3bb6a28cffe7dc175259df7e03f Mon Sep 17 00:00:00 2001 From: Thomansky <73141171+Thomansky@users.noreply.github.com> Date: Sat, 26 Sep 2026 08:56:48 +0200 Subject: [PATCH] File Manager: external link, notes and photos on library files (#3128) --- backend/app/api/routes/library.py | 264 +++++++++++ backend/app/api/routes/users.py | 11 + backend/app/core/database.py | 6 + backend/app/main.py | 31 +- backend/app/models/library.py | 6 + backend/app/schemas/library.py | 37 +- backend/app/services/library_trash.py | 4 +- backend/app/services/print_scheduler.py | 71 +++ backend/app/utils/library_paths.py | 88 ++++ .../test_library_file_metadata_3077.py | 436 ++++++++++++++++++ .../test_library_preview_thumbnail_api.py | 171 +++++++ .../integration/test_ownership_permissions.py | 108 +++++ .../integration/test_security_headers.py | 55 +++ backend/tests/unit/test_library_photo_move.py | 67 +++ .../unit/test_outbound_url_ssrf_guards.py | 1 + .../unit/test_scheduler_cleanup_library.py | 231 +++++++++- frontend/package-lock.json | 318 ++++++++++++- frontend/package.json | 7 +- frontend/scripts/check-i18n-parity.mjs | 1 + .../LibraryFileDetailsModal.test.tsx | 324 +++++++++++++ .../components/PdfPreviewModal.test.tsx | 101 ++++ .../SpreadsheetPreviewModal.test.tsx | 114 +++++ .../pages/FileManagerFileDetails.test.tsx | 183 ++++++++ frontend/src/api/client.ts | 57 +++ .../components/LibraryFileDetailsModal.tsx | 346 ++++++++++++++ frontend/src/components/ModelViewer.tsx | 186 +++++++- frontend/src/components/ModelViewerModal.tsx | 9 +- frontend/src/components/PdfPreviewModal.tsx | 244 ++++++++++ frontend/src/components/PhotoGalleryModal.tsx | 83 ++-- .../components/SpreadsheetPreviewModal.tsx | 305 ++++++++++++ frontend/src/i18n/locales/de.ts | 43 ++ frontend/src/i18n/locales/en.ts | 43 ++ frontend/src/i18n/locales/es.ts | 43 ++ frontend/src/i18n/locales/fr.ts | 43 ++ frontend/src/i18n/locales/it.ts | 43 ++ frontend/src/i18n/locales/ja.ts | 43 ++ frontend/src/i18n/locales/ko.ts | 43 ++ frontend/src/i18n/locales/nl.ts | 43 ++ frontend/src/i18n/locales/pt-BR.ts | 43 ++ frontend/src/i18n/locales/ru.ts | 43 ++ frontend/src/i18n/locales/sv.ts | 43 ++ frontend/src/i18n/locales/tr.ts | 43 ++ frontend/src/i18n/locales/uk.ts | 43 ++ frontend/src/i18n/locales/zh-CN.ts | 43 ++ frontend/src/i18n/locales/zh-TW.ts | 43 ++ frontend/src/pages/FileManagerPage.tsx | 248 +++++++++- frontend/src/types/occt-import-js.d.ts | 31 ++ frontend/src/workers/stepPreview.worker.ts | 77 ++++ frontend/vite.config.ts | 8 + 49 files changed, 4817 insertions(+), 57 deletions(-) create mode 100644 backend/app/utils/library_paths.py create mode 100644 backend/tests/integration/test_library_file_metadata_3077.py create mode 100644 backend/tests/integration/test_library_preview_thumbnail_api.py create mode 100644 backend/tests/unit/test_library_photo_move.py create mode 100644 frontend/src/__tests__/components/LibraryFileDetailsModal.test.tsx create mode 100644 frontend/src/__tests__/components/PdfPreviewModal.test.tsx create mode 100644 frontend/src/__tests__/components/SpreadsheetPreviewModal.test.tsx create mode 100644 frontend/src/__tests__/pages/FileManagerFileDetails.test.tsx create mode 100644 frontend/src/components/LibraryFileDetailsModal.tsx create mode 100644 frontend/src/components/PdfPreviewModal.tsx create mode 100644 frontend/src/components/SpreadsheetPreviewModal.tsx create mode 100644 frontend/src/types/occt-import-js.d.ts create mode 100644 frontend/src/workers/stepPreview.worker.ts diff --git a/backend/app/api/routes/library.py b/backend/app/api/routes/library.py index acb4c4205..aeba58569 100644 --- a/backend/app/api/routes/library.py +++ b/backend/app/api/routes/library.py @@ -49,6 +49,7 @@ from backend.app.schemas.library import ( BatchThumbnailResult, BulkDeleteRequest, BulkDeleteResponse, + ClientThumbnailResponse, ExternalFolderCreate, FileDuplicate, FileListResponse, @@ -90,6 +91,7 @@ from backend.app.utils.filename import ( safe_path_component, validate_print_filename, ) +from backend.app.utils.library_paths import library_photos_dir, remove_library_photos_dir from backend.app.utils.printer_models import is_gcode_compatible from backend.app.utils.safe_path import PathTraversalError, assert_under, safe_join_under from backend.app.utils.threemf_tools import ( @@ -808,6 +810,29 @@ def create_image_thumbnail(file_path: Path, thumbnails_dir: Path, max_size: int # Supported image extensions for thumbnails IMAGE_EXTENSIONS = {".png", ".jpg", ".jpeg", ".gif", ".webp", ".bmp", ".tiff", ".tif"} +# File types whose thumbnails are rendered client-side and uploaded back +# (#2976). The server has no renderer for these formats — STEP would need +# OpenCascade, PDF a rasteriser — so the browser posts its first preview +# render to POST /files/{id}/preview-thumbnail instead. Kept to exactly +# these types so the endpoint can never overwrite a server-generated +# STL/3MF/G-code/image thumbnail. +CLIENT_THUMBNAIL_TYPES = {"step", "stp", "pdf", "csv", "xlsx", "ods"} + +# Photos of the printed result (#3077): same allowlist and naming as the +# archive photo routes. 10 MB is ample for a phone camera JPEG. +PHOTO_EXTENSIONS = (".jpg", ".jpeg", ".png", ".webp") +PHOTO_MEDIA_TYPES = { + ".jpg": "image/jpeg", + ".jpeg": "image/jpeg", + ".png": "image/png", + ".webp": "image/webp", +} +MAX_PHOTO_BYTES = 10 * 1024 * 1024 + +# Upper bound for an uploaded client-rendered thumbnail. The FE sends a +# 256px PNG (a few tens of KB); anything near this limit is not a thumbnail. +MAX_CLIENT_THUMBNAIL_BYTES = 2 * 1024 * 1024 + async def _backfill_external_stl_thumbnails(folder_ids: list[int]) -> None: """Generate STL thumbnails for an external folder tree in the background. @@ -1494,6 +1519,8 @@ async def delete_folder( await delete_dependent_variants(db, doomed_file_ids) await release_queue_references(db, doomed_file_ids) + for doomed_id in doomed_file_ids: + remove_library_photos_dir(doomed_id) # Delete folder (cascade will handle files and subfolders) await db.delete(folder) @@ -1588,6 +1615,12 @@ _SCANNABLE_EXTENSIONS = { ".webp", ".svg", ".md", + # Documents that ship alongside a job folder and now have in-app + # previews (#2976): drawings/datasheets and part lists. + ".pdf", + ".csv", + ".xlsx", + ".ods", } @@ -2009,6 +2042,10 @@ async def scan_external_folder( abs_thumb.unlink() except OSError: pass + # The row is gone for good — external files skip the trash — so + # its photos go with it rather than being orphaned under an id + # nothing points at any more (#3077). + remove_library_photos_dir(db_file.id) await db.delete(db_file) removed += 1 @@ -2229,6 +2266,9 @@ async def list_files( tags=[TagSummary(id=t.id, name=t.name) for t in f.tags], variant_group_id=f.variant_group_id, variant_count=variant_counts.get(f.variant_group_id, 0) if f.variant_group_id else 0, + external_url=f.external_url, + has_notes=bool(f.notes), + photo_count=len(f.photos or []), ) ) @@ -5058,6 +5098,9 @@ async def get_file( print_count=file.print_count, last_printed_at=file.last_printed_at, notes=file.notes, + external_url=file.external_url, + photos=list(file.photos or []), + source_url=file.source_url, duplicates=duplicates if duplicates else None, duplicate_count=duplicate_count, created_by_id=file.created_by_id, @@ -5132,6 +5175,9 @@ async def update_file( if data.notes is not None: file.notes = data.notes if data.notes else None + if data.external_url is not None: + file.external_url = data.external_url.strip() or None + await db.commit() await db.refresh(file) @@ -5186,6 +5232,7 @@ async def delete_file( await delete_dependent_variants(db, [file.id]) await release_queue_references(db, [file.id]) + remove_library_photos_dir(file.id) await db.delete(file) await db.commit() return {"status": "success", "message": "File deleted", "trashed": False} @@ -5328,6 +5375,218 @@ async def get_thumbnail( return FastAPIFileResponse(str(abs_thumb_path), media_type=media_type) +@router.post("/files/{file_id}/preview-thumbnail", response_model=ClientThumbnailResponse) +async def upload_preview_thumbnail( + file_id: int, + thumbnail: UploadFile = File(...), + db: AsyncSession = Depends(get_db), + auth_result: tuple[User | None, bool] = Depends( + require_ownership_permission( + Permission.LIBRARY_UPDATE_ALL, + Permission.LIBRARY_UPDATE_OWN, + ) + ), +): + """Store a client-rendered preview thumbnail for a file (#2976). + + STEP, PDF and spreadsheet previews are rendered in the browser; the FE + posts its first render here so the grid gets a thumbnail without the + server needing OpenCascade or a PDF rasteriser. Only file types in + ``CLIENT_THUMBNAIL_TYPES`` are accepted, and only while the file has no + thumbnail yet — a stored thumbnail is never replaced by this route. + """ + user, can_modify_all = auth_result + + result = await db.execute(LibraryFile.active().where(LibraryFile.id == file_id)) + file = result.scalar_one_or_none() + + if not file: + raise HTTPException(status_code=404, detail="File not found") + + # Ownership check (same shape as update_file) + if not can_modify_all: + if file.created_by_id != user.id: + raise HTTPException(status_code=403, detail="You can only update your own files") + + if file.file_type not in CLIENT_THUMBNAIL_TYPES: + raise HTTPException(status_code=400, detail="File type does not accept client-rendered thumbnails") + + if file.thumbnail_path: + return ClientThumbnailResponse(updated=False) + + content = await thumbnail.read(MAX_CLIENT_THUMBNAIL_BYTES + 1) + if len(content) > MAX_CLIENT_THUMBNAIL_BYTES: + raise HTTPException(status_code=413, detail="Thumbnail too large") + + # Decode and re-encode through PIL: validates the bytes are a real PNG + # and strips anything that isn't pixel data before it lands on disk. + import io + + from PIL import Image, UnidentifiedImageError + + try: + with Image.open(io.BytesIO(content)) as img: + img.load() + if img.format != "PNG": + raise HTTPException(status_code=400, detail="Thumbnail must be a PNG image") + if img.mode not in ("RGB", "RGBA"): + img = img.convert("RGBA") + # The grid renders at ~256px; cap outliers instead of storing them. + if img.width > 512 or img.height > 512: + img.thumbnail((512, 512), Image.Resampling.LANCZOS) + thumbnails_dir = get_library_thumbnails_dir() + thumb_filename = f"{uuid.uuid4().hex}.png" + thumb_path = thumbnails_dir / thumb_filename # SEC-PATH-OK: thumb_filename = uuid.uuid4().hex + ".png" + img.save(thumb_path, "PNG", optimize=True) + except HTTPException: + raise + except (UnidentifiedImageError, OSError, ValueError) as e: + raise HTTPException(status_code=400, detail="Invalid thumbnail image") from e + + file.thumbnail_path = to_relative_path(thumb_path) + await db.commit() + + return ClientThumbnailResponse(updated=True) + + +# ============ Photo Endpoints (#3077) ============ + + +@router.post("/files/{file_id}/photos") +async def upload_file_photo( + file_id: int, + file: UploadFile = File(...), + db: AsyncSession = Depends(get_db), + auth_result: tuple[User | None, bool] = Depends( + require_ownership_permission( + Permission.LIBRARY_UPDATE_ALL, + Permission.LIBRARY_UPDATE_OWN, + ) + ), +): + """Attach a photo of the printed result to a library file. + + Photos are Bambuddy-side metadata, so external files take them too. Same + shape as the archive photo upload: extension allowlist, uuid-named on + disk, and the ``photos`` list re-assigned so SQLAlchemy sees the change. + """ + user, can_modify_all = auth_result + + result = await db.execute(LibraryFile.active().where(LibraryFile.id == file_id)) + library_file = result.scalar_one_or_none() + + if not library_file: + raise HTTPException(status_code=404, detail="File not found") + + # Ownership check (same shape as update_file) + if not can_modify_all: + if library_file.created_by_id != user.id: + raise HTTPException(status_code=403, detail="You can only update your own files") + + if not file.filename or not file.filename.lower().endswith(PHOTO_EXTENSIONS): + raise HTTPException(status_code=400, detail="File must be an image (.jpg, .jpeg, .png, .webp)") + + content = await file.read(MAX_PHOTO_BYTES + 1) + if len(content) > MAX_PHOTO_BYTES: + raise HTTPException(status_code=413, detail="Photo too large (max 10 MB)") + + photos_dir = library_photos_dir(library_file.id) + photos_dir.mkdir(parents=True, exist_ok=True) + + ext = Path(file.filename).suffix.lower() + photo_filename = f"{uuid.uuid4().hex[:8]}{ext}" + photo_path = photos_dir / photo_filename # SEC-PATH-OK: photo_filename = uuid.uuid4().hex[:8] + ext + photo_path.write_bytes(content) + + photos = list(library_file.photos or []) + photos.append(photo_filename) + library_file.photos = photos + + await db.commit() + await db.refresh(library_file) + + return {"status": "uploaded", "filename": photo_filename, "photos": library_file.photos} + + +@router.get("/files/{file_id}/photos/{filename}") +async def get_file_photo( + file_id: int, + filename: str, + db: AsyncSession = Depends(get_db), + auth_result: tuple[User | None, bool] = Depends( + require_media_token_ownership( + Permission.LIBRARY_READ_ALL, + Permission.LIBRARY_READ_OWN, + ) + ), +): + """Serve one photo. Media-token auth like the thumbnail route (#3025).""" + user, can_read_all = auth_result + result = await db.execute(LibraryFile.active().where(LibraryFile.id == file_id)) + library_file = _ensure_library_file_visible(result.scalar_one_or_none(), user, can_read_all) + + # Membership check first: names are uuid-generated on upload, so anything + # not in the stored list is not a photo, whatever is on disk. + if not library_file.photos or filename not in library_file.photos: + raise HTTPException(status_code=404, detail="Photo not found") + + try: + photo_path = safe_join_under(library_photos_dir(library_file.id), filename, http=False) + except PathTraversalError: + raise HTTPException(status_code=404, detail="Photo not found") from None + if not photo_path.is_file(): + raise HTTPException(status_code=404, detail="Photo not found") + + media_type = PHOTO_MEDIA_TYPES.get(Path(filename).suffix.lower(), "image/jpeg") + return FastAPIFileResponse(str(photo_path), media_type=media_type) + + +@router.delete("/files/{file_id}/photos/{filename}") +async def delete_file_photo( + file_id: int, + filename: str, + db: AsyncSession = Depends(get_db), + auth_result: tuple[User | None, bool] = Depends( + require_ownership_permission( + Permission.LIBRARY_UPDATE_ALL, + Permission.LIBRARY_UPDATE_OWN, + ) + ), +): + """Remove a photo from a library file.""" + user, can_modify_all = auth_result + + result = await db.execute(LibraryFile.active().where(LibraryFile.id == file_id)) + library_file = result.scalar_one_or_none() + + if not library_file: + raise HTTPException(status_code=404, detail="File not found") + + if not can_modify_all: + if library_file.created_by_id != user.id: + raise HTTPException(status_code=403, detail="You can only update your own files") + + if not library_file.photos or filename not in library_file.photos: + raise HTTPException(status_code=404, detail="Photo not found") + + try: + photo_path = safe_join_under(library_photos_dir(library_file.id), filename, http=False) + except PathTraversalError: + raise HTTPException(status_code=404, detail="Photo not found") from None + if photo_path.is_file(): + try: + photo_path.unlink() + except OSError as e: + logger.warning("Failed to delete photo from disk: %s", e) + + photos = [p for p in library_file.photos if p != filename] + library_file.photos = photos if photos else None + + await db.commit() + + return {"status": "deleted", "photos": library_file.photos or []} + + @router.get("/files/{file_id}/gcode") async def get_gcode( file_id: int, @@ -5558,6 +5817,7 @@ async def bulk_delete( await delete_dependent_variants(db, hard_deleted_ids) await release_queue_references(db, hard_deleted_ids) for file in hard_deleted: + remove_library_photos_dir(file.id) await db.delete(file) # Delete folders (cascade will handle contents). Folders have no ownership @@ -5580,6 +5840,10 @@ async def bulk_delete( tree_file_ids = await _folder_tree_file_ids(db, folder_id) await delete_dependent_variants(db, tree_file_ids) await release_queue_references(db, tree_file_ids) + # The cascade hard-deletes every row in the subtree, so their + # photos go with them — same as DELETE /folders/{id} (#3077). + for doomed_id in tree_file_ids: + remove_library_photos_dir(doomed_id) await db.delete(folder) deleted_folders += 1 diff --git a/backend/app/api/routes/users.py b/backend/app/api/routes/users.py index fdf004071..281b83821 100644 --- a/backend/app/api/routes/users.py +++ b/backend/app/api/routes/users.py @@ -50,6 +50,7 @@ from backend.app.services.email_service import ( send_email, ) from backend.app.services.finance_defaults import ensure_user_finance_defaults +from backend.app.utils.library_paths import remove_library_photos_dir router = APIRouter(prefix="/users", tags=["users"]) @@ -444,7 +445,14 @@ async def delete_user( detail="Cannot delete your own account", ) + # Photo directories of the library rows about to go, resolved while the + # rows still exist to say which ids they belong to. Removed after the + # commit, so a failed delete leaves the pictures alone (#3077). + doomed_library_file_ids: list[int] = [] if delete_items: + doomed_library_file_ids = list( + (await db.execute(select(LibraryFile.id).where(LibraryFile.created_by_id == user_id))).scalars().all() + ) # Delete all items created by this user await db.execute(delete(PrintArchive).where(PrintArchive.created_by_id == user_id)) await db.execute(delete(PrintQueueItem).where(PrintQueueItem.created_by_id == user_id)) @@ -494,6 +502,9 @@ async def delete_user( await db.delete(user) await db.commit() + for file_id in doomed_library_file_ids: + remove_library_photos_dir(file_id) + @router.post("/me/change-password", response_model=dict) async def change_own_password( diff --git a/backend/app/core/database.py b/backend/app/core/database.py index d6d0341f6..c76792b32 100644 --- a/backend/app/core/database.py +++ b/backend/app/core/database.py @@ -4955,6 +4955,12 @@ async def run_migrations(conn): conn, "ALTER TABLE notification_providers ADD COLUMN on_ams_drying_suspended BOOLEAN DEFAULT TRUE" ) + # Migration: user link + photos on library files (#3077), the same trio + # print_archives carries (external_url / photos). Photos are stored under + # /library/photos// — the column only holds the names. + await _safe_execute(conn, "ALTER TABLE library_files ADD COLUMN external_url VARCHAR(500)") + await _safe_execute(conn, "ALTER TABLE library_files ADD COLUMN photos JSON") + # Migration: storage location sensor alerts (#2824), own column rather than # reusing on_ha_sensor_alert. That column can be scoped to one printer # (printer_id), and a location alert has no printer to scope by — sharing diff --git a/backend/app/main.py b/backend/app/main.py index 1833c1433..7405e4d25 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -4,6 +4,7 @@ import logging import math import os import posixpath +import re import secrets import time from contextlib import asynccontextmanager @@ -9609,6 +9610,12 @@ def _frame_ancestors(default_value: str) -> str: return f"frame-ancestors {default_value};" +# The Vite-emitted STEP preview worker chunk (#2976): src/workers/ +# stepPreview.worker.ts becomes /assets/stepPreview.worker-.js. Matched +# exactly so the eval-relaxed CSP below can never apply to any other asset. +_STEP_WORKER_ASSET_RE = re.compile(r"^/assets/stepPreview\.worker-[\w-]+\.js$") + + @app.middleware("http") async def security_headers_middleware(request, call_next): """Add standard HTTP security headers to every response.""" @@ -9655,6 +9662,23 @@ async def security_headers_middleware(request, call_next): "object-src 'none'; " "base-uri 'self'; " + _frame_ancestors("'none'") ) + elif _STEP_WORKER_ASSET_RE.match(request.url.path): + # The STEP preview worker (#2976) runs OpenCascade compiled to WASM; + # its emscripten/embind glue generates invoker functions with `new + # Function(...)`, which needs 'unsafe-eval'. Per CSP3 a dedicated + # worker is governed by the policy delivered with the WORKER SCRIPT's + # own response — not the document's — so relaxing it here confines + # eval to that DOM-less worker context. The document policy below + # stays nonce-strict, and this response header has no effect when the + # file is merely fetched (a fetch's CSP is enforced against the + # requesting document, not the resource's own headers). + response.headers["Content-Security-Policy"] = ( + "default-src 'self'; " + "script-src 'self' 'wasm-unsafe-eval' 'unsafe-eval'; " + "connect-src 'self'; " + "object-src 'none'; " + "base-uri 'self'; " + _frame_ancestors("'none'") + ) else: # The streaming overlay is embedded same-origin by the URL builder's # preview in Settings (#1422), so this branch allows 'self'. @@ -9668,9 +9692,14 @@ async def security_headers_middleware(request, call_next): # TRUSTED_FRAME_ORIGINS is for, and _frame_ancestors already folds that # allowlist in. embeddable_same_origin = request.url.path.startswith("/overlay/") + # 'wasm-unsafe-eval' permits WebAssembly compilation ONLY — it does + # not allow eval()/Function() for JS, unlike 'unsafe-eval'. Needed by + # the STEP preview, which triangulates in the browser via OpenCascade + # compiled to WASM (#2976). Browsers that predate the keyword ignore + # it and simply keep blocking wasm, so this never widens JS execution. response.headers["Content-Security-Policy"] = ( "default-src 'self'; " - f"script-src 'self' 'nonce-{csp_nonce}'; " + f"script-src 'self' 'wasm-unsafe-eval' 'nonce-{csp_nonce}'; " "style-src 'self' 'unsafe-inline'; " "img-src 'self' data: blob:; " "media-src 'self' blob:; " diff --git a/backend/app/models/library.py b/backend/app/models/library.py index 0ceffbc22..144caa52d 100644 --- a/backend/app/models/library.py +++ b/backend/app/models/library.py @@ -147,6 +147,12 @@ class LibraryFile(Base): # User notes notes: Mapped[str | None] = mapped_column(Text, nullable=True) + # User-provided link (Printables, Thingiverse, ...) and photos of the printed + # result (#3077) — the same trio archives carry. ``photos`` is a list of + # stored filenames under ``library_paths.library_photos_dir(id)``. + external_url: Mapped[str | None] = mapped_column(String(500), nullable=True) + photos: Mapped[list | None] = mapped_column(JSON, nullable=True) + # Provenance — when the file was imported from an external source (e.g. # MakerWorld), ``source_type`` identifies the source and ``source_url`` is # the canonical public URL. Used for "already imported" detection and diff --git a/backend/app/schemas/library.py b/backend/app/schemas/library.py index 067abc241..ec4300299 100644 --- a/backend/app/schemas/library.py +++ b/backend/app/schemas/library.py @@ -2,7 +2,7 @@ from datetime import datetime -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, field_validator # ============ Folder Schemas ============ @@ -122,6 +122,20 @@ class FileUpdate(BaseModel): folder_id: int | None = None project_id: int | None = None notes: str | None = None + # Empty string clears the link, like ``notes`` (#3077). + external_url: str | None = Field(None, max_length=500) + + @field_validator("external_url") + @classmethod + def validate_external_url(cls, v: str | None) -> str | None: + # The link is rendered as an href for every reader of the library, so + # only web URLs are accepted (no javascript:/data: schemes). + if v is None: + return None + v = v.strip() + if v and not v.lower().startswith(("http://", "https://")): + raise ValueError("external_url must start with http:// or https://") + return v class FileDuplicate(BaseModel): @@ -157,6 +171,11 @@ class FileResponse(BaseModel): last_printed_at: datetime | None notes: str | None + # User link + photos of the printed result (#3077); ``source_url`` is the + # read-only import provenance (MakerWorld) shown next to it. + external_url: str | None = None + photos: list[str] = [] + source_url: str | None = None # Duplicate detection duplicates: list[FileDuplicate] | None = None @@ -227,6 +246,12 @@ class FileListResponse(BaseModel): variant_group_id: int | None = None variant_count: int = 0 + # Metadata indicators (#3077). The list never ships the notes text itself — + # ``has_notes`` is enough for the card badge; the details modal loads the rest. + external_url: str | None = None + has_notes: bool = False + photo_count: int = 0 + class Config: from_attributes = True @@ -413,6 +438,16 @@ class BatchThumbnailResponse(BaseModel): results: list[BatchThumbnailResult] +class ClientThumbnailResponse(BaseModel): + """Schema for the client-rendered preview thumbnail upload response (#2976). + + ``updated`` is false when the file already had a thumbnail — the upload is + skipped so a stored thumbnail is never silently replaced. + """ + + updated: bool + + # ============ Variant Group Schemas (#671 / #2570) ============ diff --git a/backend/app/services/library_trash.py b/backend/app/services/library_trash.py index bcbb11be6..b40454ab9 100644 --- a/backend/app/services/library_trash.py +++ b/backend/app/services/library_trash.py @@ -29,6 +29,7 @@ from backend.app.core.database import async_session from backend.app.models.library import LibraryFile from backend.app.models.print_queue import PrintQueueItem, PrintQueueVariant from backend.app.models.settings import Settings +from backend.app.utils.library_paths import remove_library_photos_dir from backend.app.utils.local_time import utcnow_naive logger = logging.getLogger(__name__) @@ -364,7 +365,7 @@ class LibraryTrashService: @staticmethod def _unlink_on_disk(row: LibraryFile) -> None: - """Best-effort cleanup of the file + thumbnail on disk.""" + """Best-effort cleanup of the file, thumbnail and photos (#3077) on disk.""" for rel in (row.file_path, row.thumbnail_path): abs_path = _to_absolute_path(rel) if abs_path is None: @@ -374,6 +375,7 @@ class LibraryTrashService: abs_path.unlink() except OSError as e: logger.warning("Trash sweep: failed to unlink %s: %s", abs_path, e) + remove_library_photos_dir(row.id) # ---- User-facing trash ops ---------------------------------------- diff --git a/backend/app/services/print_scheduler.py b/backend/app/services/print_scheduler.py index 0680644ff..482701b43 100644 --- a/backend/app/services/print_scheduler.py +++ b/backend/app/services/print_scheduler.py @@ -60,9 +60,11 @@ from backend.app.services.printer_manager import ( ) from backend.app.services.smart_plug_manager import smart_plug_manager from backend.app.utils.ams_humidity import ams_humidity_percent +from backend.app.utils.archive_paths import archive_photos_dir from backend.app.utils.color_utils import perceptual_color_distance from backend.app.utils.filament_types import canonical_filament_type from backend.app.utils.filename import derive_remote_filename +from backend.app.utils.library_paths import move_library_photos from backend.app.utils.local_time import utcnow_naive from backend.app.utils.printer_models import ( is_dual_nozzle_model, @@ -6351,6 +6353,10 @@ class PrintScheduler: file_path = None filename = None cleanup_disk_paths: list[Path] = [] + # Set when a dispatch consumes its library file, so the photos can be + # carried over after the commit that removes the row (#3077). + consumed_library_file_id: int | None = None + consumed_photos: list[str] = [] if item.archive_id: # Print from archive @@ -6452,6 +6458,9 @@ class PrintScheduler: archive_id=archive.id, dispatched_item_id=item.id, ) + # Read while the row is still here; the photos move + # below, once the delete has actually committed. + consumed_photos = list(library_file.photos or []) await db.delete(library_file) file_path = settings.base_dir / archive.file_path filename = archive.filename @@ -6493,6 +6502,68 @@ class PrintScheduler: await self._power_off_if_needed(db, item) return + # The photos follow the file into the archive that replaces it, for + # the same reason the siblings do (#3077). After the commit above, + # never before it: that commit can fail ("database is locked", + # #1853) and roll the library row back, and photos already moved + # would leave it naming a directory that no longer exists. The + # file and thumbnail unlinks are deferred for the same reason. + if consumed_library_file_id is not None and consumed_photos: + # Held as a plain int, read here while the session is still + # healthy, because the handler below may not touch an ORM + # instance at all. The commit it exists for fails inside the + # FLUSH, not at COMMIT: SQLite takes the write lock at the + # first DML statement, so a busy writer surfaces as "database + # is locked" on the UPDATE (#1853). SQLAlchemy rolls that back + # internally through safe_reraise before re-raising, which + # expires every loaded instance and leaves the session in + # pending-rollback state -- so `archive.id` inside the except + # would itself raise PendingRollbackError and the rollback + # below would never be reached. + archive_id = archive.id + try: + carried_photos = move_library_photos( + consumed_library_file_id, + consumed_photos, + archive_photos_dir(archive), + ) + if carried_photos: + archive.photos = list(archive.photos or []) + carried_photos + await db.commit() + except Exception as e: + # The archive and the delete are already committed; the + # print goes ahead either way. Worst case the pictures sit + # unnamed in the archive's own directory. + # + # Ints only until the rollback has run, per the note above, + # which is why this logs queue_item_id and not item.id -- + # the sibling handler forty lines up does the same. + logger.warning( + "Queue item %s: failed to carry library photos into archive %s: %s", + queue_item_id, + archive_id, + e, + ) + await db.rollback() + # rollback() expires every loaded instance, and in async + # SQLAlchemy the next plain attribute read is lazy IO + # outside the greenlet -- MissingGreenlet, which would turn + # this cosmetic failure into a dispatch crash in exactly the + # "database is locked" case the block exists for (#1853). + # The nozzle guard, the upload and the start all keep + # reading item, archive and printer, so all three go back + # into the session before falling through. + item = await db.get(PrintQueueItem, queue_item_id) + archive = await db.get(PrintArchive, archive_id) + printer = await db.get(Printer, item.printer_id) if item else None + if not item or not archive or not printer: + logger.error( + "Queue item %s: item, archive %s or printer gone after the photo rollback", + queue_item_id, + archive_id, + ) + return + else: # Neither archive nor library file specified item.status = "failed" diff --git a/backend/app/utils/library_paths.py b/backend/app/utils/library_paths.py new file mode 100644 index 000000000..dc482a684 --- /dev/null +++ b/backend/app/utils/library_paths.py @@ -0,0 +1,88 @@ +"""Where a library file's user photos live on disk (#3077). + +Photos are Bambuddy-side metadata, so they sit inside the library data dir +regardless of whether the file itself is managed or external: +``/library/photos//``. The routes, the trash sweeper, +the external-folder scan and the dispatch cleanup all derive the directory +from here — see ``archive_paths`` for why one path derived in several places +is a bug waiting to happen. +""" + +from __future__ import annotations + +import logging +import shutil +import uuid +from collections.abc import Sequence +from pathlib import Path + +from backend.app.core.config import settings +from backend.app.utils.safe_path import PathTraversalError, safe_join_under + +logger = logging.getLogger(__name__) + + +def library_photos_dir(file_id: int) -> Path: + """The photo directory for library file *file_id* (not created).""" + library_dir = Path(settings.archive_dir) / "library" + return library_dir / "photos" / str(file_id) # SEC-PATH-OK: file_id is an int primary key + + +def remove_library_photos_dir(file_id: int) -> None: + """Best-effort removal of a file's photo directory and everything in it.""" + photos_dir = library_photos_dir(file_id) + if not photos_dir.is_dir(): + return + try: + shutil.rmtree(photos_dir) + except OSError as e: + logger.warning("Failed to remove library photos dir %s: %s", photos_dir, e) + + +def move_library_photos(file_id: int, photos: Sequence[str], destination: Path) -> list[str]: + """Move a library file's photos into *destination*, emptying its directory. + + Used where a library row is consumed by the archive that replaces it + (``cleanup_library_after_dispatch``): the photos follow the file instead + of being orphaned under an id nothing points at any more. Returns the + names the photos ended up under, in order — a name already taken in + *destination* gets a fresh one, because both sides draw photo names from + the same 8-hex-digit alphabet. + + Best-effort: a photo that cannot be moved is left out of the returned + list, so it is never named by an archive that does not have it. The + caller is mid-dispatch and has nowhere to report to. The source + directory is only removed once everything in the list did move, so a + failure orphans the pictures rather than destroying them. + """ + source_dir = library_photos_dir(file_id) + if not source_dir.is_dir(): + return [] + moved: list[str] = [] + failed = False + for filename in photos: + try: + source = safe_join_under(source_dir, filename, http=False) + except PathTraversalError: + failed = True + continue + if not source.is_file(): + continue + target_name = filename + try: + destination.mkdir(parents=True, exist_ok=True) + target = safe_join_under(destination, target_name, http=False) + if target.exists(): + target_name = f"{uuid.uuid4().hex[:8]}{source.suffix.lower()}" + target = destination / target_name # SEC-PATH-OK: uuid.uuid4().hex[:8] + suffix + shutil.move(str(source), str(target)) + except (OSError, PathTraversalError) as e: + logger.warning("Failed to move library photo %s to %s: %s", source, destination, e) + failed = True + continue + moved.append(target_name) + if failed: + logger.warning("Kept library photos dir %s: not every photo reached %s", source_dir, destination) + else: + remove_library_photos_dir(file_id) + return moved diff --git a/backend/tests/integration/test_library_file_metadata_3077.py b/backend/tests/integration/test_library_file_metadata_3077.py new file mode 100644 index 000000000..898295a8b --- /dev/null +++ b/backend/tests/integration/test_library_file_metadata_3077.py @@ -0,0 +1,436 @@ +"""Integration tests for library file notes, external link and photos (#3077). + +Pins the contracts of the details modal's backend: the PUT round-trip for +``external_url`` (empty string clears), the list-view indicators +(``has_notes`` / ``photo_count``), and the photo routes — membership check +before any disk access, extension allowlist, size cap, and the photo +directory going away with the file. +""" + +import io + +import pytest +from httpx import AsyncClient +from PIL import Image + +from backend.app.core.config import settings as app_settings +from backend.app.models.library import LibraryFile +from backend.app.models.user import User +from backend.app.utils.library_paths import library_photos_dir + + +def _jpeg_bytes() -> bytes: + buf = io.BytesIO() + Image.new("RGB", (32, 32), "red").save(buf, "JPEG") + return buf.getvalue() + + +@pytest.fixture +def isolated_storage(monkeypatch, tmp_path): + """Point library storage at a throwaway directory.""" + monkeypatch.setattr(app_settings, "base_dir", tmp_path) + monkeypatch.setattr(app_settings, "archive_dir", tmp_path / "archive") + return tmp_path + + +@pytest.fixture +async def file_factory(db_session): + """Factory for LibraryFile rows with sensible defaults.""" + _counter = [0] + + async def _create_file(**kwargs): + _counter[0] += 1 + counter = _counter[0] + defaults = { + "filename": f"part{counter}.3mf", + "file_path": f"library/files/part{counter}.3mf", + "file_type": "3mf", + "file_size": 100, + } + defaults.update(kwargs) + library_file = LibraryFile(**defaults) + db_session.add(library_file) + await db_session.commit() + await db_session.refresh(library_file) + return library_file + + return _create_file + + +async def _upload(async_client: AsyncClient, file_id: int, name: str = "result.jpg", content: bytes | None = None): + return await async_client.post( + f"/api/v1/library/files/{file_id}/photos", + files={"file": (name, content if content is not None else _jpeg_bytes(), "image/jpeg")}, + ) + + +class TestExternalUrlAndNotes: + @pytest.mark.asyncio + @pytest.mark.integration + async def test_external_url_round_trip(self, async_client: AsyncClient, file_factory, isolated_storage): + library_file = await file_factory() + + response = await async_client.put( + f"/api/v1/library/files/{library_file.id}", + json={"external_url": "https://www.printables.com/model/1234", "notes": "Print at 0.2mm"}, + ) + assert response.status_code == 200 + body = response.json() + assert body["external_url"] == "https://www.printables.com/model/1234" + assert body["notes"] == "Print at 0.2mm" + assert body["photos"] == [] + + detail = await async_client.get(f"/api/v1/library/files/{library_file.id}") + assert detail.json()["external_url"] == "https://www.printables.com/model/1234" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_empty_string_clears_external_url(self, async_client: AsyncClient, file_factory, isolated_storage): + library_file = await file_factory(external_url="https://example.com/x") + + response = await async_client.put(f"/api/v1/library/files/{library_file.id}", json={"external_url": ""}) + assert response.status_code == 200 + assert response.json()["external_url"] is None + + @pytest.mark.asyncio + @pytest.mark.integration + @pytest.mark.parametrize( + "url", + ["javascript:alert(1)", "data:text/html,hi", "ftp://example.com/x", "www.printables.com/model/1"], + ) + async def test_non_http_external_url_is_rejected( + self, async_client: AsyncClient, file_factory, isolated_storage, url: str + ): + library_file = await file_factory(external_url="https://example.com/x") + + response = await async_client.put(f"/api/v1/library/files/{library_file.id}", json={"external_url": url}) + assert response.status_code == 422 + + detail = await async_client.get(f"/api/v1/library/files/{library_file.id}") + assert detail.json()["external_url"] == "https://example.com/x" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_omitted_external_url_is_left_alone(self, async_client: AsyncClient, file_factory, isolated_storage): + library_file = await file_factory(external_url="https://example.com/x") + + response = await async_client.put(f"/api/v1/library/files/{library_file.id}", json={"notes": "hi"}) + assert response.status_code == 200 + assert response.json()["external_url"] == "https://example.com/x" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_detail_exposes_source_url_read_only(self, async_client: AsyncClient, file_factory, isolated_storage): + library_file = await file_factory(source_type="makerworld", source_url="https://makerworld.com/models/1") + + detail = await async_client.get(f"/api/v1/library/files/{library_file.id}") + assert detail.json()["source_url"] == "https://makerworld.com/models/1" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_list_carries_indicators_but_not_notes( + self, async_client: AsyncClient, file_factory, isolated_storage + ): + with_meta = await file_factory( + notes="secret notes", external_url="https://example.com/a", photos=["a.jpg", "b.png"] + ) + bare = await file_factory() + + response = await async_client.get("/api/v1/library/files") + assert response.status_code == 200 + by_id = {item["id"]: item for item in response.json()} + + assert by_id[with_meta.id]["has_notes"] is True + assert by_id[with_meta.id]["photo_count"] == 2 + assert by_id[with_meta.id]["external_url"] == "https://example.com/a" + assert "notes" not in by_id[with_meta.id] + assert "photos" not in by_id[with_meta.id] + + assert by_id[bare.id]["has_notes"] is False + assert by_id[bare.id]["photo_count"] == 0 + assert by_id[bare.id]["external_url"] is None + + +class TestPhotos: + @pytest.mark.asyncio + @pytest.mark.integration + async def test_upload_serve_delete_round_trip( + self, async_client: AsyncClient, db_session, file_factory, isolated_storage + ): + library_file = await file_factory() + + upload = await _upload(async_client, library_file.id) + assert upload.status_code == 200 + body = upload.json() + filename = body["filename"] + assert body["status"] == "uploaded" + assert body["photos"] == [filename] + assert filename.endswith(".jpg") + assert (library_photos_dir(library_file.id) / filename).is_file() + + await db_session.refresh(library_file) + assert library_file.photos == [filename] + + served = await async_client.get(f"/api/v1/library/files/{library_file.id}/photos/{filename}") + assert served.status_code == 200 + assert served.headers["content-type"] == "image/jpeg" + assert served.content == _jpeg_bytes() + + detail = await async_client.get(f"/api/v1/library/files/{library_file.id}") + assert detail.json()["photos"] == [filename] + + deleted = await async_client.delete(f"/api/v1/library/files/{library_file.id}/photos/{filename}") + assert deleted.status_code == 200 + assert deleted.json() == {"status": "deleted", "photos": []} + assert not (library_photos_dir(library_file.id) / filename).exists() + + gone = await async_client.get(f"/api/v1/library/files/{library_file.id}/photos/{filename}") + assert gone.status_code == 404 + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_external_file_takes_photos_too(self, async_client: AsyncClient, file_factory, isolated_storage): + library_file = await file_factory(is_external=True, file_path="/mnt/nas/part.stl", file_type="stl") + + upload = await _upload(async_client, library_file.id, name="shot.png") + assert upload.status_code == 200 + assert (library_photos_dir(library_file.id) / upload.json()["filename"]).is_file() + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_unlisted_filename_is_404_even_when_on_disk( + self, async_client: AsyncClient, file_factory, isolated_storage + ): + library_file = await file_factory() + photos_dir = library_photos_dir(library_file.id) + photos_dir.mkdir(parents=True) + (photos_dir / "stray.jpg").write_bytes(_jpeg_bytes()) + + response = await async_client.get(f"/api/v1/library/files/{library_file.id}/photos/stray.jpg") + assert response.status_code == 404 + + response = await async_client.delete(f"/api/v1/library/files/{library_file.id}/photos/stray.jpg") + assert response.status_code == 404 + assert (photos_dir / "stray.jpg").exists() + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_traversal_filename_is_rejected(self, async_client: AsyncClient, file_factory, isolated_storage): + # Even a traversal-looking name that IS in the stored list never leaves + # the photo directory — the membership check is not the only guard. + library_file = await file_factory(photos=["../../secret.jpg"]) + (isolated_storage / "archive" / "secret.jpg").parent.mkdir(parents=True, exist_ok=True) + (isolated_storage / "archive" / "secret.jpg").write_bytes(_jpeg_bytes()) + + response = await async_client.get( + f"/api/v1/library/files/{library_file.id}/photos/..%2F..%2Fsecret.jpg", + ) + assert response.status_code in (400, 404) + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_wrong_extension_is_rejected(self, async_client: AsyncClient, file_factory, isolated_storage): + library_file = await file_factory() + + response = await _upload(async_client, library_file.id, name="notes.txt", content=b"hello") + assert response.status_code == 400 + assert not library_photos_dir(library_file.id).exists() + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_oversized_upload_is_rejected(self, async_client: AsyncClient, file_factory, isolated_storage): + library_file = await file_factory() + + response = await _upload(async_client, library_file.id, content=b"\xff" * (10 * 1024 * 1024 + 1)) + assert response.status_code == 413 + assert not library_photos_dir(library_file.id).exists() + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_upload_to_missing_file_is_404(self, async_client: AsyncClient, isolated_storage): + response = await _upload(async_client, 999999) + assert response.status_code == 404 + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_trash_purge_removes_photo_dir( + self, async_client: AsyncClient, db_session, file_factory, isolated_storage + ): + library_file = await file_factory() + upload = await _upload(async_client, library_file.id) + assert upload.status_code == 200 + photos_dir = library_photos_dir(library_file.id) + assert photos_dir.is_dir() + + trashed = await async_client.delete(f"/api/v1/library/files/{library_file.id}") + assert trashed.status_code == 200 + # Soft-delete keeps the photos, like the file bytes and thumbnail. + assert photos_dir.is_dir() + + purged = await async_client.delete(f"/api/v1/library/trash/{library_file.id}") + assert purged.status_code == 200 + assert not photos_dir.exists() + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_external_file_delete_removes_photo_dir( + self, async_client: AsyncClient, file_factory, isolated_storage + ): + library_file = await file_factory(is_external=True, file_path="/mnt/nas/part.stl", file_type="stl") + upload = await _upload(async_client, library_file.id) + assert upload.status_code == 200 + photos_dir = library_photos_dir(library_file.id) + + response = await async_client.delete(f"/api/v1/library/files/{library_file.id}") + assert response.status_code == 200 + assert response.json()["trashed"] is False + assert not photos_dir.exists() + + +class TestPhotoDirectoryCleanup: + """Every path that hard-deletes a library row takes its photos with it. + + The upload/delete round-trip, the trash purge and the external single-file + delete are covered above; these are the remaining ones — folder delete, + bulk delete of files and of whole folders, the external-folder scan that + drops rows for files that vanished from the share, and the admin user + delete that takes the user's items with them. + """ + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_folder_delete_removes_photo_dir(self, async_client: AsyncClient, file_factory, isolated_storage): + folder = await async_client.post("/api/v1/library/folders", json={"name": "Brackets"}) + assert folder.status_code == 200 + folder_id = folder.json()["id"] + library_file = await file_factory(folder_id=folder_id) + assert (await _upload(async_client, library_file.id)).status_code == 200 + photos_dir = library_photos_dir(library_file.id) + assert photos_dir.is_dir() + + response = await async_client.delete(f"/api/v1/library/folders/{folder_id}") + assert response.status_code == 200 + assert not photos_dir.exists() + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_bulk_delete_removes_photo_dir_of_hard_deleted_file( + self, async_client: AsyncClient, file_factory, isolated_storage + ): + # External files bypass the trash, so bulk delete hard-deletes them; + # a managed file is only soft-deleted and keeps its photos until the + # sweeper runs. + external = await file_factory(is_external=True, file_path="/mnt/nas/ext.stl", file_type="stl") + managed = await file_factory() + for library_file in (external, managed): + assert (await _upload(async_client, library_file.id)).status_code == 200 + + response = await async_client.post( + "/api/v1/library/bulk-delete", + json={"file_ids": [external.id, managed.id], "folder_ids": []}, + ) + assert response.status_code == 200 + assert response.json()["deleted_files"] == 2 + assert not library_photos_dir(external.id).exists() + assert library_photos_dir(managed.id).is_dir() + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_bulk_delete_removes_photo_dirs_under_a_deleted_folder( + self, async_client: AsyncClient, file_factory, isolated_storage + ): + # The folder branch of bulk-delete lets the cascade hard-delete every + # row in the subtree, so it owes the same photo cleanup the file + # branch above it does — including files nested a level down. + parent = await async_client.post("/api/v1/library/folders", json={"name": "Jigs"}) + assert parent.status_code == 200 + parent_id = parent.json()["id"] + child = await async_client.post("/api/v1/library/folders", json={"name": "V2", "parent_id": parent_id}) + assert child.status_code == 200 + + top_file = await file_factory(folder_id=parent_id) + nested_file = await file_factory(folder_id=child.json()["id"]) + for library_file in (top_file, nested_file): + assert (await _upload(async_client, library_file.id)).status_code == 200 + assert library_photos_dir(library_file.id).is_dir() + + response = await async_client.post( + "/api/v1/library/bulk-delete", + json={"file_ids": [], "folder_ids": [parent_id]}, + ) + assert response.status_code == 200 + assert response.json()["deleted_folders"] == 1 + assert (await async_client.get(f"/api/v1/library/files/{top_file.id}")).status_code == 404 + assert not library_photos_dir(top_file.id).exists() + assert not library_photos_dir(nested_file.id).exists() + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_deleting_a_user_with_their_items_removes_photo_dirs( + self, async_client: AsyncClient, db_session, file_factory, isolated_storage + ): + # DELETE /users/{id}?delete_items=true bulk-deletes the rows, which is + # a hard delete like any other and owes the photos with it. + owner = User(username="photo-owner", password_hash="x", role="user") + db_session.add(owner) + await db_session.commit() + await db_session.refresh(owner) + + owned = await file_factory(created_by_id=owner.id) + someone_elses = await file_factory() + for library_file in (owned, someone_elses): + assert (await _upload(async_client, library_file.id)).status_code == 200 + assert library_photos_dir(library_file.id).is_dir() + + response = await async_client.delete(f"/api/v1/users/{owner.id}?delete_items=true") + assert response.status_code == 204 + assert (await async_client.get(f"/api/v1/library/files/{owned.id}")).status_code == 404 + assert not library_photos_dir(owned.id).exists() + assert library_photos_dir(someone_elses.id).is_dir() + + @pytest.fixture + def external_share(self, monkeypatch, tmp_path): + """Bambuddy's data dir and an opted-in external share, as siblings. + + The share cannot live under ``base_dir`` — ``_validate_external_path`` + refuses to mount a Bambuddy-managed directory, and the module's + ``isolated_storage`` points ``base_dir`` at ``tmp_path`` itself. + """ + data_dir = tmp_path / "data" + data_dir.mkdir() + monkeypatch.setattr(app_settings, "base_dir", data_dir) + monkeypatch.setattr(app_settings, "archive_dir", data_dir / "archive") + share = tmp_path / "share" + share.mkdir() + monkeypatch.setenv("BAMBUDDY_EXTERNAL_ROOTS", str(share)) + return share + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_external_scan_removes_photo_dir_of_vanished_file(self, async_client: AsyncClient, external_share): + share = external_share + (share / "bracket.stl").write_bytes(b"fakestl") + + folder = await async_client.post( + "/api/v1/library/folders/external", + json={"name": "Share", "external_path": str(share), "readonly": True, "show_hidden": False}, + ) + assert folder.status_code == 200 + folder_id = folder.json()["id"] + + scan = await async_client.post(f"/api/v1/library/folders/{folder_id}/scan") + assert scan.status_code == 200 + assert scan.json()["added"] == 1 + + listing = await async_client.get(f"/api/v1/library/files?folder_id={folder_id}") + file_id = listing.json()[0]["id"] + assert (await _upload(async_client, file_id)).status_code == 200 + photos_dir = library_photos_dir(file_id) + assert photos_dir.is_dir() + + (share / "bracket.stl").unlink() + + rescan = await async_client.post(f"/api/v1/library/folders/{folder_id}/scan") + assert rescan.status_code == 200 + assert rescan.json()["removed"] == 1 + assert not photos_dir.exists() diff --git a/backend/tests/integration/test_library_preview_thumbnail_api.py b/backend/tests/integration/test_library_preview_thumbnail_api.py new file mode 100644 index 000000000..b59fe5a7d --- /dev/null +++ b/backend/tests/integration/test_library_preview_thumbnail_api.py @@ -0,0 +1,171 @@ +"""Integration tests for the client-rendered preview thumbnail upload (#2976). + +STEP/PDF/spreadsheet previews render in the browser and post their first +render to POST /library/files/{id}/preview-thumbnail. These tests pin the +endpoint's contract: PNG-only, capped size, only for the client-preview file +types, and never replacing an existing thumbnail. +""" + +import io + +import pytest +from httpx import AsyncClient +from PIL import Image + +from backend.app.core.config import settings as app_settings +from backend.app.models.library import LibraryFile + + +def _png_bytes(size: tuple[int, int] = (300, 300), color: str = "red") -> bytes: + buf = io.BytesIO() + Image.new("RGB", size, color).save(buf, "PNG") + return buf.getvalue() + + +@pytest.fixture +def isolated_storage(monkeypatch, tmp_path): + """Point thumbnail storage at a throwaway directory.""" + monkeypatch.setattr(app_settings, "base_dir", tmp_path) + monkeypatch.setattr(app_settings, "archive_dir", tmp_path / "archive") + return tmp_path + + +@pytest.fixture +async def file_factory(db_session): + """Factory for LibraryFile rows of arbitrary file_type.""" + _counter = [0] + + async def _create_file(**kwargs): + _counter[0] += 1 + counter = _counter[0] + defaults = { + "filename": f"part{counter}.step", + "file_path": f"library/files/part{counter}.step", + "file_type": "step", + "file_size": 100, + } + defaults.update(kwargs) + library_file = LibraryFile(**defaults) + db_session.add(library_file) + await db_session.commit() + await db_session.refresh(library_file) + return library_file + + return _create_file + + +class TestPreviewThumbnailUpload: + @pytest.mark.asyncio + @pytest.mark.integration + async def test_upload_sets_thumbnail_path( + self, async_client: AsyncClient, db_session, file_factory, isolated_storage + ): + library_file = await file_factory(file_type="step") + + response = await async_client.post( + f"/api/v1/library/files/{library_file.id}/preview-thumbnail", + files={"thumbnail": ("preview.png", _png_bytes(), "image/png")}, + ) + assert response.status_code == 200 + assert response.json() == {"updated": True} + + await db_session.refresh(library_file) + assert library_file.thumbnail_path + stored = isolated_storage / library_file.thumbnail_path + assert stored.exists() + with Image.open(stored) as img: + assert img.format == "PNG" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_upload_downscales_oversized_image( + self, async_client: AsyncClient, db_session, file_factory, isolated_storage + ): + library_file = await file_factory(file_type="pdf", filename="doc.pdf", file_path="library/files/doc.pdf") + + response = await async_client.post( + f"/api/v1/library/files/{library_file.id}/preview-thumbnail", + files={"thumbnail": ("preview.png", _png_bytes(size=(1024, 1024)), "image/png")}, + ) + assert response.status_code == 200 + + await db_session.refresh(library_file) + with Image.open(isolated_storage / library_file.thumbnail_path) as img: + assert max(img.size) <= 512 + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_upload_skips_when_thumbnail_exists( + self, async_client: AsyncClient, db_session, file_factory, isolated_storage + ): + library_file = await file_factory(file_type="csv", thumbnail_path="archive/library/thumbnails/existing.png") + + response = await async_client.post( + f"/api/v1/library/files/{library_file.id}/preview-thumbnail", + files={"thumbnail": ("preview.png", _png_bytes(), "image/png")}, + ) + assert response.status_code == 200 + assert response.json() == {"updated": False} + + await db_session.refresh(library_file) + assert library_file.thumbnail_path == "archive/library/thumbnails/existing.png" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_upload_rejected_for_server_rendered_types( + self, async_client: AsyncClient, file_factory, isolated_storage + ): + # STL thumbnails are generated server-side; the client route must not + # be able to overwrite them. + library_file = await file_factory(file_type="stl", filename="part.stl") + + response = await async_client.post( + f"/api/v1/library/files/{library_file.id}/preview-thumbnail", + files={"thumbnail": ("preview.png", _png_bytes(), "image/png")}, + ) + assert response.status_code == 400 + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_upload_rejects_non_png(self, async_client: AsyncClient, file_factory, isolated_storage): + library_file = await file_factory(file_type="step") + + buf = io.BytesIO() + Image.new("RGB", (64, 64), "blue").save(buf, "JPEG") + response = await async_client.post( + f"/api/v1/library/files/{library_file.id}/preview-thumbnail", + files={"thumbnail": ("preview.png", buf.getvalue(), "image/png")}, + ) + assert response.status_code == 400 + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_upload_rejects_garbage_bytes(self, async_client: AsyncClient, file_factory, isolated_storage): + library_file = await file_factory(file_type="xlsx") + + response = await async_client.post( + f"/api/v1/library/files/{library_file.id}/preview-thumbnail", + files={"thumbnail": ("preview.png", b"not an image at all", "image/png")}, + ) + assert response.status_code == 400 + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_upload_rejects_oversized_payload(self, async_client: AsyncClient, file_factory, isolated_storage): + library_file = await file_factory(file_type="ods") + + oversized = b"\x89PNG\r\n\x1a\n" + b"\x00" * (2 * 1024 * 1024) + response = await async_client.post( + f"/api/v1/library/files/{library_file.id}/preview-thumbnail", + files={"thumbnail": ("preview.png", oversized, "image/png")}, + ) + assert response.status_code == 413 + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_upload_missing_file_returns_404(self, async_client: AsyncClient, isolated_storage): + response = await async_client.post( + "/api/v1/library/files/999999/preview-thumbnail", + files={"thumbnail": ("preview.png", _png_bytes(), "image/png")}, + ) + assert response.status_code == 404 diff --git a/backend/tests/integration/test_ownership_permissions.py b/backend/tests/integration/test_ownership_permissions.py index 37204c005..79daf7ab9 100644 --- a/backend/tests/integration/test_ownership_permissions.py +++ b/backend/tests/integration/test_ownership_permissions.py @@ -944,6 +944,114 @@ class TestLibraryOwnershipPermissions(TestOwnershipPermissionsSetup): assert response.status_code == 403 + # ======================================================================== + # Photo routes (#3077). Upload and delete are gated on LIBRARY_UPDATE_*, + # so a non-owner is refused with 403 exactly like ``update_file``. The + # read path goes through ``_ensure_library_file_visible`` and answers 404 + # instead, so an id that exists tells an outsider nothing. + # ======================================================================== + + @pytest.fixture + def photo_storage(self, monkeypatch, tmp_path): + """Keep uploaded photos out of the real data directory.""" + from backend.app.core.config import settings as app_settings + + monkeypatch.setattr(app_settings, "base_dir", tmp_path) + monkeypatch.setattr(app_settings, "archive_dir", tmp_path / "archive") + return tmp_path + + @staticmethod + def _photo_upload(): + import io + + from PIL import Image + + buf = io.BytesIO() + Image.new("RGB", (8, 8), "blue").save(buf, "JPEG") + return {"file": ("result.jpg", buf.getvalue(), "image/jpeg")} + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_operator_can_upload_photo_to_own_library_file( + self, async_client: AsyncClient, auth_setup, library_file_factory, photo_storage + ): + file = await library_file_factory(created_by_id=auth_setup["operator_user"]["id"]) + + response = await async_client.post( + f"/api/v1/library/files/{file.id}/photos", + headers={"Authorization": f"Bearer {auth_setup['operator_token']}"}, + files=self._photo_upload(), + ) + + assert response.status_code == 200 + assert len(response.json()["photos"]) == 1 + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_operator_cannot_upload_photo_to_others_library_file( + self, async_client: AsyncClient, auth_setup, library_file_factory, photo_storage + ): + from backend.app.utils.library_paths import library_photos_dir + + file = await library_file_factory(created_by_id=auth_setup["operator2_user"]["id"]) + + response = await async_client.post( + f"/api/v1/library/files/{file.id}/photos", + headers={"Authorization": f"Bearer {auth_setup['operator_token']}"}, + files=self._photo_upload(), + ) + + assert response.status_code == 403 + assert not library_photos_dir(file.id).exists() + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_operator_cannot_delete_photo_from_others_library_file( + self, async_client: AsyncClient, auth_setup, library_file_factory, photo_storage + ): + from backend.app.utils.library_paths import library_photos_dir + + file = await library_file_factory(created_by_id=auth_setup["operator2_user"]["id"]) + upload = await async_client.post( + f"/api/v1/library/files/{file.id}/photos", + headers={"Authorization": f"Bearer {auth_setup['operator2_token']}"}, + files=self._photo_upload(), + ) + filename = upload.json()["filename"] + + response = await async_client.delete( + f"/api/v1/library/files/{file.id}/photos/{filename}", + headers={"Authorization": f"Bearer {auth_setup['operator_token']}"}, + ) + + assert response.status_code == 403 + assert (library_photos_dir(file.id) / filename).is_file() + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_operator_reading_others_library_file_photo_gets_404( + self, async_client: AsyncClient, auth_setup, library_file_factory, photo_storage + ): + file = await library_file_factory(created_by_id=auth_setup["operator2_user"]["id"]) + upload = await async_client.post( + f"/api/v1/library/files/{file.id}/photos", + headers={"Authorization": f"Bearer {auth_setup['operator2_token']}"}, + files=self._photo_upload(), + ) + filename = upload.json()["filename"] + + owner = await async_client.get( + f"/api/v1/library/files/{file.id}/photos/{filename}", + headers={"Authorization": f"Bearer {auth_setup['operator2_token']}"}, + ) + assert owner.status_code == 200 + + stranger = await async_client.get( + f"/api/v1/library/files/{file.id}/photos/{filename}", + headers={"Authorization": f"Bearer {auth_setup['operator_token']}"}, + ) + assert stranger.status_code == 404 + # ======================================================================== # Folder deletion (#1781): folders have no ownership tracking, so users # with only library:delete_own may delete empty, non-external, non-linked diff --git a/backend/tests/integration/test_security_headers.py b/backend/tests/integration/test_security_headers.py index 09ed50ed1..18a74ce92 100644 --- a/backend/tests/integration/test_security_headers.py +++ b/backend/tests/integration/test_security_headers.py @@ -289,6 +289,61 @@ async def test_spa_csp_nonce_changes_per_request(async_client: AsyncClient): assert len(nonces) == 5, f"nonces should be per-request, got {nonces!r}" +# ─── #2976: STEP preview needs WebAssembly, and only WebAssembly ───────── + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_spa_csp_allows_wasm_but_not_eval(async_client: AsyncClient): + """script-src must carry 'wasm-unsafe-eval' but never 'unsafe-eval' (#2976). + + The STEP preview triangulates in the browser via OpenCascade compiled to + WASM; without 'wasm-unsafe-eval' the nonce-based CSP blocks + WebAssembly.instantiate() and the preview dies with a CompileError. + 'wasm-unsafe-eval' permits wasm compilation only — JS eval()/Function() + stay blocked, which is what the second assertion pins. + """ + resp = await async_client.get("/api/v1/auth/status") + csp = resp.headers.get("Content-Security-Policy", "") + script_src = next( + (d.strip() for d in csp.split(";") if d.strip().startswith("script-src")), + "", + ) + assert "'wasm-unsafe-eval'" in script_src, f"script-src must allow wasm compilation: {script_src!r}" + # Substring check must not be fooled by 'wasm-unsafe-eval' containing + # "unsafe-eval" — compare whole tokens. + tokens = script_src.split() + assert "'unsafe-eval'" not in tokens, f"script-src must not allow JS eval: {script_src!r}" + + +@pytest.mark.asyncio +@pytest.mark.integration +async def test_step_worker_asset_csp_relaxes_eval_only_for_that_file(async_client: AsyncClient): + """Only the STEP worker script's own response may carry 'unsafe-eval' (#2976). + + The occt-import-js embind glue generates invokers with `new Function`, + so the dedicated worker needs an eval-permitting policy. Per CSP3 a + worker is governed by the policy delivered with its own script response, + which confines eval to that DOM-less context. Any other asset — and the + SPA document itself — must stay nonce-strict. Both requests 404 in the + test checkout; the security middleware stamps headers regardless. + """ + + def script_src_tokens(resp) -> list[str]: + csp = resp.headers.get("Content-Security-Policy", "") + directive = next( + (d.strip() for d in csp.split(";") if d.strip().startswith("script-src")), + "", + ) + return directive.split() + + worker = await async_client.get("/assets/stepPreview.worker-Ck9aB12c.js") + assert "'unsafe-eval'" in script_src_tokens(worker), "step worker script must be allowed to eval" + + other = await async_client.get("/assets/index-Ck9aB12c.js") + assert "'unsafe-eval'" not in script_src_tokens(other), "ordinary assets must stay eval-free" + + # ─── #1460: HEAD on PWA bootstrap routes (manifest / sw / sw-register) ─── diff --git a/backend/tests/unit/test_library_photo_move.py b/backend/tests/unit/test_library_photo_move.py new file mode 100644 index 000000000..fe1e8e5b2 --- /dev/null +++ b/backend/tests/unit/test_library_photo_move.py @@ -0,0 +1,67 @@ +"""``move_library_photos`` edge cases (#3077). + +The happy path is pinned by the scheduler's cleanup tests; this covers what +they cannot reach — a name already taken in the destination, and a move that +fails halfway. +""" + +import pytest + +from backend.app.core.config import settings +from backend.app.utils.library_paths import library_photos_dir, move_library_photos + + +@pytest.fixture +def photo_dirs(monkeypatch, tmp_path): + monkeypatch.setattr(settings, "archive_dir", tmp_path / "archive") + destination = tmp_path / "archives" / "1" / "photos" + source = library_photos_dir(7) + source.mkdir(parents=True) + return source, destination + + +def test_moves_every_photo_and_drops_the_directory(photo_dirs): + source, destination = photo_dirs + (source / "a1b2c3d4.jpg").write_bytes(b"one") + (source / "e5f6a7b8.png").write_bytes(b"two") + + moved = move_library_photos(7, ["a1b2c3d4.jpg", "e5f6a7b8.png"], destination) + + assert moved == ["a1b2c3d4.jpg", "e5f6a7b8.png"] + assert (destination / "a1b2c3d4.jpg").read_bytes() == b"one" + assert not source.exists() + + +def test_renames_around_a_name_the_destination_already_holds(photo_dirs): + source, destination = photo_dirs + (source / "a1b2c3d4.jpg").write_bytes(b"library") + destination.mkdir(parents=True) + (destination / "a1b2c3d4.jpg").write_bytes(b"archive") + + moved = move_library_photos(7, ["a1b2c3d4.jpg"], destination) + + assert moved != ["a1b2c3d4.jpg"] + assert moved[0].endswith(".jpg") + assert (destination / "a1b2c3d4.jpg").read_bytes() == b"archive" + assert (destination / moved[0]).read_bytes() == b"library" + + +def test_a_traversal_name_is_skipped_and_keeps_the_directory(photo_dirs): + # A stored name is uuid-generated, so this only happens to a row someone + # has written to by hand — but the photos are then left alone rather than + # swept away by a cleanup that could not move them. + source, destination = photo_dirs + (source / "a1b2c3d4.jpg").write_bytes(b"one") + + moved = move_library_photos(7, ["../escape.jpg", "a1b2c3d4.jpg"], destination) + + assert moved == ["a1b2c3d4.jpg"] + assert source.is_dir() + + +def test_a_file_without_photos_is_a_no_op(photo_dirs): + source, destination = photo_dirs + source.rmdir() + + assert move_library_photos(7, [], destination) == [] + assert not destination.exists() diff --git a/backend/tests/unit/test_outbound_url_ssrf_guards.py b/backend/tests/unit/test_outbound_url_ssrf_guards.py index 5d913d149..c15f03f97 100644 --- a/backend/tests/unit/test_outbound_url_ssrf_guards.py +++ b/backend/tests/unit/test_outbound_url_ssrf_guards.py @@ -607,6 +607,7 @@ NOT_A_FETCH_TARGET = { ("SystemConfigRequest", "backend_url"), ("ExternalLinkCreate", "url"), # sidebar link, rendered in the UI, never requested ("ExternalLinkUpdate", "url"), + ("FileUpdate", "external_url"), # library file link (#3077), rendered in the UI, never fetched ("MaintenanceTypeCreate", "wiki_url"), # documentation link surfaced in the UI/notifications ("MaintenanceTypeUpdate", "wiki_url"), ("ArchiveUpdate", "external_url"), # stored source link for the model, never fetched diff --git a/backend/tests/unit/test_scheduler_cleanup_library.py b/backend/tests/unit/test_scheduler_cleanup_library.py index 8798ca1ae..b712e60f8 100644 --- a/backend/tests/unit/test_scheduler_cleanup_library.py +++ b/backend/tests/unit/test_scheduler_cleanup_library.py @@ -1,10 +1,12 @@ +import logging from contextlib import ExitStack from pathlib import Path from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock, patch import pytest -from sqlalchemy import select +from sqlalchemy import event, select +from sqlalchemy.exc import OperationalError from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine import backend.app.models # noqa: F401 - populate Base.metadata @@ -15,6 +17,8 @@ from backend.app.models.library import LibraryFile from backend.app.models.print_queue import PrintQueueItem, PrintQueueVariant from backend.app.models.printer import Printer from backend.app.services.print_scheduler import PrintScheduler +from backend.app.utils.archive_paths import archive_photos_dir +from backend.app.utils.library_paths import library_photos_dir from backend.tests._fixtures.background_tasks import discarding_spawn_patch @@ -27,12 +31,13 @@ async def queue_factory(tmp_path): session_maker = async_sessionmaker(engine, expire_on_commit=False) case_counter = 0 - async def make_case(*, cleanup=True, is_external=False, thumbnail_path=None, siblings=()): + async def make_case(*, cleanup=True, is_external=False, thumbnail_path=None, siblings=(), photos=()): nonlocal case_counter case_counter += 1 base_dir = tmp_path / f"case-{case_counter}" base_dir.mkdir() + archive_dir = base_dir / "archive" source_path = base_dir / "library" / f"source-{case_counter}.3mf" source_path.parent.mkdir() source_path.write_bytes(b"library source") @@ -70,10 +75,21 @@ async def queue_factory(tmp_path): thumbnail_path=thumbnail_db_path, file_metadata=None, is_external=is_external, + photos=list(photos) or None, ) db.add_all([printer, library_file]) await db.flush() + # Photos of the printed result (#3077). Written through the real + # helper so the test cannot drift from the layout the code uses. + photos_dir = None + if photos: + with patch.object(scheduler_module.settings, "archive_dir", archive_dir): + photos_dir = library_photos_dir(library_file.id) + photos_dir.mkdir(parents=True, exist_ok=True) + for name in photos: + (photos_dir / name).write_bytes(f"photo {name}".encode()) + item = PrintQueueItem( printer_id=printer.id, library_file_id=library_file.id, @@ -152,7 +168,10 @@ async def queue_factory(tmp_path): return SimpleNamespace( session_maker=session_maker, base_dir=base_dir, + archive_dir=archive_dir, source_path=source_path, + photos_dir=photos_dir, + photo_names=list(photos), thumbnail_path=thumbnail_actual_path, printer_id=printer.id, library_file_id=library_file.id, @@ -170,7 +189,14 @@ async def queue_factory(tmp_path): await engine.dispose() -async def _dispatch_library_item(ctx, *, archive_failure=False, unlink_side_effect=None): +async def _dispatch_library_item( + ctx, + *, + archive_failure=False, + unlink_side_effect=None, + cleanup_commit_failure=False, + photo_commit_failure=False, +): scheduler = PrintScheduler() async def archive_print( @@ -213,6 +239,7 @@ async def _dispatch_library_item(ctx, *, archive_failure=False, unlink_side_effe patches = [ patch.object(scheduler_module.settings, "base_dir", ctx.base_dir), + patch.object(scheduler_module.settings, "archive_dir", ctx.archive_dir), patch("backend.app.services.archive.ArchiveService.archive_print", new=archive_print), patch("backend.app.services.print_scheduler.printer_manager.is_connected", MagicMock(return_value=True)), patch("backend.app.services.print_scheduler.printer_manager.get_status", MagicMock(return_value=None)), @@ -239,10 +266,99 @@ async def _dispatch_library_item(ctx, *, archive_failure=False, unlink_side_effe stack.enter_context(patcher) async with ctx.session_maker() as db: + if cleanup_commit_failure: + _arm_commit_failure_on_library_delete(db) + if photo_commit_failure: + _arm_commit_failure_on_photo_move(db, stack) item = await db.get(PrintQueueItem, ctx.queue_item_id) await scheduler._start_print(db, item) +def _fail_inside_the_next_flush(db, statement): + """Make the next flush on this session fail, once, the way SQLite does. + + Raising *instead of* calling `db.commit()` does not reproduce a failed + commit and is not the dangerous case: the session stays ACTIVE, nothing is + expired, and every loaded instance still reads out of `__dict__`. What a + busy writer actually gives you is a statement error raised inside the + flush — SQLite takes the write lock at the first DML statement, not at + COMMIT, so "database is locked" surfaces there (#1853). SQLAlchemy rolls + that back internally through `safe_reraise` before re-raising, which + expires every loaded instance and leaves the session in pending-rollback + state: the next ORM attribute read raises PendingRollbackError, *before* + the handler's own rollback can run. That is the state a handler on this + path has to survive, so it is the state these tests have to produce. + """ + sync_session = db.sync_session + fired = False + + def after_flush(session, flush_context): + nonlocal fired + if fired: + return + fired = True + raise OperationalError(statement, {}, Exception("database is locked")) + + event.listen(sync_session, "after_flush", after_flush) + + +def _arm_commit_failure_on_library_delete(db): + """Make the one commit that removes the library row fail, once. + + Stands in for the "database is locked" cascades the commit's own comment + cites (#1853). Armed by the delete rather than by a call count so it + cannot drift onto a different commit. + """ + original_delete = db.delete + original_commit = db.commit + armed = False + + async def delete(obj): + nonlocal armed + if isinstance(obj, LibraryFile): + armed = True + return await original_delete(obj) + + async def commit(): + nonlocal armed + if armed: + armed = False + _fail_inside_the_next_flush(db, "DELETE FROM library_files WHERE library_files.id = ?") + return await original_commit() + + db.delete = delete + db.commit = commit + + +def _arm_commit_failure_on_photo_move(db, stack): + """Make the commit that records the carried photos fail, once. + + The second commit of this path (#3077): the archive and the delete are + already committed, the pictures are already on disk under the archive, + and only `archive.photos` is pending. Armed by the move itself so it + cannot drift onto the delete's commit. + """ + original_commit = db.commit + original_move = scheduler_module.move_library_photos + armed = False + + def move_library_photos(file_id, photos, destination): + nonlocal armed + carried = original_move(file_id, photos, destination) + armed = bool(carried) + return carried + + async def commit(): + nonlocal armed + if armed: + armed = False + _fail_inside_the_next_flush(db, "UPDATE print_archives SET photos=? WHERE print_archives.id = ?") + return await original_commit() + + stack.enter_context(patch.object(scheduler_module, "move_library_photos", move_library_photos)) + db.commit = commit + + async def _queue_snapshot(ctx): async with ctx.session_maker() as db: item = await db.get(PrintQueueItem, ctx.queue_item_id) @@ -279,6 +395,115 @@ async def test_external_library_file_skips_cleanup(queue_factory): assert ctx.source_path.exists() +@pytest.mark.asyncio +async def test_cleanup_moves_the_photos_into_the_archive(queue_factory): + """Photos follow the consumed file into the archive that replaces it (#3077). + + The row is hard-deleted here, so leaving the photo directory alone + orphaned it under an id nothing points at any more — and the pictures + of a print that still has a record disappeared from the UI. + """ + ctx = await queue_factory(cleanup=True, photos=["a1b2c3d4.jpg", "e5f6a7b8.png"]) + + await _dispatch_library_item(ctx) + + _, library_file, archive = await _queue_snapshot(ctx) + assert library_file is None + assert not ctx.photos_dir.exists() + assert archive.photos == ctx.photo_names + with patch.object(scheduler_module.settings, "base_dir", ctx.base_dir): + destination = archive_photos_dir(archive) + for name in ctx.photo_names: + assert (destination / name).read_bytes() == f"photo {name}".encode() + + +@pytest.mark.asyncio +async def test_external_library_file_keeps_its_photos(queue_factory): + ctx = await queue_factory(cleanup=True, is_external=True, photos=["a1b2c3d4.jpg"]) + + await _dispatch_library_item(ctx) + + _, library_file, archive = await _queue_snapshot(ctx) + assert library_file is not None + assert (ctx.photos_dir / "a1b2c3d4.jpg").is_file() + assert archive.photos is None + + +@pytest.mark.asyncio +async def test_archive_creation_failure_keeps_the_photos(queue_factory): + ctx = await queue_factory(cleanup=True, photos=["a1b2c3d4.jpg"]) + + await _dispatch_library_item(ctx, archive_failure=True) + + _, library_file, archive = await _queue_snapshot(ctx) + assert archive is None + assert library_file is not None + assert (ctx.photos_dir / "a1b2c3d4.jpg").is_file() + + +@pytest.mark.asyncio +async def test_cleanup_commit_failure_keeps_the_photos_with_the_library_file(queue_factory): + """The photos move after the delete commits, not before it (#3077). + + The commit that removes the library row can fail; the except branch rolls + it back and the file is in the library again. Photos moved ahead of that + commit would be gone from under it — the row would name a directory that + no longer exists, and the pictures would sit under an archive that was + rolled back too. + """ + ctx = await queue_factory(cleanup=True, photos=["a1b2c3d4.jpg"]) + + await _dispatch_library_item(ctx, cleanup_commit_failure=True) + + item, library_file, archive = await _queue_snapshot(ctx) + assert item.status == "failed" + assert archive is None + assert library_file is not None + assert library_file.photos == ["a1b2c3d4.jpg"] + assert (ctx.photos_dir / "a1b2c3d4.jpg").read_bytes() == b"photo a1b2c3d4.jpg" + assert not (ctx.base_dir / "archives" / "photos").exists() + + +@pytest.mark.asyncio +async def test_photo_commit_failure_still_dispatches_the_print(queue_factory, caplog): + """A failed photos commit must not take the dispatch down with it (#3077). + + The archive and the delete are committed by then, so the print goes ahead + and the pictures sit unnamed under the archive. + + The commit fails inside the flush, which is where a locked SQLite fails — + see `_fail_inside_the_next_flush`. That expires every loaded instance + twice over: once by SQLAlchemy's internal rollback, before the handler + runs at all, and again at the handler's own `rollback()`. So the handler + may not read an ORM attribute on either side of that rollback. Before it, + a read raises PendingRollbackError; after it, MissingGreenlet — the nozzle + guard's `archive.nozzle_diameter` and the upload's `printer.name` are the + ones that used to die. + """ + ctx = await queue_factory(cleanup=True, photos=["a1b2c3d4.jpg"]) + + with caplog.at_level(logging.WARNING, logger="backend.app.services.print_scheduler"): + await _dispatch_library_item(ctx, photo_commit_failure=True) + + item, library_file, archive = await _queue_snapshot(ctx) + # The handler absorbed it rather than the failure being skipped: it names + # the queue item and the archive from ints it held before the commit. + assert any( + f"Queue item {ctx.queue_item_id}: failed to carry library photos into archive {item.archive_id}" + in record.message + for record in caplog.records + ) + assert item.status == "printing" + assert item.archive_id == archive.id + assert library_file is None + assert not archive.photos + ctx.upload.assert_awaited() + ctx.start_print.assert_called() + with patch.object(scheduler_module.settings, "base_dir", ctx.base_dir): + destination = archive_photos_dir(archive) + assert (destination / "a1b2c3d4.jpg").read_bytes() == b"photo a1b2c3d4.jpg" + + @pytest.mark.asyncio async def test_archive_creation_failure_skips_cleanup_and_dispatch(queue_factory): ctx = await queue_factory(cleanup=True, thumbnail_path="relative") diff --git a/frontend/package-lock.json b/frontend/package-lock.json index b0c0bef9a..ff7795e14 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -35,6 +35,9 @@ "micromark-extension-gfm-strikethrough": "^2.1.0", "micromark-extension-gfm-table": "^2.1.1", "micromark-extension-gfm-task-list-item": "^2.1.0", + "occt-import-js": "^0.0.23", + "papaparse": "^5.7.0", + "pdfjs-dist": "^6.2.108", "qrcode.react": "^4.2.0", "react": "^19.2.0", "react-dom": "^19.2.0", @@ -43,7 +46,8 @@ "react-router-dom": "7.18.2", "react-simple-keyboard": "^3.8.164", "recharts": "^3.5.1", - "three": "^0.181.2" + "three": "^0.181.2", + "xlsx": "https://cdn.sheetjs.com/xlsx-0.20.3/xlsx-0.20.3.tgz" }, "devDependencies": { "@eslint/js": "^9.39.1", @@ -52,6 +56,7 @@ "@testing-library/react": "^16.0.0", "@testing-library/user-event": "^14.5.0", "@types/node": "^24.10.1", + "@types/papaparse": "^5.5.2", "@types/react": "^19.2.5", "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^5.2.0", @@ -1006,6 +1011,271 @@ "node": ">=18" } }, + "node_modules/@napi-rs/canvas": { + "version": "1.0.8", + "resolved": "https://registry.npmjs.org/@napi-rs/canvas/-/canvas-1.0.8.tgz", + "integrity": "sha512-/SaLcvlqGWdm0HSCWMgHu7cjJiQXfP8/mOY+6dUyV9flQz7sPBBZ+ed2zYtoukojPmxOaL7bm+d/G4GeWWoN7g==", + "license": "MIT", + "optional": true, + "workspaces": [ + "e2e/*" + ], + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + }, + "optionalDependencies": { + "@napi-rs/canvas-android-arm64": "1.0.8", + "@napi-rs/canvas-darwin-arm64": "1.0.8", + "@napi-rs/canvas-darwin-x64": "1.0.8", + "@napi-rs/canvas-linux-arm-gnueabihf": "1.0.8", + "@napi-rs/canvas-linux-arm64-gnu": "1.0.8", + "@napi-rs/canvas-linux-arm64-musl": "1.0.8", + "@napi-rs/canvas-linux-riscv64-gnu": "1.0.8", + "@napi-rs/canvas-linux-x64-gnu": "1.0.8", + "@napi-rs/canvas-linux-x64-musl": "1.0.8", + "@napi-rs/canvas-win32-arm64-msvc": "1.0.8", + "@napi-rs/canvas-win32-x64-msvc": "1.0.8" + } + }, + "node_modules/@napi-rs/canvas-android-arm64": { + "version": "1.0.8", + "resolved": "https://registry.npmjs.org/@napi-rs/canvas-android-arm64/-/canvas-android-arm64-1.0.8.tgz", + "integrity": "sha512-5+nkh8i3gt6lqS/d2jTZ1xAn6tdgtB4Lf1mW6T0Qm5/rXNwBuV1sAEyLEWan5o9gJPU/GuvHR3rvSeZ+FaGrbw==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "android" + ], + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + } + }, + "node_modules/@napi-rs/canvas-darwin-arm64": { + "version": "1.0.8", + "resolved": "https://registry.npmjs.org/@napi-rs/canvas-darwin-arm64/-/canvas-darwin-arm64-1.0.8.tgz", + "integrity": "sha512-7jQ47gi+fZ7KJmfc/5rNyy1CYw/cu4kZ0KPIYbo9UUgSdW0bKQJpt+WihEor6s4Lyp7+xc3a+3HeyXmAEbbnPg==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + } + }, + "node_modules/@napi-rs/canvas-darwin-x64": { + "version": "1.0.8", + "resolved": "https://registry.npmjs.org/@napi-rs/canvas-darwin-x64/-/canvas-darwin-x64-1.0.8.tgz", + "integrity": "sha512-rRjDMZs9pIRKGxgijwezplKc1RnJsqUokrA9h88bbTkqQ+7ePj0ZN4ZnZDy8Vu0tXs7KRlI2tQLaK4mx9QlxHg==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + } + }, + "node_modules/@napi-rs/canvas-linux-arm-gnueabihf": { + "version": "1.0.8", + "resolved": "https://registry.npmjs.org/@napi-rs/canvas-linux-arm-gnueabihf/-/canvas-linux-arm-gnueabihf-1.0.8.tgz", + "integrity": "sha512-jGcCd+8ra6Q61xKqZeiItujTpp9a9eRLcQ0jW6qYNku+WpupqOPFPY0SrsuSnXFviJwkpKYT9p7QrB4lsf3LNQ==", + "cpu": [ + "arm" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + } + }, + "node_modules/@napi-rs/canvas-linux-arm64-gnu": { + "version": "1.0.8", + "resolved": "https://registry.npmjs.org/@napi-rs/canvas-linux-arm64-gnu/-/canvas-linux-arm64-gnu-1.0.8.tgz", + "integrity": "sha512-od6I2Y7kU7i1SwZYG2EKW8rWz6JiedtPpko4WEe1DDsiikrfaotVBCRaUTM5/yeZKaZ92EatoAS+5xG+6uJlYA==", + "cpu": [ + "arm64" + ], + "libc": [ + "glibc" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + } + }, + "node_modules/@napi-rs/canvas-linux-arm64-musl": { + "version": "1.0.8", + "resolved": "https://registry.npmjs.org/@napi-rs/canvas-linux-arm64-musl/-/canvas-linux-arm64-musl-1.0.8.tgz", + "integrity": "sha512-yYkPbJDJiWj6N0gASA3CAvRypZmVpJnxU0DQg3aBhneLDQde9TPLKADsQkobNoJUtTT/lj46aWpzT48PDb3Qcg==", + "cpu": [ + "arm64" + ], + "libc": [ + "musl" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + } + }, + "node_modules/@napi-rs/canvas-linux-riscv64-gnu": { + "version": "1.0.8", + "resolved": "https://registry.npmjs.org/@napi-rs/canvas-linux-riscv64-gnu/-/canvas-linux-riscv64-gnu-1.0.8.tgz", + "integrity": "sha512-PB00MSKAp4VwK/xwe6duKxRKmH8UH4GIl1pqHSbxng0jnU9Dr7FwaDypDiqwNFZ774N+8G7mJLGuLtg9NTcQsg==", + "cpu": [ + "riscv64" + ], + "libc": [ + "glibc" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + } + }, + "node_modules/@napi-rs/canvas-linux-x64-gnu": { + "version": "1.0.8", + "resolved": "https://registry.npmjs.org/@napi-rs/canvas-linux-x64-gnu/-/canvas-linux-x64-gnu-1.0.8.tgz", + "integrity": "sha512-TWM2XWJoitLiIPCvgJh7SriC+L/T9qkYCVzC66AidsZy0QP1hkKzBzVwshCdcA3q6fIn3yE0ISbq4lMJSy8jFw==", + "cpu": [ + "x64" + ], + "libc": [ + "glibc" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + } + }, + "node_modules/@napi-rs/canvas-linux-x64-musl": { + "version": "1.0.8", + "resolved": "https://registry.npmjs.org/@napi-rs/canvas-linux-x64-musl/-/canvas-linux-x64-musl-1.0.8.tgz", + "integrity": "sha512-hb20MxKXXb5IB7AAwN8UHz9WRsa2HmdZfjsDCzjElwJoeV1aotVEwFU4FrFQcYQVzsJQLeaCc/2Qdt/0Q72mMg==", + "cpu": [ + "x64" + ], + "libc": [ + "musl" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + } + }, + "node_modules/@napi-rs/canvas-win32-arm64-msvc": { + "version": "1.0.8", + "resolved": "https://registry.npmjs.org/@napi-rs/canvas-win32-arm64-msvc/-/canvas-win32-arm64-msvc-1.0.8.tgz", + "integrity": "sha512-WwPN08IXE4SkL+FhJyPz/iFnycMAUkbphFIT4cmKLlvbSU0Zfn1R7BGJ3Hqky1S89QUYc0Q4IOScXb/42Re9wQ==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + } + }, + "node_modules/@napi-rs/canvas-win32-x64-msvc": { + "version": "1.0.8", + "resolved": "https://registry.npmjs.org/@napi-rs/canvas-win32-x64-msvc/-/canvas-win32-x64-msvc-1.0.8.tgz", + "integrity": "sha512-XkrVqKb+pxyba7kjy2LJvABFVBTE0DNpEl7MrG4OYUmaWarrXH+t54z/Czj2YxCKtizYTV4mg6phNm3x24qjhQ==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + } + }, "node_modules/@napi-rs/wasm-runtime": { "version": "1.1.5", "resolved": "https://registry.npmjs.org/@napi-rs/wasm-runtime/-/wasm-runtime-1.1.5.tgz", @@ -2461,6 +2731,16 @@ "undici-types": "~7.16.0" } }, + "node_modules/@types/papaparse": { + "version": "5.5.2", + "resolved": "https://registry.npmjs.org/@types/papaparse/-/papaparse-5.5.2.tgz", + "integrity": "sha512-gFnFp/JMzLHCwRf7tQHrNnfhN4eYBVYYI897CGX4MY1tzY9l2aLkVyx2IlKZ/SAqDbB3I1AOZW5gTMGGsqWliA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/node": "*" + } + }, "node_modules/@types/react": { "version": "19.2.13", "resolved": "https://registry.npmjs.org/@types/react/-/react-19.2.13.tgz", @@ -6307,6 +6587,12 @@ "node": ">=12.20.0" } }, + "node_modules/occt-import-js": { + "version": "0.0.23", + "resolved": "https://registry.npmjs.org/occt-import-js/-/occt-import-js-0.0.23.tgz", + "integrity": "sha512-RFfYQXYFX5C1mB1Aywm0ShcUKzXOr/VzTnlzhBSDJOR6YCAPt1HYCzeXWg1vwwjn/cUxwqRNhhtf1dlewoZYCQ==", + "license": "LGPL-2.1" + }, "node_modules/optionator": { "version": "0.9.4", "resolved": "https://registry.npmjs.org/optionator/-/optionator-0.9.4.tgz", @@ -6376,6 +6662,12 @@ "integrity": "sha512-4hLB8Py4zZce5s4yd9XzopqwVv/yGNhV1Bl8NTmCq1763HeK2+EwVTv+leGeL13Dnh2wfbqowVPXCIO0z4taYw==", "license": "(MIT AND Zlib)" }, + "node_modules/papaparse": { + "version": "5.7.0", + "resolved": "https://registry.npmjs.org/papaparse/-/papaparse-5.7.0.tgz", + "integrity": "sha512-qBGxg/7Q3Kl9Wfhrz2Z74UnvnHTXLNG6jmKJFeBvP2+y4lV7So+7SR62+Zd47JvdrCkX+nDcnr0ObPzek/+6RA==", + "license": "MIT" + }, "node_modules/parent-module": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/parent-module/-/parent-module-1.0.1.tgz", @@ -6459,6 +6751,18 @@ "dev": true, "license": "MIT" }, + "node_modules/pdfjs-dist": { + "version": "6.2.108", + "resolved": "https://registry.npmjs.org/pdfjs-dist/-/pdfjs-dist-6.2.108.tgz", + "integrity": "sha512-YxFb+SQcodN2rnX9Tn3dHYlqfb7NjlzzfONPpJd+AKoKtUjEdevTfbC07d5TcczzOK6261auRkP/M8OBHs9vFQ==", + "license": "Apache-2.0", + "engines": { + "node": ">=22.13.0 || >=24" + }, + "optionalDependencies": { + "@napi-rs/canvas": "^1.0.0" + } + }, "node_modules/picocolors": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/picocolors/-/picocolors-1.1.1.tgz", @@ -8349,6 +8653,18 @@ } } }, + "node_modules/xlsx": { + "version": "0.20.3", + "resolved": "https://cdn.sheetjs.com/xlsx-0.20.3/xlsx-0.20.3.tgz", + "integrity": "sha512-oLDq3jw7AcLqKWH2AhCpVTZl8mf6X2YReP+Neh0SJUzV/BdZYjth94tG5toiMB1PPrYtxOCfaoUCkvtuH+3AJA==", + "license": "Apache-2.0", + "bin": { + "xlsx": "bin/xlsx.njs" + }, + "engines": { + "node": ">=0.8" + } + }, "node_modules/xml-name-validator": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/xml-name-validator/-/xml-name-validator-5.0.0.tgz", diff --git a/frontend/package.json b/frontend/package.json index 6481b3ac0..3f8d7a654 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -43,6 +43,9 @@ "micromark-extension-gfm-strikethrough": "^2.1.0", "micromark-extension-gfm-table": "^2.1.1", "micromark-extension-gfm-task-list-item": "^2.1.0", + "occt-import-js": "^0.0.23", + "papaparse": "^5.7.0", + "pdfjs-dist": "^6.2.108", "qrcode.react": "^4.2.0", "react": "^19.2.0", "react-dom": "^19.2.0", @@ -51,7 +54,8 @@ "react-router-dom": "7.18.2", "react-simple-keyboard": "^3.8.164", "recharts": "^3.5.1", - "three": "^0.181.2" + "three": "^0.181.2", + "xlsx": "https://cdn.sheetjs.com/xlsx-0.20.3/xlsx-0.20.3.tgz" }, "overrides": { "minimatch": "^10.2.1", @@ -67,6 +71,7 @@ "@testing-library/react": "^16.0.0", "@testing-library/user-event": "^14.5.0", "@types/node": "^24.10.1", + "@types/papaparse": "^5.5.2", "@types/react": "^19.2.5", "@types/react-dom": "^19.2.3", "@vitejs/plugin-react": "^5.2.0", diff --git a/frontend/scripts/check-i18n-parity.mjs b/frontend/scripts/check-i18n-parity.mjs index 8a8bbb8eb..bd00d03cd 100644 --- a/frontend/scripts/check-i18n-parity.mjs +++ b/frontend/scripts/check-i18n-parity.mjs @@ -222,6 +222,7 @@ const FR_COGNATES = [ '{{filament}} @ {{temp}}°C', // drying badge: filament code + universal °C 'Simple', 'Expert', // slicer settings visibility tiers — identical words in French 'Support', // same word in French + 'Photos', '{{count}} photo', '{{count}} photos', // file details photo strip (#3077) — same word in French ]; // Italian cognates. diff --git a/frontend/src/__tests__/components/LibraryFileDetailsModal.test.tsx b/frontend/src/__tests__/components/LibraryFileDetailsModal.test.tsx new file mode 100644 index 000000000..8da3f5c61 --- /dev/null +++ b/frontend/src/__tests__/components/LibraryFileDetailsModal.test.tsx @@ -0,0 +1,324 @@ +/** + * Tests for the LibraryFileDetailsModal component (#3077). + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { render } from '../utils'; +import { LibraryFileDetailsModal } from '../../components/LibraryFileDetailsModal'; +import type { LibraryFileListItem } from '../../api/client'; +import { http, HttpResponse } from 'msw'; +import { server } from '../mocks/server'; + +const listItem: LibraryFileListItem = { + id: 7, + folder_id: null, + is_external: false, + filename: 'benchy.gcode.3mf', + file_type: 'gcode.3mf', + file_size: 1048576, + thumbnail_path: null, + print_count: 2, + duplicate_count: 0, + created_by_id: null, + created_by_username: null, + created_at: '2024-01-01T00:00:00Z', + fs_modified_at: null, + print_name: 'Benchy', + print_time_seconds: 3600, + filament_used_grams: 12.5, + sliced_for_model: 'X1C', + tags: [], +}; + +const details = { + ...listItem, + folder_name: null, + project_id: null, + project_name: null, + file_path: 'library/files/benchy.gcode.3mf', + file_hash: null, + metadata: null, + last_printed_at: null, + notes: 'Print with brim', + external_url: 'https://www.printables.com/model/1', + photos: ['abc123.jpg'], + source_url: 'https://makerworld.com/models/42', + duplicates: null, + updated_at: '2024-02-01T00:00:00Z', +}; + +describe('LibraryFileDetailsModal', () => { + const onClose = vi.fn(); + let lastUpdate: Record | null = null; + + beforeEach(() => { + vi.clearAllMocks(); + lastUpdate = null; + server.use( + http.get('/api/v1/library/files/7', () => HttpResponse.json(details)), + http.put('/api/v1/library/files/7', async ({ request }) => { + lastUpdate = (await request.json()) as Record; + return HttpResponse.json({ ...details, ...lastUpdate }); + }), + http.delete('/api/v1/library/files/7/photos/:filename', () => + HttpResponse.json({ status: 'deleted', photos: [] }) + ) + ); + }); + + it('renders the facts, notes, link and photos from the detail response', async () => { + render(); + + expect(screen.getByText('benchy.gcode.3mf')).toBeInTheDocument(); + // Header badge and the Type fact. + expect(screen.getAllByText('GCODE.3MF')).toHaveLength(2); + expect(screen.getByText('1.0 MB')).toBeInTheDocument(); + + await waitFor(() => { + expect(screen.getByDisplayValue('Print with brim')).toBeInTheDocument(); + }); + expect(screen.getByDisplayValue('https://www.printables.com/model/1')).toBeInTheDocument(); + expect(screen.getByText('Benchy')).toBeInTheDocument(); + expect(screen.getByText('X1C')).toBeInTheDocument(); + expect(screen.getByText('12.5 g')).toBeInTheDocument(); + + // Source provenance is a link, not an editable field. + const source = screen.getByRole('link', { name: /makerworld\.com\/models\/42/ }); + expect(source).toHaveAttribute('href', 'https://makerworld.com/models/42'); + expect(source).toHaveAttribute('target', '_blank'); + + const photo = screen.getByAltText('Photos') as HTMLImageElement; + expect(photo.src).toContain('/library/files/7/photos/abc123.jpg'); + }); + + it('saves edited notes and link through updateLibraryFile', async () => { + const user = userEvent.setup(); + render(); + + const notes = await screen.findByDisplayValue('Print with brim'); + const saveButton = screen.getByRole('button', { name: /save/i }); + // Nothing changed yet, so there is nothing to save. + expect(saveButton).toBeDisabled(); + + await user.clear(notes); + await user.type(notes, 'Use 0.2 mm layers'); + const link = screen.getByDisplayValue('https://www.printables.com/model/1'); + await user.clear(link); + await user.type(link, 'https://example.com/part '); + + expect(saveButton).toBeEnabled(); + await user.click(saveButton); + + await waitFor(() => { + expect(lastUpdate).toEqual({ notes: 'Use 0.2 mm layers', external_url: 'https://example.com/part' }); + }); + await waitFor(() => expect(onClose).toHaveBeenCalled()); + }); + + it('sends an empty external_url when the link is cleared', async () => { + const user = userEvent.setup(); + render(); + + const link = await screen.findByDisplayValue('https://www.printables.com/model/1'); + await user.clear(link); + await user.click(screen.getByRole('button', { name: /save/i })); + + await waitFor(() => { + expect(lastUpdate).toEqual({ notes: 'Print with brim', external_url: '' }); + }); + }); + + it('is read-only without edit permission', async () => { + render(); + + const notes = await screen.findByDisplayValue('Print with brim'); + expect(notes).toBeDisabled(); + expect(screen.getByDisplayValue('https://www.printables.com/model/1')).toBeDisabled(); + expect(screen.queryByRole('button', { name: /save/i })).not.toBeInTheDocument(); + expect(screen.queryByLabelText('Add photo')).not.toBeInTheDocument(); + expect(screen.queryByLabelText('Delete photo')).not.toBeInTheDocument(); + }); + + it('removes a photo from the grid after deleting it', async () => { + const user = userEvent.setup(); + render(); + + await screen.findByAltText('Photos'); + await user.click(screen.getByLabelText('Delete photo')); + + await waitFor(() => { + expect(screen.queryByAltText('Photos')).not.toBeInTheDocument(); + }); + }); + + it('keeps unsaved notes when a photo change refetches the file', async () => { + // Deleting a photo invalidates the detail query; the refetched file has a + // different photo list, so it is a new object and must not reseed the form. + let serverPhotos = ['abc123.jpg']; + let detailFetches = 0; + server.use( + http.get('/api/v1/library/files/7', () => { + detailFetches += 1; + return HttpResponse.json({ ...details, photos: serverPhotos }); + }), + http.delete('/api/v1/library/files/7/photos/:filename', () => { + serverPhotos = []; + return HttpResponse.json({ status: 'deleted', photos: [] }); + }) + ); + const user = userEvent.setup(); + render(); + + const notes = await screen.findByDisplayValue('Print with brim'); + await user.clear(notes); + await user.type(notes, 'Draft not saved yet'); + + await user.click(screen.getByLabelText('Delete photo')); + await waitFor(() => { + expect(screen.queryByAltText('Photos')).not.toBeInTheDocument(); + }); + await waitFor(() => expect(detailFetches).toBeGreaterThan(1)); + + expect(screen.getByDisplayValue('Draft not saved yet')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /save/i })).toBeEnabled(); + }); + + describe('lightbox', () => { + beforeEach(() => { + server.use( + http.get('/api/v1/library/files/7', () => + HttpResponse.json({ ...details, photos: ['one.jpg', 'two.jpg', 'three.jpg'] }) + ) + ); + }); + + it('opens on the photo that was clicked, not the first one', async () => { + const user = userEvent.setup(); + render(); + + const thumbnails = await screen.findAllByAltText('Photos'); + expect(thumbnails).toHaveLength(3); + await user.click(thumbnails[2]); + + expect(await screen.findByText('Photo 3 of 3')).toBeInTheDocument(); + const shown = screen.getByAltText('Photo 3') as HTMLImageElement; + expect(shown.src).toContain('/library/files/7/photos/three.jpg'); + }); + + it('keeps the details modal open when the delete confirmation is dismissed', async () => { + // The confirmation renders inside the details overlay, whose root closes + // on a backdrop click — dismissing the confirmation used to discard the + // unsaved notes and link along with it. + const user = userEvent.setup(); + const { container } = render(); + + const notes = await screen.findByDisplayValue('Print with brim'); + await user.clear(notes); + await user.type(notes, 'Draft not saved yet'); + + await user.click(screen.getAllByAltText('Photos')[0]); + await screen.findByText('Photo 1 of 3'); + await user.click(container.querySelector('.text-red-400') as HTMLElement); + + const confirmation = await screen.findByText('Delete Photo'); + await user.click(confirmation.closest('div.fixed') as HTMLElement); + + await waitFor(() => expect(screen.queryByText('Delete Photo')).not.toBeInTheDocument()); + expect(screen.getByText('Photo 1 of 3')).toBeInTheDocument(); + expect(screen.getByDisplayValue('Draft not saved yet')).toBeInTheDocument(); + expect(onClose).not.toHaveBeenCalled(); + }); + + it('closes the details modal on Escape, and only the lightbox while that is open', async () => { + const user = userEvent.setup(); + render(); + + await screen.findByDisplayValue('Print with brim'); + await user.click(screen.getAllByAltText('Photos')[1]); + await screen.findByText('Photo 2 of 3'); + + await user.keyboard('{Escape}'); + await waitFor(() => expect(screen.queryByText('Photo 2 of 3')).not.toBeInTheDocument()); + expect(onClose).not.toHaveBeenCalled(); + + await user.keyboard('{Escape}'); + await waitFor(() => expect(onClose).toHaveBeenCalled()); + }); + + it('keeps the lightbox open when the delete confirmation is dismissed with Escape', async () => { + // The confirmation has its own Escape handler. With the gallery's still + // listening, one press cancelled the prompt and closed the gallery under + // it, losing the user's place in the photo list. + const user = userEvent.setup(); + const { container } = render(); + + await screen.findByDisplayValue('Print with brim'); + await user.click(screen.getAllByAltText('Photos')[1]); + await screen.findByText('Photo 2 of 3'); + await user.click(container.querySelector('.text-red-400') as HTMLElement); + await screen.findByText('Delete Photo'); + + await user.keyboard('{Escape}'); + + await waitFor(() => expect(screen.queryByText('Delete Photo')).not.toBeInTheDocument()); + expect(screen.getByText('Photo 2 of 3')).toBeInTheDocument(); + expect(onClose).not.toHaveBeenCalled(); + }); + + describe('after the last photo is deleted from inside the lightbox', () => { + beforeEach(() => { + server.use( + http.get('/api/v1/library/files/7', () => HttpResponse.json({ ...details, photos: ['only.jpg'] })) + ); + }); + + const deleteTheOnlyPhotoFromTheLightbox = async ( + user: ReturnType, + container: HTMLElement + ) => { + await screen.findByDisplayValue('Print with brim'); + await user.click(screen.getByAltText('Photos')); + await screen.findByText('Photo 1 of 1'); + await user.click(container.querySelector('.text-red-400') as HTMLElement); + await user.click(await screen.findByRole('button', { name: /^delete$/i })); + await waitFor(() => expect(screen.queryByText('Photo 1 of 1')).not.toBeInTheDocument()); + }; + + it('leaves Escape closing the details modal', async () => { + // Emptying the list unmounts the gallery before its own "nothing left + // to show" branch can call onClose, so the details modal never learned + // the lightbox had gone and kept its Escape handler stood down. + const user = userEvent.setup(); + const { container } = render(); + + await deleteTheOnlyPhotoFromTheLightbox(user, container); + + await user.keyboard('{Escape}'); + await waitFor(() => expect(onClose).toHaveBeenCalled()); + }); + + it('does not re-open the lightbox when the next photo is uploaded', async () => { + server.use( + http.post('/api/v1/library/files/7/photos', () => + HttpResponse.json({ status: 'uploaded', photos: ['replacement.jpg'] }) + ) + ); + const user = userEvent.setup(); + const { container } = render(); + + await deleteTheOnlyPhotoFromTheLightbox(user, container); + + await user.upload( + screen.getByLabelText('Add photo'), + new File(['jpeg'], 'replacement.jpg', { type: 'image/jpeg' }) + ); + + const photo = (await screen.findByAltText('Photos')) as HTMLImageElement; + expect(photo.src).toContain('/library/files/7/photos/replacement.jpg'); + expect(screen.queryByText('Photo 1 of 1')).not.toBeInTheDocument(); + }); + }); + }); +}); diff --git a/frontend/src/__tests__/components/PdfPreviewModal.test.tsx b/frontend/src/__tests__/components/PdfPreviewModal.test.tsx new file mode 100644 index 000000000..34f896f94 --- /dev/null +++ b/frontend/src/__tests__/components/PdfPreviewModal.test.tsx @@ -0,0 +1,101 @@ +/** + * Tests for PdfPreviewModal (#2976). + * + * pdf.js cannot rasterise inside jsdom (no real canvas), so the library is + * mocked at the module boundary; the tests cover the modal's own logic — + * loading, page navigation, and error/size fallbacks. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { PdfPreviewModal } from '../../components/PdfPreviewModal'; + +const pdfjsMocks = vi.hoisted(() => { + const render = vi.fn(() => ({ promise: Promise.resolve(), cancel: vi.fn() })); + const getPage = vi.fn(async () => ({ + getViewport: ({ scale }: { scale: number }) => ({ width: 600 * scale, height: 800 * scale }), + render, + })); + const getDocument = vi.fn(() => ({ + promise: Promise.resolve({ numPages: 3, getPage }), + destroy: vi.fn(), + })); + return { render, getPage, getDocument }; +}); + +vi.mock('pdfjs-dist', () => ({ + GlobalWorkerOptions: { workerSrc: '' }, + getDocument: pdfjsMocks.getDocument, +})); + +vi.mock('pdfjs-dist/build/pdf.worker.min.mjs?url', () => ({ default: 'pdf.worker.min.mjs' })); + +vi.mock('../../api/client', () => ({ + api: { + getLibraryFileDownloadUrl: vi.fn((id: number) => `http://test/library/files/${id}/download`), + }, + getAuthToken: () => null, +})); + +const mockOnClose = vi.fn(); + +function renderModal(props: Partial[0]> = {}) { + return render( + , + ); +} + +describe('PdfPreviewModal', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response(new Uint8Array([1, 2, 3]), { status: 200 })), + ); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('shows the page indicator once the document loads', async () => { + renderModal(); + expect(await screen.findByText('Page 1 of 3')).toBeInTheDocument(); + expect(pdfjsMocks.render).toHaveBeenCalled(); + }); + + it('navigates between pages', async () => { + const user = userEvent.setup(); + renderModal(); + await screen.findByText('Page 1 of 3'); + + await user.click(screen.getByRole('button', { name: 'Next page' })); + expect(await screen.findByText('Page 2 of 3')).toBeInTheDocument(); + expect(pdfjsMocks.getPage).toHaveBeenLastCalledWith(2); + + await user.click(screen.getByRole('button', { name: 'Previous page' })); + expect(await screen.findByText('Page 1 of 3')).toBeInTheDocument(); + }); + + it('shows an error message when the document cannot be parsed', async () => { + pdfjsMocks.getDocument.mockReturnValueOnce({ promise: Promise.reject(new Error('bad pdf')), destroy: vi.fn() } as never); + renderModal(); + expect(await screen.findByText('This file cannot be previewed.')).toBeInTheDocument(); + }); + + it('refuses oversized files without fetching them', async () => { + const fetchSpy = vi.fn(); + vi.stubGlobal('fetch', fetchSpy); + renderModal({ fileSize: 500 * 1024 * 1024 }); + + expect(await screen.findByText(/too large to preview/)).toBeInTheDocument(); + expect(fetchSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/__tests__/components/SpreadsheetPreviewModal.test.tsx b/frontend/src/__tests__/components/SpreadsheetPreviewModal.test.tsx new file mode 100644 index 000000000..d94943871 --- /dev/null +++ b/frontend/src/__tests__/components/SpreadsheetPreviewModal.test.tsx @@ -0,0 +1,114 @@ +/** + * Tests for SpreadsheetPreviewModal (#2976). + * + * CSV parsing uses the real papaparse and XLSX parsing the real SheetJS — + * only the network fetch is stubbed, so the tests cover the actual parse + * paths the preview relies on. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import * as XLSX from 'xlsx'; +import { SpreadsheetPreviewModal } from '../../components/SpreadsheetPreviewModal'; + +vi.mock('../../api/client', () => ({ + api: { + getLibraryFileDownloadUrl: vi.fn((id: number) => `http://test/library/files/${id}/download`), + }, + getAuthToken: () => null, +})); + +const mockOnClose = vi.fn(); + +function stubFetchWith(bytes: ArrayBuffer | Uint8Array | string) { + const body = typeof bytes === 'string' ? new TextEncoder().encode(bytes) : bytes; + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response(body as BodyInit, { status: 200 })), + ); +} + +function renderModal(props: Partial[0]> = {}) { + return render( + , + ); +} + +describe('SpreadsheetPreviewModal', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('renders CSV cells as a read-only grid', async () => { + stubFetchWith('Article,Qty\nM3 screw,12\nBearing 608,4\n'); + renderModal(); + + expect(await screen.findByText('M3 screw')).toBeInTheDocument(); + expect(screen.getByText('Bearing 608')).toBeInTheDocument(); + expect(screen.getByText('Qty')).toBeInTheDocument(); + }); + + it('shows a truncation notice for long CSV files', async () => { + const rows = Array.from({ length: 600 }, (_, i) => `row${i},${i}`).join('\n'); + stubFetchWith(`name,value\n${rows}\n`); + renderModal(); + + expect(await screen.findByText('row0')).toBeInTheDocument(); + expect(screen.getByText(/Showing the first/)).toBeInTheDocument(); + expect(screen.queryByText('row599')).not.toBeInTheDocument(); + }); + + it('renders XLSX workbooks with one tab per sheet', async () => { + const workbook = XLSX.utils.book_new(); + XLSX.utils.book_append_sheet( + workbook, + XLSX.utils.aoa_to_sheet([ + ['Part', 'Price'], + ['Nozzle', '12.50'], + ]), + 'Parts', + ); + XLSX.utils.book_append_sheet(workbook, XLSX.utils.aoa_to_sheet([['SupplierList']]), 'Suppliers'); + const bytes = XLSX.write(workbook, { type: 'array', bookType: 'xlsx' }) as ArrayBuffer; + stubFetchWith(bytes); + + const user = userEvent.setup(); + renderModal({ filename: 'bom.xlsx', fileType: 'xlsx' }); + + expect(await screen.findByText('Nozzle')).toBeInTheDocument(); + // Both sheets appear as tabs; switching shows the second sheet's content. + await user.click(screen.getByRole('button', { name: 'Suppliers' })); + expect(await screen.findByText('SupplierList')).toBeInTheDocument(); + expect(screen.queryByText('Nozzle')).not.toBeInTheDocument(); + }); + + it('shows an error message for a broken workbook', async () => { + // A truncated ZIP: SheetJS recognises the PK magic, then fails to parse. + // (Plain text bytes would be leniently read as CSV, not rejected.) + stubFetchWith(new Uint8Array([0x50, 0x4b, 0x03, 0x04, 0x01, 0x02, 0x03])); + renderModal({ filename: 'broken.xlsx', fileType: 'xlsx' }); + + expect(await screen.findByText('This file cannot be previewed.')).toBeInTheDocument(); + }); + + it('refuses oversized files without fetching them', async () => { + const fetchSpy = vi.fn(); + vi.stubGlobal('fetch', fetchSpy); + renderModal({ fileSize: 100 * 1024 * 1024 }); + + expect(await screen.findByText(/too large to preview/)).toBeInTheDocument(); + expect(fetchSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/__tests__/pages/FileManagerFileDetails.test.tsx b/frontend/src/__tests__/pages/FileManagerFileDetails.test.tsx new file mode 100644 index 000000000..66019037d --- /dev/null +++ b/frontend/src/__tests__/pages/FileManagerFileDetails.test.tsx @@ -0,0 +1,183 @@ +/** + * File details entry points and indicators on the File Manager (#3077). + * + * The card kebab and the list row both offer "File details"; a card or row + * shows a globe (external link, opens in a new tab), a sticky-note icon + * (has notes) and a photo count only when the listing says so. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { screen, waitFor, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { render } from '../utils'; +import { FileManagerPage } from '../../pages/FileManagerPage'; +import { http, HttpResponse } from 'msw'; +import { server } from '../mocks/server'; + +vi.mock('../../components/LibraryFileDetailsModal', () => ({ + LibraryFileDetailsModal: ({ file }: { file: { filename: string } }) => ( +
{file.filename}
+ ), +})); + +const base = { + file_path: '/library/x', + file_size: 1048576, + folder_id: null, + thumbnail_path: null, + print_name: null, + print_time_seconds: null, + print_count: 0, + duplicate_count: 0, + created_at: '2024-01-01T00:00:00Z', +}; + +const files = [ + { + ...base, + id: 1, + filename: 'documented.stl', + file_type: 'stl', + external_url: 'https://www.printables.com/model/1', + has_notes: true, + photo_count: 3, + }, + { ...base, id: 2, filename: 'bare.stl', file_type: 'stl', external_url: null, has_notes: false, photo_count: 0 }, +]; + +function serve() { + server.use( + http.get('/api/v1/library/folders', () => HttpResponse.json([])), + http.get('/api/v1/library/files', () => HttpResponse.json(files)), + http.get('/api/v1/library/stats', () => + HttpResponse.json({ + total_files: files.length, + total_folders: 0, + total_size_bytes: 1, + disk_free_bytes: 1, + disk_total_bytes: 2, + }), + ), + ); +} + +function cardFor(filename: string): HTMLElement { + return screen.getByText(filename).closest('.group') as HTMLElement; +} + +function rowFor(filename: string): HTMLElement { + return screen.getByText(filename).closest('div[class*="grid-cols-"]') as HTMLElement; +} + +describe('FileManagerPage — file details (#3077)', () => { + beforeEach(() => { + serve(); + }); + + afterEach(() => { + (localStorage.getItem as ReturnType).mockReset(); + }); + + describe('grid view', () => { + it('opens the details modal from the card menu', async () => { + const user = userEvent.setup(); + render(); + + await waitFor(() => expect(screen.getByText('bare.stl')).toBeInTheDocument()); + const card = cardFor('bare.stl'); + const kebab = card.querySelector('.lucide-ellipsis-vertical')?.closest('button') as HTMLButtonElement; + await user.click(kebab); + await user.click(within(card).getByText('File details')); + + expect(await screen.findByTestId('details-modal')).toHaveTextContent('bare.stl'); + }); + + it('shows the link, notes and photo indicators only when the listing carries them', async () => { + render(); + + await waitFor(() => expect(screen.getByText('documented.stl')).toBeInTheDocument()); + + const documented = cardFor('documented.stl'); + const globe = within(documented).getByLabelText('Open link'); + expect(globe).toHaveAttribute('href', 'https://www.printables.com/model/1'); + expect(globe).toHaveAttribute('target', '_blank'); + expect(within(documented).getByLabelText('Has notes')).toBeInTheDocument(); + expect(within(documented).getByLabelText('3 photos')).toHaveTextContent('3'); + + const bare = cardFor('bare.stl'); + expect(within(bare).queryByLabelText('Open link')).not.toBeInTheDocument(); + expect(within(bare).queryByLabelText('Has notes')).not.toBeInTheDocument(); + expect(within(bare).queryByLabelText(/photos?$/)).not.toBeInTheDocument(); + }); + + it('offers the link in the card menu only when the file has one', async () => { + const user = userEvent.setup(); + render(); + + await waitFor(() => expect(screen.getByText('documented.stl')).toBeInTheDocument()); + + const documented = cardFor('documented.stl'); + await user.click(documented.querySelector('.lucide-ellipsis-vertical')?.closest('button') as HTMLButtonElement); + expect(within(documented).getByText('Open link')).toBeInTheDocument(); + await user.keyboard('{Escape}'); + + const bare = cardFor('bare.stl'); + await user.click(bare.querySelector('.lucide-ellipsis-vertical')?.closest('button') as HTMLButtonElement); + expect(within(bare).getByText('File details')).toBeInTheDocument(); + expect(within(bare).queryByText('Open link')).not.toBeInTheDocument(); + }); + + it('opens the stored link without handing the new tab a window.opener', async () => { + // The URL comes from whoever owns the file, so the page it opens must + // not get a handle back on Bambuddy's window. + const open = vi.spyOn(window, 'open').mockReturnValue(null); + const user = userEvent.setup(); + render(); + + await waitFor(() => expect(screen.getByText('documented.stl')).toBeInTheDocument()); + + const documented = cardFor('documented.stl'); + await user.click(documented.querySelector('.lucide-ellipsis-vertical')?.closest('button') as HTMLButtonElement); + await user.click(within(documented).getByText('Open link')); + + expect(open).toHaveBeenCalledWith('https://www.printables.com/model/1', '_blank', 'noopener,noreferrer'); + open.mockRestore(); + }); + }); + + describe('list view', () => { + beforeEach(() => { + (localStorage.getItem as ReturnType).mockImplementation((key: string) => + key === 'library-view-mode' ? 'list' : null, + ); + }); + + it('opens the details modal from the row action', async () => { + const user = userEvent.setup(); + render(); + + await waitFor(() => expect(screen.getByText('bare.stl')).toBeInTheDocument()); + await user.click(within(rowFor('bare.stl')).getByTitle('File details')); + + expect(await screen.findByTestId('details-modal')).toHaveTextContent('bare.stl'); + }); + + it('shows the indicators next to the name', async () => { + render(); + + await waitFor(() => expect(screen.getByText('documented.stl')).toBeInTheDocument()); + + const documented = rowFor('documented.stl'); + expect(within(documented).getByLabelText('Open link')).toHaveAttribute( + 'href', + 'https://www.printables.com/model/1', + ); + expect(within(documented).getByLabelText('Has notes')).toBeInTheDocument(); + expect(within(documented).getByTitle('3 photos')).toHaveTextContent('3'); + + const bare = rowFor('bare.stl'); + expect(within(bare).queryByLabelText('Open link')).not.toBeInTheDocument(); + expect(within(bare).queryByLabelText('Has notes')).not.toBeInTheDocument(); + }); + }); +}); diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 1eea42ae4..5925dd4da 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -7479,8 +7479,54 @@ export const api = { window.URL.revokeObjectURL(url); }, getLibraryFileThumbnailUrl: (id: number) => withMediaToken(`${API_BASE}/library/files/${id}/thumbnail`), + // Client-rendered preview thumbnail upload (#2976). STEP/PDF/spreadsheet + // previews render in the browser; the first render is posted back so the + // grid gets a thumbnail without a server-side renderer for those formats. + uploadLibraryPreviewThumbnail: async (fileId: number, thumbnail: Blob): Promise<{ updated: boolean }> => { + const formData = new FormData(); + formData.append('thumbnail', thumbnail, 'preview.png'); + const headers: Record = {}; + if (authToken) { + headers['Authorization'] = `Bearer ${authToken}`; + } + const response = await fetch(`${API_BASE}/library/files/${fileId}/preview-thumbnail`, { + method: 'POST', + headers, + body: formData, + }); + if (!response.ok) { + const error = await response.json().catch(() => ({})); + throw new Error(error.detail || `HTTP ${response.status}`); + } + return response.json(); + }, getLibraryFilePlateThumbnail: (id: number, plateIndex: number) => withMediaToken(`${API_BASE}/library/files/${id}/plate-thumbnail/${plateIndex}`), + // Photos of the printed result (#3077) — same shape as the archive photo API. + getLibraryFilePhotoUrl: (fileId: number, filename: string) => + withMediaToken(`${API_BASE}/library/files/${fileId}/photos/${encodeURIComponent(filename)}`), + uploadLibraryFilePhoto: async (fileId: number, file: File): Promise<{ status: string; filename: string; photos: string[] }> => { + const formData = new FormData(); + formData.append('file', file); + const headers: Record = {}; + if (authToken) { + headers['Authorization'] = `Bearer ${authToken}`; + } + const response = await fetch(`${API_BASE}/library/files/${fileId}/photos`, { + method: 'POST', + headers, + body: formData, + }); + if (!response.ok) { + const error = await response.json().catch(() => ({})); + throw new Error(error.detail || `HTTP ${response.status}`); + } + return response.json(); + }, + deleteLibraryFilePhoto: (fileId: number, filename: string) => + request<{ status: string; photos: string[] }>(`/library/files/${fileId}/photos/${encodeURIComponent(filename)}`, { + method: 'DELETE', + }), getLibraryFileGcodeUrl: (id: number) => `${API_BASE}/library/files/${id}/gcode`, moveLibraryFiles: (fileIds: number[], folderId: number | null) => request<{ status: string; moved: number }>('/library/files/move', { @@ -8046,6 +8092,11 @@ export interface LibraryFile { print_count: number; last_printed_at: string | null; notes: string | null; + // User link + photos of the printed result (#3077); source_url is the + // read-only import provenance (MakerWorld). + external_url: string | null; + photos: string[]; + source_url: string | null; duplicates: LibraryFileDuplicate[] | null; duplicate_count: number; // User tracking (Issue #206) @@ -8096,6 +8147,11 @@ export interface LibraryFileListItem { // matching rows on screen. 0 when the file is not grouped. variant_group_id?: number | null; variant_count?: number; + // Metadata indicators (#3077). Optional for the same reason as `tags`: older + // mocks construct list items without them. Read sites default to falsy. + external_url?: string | null; + has_notes?: boolean; + photo_count?: number; } // Variant groups (#671 / #2570): the same job sliced for different printers. @@ -8133,6 +8189,7 @@ export interface LibraryFileUpdate { folder_id?: number | null; project_id?: number | null; notes?: string | null; + external_url?: string | null; } // Library trash (#1008) diff --git a/frontend/src/components/LibraryFileDetailsModal.tsx b/frontend/src/components/LibraryFileDetailsModal.tsx new file mode 100644 index 000000000..e5f51ce81 --- /dev/null +++ b/frontend/src/components/LibraryFileDetailsModal.tsx @@ -0,0 +1,346 @@ +import { useEffect, useRef, useState } from 'react'; +import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; +import { useTranslation } from 'react-i18next'; +import { X, Save, Link, Camera, Trash2, Loader2, Plus, ExternalLink, StickyNote } from 'lucide-react'; +import { api } from '../api/client'; +import type { LibraryFileListItem } from '../api/client'; +import { Button } from './Button'; +import { PhotoGalleryModal } from './PhotoGalleryModal'; +import { useToast } from '../contexts/ToastContext'; +import { formatDate, formatDuration } from '../utils/date'; +import { formatFileSize } from '../utils/file'; + +interface LibraryFileDetailsModalProps { + file: LibraryFileListItem; + // Notes, link and photos are edits to the file, so they follow the same + // ownership gate as rename (`canModify('library', 'update', ...)`). + canEdit: boolean; + onClose: () => void; +} + +// Notes, an external link and photos of the printed result on a library +// file (#3077) — the same trio EditArchiveModal offers for an archive. Photos +// are saved as they are added; notes and the link on Save. +export function LibraryFileDetailsModal({ file, canEdit, onClose }: LibraryFileDetailsModalProps) { + const { t } = useTranslation(); + const { showToast } = useToast(); + const queryClient = useQueryClient(); + + const { data: details } = useQuery({ + queryKey: ['library-file', file.id], + queryFn: () => api.getLibraryFile(file.id), + }); + + const [notes, setNotes] = useState(''); + const [externalUrl, setExternalUrl] = useState(''); + const [photos, setPhotos] = useState([]); + const [uploadingPhoto, setUploadingPhoto] = useState(false); + // The photo the lightbox opens on, or null while it is closed. + const [galleryIndex, setGalleryIndex] = useState(null); + const photoInputRef = useRef(null); + + // Seed the form once per file. Later refetches (photo changes below, window + // focus) must not overwrite notes or a link the user is still typing. + const seededForId = useRef(null); + useEffect(() => { + if (!details || seededForId.current === details.id) return; + seededForId.current = details.id; + setNotes(details.notes ?? ''); + setExternalUrl(details.external_url ?? ''); + setPhotos(details.photos ?? []); + }, [details]); + + // Escape closes the modal, as it does in the rest of the app. Not while the + // lightbox is open on top of it — that handles Escape itself, and a second + // listener here would close both at once. + useEffect(() => { + if (galleryIndex !== null) return; + const handleKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') onClose(); + }; + window.addEventListener('keydown', handleKeyDown); + return () => window.removeEventListener('keydown', handleKeyDown); + }, [galleryIndex, onClose]); + + const invalidate = () => { + queryClient.invalidateQueries({ queryKey: ['library-files'] }); + queryClient.invalidateQueries({ queryKey: ['library-file', file.id] }); + }; + + const saveMutation = useMutation({ + mutationFn: () => api.updateLibraryFile(file.id, { notes, external_url: externalUrl.trim() }), + onSuccess: () => { + invalidate(); + showToast(t('fileManager.details.saved'), 'success'); + onClose(); + }, + onError: () => { + showToast(t('fileManager.details.saveFailed'), 'error'); + }, + }); + + const hasChanges = + !!details && (notes !== (details.notes ?? '') || externalUrl.trim() !== (details.external_url ?? '')); + + const handlePhotoUpload = async (e: React.ChangeEvent) => { + const picked = e.target.files?.[0]; + if (!picked) return; + setUploadingPhoto(true); + try { + const result = await api.uploadLibraryFilePhoto(file.id, picked); + setPhotos(result.photos); + invalidate(); + } catch { + showToast(t('fileManager.details.uploadFailed'), 'error'); + } finally { + setUploadingPhoto(false); + if (photoInputRef.current) { + photoInputRef.current.value = ''; + } + } + }; + + const handlePhotoDelete = async (filename: string) => { + try { + const result = await api.deleteLibraryFilePhoto(file.id, filename); + const remaining = result.photos ?? []; + setPhotos(remaining); + // Deleting the last photo unmounts the lightbox through the render + // guard below before its own "nothing left to show" branch can call + // onClose, so the index has to be cleared here. Left set, it keeps + // Escape disabled for good and re-opens the lightbox unasked as soon + // as another photo is uploaded. + if (remaining.length === 0) setGalleryIndex(null); + invalidate(); + } catch { + showToast(t('fileManager.details.deleteFailed'), 'error'); + } + }; + + const handleSubmit = (e: React.FormEvent) => { + e.preventDefault(); + if (!canEdit || !hasChanges) return; + saveMutation.mutate(); + }; + + const trimmedUrl = externalUrl.trim(); + const facts: Array<{ label: string; value: React.ReactNode }> = [ + { label: t('fileManager.details.size'), value: formatFileSize(file.file_size) }, + { label: t('fileManager.details.type'), value: file.file_type.toUpperCase() }, + ]; + if (details?.print_name) facts.push({ label: t('fileManager.details.printName'), value: details.print_name }); + if (details?.print_time_seconds) { + facts.push({ label: t('fileManager.details.printTime'), value: formatDuration(details.print_time_seconds) }); + } + if (details?.filament_used_grams) { + facts.push({ label: t('fileManager.details.filament'), value: `${details.filament_used_grams.toFixed(1)} g` }); + } + if (details?.sliced_for_model) facts.push({ label: t('fileManager.details.slicedFor'), value: details.sliced_for_model }); + if (details?.source_url) { + facts.push({ + label: t('fileManager.details.source'), + value: ( + + {details.source_url} + + + ), + }); + } + facts.push({ label: t('fileManager.details.created'), value: formatDate(file.created_at) }); + facts.push({ + label: t('fileManager.details.modified'), + value: formatDate(file.fs_modified_at ?? details?.updated_at ?? file.created_at), + }); + + return ( +
+
e.stopPropagation()} + > + {/* Header */} +
+
+

{t('fileManager.details.title')}

+
+

+ {file.filename} +

+ + {file.file_type.toUpperCase()} + +
+
+ +
+ +
+ {/* Facts */} +
+ {facts.map((fact) => ( +
+
{fact.label}
+
{fact.value}
+
+ ))} +
+ + {/* Notes */} +
+ +