diff --git a/changelog.d/2879-adoption-suggestion-pool.md b/changelog.d/2879-adoption-suggestion-pool.md new file mode 100644 index 000000000..ad8e26cc0 --- /dev/null +++ b/changelog.d/2879-adoption-suggestion-pool.md @@ -0,0 +1,2 @@ +### Fixed +- **Import suggestions now offer every book by the matched author** (#2879): an untracked "The Matsumoto - Sarah K. L. Wilson.epub" was offered three other books by her instead of "The Matsumoto", because suggestions on **Import → In your library** only came from books still waiting for a file. A book you had skipped, or one already imported with a file somewhere else, was never offered, however exact the title. Suggestions now come from all of the author's books, an exact title comes first, and each suggestion shows its status. The scan still only attaches files on its own to books that are wanted or missing a file, so nothing changes without you choosing it. A book that already has its files or was skipped is never a one click Confirm, and adopting into a book that already has a file adds the new one alongside it and says so. The author's spelling with or without a space between initials was already treated as the same name. Thanks bhaveman for the report. diff --git a/docs/User-Guide-Wiki.md b/docs/User-Guide-Wiki.md index f02d68d6d..db05ec5c6 100644 --- a/docs/User-Guide-Wiki.md +++ b/docs/User-Guide-Wiki.md @@ -533,6 +533,12 @@ How to work through the list: - **Possible match** means the title is only similar, or the author differs. Click the suggested title to check it in the editor, where it is already selected, and adopt it from there. +- Suggestions come from **every book by the author the scan matched**, + whatever its status, and each shows that status. A book you skipped or one + that is already **Imported** is never a one click Confirm; adopting into a + book that already has a file adds the new file alongside it (#2879). + Suggestions are worked out by the scan, so a book added since the last scan + appears after the next one; until then, search for it in the editor. - **Choose book** opens the row in place: the suggestions with their scores, a search of your library (prefilled from the file), and a collapsed **Search metadata**. Metadata providers are only asked when you press Search diff --git a/internal/api/adoption_adopt.go b/internal/api/adoption_adopt.go index 23b957f5e..d3ff0d734 100644 --- a/internal/api/adoption_adopt.go +++ b/internal/api/adoption_adopt.go @@ -84,7 +84,8 @@ func (h *AdoptionHandler) Adopt(w http.ResponseWriter, r *http.Request) { // The request context may be cancelled once work has started; releasing // the claim and compensating must still happen. work := context.WithoutCancel(ctx) - if err := h.adopt(work, id, token, req); err != nil { + message, err := h.adopt(work, id, token, req) + if err != nil { var unfinished *unfinishedReversalError if errors.As(err, &unfinished) { // The claim and its record stay, so stale claim recovery can finish @@ -100,42 +101,51 @@ func (h *AdoptionHandler) Adopt(w http.ResponseWriter, r *http.Request) { h.writeAdoptionError(w, r, err) return } - h.writeUnit(w, r, id) + h.writeUnitWithMessage(w, r, id, message) } -func (h *AdoptionHandler) adopt(ctx context.Context, id int64, token string, req adoptRequest) error { +// alreadyHasFileMessage explains adopting into a book that already has a file +// of the same format on disk. Suggestions offer such books (#2879), because an +// untracked copy of a book already imported is common. The adopted files are +// added alongside: book_files is additive, the file the book already shows +// keeps showing, and nothing is deleted. Undo removes only what was added. +const alreadyHasFileMessage = "This book already had a file of this format. The adopted files were added alongside it, and the book still shows its existing file." + +// adopt does the work of Adopt. The message it returns, when not empty, +// explains an outcome that is not the obvious one. +func (h *AdoptionHandler) adopt(ctx context.Context, id int64, token string, req adoptRequest) (string, error) { unit, err := h.units.Get(ctx, id) if err != nil { - return err + return "", err } if unit == nil { - return refuse(http.StatusNotFound, "unmatched book not found") + return "", refuse(http.StatusNotFound, "unmatched book not found") } if req.Format != "" && req.Format != unit.Format { - return refuse(http.StatusBadRequest, "These files are "+unit.Format+" files, so they can only be adopted as "+unit.Format+".") + return "", refuse(http.StatusBadRequest, "These files are "+unit.Format+" files, so they can only be adopted as "+unit.Format+".") } format := unit.Format paths, err := h.registrationPaths(ctx, unit) if err != nil { - return err + return "", err } // Refuse before any side effect when a file already belongs to a book. if err := h.checkOwnership(ctx, append(paths, unit.MemberPaths...), 0); err != nil { - return err + return "", err } var book *models.Book var created addBookResult if req.BookID > 0 { if book, err = h.books.GetByID(ctx, req.BookID); err != nil { - return err + return "", err } if book == nil { - return refuse(http.StatusNotFound, "That book is no longer in your library.") + return "", refuse(http.StatusNotFound, "That book is no longer in your library.") } } else { if h.adder == nil { - return refuse(http.StatusServiceUnavailable, "Adding books is not available.") + return "", refuse(http.StatusServiceUnavailable, "Adding books is not available.") } unmonitored := false created, err = h.adder.addBookCore(ctx, addBookParams{ @@ -153,12 +163,23 @@ func (h *AdoptionHandler) adopt(ctx context.Context, id int64, token string, req // Already in the library: adopt into that row, as if picked. book = inLibrary.Book case err != nil: - return err + return "", err default: book = created.Book } } + message := "" + if !created.BookCreated { + had, err := h.hasLiveFile(ctx, book.ID, format) + if err != nil { + return "", err + } + if had { + message = alreadyHasFileMessage + } + } + rec := db.AdoptionRecord{BookID: book.ID} if created.BookCreated { rec.CreatedBookID = book.ID @@ -168,7 +189,7 @@ func (h *AdoptionHandler) adopt(ctx context.Context, id int64, token string, req } // The created rows are on record before any file is registered. if err := h.progress(ctx, id, token, rec); err != nil { - return h.failAdopt(ctx, id, rec, err) + return "", h.failAdopt(ctx, id, rec, err) } regErr := h.register(ctx, id, token, book.ID, format, paths, unit.MemberPaths, &rec) @@ -191,18 +212,34 @@ func (h *AdoptionHandler) adopt(ctx context.Context, id int64, token string, req } } if regErr != nil { - return h.failAdopt(ctx, id, rec, regErr) + return "", h.failAdopt(ctx, id, rec, regErr) } done, err := h.units.CompleteAdoption(ctx, id, token, rec) if err != nil || !done { if err == nil { err = refuse(http.StatusConflict, "This book changed while it was being adopted. Try again.") } - return h.failAdopt(ctx, id, rec, err) + return "", h.failAdopt(ctx, id, rec, err) } slog.Info("adoption: registered files in place", "unit", id, "book_id", book.ID, - "files", len(rec.Registered), "book_created", rec.CreatedBookID > 0, "author_created", rec.CreatedAuthorID > 0) - return nil + "files", len(rec.Registered), "book_created", rec.CreatedBookID > 0, "author_created", rec.CreatedAuthorID > 0, + "added_alongside_existing", message != "") + return message, nil +} + +// hasLiveFile reports whether the book has a registered file of format that +// still exists on disk. +func (h *AdoptionHandler) hasLiveFile(ctx context.Context, bookID int64, format string) (bool, error) { + files, err := h.books.ListFiles(ctx, bookID) + if err != nil { + return false, err + } + for _, f := range files { + if f.Format == format && db.BookFilePathResolves(f.Path) { + return true, nil + } + } + return false, nil } // failAdopt reverses a failed adopt's writes. If the reversal itself fails diff --git a/internal/api/adoption_test.go b/internal/api/adoption_test.go index 585ca8189..ca2f3b4b9 100644 --- a/internal/api/adoption_test.go +++ b/internal/api/adoption_test.go @@ -515,3 +515,68 @@ func TestAdoptionList_NoProviderCallsAndOneHydrationQuery(t *testing.T) { t.Fatalf("provider calls during list = %d, want 0", n) } } + +// TestAdopt_IntoABookThatAlreadyHasItsFile is the outcome #2879 makes +// reachable from a suggestion: the scan now offers a book already Imported +// with a file, because an untracked copy of it is common. Adopting there adds +// the copy alongside the existing file, keeps the book Imported and still +// showing that file, says so in the response, and Undo removes only the copy. +func TestAdopt_IntoABookThatAlreadyHasItsFile(t *testing.T) { + f := newAdoptionFixture(t, &stubMetaProvider{name: "openlibrary"}) + ctx := context.Background() + book := f.seedBook(t, "The Matsumoto") + existing := f.write(t, "Sarah K.L. Wilson/The Matsumoto (13110)/The Matsumoto - Sarah K.L. Wilson.epub") + if err := f.books.AddBookFile(ctx, book.ID, models.MediaTypeEbook, existing); err != nil { + t.Fatal(err) + } + copyPath := f.write(t, "Sarah K. L. Wilson/The Matsumoto (13110)/The Matsumoto - Sarah K. L. Wilson.epub") + id := f.seedUnit(t, db.UnmatchedUnitScan{UnitPath: copyPath, MemberPaths: []string{copyPath}}) + + rec := f.post(t, fmt.Sprintf("/library/unmatched/%d/adopt", id), map[string]any{"bookId": book.ID}) + if rec.Code != http.StatusOK { + t.Fatalf("adopt = %d %s", rec.Code, rec.Body.String()) + } + it := decodeItem(t, rec) + if it.State != db.UnmatchedStateAdopted || it.Message != alreadyHasFileMessage { + t.Fatalf("adopted item = state %s message %q, want adopted with the already had a file message", it.State, it.Message) + } + if got := filePaths(t, f.books, book.ID); len(got) != 2 || got[0] != existing || got[1] != copyPath { + t.Fatalf("book files = %v, want the existing file then the adopted copy", got) + } + after, _ := f.books.GetByID(ctx, book.ID) + if after.Status != models.BookStatusImported || after.EbookFilePath != existing { + t.Fatalf("book after adopt = status %s showing %q, want imported still showing %s", after.Status, after.EbookFilePath, existing) + } + + rec = f.post(t, fmt.Sprintf("/library/unmatched/%d/undo", id), nil) + if rec.Code != http.StatusOK { + t.Fatalf("undo = %d %s", rec.Code, rec.Body.String()) + } + if got := filePaths(t, f.books, book.ID); len(got) != 1 || got[0] != existing { + t.Fatalf("book files after undo = %v, want only the existing file", got) + } + if b, _ := f.books.GetByID(ctx, book.ID); b.Status != models.BookStatusImported { + t.Fatalf("book after undo = status %s, want imported", b.Status) + } + for _, p := range []string{existing, copyPath} { + if _, err := os.Stat(p); err != nil { + t.Fatalf("%s touched on disk: %v", p, err) + } + } +} + +// TestAdopt_IntoAWantedBookSaysNothingExtra: the message is only for a book +// that already had a file, so an ordinary adoption stays quiet. +func TestAdopt_IntoAWantedBookSaysNothingExtra(t *testing.T) { + f := newAdoptionFixture(t, &stubMetaProvider{name: "openlibrary"}) + book := f.seedBook(t, "Ancillary Sword") + path := f.write(t, "Ann Leckie/Ancillary Sword.epub") + id := f.seedUnit(t, db.UnmatchedUnitScan{UnitPath: path, MemberPaths: []string{path}}) + rec := f.post(t, fmt.Sprintf("/library/unmatched/%d/adopt", id), map[string]any{"bookId": book.ID}) + if rec.Code != http.StatusOK { + t.Fatalf("adopt = %d %s", rec.Code, rec.Body.String()) + } + if it := decodeItem(t, rec); it.Message != "" { + t.Fatalf("message = %q, want none", it.Message) + } +} diff --git a/internal/importer/scanner.go b/internal/importer/scanner.go index 6f43ed8fc..54e401360 100644 --- a/internal/importer/scanner.go +++ b/internal/importer/scanner.go @@ -2842,6 +2842,16 @@ type scanBook struct { book *models.Book normTitle string normLen int + // reconcilable is isReconcileCandidate for the book: the scan may claim + // a file for it on its own. Only the suggestion pool holds books without + // it (#2879). + reconcilable bool +} + +func newScanBook(b *models.Book, reconcilable bool) scanBook { + sb := scanBook{book: b, normTitle: normalizeTitle(b.Title), reconcilable: reconcilable} + sb.normLen = len(sb.normTitle) + return sb } // cleanLayoutTitle strips bracket/paren annotations from a book-folder name — @@ -3205,8 +3215,7 @@ func (s *Scanner) scanLibrary(ctx context.Context) { if !isReconcileCandidate(b) { continue } - sb := scanBook{book: b, normTitle: normalizeTitle(b.Title)} - sb.normLen = len(sb.normTitle) + sb := newScanBook(b, true) idx := len(wantedBooks) wantedBooks = append(wantedBooks, sb) booksByAuthor[b.AuthorID] = append(booksByAuthor[b.AuthorID], idx) @@ -3751,11 +3760,22 @@ func (s *Scanner) scanLibrary(ctx context.Context) { "reconciled", reconciled, "unmatched", unmatched, "tagReadFailed", tagReadFailed) // Suggestions come from the catalogue already in memory, ranked once per - // unit rather than per file. + // unit rather than per file. A person confirms a suggestion, so for a + // resolved author they are drawn from every book of that author, not only + // the ones the scan may claim by itself: a Skipped book, or one already + // Imported with a file elsewhere, is often exactly what an untracked copy + // is (#2879). Built on first use, so a scan with nothing unmatched pays + // nothing for it. Excluded books are not in allBooks and so are never + // offered. + var catalogue []scanBook + var catalogueByAuthor map[int64][]int units := s.recordUnmatchedUnits(ctx, &unmatchedFiles, scanRoots, rootsWithFiles, scanStartedAt, func(title, layoutTitle, author, layoutAuthor string) []db.UnmatchedCandidate { + if catalogueByAuthor == nil { + catalogue, catalogueByAuthor = suggestionCatalogue(allBooks, wantedBooks) + } authorSet, _ := resolveAuthors(author, layoutAuthor) - return rankCandidates(title, layoutTitle, wantedBooks, booksByAuthor, authorSet) + return rankCandidates(title, layoutTitle, wantedBooks, catalogue, catalogueByAuthor, authorSet) }) s.writeScanResult(ctx, len(foundFiles), reconciled, unmatched, alreadyTracked, tagReadFailed, units) diff --git a/internal/importer/unmatched_units.go b/internal/importer/unmatched_units.go index 1c63c67ea..e0e7a9452 100644 --- a/internal/importer/unmatched_units.go +++ b/internal/importer/unmatched_units.go @@ -332,24 +332,55 @@ func eligibleUnmatched(files []unmatchedScanFile, roots []string) []unmatchedSca return out } +// suggestionCatalogue indexes every book the scan loaded by author, for +// suggestions only. The reconcile's own candidates (wanted) are marked so a +// tie goes to the book still waiting for a file; they are taken from the set +// the scan already built rather than asking isReconcileCandidate again, which +// stats files on disk. +func suggestionCatalogue(books []models.Book, wanted []scanBook) ([]scanBook, map[int64][]int) { + reconcilable := make(map[int64]bool, len(wanted)) + for i := range wanted { + reconcilable[wanted[i].book.ID] = true + } + out := make([]scanBook, 0, len(books)) + byAuthor := make(map[int64][]int) + for i := range books { + b := &books[i] + byAuthor[b.AuthorID] = append(byAuthor[b.AuthorID], len(out)) + out = append(out, newScanBook(b, reconcilable[b.ID])) + } + return out, byAuthor +} + // rankCandidates returns up to maxCandidates catalogue books whose title // scores at least candidateThreshold against normParsed, best first. It uses -// the reconcile's own measure (Jaro-Winkler over normalizeTitle) and its own -// candidate set: the books of the authors the file's author resolved to, or -// every reconcile candidate when the file named no author. In that last case -// titles of wildly different length are skipped, the same cheap gate the -// reconcile uses. No provider is asked anything. +// the reconcile's own measure (Jaro-Winkler over normalizeTitle). When the +// file's author resolved, the candidates are every catalogue book of those +// authors, whatever its status (#2879): the reconcile only claims books still +// waiting for a file, but a person confirming a suggestion may well be +// pointing an untracked copy at a book that is Skipped or already has one. +// When the file named no author they are the reconcile's own candidates +// (wanted), and titles of wildly different length are skipped, the same +// cheap gate the reconcile uses; the whole library is too wide a net without +// an author. No provider is asked anything. +// +// An exact title scores 1, so it ranks first; between equal scores a book the +// reconcile could claim goes ahead of one that already has its files. // // A book that is provably another volume of the same series is never // suggested, by the same rule the reconcile applies (libraryVolumeConflict): // volume 1's folder scores 0.983 against a wanted volume 17, and offering it // as the top suggestion would invite the user to adopt it there (#2860). -func rankCandidates(title, layoutTitle string, wanted []scanBook, byAuthor map[int64][]int, authorSet map[int64]bool) []db.UnmatchedCandidate { +func rankCandidates(title, layoutTitle string, wanted, catalogue []scanBook, catalogueByAuthor map[int64][]int, authorSet map[int64]bool) []db.UnmatchedCandidate { normParsed := normalizeTitle(title) if normParsed == "" { return nil } - var out []db.UnmatchedCandidate + type scored struct { + db.UnmatchedCandidate + reconcilable bool + } + var out []scored consider := func(sb *scanBook) { score := textutil.JaroWinkler(sb.normTitle, normParsed) if score < candidateThreshold { @@ -358,7 +389,7 @@ func rankCandidates(title, layoutTitle string, wanted []scanBook, byAuthor map[i if libraryVolumeConflict(title, layoutTitle, sb.book.Title) { return } - out = append(out, db.UnmatchedCandidate{BookID: sb.book.ID, Score: score}) + out = append(out, scored{db.UnmatchedCandidate{BookID: sb.book.ID, Score: score}, sb.reconcilable}) } if authorSet == nil { for i := range wanted { @@ -378,24 +409,35 @@ func rankCandidates(title, layoutTitle string, wanted []scanBook, byAuthor map[i } slices.Sort(ids) for _, id := range ids { - for _, idx := range byAuthor[id] { - consider(&wanted[idx]) + for _, idx := range catalogueByAuthor[id] { + consider(&catalogue[idx]) } } } - slices.SortStableFunc(out, func(a, b db.UnmatchedCandidate) int { + slices.SortStableFunc(out, func(a, b scored) int { switch { case a.Score > b.Score: return -1 case a.Score < b.Score: return 1 + case a.reconcilable && !b.reconcilable: + return -1 + case b.reconcilable && !a.reconcilable: + return 1 } return 0 }) if len(out) > maxCandidates { out = out[:maxCandidates] } - return out + if len(out) == 0 { + return nil + } + res := make([]db.UnmatchedCandidate, len(out)) + for i := range out { + res[i] = out[i].UnmatchedCandidate + } + return res } // unitCounts is what the scan result blob reports about stored units. diff --git a/internal/importer/unmatched_units_test.go b/internal/importer/unmatched_units_test.go index 24a7a345f..0e0c80ab0 100644 --- a/internal/importer/unmatched_units_test.go +++ b/internal/importer/unmatched_units_test.go @@ -1,6 +1,7 @@ package importer import ( + "context" "fmt" "math/rand/v2" "net/http" @@ -9,6 +10,7 @@ import ( "slices" "testing" + "github.com/vavallee/bindery/internal/db" "github.com/vavallee/bindery/internal/models" ) @@ -327,3 +329,148 @@ func BenchmarkGroupUnmatched(b *testing.B) { } } } + +// wilsonCatalogue is the #2879 library: the author is catalogued as "Sarah +// K.L. Wilson" and her files sit under "Sarah K. L. Wilson". matsumotoStatus +// sets the state of the one book whose title the untracked file carries; the +// others are Wanted, so on their own they are exactly the reconcile pool. +func wilsonCatalogue(t *testing.T, matsumotoStatus string) (s *Scanner, books *db.BookRepo, libraryDir string, ctx context.Context, matsumoto *models.Book, untracked string) { + t.Helper() + s, _, books, authors, _, libraryDir, ctx := unmatchedFixture(t) + author := &models.Author{ForeignID: "ol:skl-wilson", Name: "Sarah K.L. Wilson", SortName: "Wilson, Sarah K.L.", MetadataProvider: "openlibrary"} + if err := authors.Create(ctx, author); err != nil { + t.Fatal(err) + } + for _, title := range []string{"Paths of Deception", "The Matsumoto", "Chase the Moon", "Mist of Power"} { + b := &models.Book{ForeignID: "ol:" + title, AuthorID: author.ID, Title: title, Status: models.BookStatusWanted, + Monitored: true, MediaType: models.MediaTypeEbook, MetadataProvider: "openlibrary"} + if title == "The Matsumoto" { + b.Status = matsumotoStatus + b.Monitored = matsumotoStatus == models.BookStatusWanted + } + if err := books.Create(ctx, b); err != nil { + t.Fatal(err) + } + if title == "The Matsumoto" { + matsumoto = b + } + } + untracked = filepath.Join(libraryDir, "Sarah K. L. Wilson", "The Matsumoto (13110)", "The Matsumoto - Sarah K. L. Wilson.epub") + if err := os.MkdirAll(filepath.Dir(untracked), 0o755); err != nil { + t.Fatal(err) + } + writeEpubAt(t, untracked, "", "", "") + return s, books, libraryDir, ctx, matsumoto, untracked +} + +// TestScanLibrary_SuggestsTheExactTitleWhateverItsStatus is #2879. The author +// resolved (the spelling "K. L." against "K.L." was never the problem: the +// other three suggestions are her books), but suggestions were ranked only +// from the books the scan may claim on its own, so a Skipped or already +// Imported "The Matsumoto" was never offered and three weaker titles were. +// Suggestions are confirmed by a person, so they come from every book of the +// author; the automatic claim must still leave those books alone. +func TestScanLibrary_SuggestsTheExactTitleWhateverItsStatus(t *testing.T) { + for _, tc := range []struct { + name string + status string + // existing, when set, is a file the book already has elsewhere in + // the library, so it is Imported with a file that resolves. + existing string + }{ + {name: "skipped", status: models.BookStatusSkipped}, + {name: "imported with a file elsewhere", status: models.BookStatusWanted, + existing: filepath.Join("Sarah K.L. Wilson", "The Matsumoto (13110)", "The Matsumoto - Sarah K.L. Wilson.epub")}, + } { + t.Run(tc.name, func(t *testing.T) { + s, books, libraryDir, ctx, matsumoto, untracked := wilsonCatalogue(t, tc.status) + if tc.existing != "" { + p := filepath.Join(libraryDir, tc.existing) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + writeEpubAt(t, p, "", "", "") + if err := books.AddBookFile(ctx, matsumoto.ID, models.MediaTypeEbook, p); err != nil { + t.Fatal(err) + } + } + before, err := books.GetByID(ctx, matsumoto.ID) + if err != nil { + t.Fatal(err) + } + if tc.existing != "" && before.Status != models.BookStatusImported { + t.Fatalf("setup: book status = %s, want imported", before.Status) + } + + s.ScanLibrary(ctx) + + units := readUnmatchedFiles(t, ctx, s) + if len(units) != 1 || units[0].UnitPath != untracked { + t.Fatalf("units = %+v, want the untracked Matsumoto left for a person to decide", units) + } + c := units[0].Candidates + if len(c) == 0 || c[0].BookID != matsumoto.ID || c[0].Score < 0.999 { + t.Fatalf("candidates = %+v, want The Matsumoto (book %d) first with an exact title score", c, matsumoto.ID) + } + if len(c) > maxCandidates { + t.Errorf("kept %d candidates, cap is %d", len(c), maxCandidates) + } + after, err := books.GetByID(ctx, matsumoto.ID) + if err != nil { + t.Fatal(err) + } + if after.Status != before.Status || after.Monitored != before.Monitored { + t.Errorf("book changed by the scan: status %s -> %s, monitored %v -> %v", before.Status, after.Status, before.Monitored, after.Monitored) + } + files, err := books.ListFiles(ctx, matsumoto.ID) + if err != nil { + t.Fatal(err) + } + for _, f := range files { + if f.Path == untracked { + t.Fatalf("the scan claimed %s for a %s book on its own", untracked, before.Status) + } + } + }) + } +} + +// TestScanLibrary_WantedExactTitleStillReconciles: widening the suggestions +// changes nothing for the automatic claim. A Wanted book whose title the file +// carries is claimed by the scan exactly as before, across the same "K. L." +// and "K.L." spellings, and no unit is stored. +func TestScanLibrary_WantedExactTitleStillReconciles(t *testing.T) { + s, books, _, ctx, matsumoto, untracked := wilsonCatalogue(t, models.BookStatusWanted) + + s.ScanLibrary(ctx) + + if units := readUnmatchedFiles(t, ctx, s); len(units) != 0 { + t.Fatalf("units = %+v, want none: the Wanted book should have claimed the file", units) + } + after, err := books.GetByID(ctx, matsumoto.ID) + if err != nil { + t.Fatal(err) + } + if after.Status != models.BookStatusImported || after.EbookFilePath != untracked { + t.Fatalf("book = status %s path %q, want imported at %s", after.Status, after.EbookFilePath, untracked) + } +} + +// TestRankCandidates_ReconcilableBookWinsATie: two books of the author carry +// the same title, one Wanted and one that already has its file. The Wanted +// one is the likelier home for a new file, so it is offered first. +func TestRankCandidates_ReconcilableBookWinsATie(t *testing.T) { + imported := &models.Book{ID: 1, AuthorID: 7, Title: "The Matsumoto", Status: models.BookStatusImported} + wanted := &models.Book{ID: 2, AuthorID: 7, Title: "The Matsumoto", Status: models.BookStatusWanted} + other := &models.Book{ID: 3, AuthorID: 7, Title: "Mist of Power", Status: models.BookStatusWanted} + var catalogue []scanBook + byAuthor := map[int64][]int{} + for _, b := range []*models.Book{imported, wanted, other} { + byAuthor[b.AuthorID] = append(byAuthor[b.AuthorID], len(catalogue)) + catalogue = append(catalogue, newScanBook(b, b.Status == models.BookStatusWanted)) + } + got := rankCandidates("The Matsumoto", "", nil, catalogue, byAuthor, map[int64]bool{7: true}) + if len(got) < 2 || got[0].BookID != wanted.ID || got[1].BookID != imported.ID { + t.Fatalf("candidates = %+v, want the wanted book then the imported one", got) + } +} diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 1853c0b94..b649def00 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -209,7 +209,8 @@ "ignored": "Ignored. Later scans keep it out of this list.", "undoing": "Undoing…", "restored": "Back in Needs a decision.", - "keptBook": "Files removed. The book stayed because it is now in use." + "keptBook": "Files removed. The book stayed because it is now in use.", + "addedAlongside": "Added alongside the file it already had." }, "editor": { "heading": "Which book is {{name}}?", @@ -225,7 +226,8 @@ "moreFiles": "and {{count}} more", "inPlace": "Files stay where they are. Picking a book already in your library leaves its owner and monitoring as they are.", "adoptAs": "Adopt as {{title}}", - "pickFirst": "Pick a book" + "pickFirst": "Pick a book", + "alreadyImported": "This book already has its files. Adopting adds these alongside them, and nothing is replaced or deleted." }, "col": { "book": "Book", diff --git a/web/src/pages/import/AdoptionEditor.tsx b/web/src/pages/import/AdoptionEditor.tsx index 66c02e95d..31d2b47c4 100644 --- a/web/src/pages/import/AdoptionEditor.tsx +++ b/web/src/pages/import/AdoptionEditor.tsx @@ -4,6 +4,7 @@ import type { AdoptionBookRef, AdoptionItem, AdoptTarget, Book } from '../../api import { btn, btnSize } from '../../components/buttons' import BookPicker from '../../components/import/BookPicker' import CatalogueAdder from '../../components/import/CatalogueAdder' +import { bookStatusBadge } from '../../components/bookStatus' import { adoptionHint, scorePercent, unitDisplayName } from './adoptionHint' interface Props { @@ -21,6 +22,18 @@ function refFromBook(b: Book): AdoptionBookRef { } } +// StatusPill shows a suggested book's status, so a book that already has its +// files or was skipped is visible as such before it is picked (#2879). +export function StatusPill({ status, monitored }: { status: string; monitored: boolean }) { + const { t } = useTranslation() + const badge = bookStatusBadge(status, monitored, t) + return ( + + {badge.label} + + ) +} + // AdoptionEditor opens in place under its row, never as a dialog: the // suggestions as radio rows with their scores, a search of the library, // prefilled and focused, and a collapsed metadata search that asks a provider @@ -80,6 +93,7 @@ export default function AdoptionEditor({ item, onAdopt, onCancel, showFiles = fa {o.book.title} {o.book.authorName && · {o.book.authorName}} + {pct !== null ? (