diff --git a/internal/api/admin_duplicates.go b/internal/api/admin_duplicates.go index de8fe2e1..87260c05 100644 --- a/internal/api/admin_duplicates.go +++ b/internal/api/admin_duplicates.go @@ -449,3 +449,90 @@ func (h *handlers) handleMergeDuplicateGroup(w http.ResponseWriter, r *http.Requ } writeJSON(w, http.StatusOK, resp) } + +// refetchDuplicateRequest is the operator's answer for one recording filling +// several of an album's slots. keep_track_id is the copy whose title the +// recording is; empty means they cannot say, and every copy goes. +type refetchDuplicateRequest struct { + KeepTrackID string `json:"keep_track_id"` +} + +// refetchDuplicateResponse reports what went. Blocked names the releases set +// aside; empty when the album came from before Minstrel downloaded anything. +type refetchDuplicateResponse struct { + KeptTrackID *string `json:"kept_track_id"` + RemovedPaths []string `json:"removed_paths"` + Blocked []string `json:"blocked"` +} + +// handleRefetchDuplicateGroup implements POST +// /api/admin/library/duplicates/{id}/refetch (#6184): settle a group of one +// recording in several slots of an album the way the resolver would, with the +// operator's answer. The copies in slots that are not the recording's are +// removed and marked missing, the release they came from is set aside, and the +// album is wanted again. Merge is the wrong action there: it keeps one title +// whether or not it is the audio's, and fetches nothing. +// +// Errors: +// - 409 library_not_writable / 500 file_delete_failed when a file could not be +// removed (fileRemoveAPIError) +// - 404 duplicate_group_not_pending when the group was already resolved +// - 400 not_wrong_slot when the group is not one recording in several slots +// - 400 survivor_not_in_group, invalid_id, invalid_body +func (h *handlers) handleRefetchDuplicateGroup(w http.ResponseWriter, r *http.Request) { + admin, ok := requireUser(w, r) + if !ok { + return + } + groupID, ok := parseUUID(chi.URLParam(r, "id")) + if !ok { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id") + return + } + var body refetchDuplicateRequest + if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, mergeRequestBodyLimit)).Decode(&body); err != nil && !errors.Is(err, io.EOF) { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_body") + return + } + var keepID pgtype.UUID // invalid: every copy goes + if body.KeepTrackID != "" { + if keepID, ok = parseUUID(body.KeepTrackID); !ok { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id") + return + } + } + + res, err := library.SettleWrongSlot(r.Context(), h.pool, h.logger, groupID, keepID, admin.ID) + if err != nil { + if apiErr, ok := fileRemoveAPIError(err); ok { + logFileRemoveFailure(h.logger, apiErr, "group_id", uuidToString(groupID)) + writeErr(w, apiErr) + return + } + switch { + case errors.Is(err, library.ErrDuplicateGroupNotPending): + writeAdminJSONErr(w, http.StatusNotFound, "duplicate_group_not_pending") + case errors.Is(err, library.ErrNotWrongSlot): + writeAdminJSONErr(w, http.StatusBadRequest, "not_wrong_slot") + case errors.Is(err, library.ErrSurvivorNotInGroup): + writeAdminJSONErr(w, http.StatusBadRequest, "survivor_not_in_group") + default: + h.logger.Error("admin: refetch duplicate group", "group_id", uuidToString(groupID), "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + } + return + } + + resp := refetchDuplicateResponse{RemovedPaths: make([]string, 0, len(res.Removed)), Blocked: res.Blocked} + if resp.Blocked == nil { + resp.Blocked = []string{} + } + if res.Kept != nil { + id := uuidToString(res.Kept.TrackID) + resp.KeptTrackID = &id + } + for _, c := range res.Removed { + resp.RemovedPaths = append(resp.RemovedPaths, c.FilePath) + } + writeJSON(w, http.StatusOK, resp) +} diff --git a/internal/api/api.go b/internal/api/api.go index 7dca451c..45d85b2c 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -333,6 +333,7 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev admin.Post("/library/duplicates/sweep", h.handleRunDuplicateSweep) admin.Post("/library/duplicates/{id}/dismiss", h.handleDismissDuplicateGroup) admin.Post("/library/duplicates/{id}/merge", h.handleMergeDuplicateGroup) + admin.Post("/library/duplicates/{id}/refetch", h.handleRefetchDuplicateGroup) admin.Get("/invites", h.handleListInvites) admin.Post("/invites", h.handleCreateInvite) diff --git a/internal/db/dbq/duplicates.sql.go b/internal/db/dbq/duplicates.sql.go index 880f34da..9e7b70d2 100644 --- a/internal/db/dbq/duplicates.sql.go +++ b/internal/db/dbq/duplicates.sql.go @@ -328,6 +328,7 @@ SELECT g.id AS group_id, LEFT JOIN track_acoustid_lookups l ON l.track_id = t.id WHERE g.status = 'pending' AND t.missing_since IS NULL + AND ($1::uuid IS NULL OR g.id = $1::uuid) ORDER BY g.id, t.id ` @@ -366,9 +367,10 @@ type ListDuplicateGroupsForResolveRow struct { // tags or the library listing's match; ” when neither names one. // acoustid_state and acoustid_recording are what AcoustID said of the file, // when it was looked up (#6169: which song one recording cut to several slots -// is). -func (q *Queries) ListDuplicateGroupsForResolve(ctx context.Context) ([]ListDuplicateGroupsForResolveRow, error) { - rows, err := q.db.Query(ctx, listDuplicateGroupsForResolve) +// is). group_id narrows it to one group, for the operator settling it (#6184); +// NULL reads them all. +func (q *Queries) ListDuplicateGroupsForResolve(ctx context.Context, groupID pgtype.UUID) ([]ListDuplicateGroupsForResolveRow, error) { + rows, err := q.db.Query(ctx, listDuplicateGroupsForResolve, groupID) if err != nil { return nil, err } diff --git a/internal/db/queries/duplicates.sql b/internal/db/queries/duplicates.sql index d38c6e89..12cc6f14 100644 --- a/internal/db/queries/duplicates.sql +++ b/internal/db/queries/duplicates.sql @@ -197,7 +197,8 @@ UPDATE duplicate_groups -- tags or the library listing's match; '' when neither names one. -- acoustid_state and acoustid_recording are what AcoustID said of the file, -- when it was looked up (#6169: which song one recording cut to several slots --- is). +-- is). group_id narrows it to one group, for the operator settling it (#6184); +-- NULL reads them all. SELECT g.id AS group_id, g.tier, g.worst_bit_error_rate, @@ -232,6 +233,7 @@ SELECT g.id AS group_id, LEFT JOIN track_acoustid_lookups l ON l.track_id = t.id WHERE g.status = 'pending' AND t.missing_since IS NULL + AND (sqlc.narg(group_id)::uuid IS NULL OR g.id = sqlc.narg(group_id)::uuid) ORDER BY g.id, t.id; -- name: SetDuplicateGroupClasses :exec diff --git a/internal/library/duplicate_resolve.go b/internal/library/duplicate_resolve.go index 14f71163..2fc39e05 100644 --- a/internal/library/duplicate_resolve.go +++ b/internal/library/duplicate_resolve.go @@ -160,7 +160,7 @@ func ResolveDuplicatesWith( q := dbq.New(pool) res := DuplicateResolveResult{Classes: map[DuplicateClass]int{}} - rows, err := q.ListDuplicateGroupsForResolve(ctx) + rows, err := q.ListDuplicateGroupsForResolve(ctx, pgtype.UUID{}) if err != nil { return res, fmt.Errorf("list duplicate groups: %w", err) } @@ -168,12 +168,7 @@ func ResolveDuplicatesWith( res.Groups = len(groups) for _, g := range groups { - cm := make([]ClassifyMember, len(g.members)) - for i, m := range g.members { - cm[i] = ClassifyMember{AlbumID: syncpkg.FormatUUID(m.AlbumID), Title: m.Title, ArtistName: m.ArtistName} - } - g.class = ClassifyDuplicateGroup(g.tier, cm) - g.note = resolveNote(g) + g.classify() res.Classes[g.class]++ } if err := writeResolveVerdicts(ctx, q, groups); err != nil { @@ -444,6 +439,16 @@ func foldResolveGroups(rows []dbq.ListDuplicateGroupsForResolveRow) []*resolveGr return out } +// classify sets the group's class and the note the report shows beside it. +func (g *resolveGroup) classify() { + cm := make([]ClassifyMember, len(g.members)) + for i, m := range g.members { + cm[i] = ClassifyMember{AlbumID: syncpkg.FormatUUID(m.AlbumID), Title: m.Title, ArtistName: m.ArtistName} + } + g.class = ClassifyDuplicateGroup(g.tier, cm) + g.note = resolveNote(g) +} + // resolveNote is what the report says beside a group, when there is something // to say. func resolveNote(g *resolveGroup) string { diff --git a/internal/library/duplicate_settle.go b/internal/library/duplicate_settle.go index b73ac791..f6812b5d 100644 --- a/internal/library/duplicate_settle.go +++ b/internal/library/duplicate_settle.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "log/slog" - "os" "slices" "github.com/jackc/pgx/v5" @@ -45,7 +44,8 @@ import ( // Whenever copies go, the release the files came from is set aside, so the // album fetched again is not the same files (#6169). // -// Anything less certain stays with the operator. +// Anything less certain stays with the operator, who settles it the same way +// with their own answer (SettleWrongSlot). const ( // trackListFetchesPerPass is how many albums' track lists a pass may ask @@ -239,7 +239,7 @@ func settleWrongSlots( // still does not fetch the same files again. blocked := blockReleasesHolding(ctx, pool, logger, g) emptied := emptiedSlots(ctx, q, logger, g, losers, tracks) - n, err := removeWrongSlots(ctx, pool, logger, g, keep, losers, emptied, blocked) + n, err := removeWrongSlots(ctx, pool, logger, g, keep, losers, emptied, blocked, pgtype.UUID{}) removed += n if err != nil { logger.Warn("duplicate resolve: removing wrong-slot copies failed", "group_id", syncpkg.FormatUUID(g.id), "err", err) @@ -253,6 +253,86 @@ func settleWrongSlots( return removed, names } +// ErrNotWrongSlot means a group is not one recording filling several of an +// album's track slots, so there is no slot to empty and nothing to fetch again. +var ErrNotWrongSlot = errors.New("library: duplicate group is not one recording in several slots") + +// WrongSlotResult is what settling a wrong slot by hand did. Kept is nil when +// every copy went. +type WrongSlotResult struct { + Kept *MergedCopy + Removed []MergedCopy + // Blocked names the releases set aside. None when the album was imported + // before Minstrel downloaded anything: the same files may come back. + Blocked []string +} + +// SettleWrongSlot settles a one-album mismatch the resolver left for the +// operator (#6184), with their answer in place of MusicBrainz's lengths and +// AcoustID's. keepID is the copy whose title they say the recording is; an +// invalid id means they cannot say, and every copy goes. Either way it is the +// resolver's own path: the release the files came from is set aside, the +// copies removed are marked missing, and the album is wanted again for them. +// A merge does none of that, and keeps one copy's title whether or not it is +// the audio's. +// +// Copies sharing the kept copy's title are one song held twice and stay, as in +// wrongSlots: the next sweep finds the pair and the resolver merges it. +func SettleWrongSlot( + ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, groupID, keepID, actorID pgtype.UUID, +) (WrongSlotResult, error) { + if logger == nil { + logger = slog.Default() + } + q := dbq.New(pool) + rows, err := q.ListDuplicateGroupsForResolve(ctx, groupID) + if err != nil { + return WrongSlotResult{}, fmt.Errorf("read duplicate group: %w", err) + } + groups := foldResolveGroups(rows) + if len(groups) == 0 { + return WrongSlotResult{}, ErrDuplicateGroupNotPending + } + g := groups[0] + if g.classify(); g.class != ClassMismatch { + return WrongSlotResult{}, ErrNotWrongSlot + } + keep := -1 + if keepID.Valid { + keep = slices.IndexFunc(g.members, func(m dbq.ListDuplicateGroupsForResolveRow) bool { return m.TrackID == keepID }) + if keep < 0 { + return WrongSlotResult{}, ErrSurvivorNotInGroup + } + } + var losers []int + for i, m := range g.members { + if keep >= 0 && MatchTitleKey(m.Title, m.ArtistName) == MatchTitleKey(g.members[keep].Title, m.ArtistName) { + continue + } + losers = append(losers, i) + } + if len(losers) == 0 { + return WrongSlotResult{}, ErrNotWrongSlot + } + var tracks []dbq.MbReleaseTrack + if rg := g.members[0].ReleaseGroupMbid; rg != "" { + if tracks, err = q.ListMBReleaseTracksForGroup(ctx, rg); err != nil { + return WrongSlotResult{}, fmt.Errorf("read the album's track list: %w", err) + } + } + + res := WrongSlotResult{Blocked: blockReleasesHolding(ctx, pool, logger, g)} + emptied := emptiedSlots(ctx, q, logger, g, losers, tracks) + n, err := removeWrongSlots(ctx, pool, logger, g, keep, losers, emptied, res.Blocked, actorID) + if keep >= 0 { + res.Kept = &MergedCopy{TrackID: g.members[keep].TrackID, FilePath: g.members[keep].FilePath} + } + for _, i := range losers[:n] { + res.Removed = append(res.Removed, MergedCopy{TrackID: g.members[i].TrackID, FilePath: g.members[i].FilePath}) + } + return res, err +} + // sameAudio reports whether a group's copies are one recording, cut however: // identical audio, or fingerprints all but identical (sameAudioBER). func sameAudio(g *resolveGroup) bool { @@ -442,18 +522,19 @@ func neverRead(ctx context.Context, q *dbq.Queries, rg string) bool { // are stamped to wait for a copy fetched again (emptiedSlots). blocked names // the releases set aside, for the record. A file that cannot be removed stops // the group; the ones already gone are still marked, and the group waits for -// the next pass. +// the next pass. actorID is the operator settling it by hand, invalid for the +// resolver. func removeWrongSlots( ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, g *resolveGroup, keep int, losers []int, - emptied []pgtype.UUID, blocked []string, + emptied []pgtype.UUID, blocked []string, actorID pgtype.UUID, ) (int, error) { var gone []pgtype.UUID var removedFiles []map[string]string var removeErr error for _, i := range losers { m := g.members[i] - if err := removeFile(m.FilePath); err != nil && !errors.Is(err, os.ErrNotExist) { - removeErr = fmt.Errorf("remove %s: %w", m.FilePath, err) + if err := removeTrackFileOnDisk(m.FilePath); err != nil { + removeErr = err break } gone = append(gone, m.TrackID) @@ -481,6 +562,10 @@ func removeWrongSlots( if removeErr != nil { return nil } + if actorID.Valid { + _, err := tq.MarkDuplicateGroupMerged(ctx, g.id) + return err + } _, err := tq.SettleDuplicateGroupAutomatically(ctx, dbq.SettleDuplicateGroupAutomaticallyParams{ ID: g.id, Status: "merged", }) @@ -490,7 +575,7 @@ func removeWrongSlots( return 0, err } details := map[string]any{ - "automatic": true, + "automatic": !actorID.Valid, "group_id": syncpkg.FormatUUID(g.id), "artist": g.members[0].ArtistName, "removed": removedFiles, @@ -504,7 +589,7 @@ func removeWrongSlots( details["kept_title"], details["kept_path"] = kept.Title, kept.FilePath keptPath = kept.FilePath } - audit.WriteOrLog(ctx, pool, logger, pgtype.UUID{}, pgtype.UUID{}, audit.ActionDuplicateWrongFile, details) + audit.WriteOrLog(ctx, pool, logger, actorID, pgtype.UUID{}, audit.ActionDuplicateWrongFile, details) logger.Info("duplicate resolve: removed copies from slots that were not theirs", "group_id", syncpkg.FormatUUID(g.id), "kept", keptPath, "removed", len(gone)) return len(gone), removeErr diff --git a/internal/library/duplicate_settle_test.go b/internal/library/duplicate_settle_test.go new file mode 100644 index 00000000..8c623ff4 --- /dev/null +++ b/internal/library/duplicate_settle_test.go @@ -0,0 +1,148 @@ +package library + +import ( + "context" + "errors" + "path/filepath" + "strings" + "testing" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" + syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" +) + +// undecidedSlot is one recording filed as Void and as Reach, two songs +// MusicBrainz lists at lengths that cannot tell them apart, from a download +// Minstrel imported: the resolver leaves it pending for the operator (#6184). +func undecidedSlot(t *testing.T) (oneAlbumFixture, dbq.User) { + t.Helper() + f := newOneAlbumFixture(t, "acoustic", "Void", "Reach") + ctx := context.Background() + seedTrackList(t, f.pool, f.album, map[string]int{"Void": 220500, "Reach": 221000}) + if _, err := f.pool.Exec(ctx, + `INSERT INTO grabs (info_hash, name, key, client, save_path, origin, import_state, destination) + VALUES ('abc123', 'Savant - Void [WEB]', 'k', 'qbit', '/dl', 'schedule', 'imported', $1)`, + filepath.Dir(f.aPath)); err != nil { + t.Fatal(err) + } + if res, err := ResolveDuplicates(ctx, f.pool, nil, "", true); err != nil || res.WrongFiles != 0 { + t.Fatalf("resolve = %+v, %v; want the group left for the operator", res, err) + } + name := strings.NewReplacer("/", "-", " ", "-").Replace(t.Name()) + u, err := dbq.New(f.pool).CreateUser(ctx, dbq.CreateUserParams{ + Username: dbtest.TestUserPrefix + name, PasswordHash: "x", ApiTokenHash: name + "-token", + }) + if err != nil { + t.Fatal(err) + } + return f, u +} + +// emptiedTrack says whether a copy's track is marked missing and waiting for +// the album to be fetched again. +func emptiedTrack(t *testing.T, f oneAlbumFixture, id pgtype.UUID) bool { + t.Helper() + var emptied bool + if err := f.pool.QueryRow(context.Background(), + `SELECT missing_since IS NOT NULL AND emptied_at IS NOT NULL FROM tracks WHERE id = $1`, id).Scan(&emptied); err != nil { + t.Fatalf("track %s is gone: %v", syncpkg.FormatUUID(id), err) + } + return emptied +} + +// settledByHand checks the group closed as the operator's and the audit row +// names them. +func settledByHand(t *testing.T, f oneAlbumFixture, actor dbq.User) { + t.Helper() + if status, _, _, auto := (resolveFixture{pool: f.pool, groupID: f.groupID}).group(t); status != "merged" || auto { + t.Errorf("group = (%s, auto=%v), want merged by the operator", status, auto) + } + var n int + if err := f.pool.QueryRow(context.Background(), + `SELECT count(*) FROM audit_log WHERE action = 'duplicate_wrong_file' AND actor_id = $1 + AND metadata->>'group_id' = $2 AND NOT (metadata->>'automatic')::boolean`, + actor.ID, syncpkg.FormatUUID(f.groupID)).Scan(&n); err != nil { + t.Fatal(err) + } + if n != 1 { + t.Errorf("operator's wrong-file audit rows = %d, want 1", n) + } +} + +// "Get it again": the operator cannot say which song the recording is, so +// every copy goes, the download they came from is set aside, and both slots +// wait for the album fetched again. A merge would have kept one title. +func TestSettleWrongSlot_GetItAgainRemovesEveryCopy_Integration(t *testing.T) { + f, op := undecidedSlot(t) + ctx := context.Background() + res, err := SettleWrongSlot(ctx, f.pool, nil, f.groupID, pgtype.UUID{}, op.ID) + if err != nil { + t.Fatal(err) + } + if res.Kept != nil || len(res.Removed) != 2 || exists(f.aPath) || exists(f.bPath) { + t.Fatalf("result %+v, void exists %v, reach exists %v; want both copies removed", + res, exists(f.aPath), exists(f.bPath)) + } + for _, id := range []pgtype.UUID{f.a.ID, f.b.ID} { + if !emptiedTrack(t, f, id) { + t.Errorf("track %s is not waiting to be fetched again", syncpkg.FormatUUID(id)) + } + } + if !releaseBlocked(t, f.pool, "abc123") || len(res.Blocked) != 1 { + t.Errorf("blocked %v; want the download the files came from set aside", res.Blocked) + } + settledByHand(t, f, op) + + if _, err := SettleWrongSlot(ctx, f.pool, nil, f.groupID, pgtype.UUID{}, op.ID); !errors.Is(err, ErrDuplicateGroupNotPending) { + t.Errorf("settling it twice: err = %v, want ErrDuplicateGroupNotPending", err) + } +} + +// "This one is Reach": the copy the operator names stays, and only Void's slot +// is emptied and wanted again. +func TestSettleWrongSlot_KeepsTheCopyTheOperatorNames_Integration(t *testing.T) { + f, op := undecidedSlot(t) + res, err := SettleWrongSlot(context.Background(), f.pool, nil, f.groupID, f.b.ID, op.ID) + if err != nil { + t.Fatal(err) + } + if res.Kept == nil || res.Kept.TrackID != f.b.ID || len(res.Removed) != 1 || exists(f.aPath) || !exists(f.bPath) { + t.Fatalf("result %+v, void exists %v, reach exists %v; want only Void's copy removed", + res, exists(f.aPath), exists(f.bPath)) + } + if !emptiedTrack(t, f, f.a.ID) { + t.Error("Void's track is not waiting to be fetched again") + } + if !releaseBlocked(t, f.pool, "abc123") { + t.Error("the download the files came from is not set aside") + } + settledByHand(t, f, op) +} + +// Only a group of one recording in several slots is settled this way, and only +// with a copy from it. +func TestSettleWrongSlot_RefusesWhatItCannotSettle_Integration(t *testing.T) { + ctx := context.Background() + t.Run("two versions of a song", func(t *testing.T) { + f := newOneAlbumFixture(t, "acoustic", "Lovesick", "Lovesick (Instrumental)") + if _, err := SettleWrongSlot(ctx, f.pool, nil, f.groupID, pgtype.UUID{}, pgtype.UUID{}); !errors.Is(err, ErrNotWrongSlot) { + t.Errorf("err = %v, want ErrNotWrongSlot", err) + } + if !exists(f.aPath) || !exists(f.bPath) { + t.Error("a file was removed") + } + }) + t.Run("a copy from another group", func(t *testing.T) { + f, op := undecidedSlot(t) + other := pgtype.UUID{Bytes: [16]byte{1}, Valid: true} + if _, err := SettleWrongSlot(ctx, f.pool, nil, f.groupID, other, op.ID); !errors.Is(err, ErrSurvivorNotInGroup) { + t.Errorf("err = %v, want ErrSurvivorNotInGroup", err) + } + if !exists(f.aPath) || !exists(f.bPath) || releaseBlocked(t, f.pool, "abc123") { + t.Error("something changed") + } + }) +} diff --git a/web/src/lib/api/admin.ts b/web/src/lib/api/admin.ts index 317a25b2..8e71dfb1 100644 --- a/web/src/lib/api/admin.ts +++ b/web/src/lib/api/admin.ts @@ -9,6 +9,7 @@ import type { AdminDuplicatesResponse, DuplicatesView, MergeDuplicateResult, + RefetchDuplicateResult, ResolvedDuplicatesResponse, AdminPlaybackError, AdminQuarantineRow, @@ -893,6 +894,18 @@ export async function mergeDuplicateGroup( ); } +// One recording in several of an album's slots (#6184): keep the copy whose +// title it is, or none (empty keep_track_id), and fetch the album again. +export async function refetchDuplicateGroup( + id: string, + body: { keep_track_id: string } +): Promise { + return api.post( + `/api/admin/library/duplicates/${encodeURIComponent(id)}/refetch`, + body + ); +} + export async function dismissDuplicateGroup(id: string): Promise { await api.post(`/api/admin/library/duplicates/${encodeURIComponent(id)}/dismiss`, {}); } diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index 5b8848e8..48dbd69e 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -485,6 +485,15 @@ export type MergeDuplicateResult = { removed_paths: string[]; }; +// What settling one recording in several slots did (#6184). kept_track_id is +// null when every copy went; blocked names the releases set aside, empty when +// the album came from before Minstrel downloaded anything. +export type RefetchDuplicateResult = { + kept_track_id: string | null; + removed_paths: string[]; + blocked: string[]; +}; + export type AdminDuplicatesResponse = { sweep: AdminDuplicateSweep; fingerprints: { diff --git a/web/src/routes/collection/duplicates/+page.svelte b/web/src/routes/collection/duplicates/+page.svelte index 63ffd1c3..9510eba8 100644 --- a/web/src/routes/collection/duplicates/+page.svelte +++ b/web/src/routes/collection/duplicates/+page.svelte @@ -9,7 +9,8 @@ createResolvedDuplicatesQuery, runDuplicateSweep, dismissDuplicateGroup, - mergeDuplicateGroup + mergeDuplicateGroup, + refetchDuplicateGroup } from '#lib/api/admin.js'; import { errMessage } from '#lib/api/errors.js'; import { pushToast } from '#lib/stores/toast.svelte.js'; @@ -38,6 +39,14 @@ // song" and also undoes the link that made the copies one song (#5438). Merging (#3911) keeps one copy, moves the others' // likes, plays and playlist entries onto it, and deletes their files, so it // asks twice. + // + // Except for one recording filling several of an album's slots (class + // mismatch, #6184), where a merge is the wrong answer: it keeps one copy's + // title whether or not it is the audio's, and fetches nothing. There the + // operator says which song the recording is, or that they can't tell, and the + // group settles as the resolver would have: copies in other songs' slots are + // removed, the release they came from is set aside, and the album is fetched + // again. Operator, 2026-10-21: "for some of these I don't have any idea". type Tab = DuplicatesView | 'resolved'; const fromUrl = page.url.searchParams.get('view'); @@ -53,6 +62,11 @@ let keepChoice = $state>({}); let confirmingMerge = $state(null); let merging = $state(null); + // Per wrong-slot group: the copy whose title the recording is, absent when + // the operator can't tell, and the same two-click confirm. + let slotPick = $state>({}); + let confirmingRefetch = $state(null); + let refetching = $state(null); const queryStore = $derived(createDuplicatesQuery(groupView)); const query = $derived($queryStore); @@ -126,6 +140,51 @@ } } + function wrongSlot(g: AdminDuplicateGroup): boolean { + return g.class === 'mismatch'; + } + + function pickedOf(g: AdminDuplicateGroup): AdminDuplicateMember | undefined { + return g.members.find((m) => m.track_id === slotPick[g.id]); + } + + // Close to the server's MatchTitleKey: case, "&" and punctuation aside. + function songKey(title: string): string { + return title.toLowerCase().replace(/&/g, 'and').replace(/[^\p{L}\p{N}]/gu, ''); + } + + // The copies that go: every one when the operator can't tell, else those + // under another title. A copy under the picked title is the song held twice, + // and the next sweep merges it. + function refetchRemoves(g: AdminDuplicateGroup): AdminDuplicateMember[] { + const picked = pickedOf(g); + if (!picked) return g.members; + return g.members.filter((m) => songKey(m.title) !== songKey(picked.title)); + } + + async function onRefetch(group: AdminDuplicateGroup) { + if (confirmingRefetch !== group.id) { + confirmingRefetch = group.id; + return; + } + confirmingRefetch = null; + refetching = group.id; + try { + const result = await refetchDuplicateGroup(group.id, { keep_track_id: slotPick[group.id] ?? '' }); + const removed = `Removed ${plural(result.removed_paths.length, 'file', 'files')}`; + pushToast( + result.blocked.length > 0 + ? `${removed} and set aside the release they came from. The album will be fetched again.` + : `${removed}. The album will be fetched again, but Minstrel didn't download these files, so the same ones may come back.` + ); + query.refetch(); + } catch (e: unknown) { + pushToast(errMessage(e), 'error'); + } finally { + refetching = null; + } + } + // "Identical audio" and "same recording" are different claims, and an // operator deciding whether to merge needs to know which one they are // looking at before anything else. @@ -377,27 +436,49 @@ - + {#if wrongSlot(group)} + {@const picked = pickedOf(group)} + + {:else} + + {/if} @@ -424,12 +505,40 @@ {group.tier === 'exact' ? 'The audio is identical' : 'The music is the same'}, but the copies are titled {quotedList(titles)}. If those name different recordings, one file is - mislabelled and the other recording isn't in your library. Play each and keep the copy - whose title matches what you hear. + mislabelled and the other recording isn't in your library. + {#if wrongSlot(group)} + Play them. If you know which song this is, pick the copy with that title. If you can't + tell, get the album again: every copy goes, and it's fetched from another release. + {:else} + Play each and keep the copy whose title matches what you hear. + {/if}

{/if} + {#if confirmingRefetch === group.id} + {@const picked = pickedOf(group)} +
+

+ {#if picked} + The copy titled “{picked.title}” stays, and {plural(refetchRemoves(group).length, 'copy', 'copies')} + under other titles will be deleted from disk. + {:else} + All {plural(group.members.length, 'copy', 'copies')} will be deleted from disk. + {/if} + Their tracks stay, marked missing, with their likes and plays. The release the files came + from is set aside, and the album is fetched again from another one. +

+ +
+ {/if} + {#if confirmingMerge === group.id}
@@ -449,20 +558,38 @@
    {#each group.members as m (m.track_id)} - {@const keep = keeperOf(group) === m.track_id} + {@const keep = !wrongSlot(group) && keeperOf(group) === m.track_id} {@const proposed = m.track_id === group.survivor_track_id}
  • - (keepChoice[group.id] = m.track_id)} - aria-label="Keep {m.title}, {m.file_format.toUpperCase()}, {m.file_path}" - /> + {#if wrongSlot(group)} + + (slotPick[group.id] = m.track_id)} + aria-label="This is {m.title}, {m.file_path}" + /> + {:else} + (keepChoice[group.id] = m.track_id)} + aria-label="Keep {m.title}, {m.file_format.toUpperCase()}, {m.file_path}" + /> + {/if}
    {m.title} + {#if wrongSlot(group) && slotPick[group.id] === m.track_id} + + The recording is this song + + {/if} {#if keep}