From c6f31b5dfbc04939c6b2efc1135d87acdf40d3d7 Mon Sep 17 00:00:00 2001 From: dtoro Date: Sun, 5 Jul 2026 09:56:21 +0200 Subject: [PATCH] feat(move): undoable moves + Lightroom-style move/copy dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend correctness was already sound (companion files travel as a group, coordinated collision suffixes, EXDEV fallback, per-file scope checks, blocking scoped reindex) — this pass adds reversibility and brings the modal up to standard. Sidecar: - movePhotoFiles records per-file {from,to} pairs (move mode) — including siblings of photos that failed partway, since undo must restore whatever actually left its folder. Both POST /photos/move and POST /albums/:uid/convert return them as movedFiles. - New POST /files/restore-moves plays those pairs backwards: both ends scope-checked (sources aren't quarantined like the duplicates restore), never clobbers an existing destination, EXDEV fallback, blocking reindex of affected parents so the client's refetch already sees the restored layout. Dialog (all three subjects — photos, heap convert, folder reparent): - Search field on top (autofocused) filtering the tree live: matches + ancestors, force-expanded without touching the sidebar's persisted open/collapse state (new FolderTree forceExpand prop). - Arrow keys rove through visible rows with selection following focus (data-move-row attributes in FolderTree's readonly picker mode); Enter confirms from anywhere once a destination is set. - Recent destinations as one-click chips (last 5, per library base). - Live destination preview line and count-labeled confirm buttons ("Move 12 photos", "Move “2024”") with a disabled-reason tooltip. - Client-side subfolder validation mirroring the sidecar's sanitizeFilename rules (inline error, aria-invalid, confirm gated). - Pre-disables Move when every selected photo is already in the target. - Undo everywhere it's safe: photo/heap moves restore via the new endpoint, folder moves invert to another folder move, copies stay toast-only (their inverse would be deletion). Success toasts carry an inline Undo action; ⌘Z works through the shared undo stack. Co-Authored-By: Claude Sonnet 5 --- sidecar/handlers_heap.go | 18 +- sidecar/handlers_move.go | 102 +++- sidecar/main.go | 1 + .../lib/components/layout/FolderTree.svelte | 16 +- .../layout/MoveToFolderDialog.svelte | 554 ++++++++++++++---- web/src/lib/services/photoprism.ts | 19 + 6 files changed, 577 insertions(+), 133 deletions(-) diff --git a/sidecar/handlers_heap.go b/sidecar/handlers_heap.go index cd2ee97..b1b0fe1 100644 --- a/sidecar/handlers_heap.go +++ b/sidecar/handlers_heap.go @@ -127,7 +127,7 @@ func handleHeapConvert(cfg *Config, pp *ppClient) gin.HandlerFunc { return } - moved, copied, errs, err := movePhotoFiles(cfg, pp, token, photos, targetAbs, subfolder, mode, userScopeRoot(c, cfg)) + moved, copied, movedPairs, errs, err := movePhotoFiles(cfg, pp, token, photos, targetAbs, subfolder, mode, userScopeRoot(c, cfg)) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return @@ -157,6 +157,7 @@ func handleHeapConvert(cfg *Config, pp *ppClient) gin.HandlerFunc { c.JSON(http.StatusOK, gin.H{ "moved": moved, "copied": copied, + "movedFiles": movedPairs, "errors": errs, "heap_deleted": heapDeleted, }) @@ -173,17 +174,23 @@ func handleHeapConvert(cfg *Config, pp *ppClient) gin.HandlerFunc { // `scopeAbs` is the caller's userScopeRoot — source files outside it fail // per-photo, so a UID that resolves outside the user's BasePath (however // PhotoPrism came to return it) can't be used to pull files across users. -func movePhotoFiles(cfg *Config, pp *ppClient, token string, photos []heapPhoto, targetAbs, subfolder, mode, scopeAbs string) (moved, copied int, errs []heapErr, err error) { +// +// `movedPairs` records every file that physically moved (move mode only — +// copies have no inverse pair) as originals-relative {from,to}, including +// siblings of photos that later failed partway: undo must restore whatever +// actually left its folder, not just fully-successful photos. +func movePhotoFiles(cfg *Config, pp *ppClient, token string, photos []heapPhoto, targetAbs, subfolder, mode, scopeAbs string) (moved, copied int, movedPairs []dupMoved, errs []heapErr, err error) { destAbs := targetAbs if subfolder != "" { destAbs = filepath.Join(targetAbs, subfolder) if e := os.MkdirAll(destAbs, 0o755); e != nil { - return 0, 0, nil, e + return 0, 0, nil, nil, e } } sourceParents := map[string]struct{}{} errs = []heapErr{} + movedPairs = []dupMoved{} for _, photo := range photos { // Gather *every* originals-rooted file of the photo, not just the @@ -282,6 +289,9 @@ func movePhotoFiles(cfg *Config, pp *ppClient, token string, photos []heapPhoto, break } } + if dstRel, relErr := filepath.Rel(cfg.OriginalsRoot, dstAbs); relErr == nil { + movedPairs = append(movedPairs, dupMoved{From: srcRel, To: dstRel}) + } } else { if cpErr := copyFile(srcAbs, dstAbs); cpErr != nil { failure = cpErr.Error() @@ -329,7 +339,7 @@ func movePhotoFiles(cfg *Config, pp *ppClient, token string, photos []heapPhoto, fireReindex(cfg, pp, token, reindex) } - return moved, copied, errs, nil + return moved, copied, movedPairs, errs, nil } // resolveMoveTarget translates a targetFolder (Originals-relative; ""/"/"/"." diff --git a/sidecar/handlers_move.go b/sidecar/handlers_move.go index b72bfe1..4942d46 100644 --- a/sidecar/handlers_move.go +++ b/sidecar/handlers_move.go @@ -70,7 +70,7 @@ func handlePhotosMove(cfg *Config, pp *ppClient) gin.HandlerFunc { return } - moved, copied, errs, err := movePhotoFiles(cfg, pp, token, photos, targetAbs, subfolder, mode, userScopeRoot(c, cfg)) + moved, copied, movedPairs, errs, err := movePhotoFiles(cfg, pp, token, photos, targetAbs, subfolder, mode, userScopeRoot(c, cfg)) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return @@ -87,9 +87,10 @@ func handlePhotosMove(cfg *Config, pp *ppClient) gin.HandlerFunc { "errors", len(errs), ) c.JSON(http.StatusOK, gin.H{ - "moved": moved, - "copied": copied, - "errors": errs, + "moved": moved, + "copied": copied, + "movedFiles": movedPairs, + "errors": errs, }) } } @@ -203,3 +204,96 @@ func handleFolderMove(cfg *Config, pp *ppClient) gin.HandlerFunc { }) } } + +type restoreMovesBody struct { + // Moves mirror the movedFiles pairs from photos-move / heap-convert + // responses verbatim; the handler renames each `to` (current location) + // back to its `from` (original location). + Moves []dupMoved `json:"moves"` +} + +// handleRestoreMoves is the generic inverse of movePhotoFiles: it moves +// files back to where they came from, powering ⌘Z undo for photo/heap +// moves. Unlike the duplicates restore (whose sources must live in the +// .duplicates/ quarantine), both ends here are arbitrary library paths — +// so BOTH are validated against the caller's scope, and existing +// destinations are never clobbered. +// +// Route: POST /api/sidecar/files/restore-moves (behind requireSession) +func handleRestoreMoves(cfg *Config, pp *ppClient) gin.HandlerFunc { + return func(c *gin.Context) { + token := ctxToken(c) + var body restoreMovesBody + if err := c.ShouldBindJSON(&body); err != nil || len(body.Moves) == 0 { + c.JSON(http.StatusBadRequest, gin.H{"error": "moves[] required"}) + return + } + + scope := userScopeRoot(c, cfg) + type resolved struct { + srcAbs, dstAbs string + srcRel, dstRel string + } + items := make([]resolved, 0, len(body.Moves)) + for _, m := range body.Moves { + srcAbs, err := resolveUnderRoot(cfg.OriginalsRoot, m.To, true) + if err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid source path: " + m.To}) + return + } + dstAbs, err := resolveUnderRoot(cfg.OriginalsRoot, m.From, false) + if err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid destination path: " + m.From}) + return + } + if !sameOrUnder(srcAbs, scope) || !sameOrUnder(dstAbs, scope) { + c.JSON(http.StatusForbidden, gin.H{"error": "path outside your library"}) + return + } + items = append(items, resolved{srcAbs: srcAbs, dstAbs: dstAbs, srcRel: m.To, dstRel: m.From}) + } + + restored := []dupMoved{} + errs := []dupArchiveErr{} + parents := map[string]struct{}{} + for _, it := range items { + if _, err := os.Stat(it.dstAbs); err == nil { + errs = append(errs, dupArchiveErr{Path: it.srcRel, Error: "destination already exists"}) + continue + } else if !os.IsNotExist(err) { + errs = append(errs, dupArchiveErr{Path: it.srcRel, Error: err.Error()}) + continue + } + if err := os.MkdirAll(filepath.Dir(it.dstAbs), 0o755); err != nil { + errs = append(errs, dupArchiveErr{Path: it.srcRel, Error: err.Error()}) + continue + } + if err := os.Rename(it.srcAbs, it.dstAbs); err != nil { + if err2 := copyFile(it.srcAbs, it.dstAbs); err2 != nil { + errs = append(errs, dupArchiveErr{Path: it.srcRel, Error: err.Error()}) + continue + } + if err2 := os.Remove(it.srcAbs); err2 != nil { + errs = append(errs, dupArchiveErr{Path: it.srcRel, Error: "restored but source remove failed: " + err2.Error()}) + continue + } + } + restored = append(restored, dupMoved{From: it.srcRel, To: it.dstRel}) + parents[filepath.Dir(it.srcRel)] = struct{}{} + parents[filepath.Dir(it.dstRel)] = struct{}{} + slog.Info("files.restore-moves", "from", it.srcRel, "to", it.dstRel) + } + + // Block on the reindex like movePhotoFiles does — the client + // invalidates its photo queries right after this returns, and the + // refetch must already see the restored locations. + for p := range parents { + reindex := "/" + if p != "" && p != "." { + reindex = "/" + p + } + fireReindex(cfg, pp, token, reindex) + } + c.JSON(http.StatusOK, gin.H{"restored": restored, "errors": errs}) + } +} diff --git a/sidecar/main.go b/sidecar/main.go index 1f9df69..1d2f0ec 100644 --- a/sidecar/main.go +++ b/sidecar/main.go @@ -99,6 +99,7 @@ func main() { auth.POST("/albums/:uid/convert", handleHeapConvert(cfg, pp)) auth.POST("/photos/move", handlePhotosMove(cfg, pp)) + auth.POST("/files/restore-moves", handleRestoreMoves(cfg, pp)) auth.GET("/duplicates/scan", handleDupScan(cfg, pp, db)) auth.POST("/duplicates/archive", handleDupArchive(cfg, pp, db)) diff --git a/web/src/lib/components/layout/FolderTree.svelte b/web/src/lib/components/layout/FolderTree.svelte index 2bd2306..15e05a0 100644 --- a/web/src/lib/components/layout/FolderTree.svelte +++ b/web/src/lib/components/layout/FolderTree.svelte @@ -69,6 +69,10 @@ * "{n} photos" affordance. Undefined keeps the badge off entirely * (the picker dialog doesn't need it). */ counts?: Record; + /** Render every branch expanded regardless of the persisted openSet — + * the picker turns this on while a search filter is active so matches + * buried in collapsed branches stay visible. */ + forceExpand?: boolean; } let { nodes, @@ -80,7 +84,8 @@ onMove, readonly = false, selectedPath, - counts + counts, + forceExpand = false }: Props = $props(); // Auto-expanded folders, persisted to localStorage so the tree state @@ -145,7 +150,7 @@
    {#each nodes as node (node.path)} - {@const open = openSet.has(node.path)} + {@const open = forceExpand || openSet.has(node.path)} {@const active = isActive(node.path)} {@const hasChildren = node.children.length > 0}
  • @@ -185,11 +190,17 @@ + badge) is one hit target — the badge was previously a dead zone right where the user's eye lands. --> +
  • diff --git a/web/src/lib/components/layout/MoveToFolderDialog.svelte b/web/src/lib/components/layout/MoveToFolderDialog.svelte index c79ab0d..f73e505 100644 --- a/web/src/lib/components/layout/MoveToFolderDialog.svelte +++ b/web/src/lib/components/layout/MoveToFolderDialog.svelte @@ -14,26 +14,36 @@ folder keeps its own name. The picker excludes the folder itself and its descendants. - Picker reuses the readonly FolderTree; the dialog owns the selection - (`pickedPath`) so it never fights the global folderPath filter. + UX model (Lightroom-style): tree is the primary surface, with a search + field on top that filters it live (matches + their ancestors, force- + expanded). Arrow keys rove through visible rows with selection following + focus; Enter confirms; recent destinations render as one-click chips. + Moves are undoable via ⌘Z / the toast's Undo action — the sidecar returns + per-file {from,to} pairs and /files/restore-moves plays them backwards. -->