From 42161e4287840364d42064df2f0c082d0a6bd827 Mon Sep 17 00:00:00 2001 From: Chris Date: Mon, 28 Sep 2026 20:20:52 -0700 Subject: [PATCH] Add renaming slave folders to the master's spelling The "N folders are named differently" warning now offers to rename the slave folders to match the master, marked as the recommended first step after a scan (also linked from the dashboard card). - A preview lists each rename with the differing characters highlighted; renames are selectable, and conflicts (the slave already has a folder with the master's name, symlinks, the quarantine) are skipped with a reason. - Only renames the server computes itself are applied; a request can only narrow them down, never name arbitrary paths. Only slave folders are renamed. - Case-only renames go through a temporary name so they also work on case-insensitive filesystems; a rename interrupted by a crash is completed on startup (the folder is never removed). - Renaming everything repeats until folders revealed inside renamed ones (an album under a renamed artist) are fixed too, then rescans. - Each rename is an operation in History that can be undone (which also rescans). Dry run records renames as planned. Also fixes the Review list not refreshing after a job that started and finished between two status polls, and logs the new path of operations. Co-Authored-By: Claude Opus 5.5 --- README.md | 7 + internal/api/server.go | 38 ++++++ internal/engine/engine.go | 5 + internal/engine/layout.go | 4 +- internal/engine/ops.go | 20 ++- internal/engine/rename.go | 233 +++++++++++++++++++++++++++++++++ internal/engine/rename_test.go | 163 +++++++++++++++++++++++ internal/store/models.go | 8 ++ web/src/api.ts | 6 +- web/src/pages/Activity.tsx | 3 + web/src/pages/Dashboard.tsx | 7 +- web/src/pages/PairPage.tsx | 20 ++- web/src/pages/Renames.tsx | 166 +++++++++++++++++++++++ web/src/pages/Warnings.tsx | 25 ++-- 14 files changed, 684 insertions(+), 21 deletions(-) create mode 100644 internal/engine/rename.go create mode 100644 internal/engine/rename_test.go create mode 100644 web/src/pages/Renames.tsx diff --git a/README.md b/README.md index a270915..1d52107 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,13 @@ codec-weighted bitrate (Opus and AAC count for more per kbps than MP3). You then libraries share the better file. - **Ignore.** The pair is hidden from review until you un-ignore it. +**Rename folders to match (recommended first step).** When a scan finds slave folders that are the same as a master folder with a +different spelling (`City Of Evil` vs `City of Evil`, `AC-DC` vs `AC_DC`, look-alike hyphens), the warning offers to rename the slave +folders to the master's spelling so their songs match by path. Do this right after a scan, before reviewing matches. A preview lists +every rename with the differences highlighted; conflicts (the slave already has a folder with the master's name) are skipped with the +reason. Only slave folders are renamed, nested folders revealed by a rename are handled in the same run, every rename appears in +History and can be undone, and the library is rescanned afterwards. + **Bulk review.** Switch the Review list from *Songs* to *Albums* to see one row per album (disc folders are folded in), with counts of upgrades, identical files, songs where the master is lower quality and approximate matches. Tick whole albums, or expand an album and tick individual songs, then **Link**, **Use slave copies as master**, **Ignore** or **Un-ignore** them in one go. Before diff --git a/internal/api/server.go b/internal/api/server.go index c354970..a323370 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -87,6 +87,8 @@ func (s *Server) Handler() http.Handler { r.Get("/groups", s.handleGroups) r.Post("/bulk/preview", s.handleBulkPreview) r.Post("/bulk", s.handleBulk) + r.Get("/renames", s.handleRenamePreview) + r.Post("/renames", s.handleRenames) r.Post("/link-identical", s.handleLinkIdentical) r.Get("/tree", s.handleTree) r.Post("/build", s.handleBuild) @@ -634,3 +636,39 @@ func (s *Server) handleBulk(w http.ResponseWriter, r *http.Request) { writeJSON(w, 202, j) } } + +// ---- folder renames ---- + +func (s *Server) handleRenamePreview(w http.ResponseWriter, r *http.Request) { + id, _ := idParam(r) + items, err := s.eng.PreviewRenames(r.Context(), id) + if err != nil { + opErr(w, err) + return + } + writeJSON(w, 200, map[string]any{"items": items, "dry_run": s.st.DryRun(r.Context())}) +} + +func (s *Server) handleRenames(w http.ResponseWriter, r *http.Request) { + id, _ := idParam(r) + var body struct { + // Only restricts the renames to these slave folders; empty renames all. + Only []string `json:"only"` + } + if !readJSON(w, r, &body) { + return + } + for _, p := range body.Only { + if _, err := fsops.CleanRel(p); err != nil { + writeErr(w, 400, err) + return + } + } + j, err := s.eng.StartRenames(id, body.Only) + if err != nil { + opErr(w, err) + return + } + s.log.Info("folder renames started", "pair", id, "selected", len(body.Only)) + writeJSON(w, 202, j) +} diff --git a/internal/engine/engine.go b/internal/engine/engine.go index e74d0ba..3df399e 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -316,6 +316,11 @@ func layoutWarnings(l *libs) []store.Warning { Detail: "Use Build to pick songs from the master library to hardlink into it."}} } if w := renamedWarning(renamedFolders(l)); w != nil { + for _, r := range planRenames(l) { + if r.Ready { + w.Fixable++ + } + } return []store.Warning{*w} } return nil diff --git a/internal/engine/layout.go b/internal/engine/layout.go index 6df62d0..6008502 100644 --- a/internal/engine/layout.go +++ b/internal/engine/layout.go @@ -100,8 +100,8 @@ func renamedWarning(pairs [][2]string) *store.Warning { if len(pairs) == 0 { return nil } - w := &store.Warning{Total: len(pairs), - Detail: "Songs inside these folders can only match by path once the names agree. Rename one side so they match."} + w := &store.Warning{Code: store.WarnRenamedFolders, Total: len(pairs), + Detail: "Songs inside these folders can only match by path once the names agree. Renaming the slave folders to the master's spelling is the recommended first step after a scan."} if len(pairs) == 1 { w.Title = "1 folder is named differently in the two libraries" } else { diff --git a/internal/engine/ops.go b/internal/engine/ops.go index 8fde97b..b8bccf2 100644 --- a/internal/engine/ops.go +++ b/internal/engine/ops.go @@ -180,7 +180,7 @@ func (e *Engine) finish(ctx context.Context, o *store.Operation, err error) erro e.log.Warn("operation failed", "op", o.ID, "action", o.Action, "slave", o.RelSlave, "err", err) } else { o.Status = store.OpDone - e.log.Info("operation done", "op", o.ID, "action", o.Action, "slave", o.RelSlave, "master", o.RelMaster) + e.log.Info("operation done", "op", o.ID, "action", o.Action, "slave", o.RelSlave, "master", o.RelMaster, "new", o.RelNew) } if ferr := e.st.FinishOp(ctx, o.ID, o.Status, o.Error); ferr != nil && err == nil { return ferr @@ -625,6 +625,17 @@ func (e *Engine) Undo(ctx context.Context, opID int64) error { if err != nil { return err } + if o.Action == store.ActRename { + if err := undoRename(l, o); err != nil { + return err + } + if err := e.st.MarkUndone(ctx, o.ID); err != nil { + return err + } + e.log.Info("folder rename undone", "op", o.ID, "folder", o.RelNew) + _, _ = e.StartScan(o.PairID, "undo") // paths changed; ErrBusy just means a job already runs + return nil + } res := func(r fsops.Root, rel string) string { p, e := r.Resolve(rel) if e != nil && err == nil { @@ -837,6 +848,13 @@ func (e *Engine) Recover(ctx context.Context) { return } for _, o := range ops { + if o.Action == store.ActRename { + if p, err := e.st.Pair(ctx, o.PairID); err == nil { + recoverRename(p, o, e.log) + } + _ = e.st.FinishOp(ctx, o.ID, store.OpInterrupted, "interrupted by shutdown; rescan to verify state") + continue + } if p, err := e.st.Pair(ctx, o.PairID); err == nil { for _, root := range []string{p.SlaveRoot, p.MasterRoot} { for _, rel := range []string{o.RelSlave, o.RelNew, o.RelMaster} { diff --git a/internal/engine/rename.go b/internal/engine/rename.go new file mode 100644 index 0000000..bbf8a0c --- /dev/null +++ b/internal/engine/rename.go @@ -0,0 +1,233 @@ +package engine + +import ( + "context" + "crypto/rand" + "encoding/hex" + "errors" + "fmt" + "log/slog" + "os" + "path/filepath" + "strings" + + "github.com/cdnutter/playlink/internal/fsops" + "github.com/cdnutter/playlink/internal/store" +) + +// Folder renames fix the "named differently" warning: slave folders that are +// the same as a master folder under another spelling ("City Of Evil" vs +// "City of Evil", "AC-DC" vs "AC_DC") are renamed to the master's spelling so +// their songs match by path. Only the slave library is ever renamed. + +// FolderRename is one proposed rename of a slave folder. +type FolderRename struct { + Slave string `json:"slave"` // current slave folder (relative) + Master string `json:"master"` // the master's spelling = the new slave name + Ready bool `json:"ready"` + Reason string `json:"reason,omitempty"` // why it cannot be renamed +} + +// maxRenamePasses bounds the loop that picks up folders revealed by renaming +// their parent (an artist rename can expose album folders that also differ). +const maxRenamePasses = 3 + +// planRenames lists the renames needed right now, marking conflicts. +func planRenames(l *libs) []FolderRename { + var out []FolderRename + for _, p := range renamedFolders(l) { + r := FolderRename{Slave: p[0], Master: p[1], Ready: true} + r.Reason = checkRename(l, r.Slave, r.Master) + r.Ready = r.Reason == "" + out = append(out, r) + } + return out +} + +// checkRename returns why a slave folder cannot be renamed, or "". +func checkRename(l *libs, from, to string) string { + src, err := l.slave.Resolve(from) + if err != nil { + return err.Error() + } + dst, err := l.slave.Resolve(to) + if err != nil { + return err.Error() + } + fi, err := os.Lstat(src) + switch { + case err != nil: + return "folder no longer exists (rescan)" + case fi.Mode()&os.ModeSymlink != 0: + return "is a symlink" + case !fi.IsDir(): + return "is not a folder" + case src == l.quarantine || fsops.Within(src, l.quarantine): + return "contains the quarantine folder" + } + if filepath.Dir(from) != filepath.Dir(to) { + return "rename would move the folder" + } + // A case-only rename "exists" on case-insensitive filesystems; that is fine. + if di, err := os.Lstat(dst); err == nil && !(strings.EqualFold(from, to) && os.SameFile(fi, di)) { + return fmt.Sprintf("the slave already has a folder named %q; merge the two by hand", filepath.Base(to)) + } + return "" +} + +// PreviewRenames lists the folder renames that would be made. +func (e *Engine) PreviewRenames(ctx context.Context, pairID int64) ([]FolderRename, error) { + l, err := e.loadPair(ctx, pairID) + if err != nil { + return nil, err + } + out := planRenames(l) + if out == nil { + out = []FolderRename{} + } + return out, nil +} + +// StartRenames renames slave folders to the master's spelling, then rescans. +// With only set, just those renames are made (by slave path); they must be +// among the renames the server computes itself, so a request can never name +// arbitrary paths. Later passes (folders revealed inside renamed ones) only +// run when renaming everything. +func (e *Engine) StartRenames(pairID int64, only []string) (*Job, error) { + return e.startJob(pairID, "rename-folders", func(ctx context.Context, j *Job) error { + l, err := e.loadPair(ctx, pairID) + if err != nil { + return err + } + want := map[string]bool{} + for _, s := range only { + want[filepath.Clean(s)] = true + } + done := map[string]bool{} + for pass := 0; pass < maxRenamePasses; pass++ { + var todo []FolderRename + for _, r := range planRenames(l) { + if done[r.Slave] || (len(want) > 0 && !want[r.Slave]) { + continue + } + todo = append(todo, r) + } + if len(todo) == 0 { + break + } + j.phase(fmt.Sprintf("Renaming slave folders (pass %d)", pass+1), int64(len(todo))) + for _, r := range todo { + if ctx.Err() != nil { + return ctx.Err() + } + done[r.Slave] = true + if !r.Ready { + j.skip(r.Slave + ": " + r.Reason) + } else if err := e.renameOne(ctx, j, l, r); err != nil { + j.skip(r.Slave + ": " + err.Error()) + } + j.tick() + } + if len(want) > 0 || e.st.DryRun(ctx) { + break // a dry run renames nothing, so there is nothing new to find + } + } + if e.st.DryRun(ctx) { + j.update(func(j *Job) { j.Message = "Dry run: renames recorded as planned in Activity" }) + return nil + } + // Paths changed, so the stored matches are stale. + return e.scan(ctx, j, pairID, "rename") + }) +} + +func (e *Engine) renameOne(ctx context.Context, j *Job, l *libs, r FolderRename) error { + e.mu.Lock() + defer e.mu.Unlock() + if why := checkRename(l, r.Slave, r.Master); why != "" { // re-check under the lock + return errors.New(why) + } + src, _ := l.slave.Resolve(r.Slave) + dst, _ := l.slave.Resolve(r.Master) + o := store.Operation{PairID: l.pair.ID, Action: store.ActRename, RelSlave: r.Slave, RelNew: r.Master, Trigger: "rename"} + if err := e.begin(ctx, &o); err != nil { + return err + } + if o.DryRun { + j.count("planned") + return nil + } + err := renameDir(src, dst) + if err = e.finish(ctx, &o, err); err != nil { + return err + } + j.count("renamed") + return nil +} + +// renameDir renames a folder. Case-only renames go through a temporary name +// so they also work on case-insensitive filesystems. The destination must not +// exist (checked by the caller under the engine lock). +func renameDir(src, dst string) error { + if strings.EqualFold(src, dst) { + var b [6]byte + if _, err := rand.Read(b[:]); err != nil { + return err + } + tmp := filepath.Join(filepath.Dir(src), fsops.TmpPrefix+"dir-"+hex.EncodeToString(b[:])) + if err := os.Rename(src, tmp); err != nil { + return err + } + if err := os.Rename(tmp, dst); err != nil { + _ = os.Rename(tmp, src) // put it back + return err + } + } else if err := os.Rename(src, dst); err != nil { + return err + } + return fsops.SyncDir(filepath.Dir(dst)) +} + +// undoRename renames a folder back to its old name. +func undoRename(l *libs, o store.Operation) error { + cur, err := l.slave.Resolve(o.RelNew) + if err != nil { + return err + } + old, err := l.slave.Resolve(o.RelSlave) + if err != nil { + return err + } + fi, err := os.Lstat(cur) + if err != nil || !fi.IsDir() { + return fmt.Errorf("the folder %q no longer exists", o.RelNew) + } + if di, err := os.Lstat(old); err == nil && !(strings.EqualFold(o.RelNew, o.RelSlave) && os.SameFile(fi, di)) { + return fmt.Errorf("%q exists again: %w", o.RelSlave, fsops.ErrExists) + } + return renameDir(cur, old) +} + +// recoverRename finishes a case-only folder rename cut short by a crash: the +// folder was moved to a temporary name and never reached its new name. The +// folder holds user data, so it is renamed, never removed. +func recoverRename(p store.Pair, o store.Operation, log *slog.Logger) { + dir := filepath.Join(p.SlaveRoot, filepath.Dir(o.RelSlave)) + from := filepath.Join(p.SlaveRoot, o.RelSlave) + to := filepath.Join(p.SlaveRoot, o.RelNew) + if _, err := os.Lstat(from); err == nil { + return // never moved + } + if _, err := os.Lstat(to); err == nil { + return // completed + } + ents, _ := os.ReadDir(dir) + for _, en := range ents { + if en.IsDir() && strings.HasPrefix(en.Name(), fsops.TmpPrefix+"dir-") { + if err := os.Rename(filepath.Join(dir, en.Name()), to); err == nil { + log.Warn("finished interrupted folder rename", "op", o.ID, "folder", o.RelNew) + } + return + } + } +} diff --git a/internal/engine/rename_test.go b/internal/engine/rename_test.go new file mode 100644 index 0000000..20ebedb --- /dev/null +++ b/internal/engine/rename_test.go @@ -0,0 +1,163 @@ +package engine + +import ( + "io" + "log/slog" + "os" + "path/filepath" + "testing" + + "github.com/cdnutter/playlink/internal/store" +) + +func renameLibrary(t *testing.T, dryRun bool) *env { + ev := setup(t, dryRun) + ev.write(ev.master, "AC_DC/Back in Black/01 Hells Bells.flac", "hb") + ev.write(ev.slave, "AC-DC/Back In Black/01 Hells Bells.flac", "hb") + ev.write(ev.master, "Avenged Sevenfold/City of Evil/03 Bat Country.flac", "bc") + ev.write(ev.slave, "Avenged Sevenfold/City Of Evil/03 Bat Country.flac", "bc") + ev.scan() + return ev +} + +func TestRenameFoldersToMasterSpelling(t *testing.T) { + ev := renameLibrary(t, false) + plan, err := ev.e.PreviewRenames(ctx, ev.pair.ID) + if err != nil { + t.Fatal(err) + } + if len(plan) != 2 || !plan[0].Ready || !plan[1].Ready { + t.Fatalf("want 2 ready renames, got %+v", plan) + } + if _, err := ev.e.StartRenames(ev.pair.ID, nil); err != nil { + t.Fatal(err) + } + j := ev.wait() + // The artist rename reveals "Back In Black" inside it: a second pass fixes it. + if j.Results["renamed"] != 3 { + t.Fatalf("want 3 renames over two passes, got %+v %v", j.Results, j.Skipped) + } + for _, rel := range []string{"AC_DC/Back in Black/01 Hells Bells.flac", "Avenged Sevenfold/City of Evil/03 Bat Country.flac"} { + if ev.read(filepath.Join(ev.slave, rel)) == "" { + t.Fatalf("%s missing after rename", rel) + } + // The job rescans, so the songs now match by path. + if m := ev.match(rel); m.Status != store.StatusIdentical || m.Kind != store.KindPath { + t.Fatalf("%s should match by path after the rename, got %s/%s", rel, m.Status, m.Kind) + } + } + p, _ := ev.st.Pair(ctx, ev.pair.ID) + for _, w := range p.LastScanWarnings { + if w.Code == store.WarnRenamedFolders { + t.Fatalf("renamed-folders warning should be gone, got %+v", w) + } + } + if ev.read(filepath.Join(ev.master, "AC_DC/Back in Black/01 Hells Bells.flac")) != "hb" { + t.Fatal("the master library must not change") + } + + // Undo the artist rename from History. + ops, _, _ := ev.st.Ops(ctx, store.OpFilter{PairID: ev.pair.ID}) + var artist store.Operation + for _, o := range ops { + if o.Action == store.ActRename && o.RelSlave == "AC-DC" { + artist = o + } + } + if artist.ID == 0 || artist.RelNew != "AC_DC" { + t.Fatalf("rename not recorded in history: %+v", ops) + } + if err := ev.e.Undo(ctx, artist.ID); err != nil { + t.Fatal(err) + } + ev.wait() // undo starts a rescan + if _, err := os.Stat(filepath.Join(ev.slave, "AC-DC/Back in Black/01 Hells Bells.flac")); err != nil { + t.Fatalf("undo should rename the folder back: %v", err) + } +} + +func TestRenameConflictIsSkipped(t *testing.T) { + ev := setup(t, false) + ev.write(ev.master, "Band/Album Name/01 a.flac", "a") + ev.write(ev.slave, "Band/Album name/01 a.flac", "a") + ev.write(ev.slave, "Band/Album Name/02 b.flac", "b") // the master spelling already exists too + ev.scan() + plan, _ := ev.e.PreviewRenames(ctx, ev.pair.ID) + if len(plan) != 1 || plan[0].Ready || plan[0].Reason == "" { + t.Fatalf("want one conflicting rename, got %+v", plan) + } + if _, err := ev.e.StartRenames(ev.pair.ID, nil); err != nil { + t.Fatal(err) + } + j := ev.wait() + if j.Results["renamed"] != 0 || j.Results["skipped"] != 1 { + t.Fatalf("conflict must be skipped, got %+v", j.Results) + } + if ev.read(filepath.Join(ev.slave, "Band/Album name/01 a.flac")) != "a" || ev.read(filepath.Join(ev.slave, "Band/Album Name/02 b.flac")) != "b" { + t.Fatal("nothing may change on a conflict") + } +} + +func TestRenameOnlySelectedAndNeverArbitraryPaths(t *testing.T) { + ev := renameLibrary(t, false) + ev.write(ev.slave, "Unrelated/x.flac", "x") + if _, err := ev.e.StartRenames(ev.pair.ID, []string{"Avenged Sevenfold/City Of Evil", "Unrelated"}); err != nil { + t.Fatal(err) + } + j := ev.wait() + if j.Results["renamed"] != 1 { + t.Fatalf("only the selected, computed rename may run, got %+v", j.Results) + } + if _, err := os.Stat(filepath.Join(ev.slave, "AC-DC")); err != nil { + t.Fatal("unselected folders must not be renamed") + } + if _, err := os.Stat(filepath.Join(ev.slave, "Unrelated/x.flac")); err != nil { + t.Fatal("a folder that is not a computed rename must never be touched") + } +} + +func TestRenameDryRun(t *testing.T) { + ev := renameLibrary(t, true) + if _, err := ev.e.StartRenames(ev.pair.ID, nil); err != nil { + t.Fatal(err) + } + j := ev.wait() + if j.Results["planned"] != 2 { + t.Fatalf("dry run should plan 2 renames, got %+v", j.Results) + } + if _, err := os.Stat(filepath.Join(ev.slave, "AC-DC")); err != nil { + t.Fatal("dry run must not rename anything") + } +} + +func TestRecoverInterruptedCaseRename(t *testing.T) { + root := t.TempDir() + os.MkdirAll(filepath.Join(root, "Band", ".playlink-tmp-dir-abc123"), 0o755) + os.WriteFile(filepath.Join(root, "Band", ".playlink-tmp-dir-abc123", "song.flac"), []byte("data"), 0o644) + p := store.Pair{SlaveRoot: root} + o := store.Operation{ID: 1, Action: store.ActRename, RelSlave: "Band/City Of Evil", RelNew: "Band/City of Evil"} + recoverRename(p, o, slog.New(slog.NewTextHandler(io.Discard, nil))) + if b, err := os.ReadFile(filepath.Join(root, "Band", "City of Evil", "song.flac")); err != nil || string(b) != "data" { + t.Fatalf("interrupted rename should be completed, got %v", err) + } +} + +func TestRenameWarningCountsFixable(t *testing.T) { + ev := setup(t, false) + ev.write(ev.master, "Band/Album Name/01 a.flac", "a") + ev.write(ev.slave, "Band/Album name/01 a.flac", "a") + ev.write(ev.slave, "Band/Album Name/02 b.flac", "b") // conflict + ev.write(ev.master, "Other/Record/01 c.flac", "c") + ev.write(ev.slave, "Other/record/01 c.flac", "c") // renameable + ev.scan() + p, _ := ev.st.Pair(ctx, ev.pair.ID) + for _, w := range p.LastScanWarnings { + if w.Code == store.WarnRenamedFolders { + if w.Total != 2 || w.Fixable != 1 { + t.Fatalf("want 2 renamed folders, 1 fixable; got total=%d fixable=%d", w.Total, w.Fixable) + } + return + } + } + t.Fatal("renamed-folders warning missing") +} diff --git a/internal/store/models.go b/internal/store/models.go index 94c918c..2414a36 100644 --- a/internal/store/models.go +++ b/internal/store/models.go @@ -93,15 +93,22 @@ func (s *Store) PruneSessions(ctx context.Context) { // ---- warnings ---- +// Warning codes. +const WarnRenamedFolders = "renamed_folders" + // Warning is a scan or validation notice, structured so the UI can lay it // out (a heading, an explanation and a table of affected folders) rather // than print one long sentence. type Warning struct { + // Code identifies warnings the UI offers a fix for. + Code string `json:"code,omitempty"` Title string `json:"title"` Detail string `json:"detail,omitempty"` Items []WarningItem `json:"items,omitempty"` // Total is the full number of items when Items was truncated. Total int `json:"total,omitempty"` + // Fixable is how many items the offered fix can handle automatically. + Fixable int `json:"fixable,omitempty"` } // WarningItem pairs a slave path with the master path it corresponds to. @@ -645,6 +652,7 @@ const ( ActPromote = "promote" // master replaced by slave's file, then linked ActBuild = "build" // master file linked into the slave (fresh library) ActRemove = "remove" // slave hardlink removed (master copy kept) + ActRename = "rename" // slave folder renamed to the master's spelling ) // Operation statuses. diff --git a/web/src/api.ts b/web/src/api.ts index 7c0e85d..daf5c1b 100644 --- a/web/src/api.ts +++ b/web/src/api.ts @@ -138,7 +138,7 @@ export type Match = { export type Operation = { id: number; pair_id: number; - action: "link" | "promote" | "build" | "remove"; + action: "link" | "promote" | "build" | "remove" | "rename"; rel_master: string; rel_slave: string; rel_new: string; @@ -201,8 +201,12 @@ export type BulkPreview = { }; export type ScanWarning = { + code?: "renamed_folders"; title: string; detail?: string; items?: { slave: string; master: string; count?: number }[]; total?: number; + fixable?: number; }; + +export type FolderRename = { slave: string; master: string; ready: boolean; reason?: string }; diff --git a/web/src/pages/Activity.tsx b/web/src/pages/Activity.tsx index cfe4e78..91c4d45 100644 --- a/web/src/pages/Activity.tsx +++ b/web/src/pages/Activity.tsx @@ -36,6 +36,7 @@ const actionLabel: Record = { promote: "Promoted slave → master", build: "Built into slave", remove: "Removed from slave", + rename: "Renamed slave folder", }; const statusTone: Record = { @@ -177,5 +178,7 @@ function undoText(o: Operation) { return "The hardlink is removed from the slave library. The master file is not affected."; case "remove": return "The hardlink is recreated in the slave library."; + case "rename": + return `The slave folder is renamed back from "${o.rel_new}" to "${o.rel_slave}", and the library is rescanned.`; } } diff --git a/web/src/pages/Dashboard.tsx b/web/src/pages/Dashboard.tsx index 9c6d2e6..834718e 100644 --- a/web/src/pages/Dashboard.tsx +++ b/web/src/pages/Dashboard.tsx @@ -104,8 +104,13 @@ function PairCard({ pair }: { pair: Pair }) { )} {pair.last_scan_warnings.length > 0 && ( -
+
{pair.last_scan_warnings.length === 1 ? pair.last_scan_warnings[0].title : `${pair.last_scan_warnings.length} warnings`} + {pair.last_scan_warnings.some((w) => w.code === "renamed_folders" && (w.fixable ?? 0) > 0) && ( + + Rename folders to match (recommended first) + + )}
)} diff --git a/web/src/pages/PairPage.tsx b/web/src/pages/PairPage.tsx index 864ec5b..d704835 100644 --- a/web/src/pages/PairPage.tsx +++ b/web/src/pages/PairPage.tsx @@ -23,20 +23,26 @@ export function usePair(id: number) { function useJobFinished(pair: Pair | undefined) { const qc = useQueryClient(); const toast = useToast(); - const prev = useRef(undefined); + // Keyed by job id as well as state: a fast job can start and finish between + // two polls, so "running" is never observed. + // null = pair not loaded yet; "none" = loaded, no job so far. + const prev = useRef(null); useEffect(() => { - const st = pair?.job?.state; - if (prev.current === "running" && st && st !== "running") { + if (!pair) return; + const st = pair.job?.state; + const key = pair.job ? `${pair.job.id}:${st}` : "none"; + const seen = prev.current; + prev.current = key; + if (seen !== null && key !== "none" && key !== seen && st !== "running") { qc.invalidateQueries({ queryKey: ["matches"] }); qc.invalidateQueries({ queryKey: ["tree"] }); qc.invalidateQueries({ queryKey: ["ops"] }); qc.invalidateQueries({ queryKey: ["pairs"] }); - const j = pair!.job!; + const j = pair.job!; if (st === "done") toast("ok", `${j.kind} finished${j.message ? `: ${j.message}` : ""}`); else toast("err", `${j.kind} ${st}: ${j.error}`); } - prev.current = st; - }, [pair?.job?.state]); // eslint-disable-line react-hooks/exhaustive-deps + }, [pair, pair?.job?.id, pair?.job?.state]); // eslint-disable-line react-hooks/exhaustive-deps } export function ScanButton({ pair }: { pair: Pair }) { @@ -175,7 +181,7 @@ export function PairPage({ id, tab }: { id: number; tab: string }) { )} {pair.last_scan_error && Last scan failed: {pair.last_scan_error}} - +
{tabs.map((t) => ( diff --git a/web/src/pages/Renames.tsx b/web/src/pages/Renames.tsx new file mode 100644 index 0000000..e8ccec8 --- /dev/null +++ b/web/src/pages/Renames.tsx @@ -0,0 +1,166 @@ +import { useEffect, useMemo, useState } from "react"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { AlertTriangle, ArrowRight, FolderPen } from "lucide-react"; +import { api, type FolderRename, type Pair } from "../api"; +import { Alert, Button, Modal, Spinner, cx } from "../ui"; +import { useToast } from "../toast"; +import { Diff } from "./Warnings"; + +function wantsFix() { + const q = window.location.hash.split("?")[1]; + return new URLSearchParams(q ?? "").get("fix") === "renames"; +} + +export function RenameButton({ pair, count }: { pair: Pair; count: number }) { + // The dashboard's "Rename folders to match" link opens the dialog directly. + const [open, setOpen] = useState(wantsFix); + const running = pair.job?.state === "running"; + return ( + <> + {count > 0 ? ( + + ) : ( + + )} + {open && setOpen(false)} />} + + ); +} + +function RenameDialog({ pair, onClose }: { pair: Pair; onClose: () => void }) { + const qc = useQueryClient(); + const toast = useToast(); + const preview = useQuery({ + queryKey: ["renames", pair.id], + queryFn: () => api.get<{ items: FolderRename[]; dry_run: boolean }>(`/pairs/${pair.id}/renames`), + gcTime: 0, + }); + const items = useMemo(() => preview.data?.items ?? [], [preview.data]); + const ready = items.filter((r) => r.ready); + const conflicts = items.filter((r) => !r.ready); + const [picked, setPicked] = useState>(new Set()); + useEffect(() => setPicked(new Set(ready.map((r) => r.slave))), [items]); // eslint-disable-line react-hooks/exhaustive-deps + const all = ready.length > 0 && picked.size === ready.length; + const dry = preview.data?.dry_run ?? true; + + const run = useMutation({ + // Renaming everything sends no list, so folders revealed inside renamed + // ones (album folders under a renamed artist) are renamed too. + mutationFn: () => api.post(`/pairs/${pair.id}/renames`, { only: all ? [] : [...picked] }), + onSuccess: () => { + toast("ok", dry ? "Started (dry run): renames are recorded as planned only" : "Renaming folders. The library is rescanned afterwards."); + qc.invalidateQueries({ queryKey: ["pair", pair.id] }); + qc.invalidateQueries({ queryKey: ["pairs"] }); + clearFix(); + onClose(); + }, + onError: (e) => toast("err", e.message), + }); + const close = () => { + clearFix(); + onClose(); + }; + + return ( + + {preview.isLoading && } + {preview.error && {preview.error.message}} + {preview.data && ( +
+

+ These slave folders are the same as a master folder with a different spelling. Renaming them to the master's spelling lets their songs + match by path, so it is best done first, right after a scan, before reviewing matches. +

+ + {items.length === 0 && Nothing to rename. The folder names already agree.} + + {ready.length > 0 && ( +
+ +
+ {ready.map((r) => ( + + ))} +
+
+ )} + + {conflicts.length > 0 && ( + }> +
+ {conflicts.length} can't be renamed automatically +
+
    + {conflicts.map((r) => ( +
  • + {r.slave}: {r.reason} +
    • + ))} +
+ + )} + + {ready.length > 0 && ( +
    +
  • Only folders in the slave library are renamed. The master library is not touched, and no files are moved or changed.
    • +
  • + Paths inside these folders change. Playlists or apps that point at the old paths will need updating. +
    • + {all &&
  • Folders that turn out to differ inside a renamed folder (for example an album inside a renamed artist) are renamed too.
    • } +
  • Every rename is listed in History and can be undone. The library is rescanned when it finishes.
    • +
+ )} + {dry && ready.length > 0 && Dry run is on, so these are recorded as planned in Activity. No folders change.} +
+ )} +
+ + +
+ + ); +} + +/** Drops ?fix=renames from the URL so the dialog doesn't reopen. */ +function clearFix() { + if (wantsFix()) window.history.replaceState(null, "", window.location.hash.split("?")[0]); +} diff --git a/web/src/pages/Warnings.tsx b/web/src/pages/Warnings.tsx index a5466c0..29c1524 100644 --- a/web/src/pages/Warnings.tsx +++ b/web/src/pages/Warnings.tsx @@ -1,23 +1,26 @@ import { useState } from "react"; import { AlertTriangle, ArrowRight, ChevronDown, ChevronRight } from "lucide-react"; -import type { ScanWarning } from "../api"; -import { cx } from "../ui"; +import type { Pair, ScanWarning } from "../api"; +import { Badge, cx } from "../ui"; +import { RenameButton } from "./Renames"; const PREVIEW_ROWS = 5; /** Scan and validation warnings: a heading, an explanation and a folder table. */ -export function WarningList({ warnings }: { warnings: ScanWarning[] }) { +export function WarningList({ warnings, pair }: { warnings: ScanWarning[]; pair?: Pair }) { if (warnings.length === 0) return null; + // Fixable warnings first: renaming folders is the recommended first step. + const sorted = [...warnings].sort((a, b) => Number(!!b.code) - Number(!!a.code)); return (
- {warnings.map((w) => ( - + {sorted.map((w) => ( + ))}
); } -function WarningCard({ w }: { w: ScanWarning }) { +function WarningCard({ w, pair }: { w: ScanWarning; pair?: Pair }) { const [expanded, setExpanded] = useState(false); const items = w.items ?? []; const grouped = items.some((it) => it.count); @@ -29,10 +32,14 @@ function WarningCard({ w }: { w: ScanWarning }) {
-
-
{w.title}
+
+
+ {w.title} + {pair && w.code === "renamed_folders" && (w.fixable ?? 0) > 0 && Recommended first step} +
{w.detail &&
{w.detail}
}
+ {pair && w.code === "renamed_folders" && }
{items.length > 0 && ( @@ -85,7 +92,7 @@ function WarningCard({ w }: { w: ScanWarning }) { * in a tooltip, so look-alikes such as "-" (U+002D) and "‐" (U+2010) can be * told apart. */ -function Diff({ a, b }: { a: string; b: string }) { +export function Diff({ a, b }: { a: string; b: string }) { const ca = Array.from(a); const cb = Array.from(b); let pre = 0;