Option B — libraries expand in the settings nav, duplicates becomes a section within a library's pane.
147 lines
7.0 KiB
Markdown
147 lines
7.0 KiB
Markdown
# Implementation brief: move Duplicates into library settings
|
|
|
|
Written for an agent picking this up cold. Read the repo-root `AGENTS.md` and
|
|
`frontend/AGENTS.md` first — this brief assumes both.
|
|
|
|
**This is a frontend-only change.** The backend already scopes everything by library
|
|
(`GET /books/duplicate-books?library_id=`), so no endpoint, schema or migration is
|
|
involved.
|
|
|
|
## Where this starts from
|
|
|
|
The duplicates review screen exists and works. It currently lives at
|
|
|
|
```
|
|
frontend/src/routes/(root)/(library)/library/[libraryId]/duplicates/
|
|
+page.server.ts loads the groups, plus the full Book records merge needs
|
|
+page.svelte group cards, "Not duplicates", "Merge…"
|
|
```
|
|
|
|
and is reached from a **Duplicates entry in the main sidebar**
|
|
(`frontend/src/lib/components/layout/nav-main.svelte`), which is what this change
|
|
removes.
|
|
|
|
Settings today is a flat, entirely global four-item nav
|
|
(`frontend/src/routes/(root)/settings/+layout.svelte`): Account, Appearance, Libraries,
|
|
Devices. `settings/libraries/+page.svelte` is a single table of every library whose rows
|
|
link *out* to the library itself. **There is nowhere that means "settings for this
|
|
library"** — that is the gap this change fills.
|
|
|
|
## What to build — option B
|
|
|
|
Libraries expands in the settings nav. Every library is a sub-item; selecting one swaps
|
|
the pane; Duplicates is a section inside that pane. All libraries and all their sections
|
|
end up one click apart.
|
|
|
|
```
|
|
/settings/libraries the existing table (leave it as the index)
|
|
/settings/libraries/[libraryId] redirects to the first section
|
|
/settings/libraries/[libraryId]/duplicates the review screen, moved
|
|
```
|
|
|
|
Suggested files:
|
|
|
|
| Path | What |
|
|
| --- | --- |
|
|
| `settings/libraries/[libraryId]/+layout.svelte` | Library name, and the section tabs |
|
|
| `settings/libraries/[libraryId]/+page.ts` | `redirect(303, …/duplicates)` |
|
|
| `settings/libraries/[libraryId]/duplicates/+page.server.ts` | Moved verbatim |
|
|
| `settings/libraries/[libraryId]/duplicates/+page.svelte` | Moved verbatim |
|
|
|
|
Duplicates is the **only** real section today. Build the tab strip so General and Danger
|
|
zone have somewhere obvious to land, but do not invent them now — an empty tab is worse
|
|
than no tab.
|
|
|
|
## The nav
|
|
|
|
In `settings/+layout.svelte`, `items` is a flat `as const` array matched on
|
|
`page.route.id`. Libraries needs to render its children beneath it:
|
|
|
|
```svelte
|
|
{#each libraryState.libraries as library (library.id)}
|
|
<a href={resolve('/(root)/settings/libraries/[libraryId]/duplicates', {
|
|
libraryId: String(library.id) })}> … </a>
|
|
{/each}
|
|
```
|
|
|
|
`getLibraryState()` **is** available under `/settings` — it is set in
|
|
`(root)/+layout.svelte`, above the settings group, and `settings/libraries/+page.svelte`
|
|
already uses it. No new load function is needed to list the libraries.
|
|
|
|
**Active state is matched on route id, not pathname.** There is a comment in
|
|
`settings/+layout.svelte` explaining why: `resolve()` returns an absolute path on the
|
|
client and a relative one during SSR, so a pathname comparison is false on the server and
|
|
true after hydration, and the highlight flashes in. A nested library item is active when
|
|
the route id matches **and** `page.params.libraryId === String(library.id)` — both, or
|
|
every library lights up at once.
|
|
|
|
## Things that will bite
|
|
|
|
1. **Remove the sidebar entry in the same change.** `nav-main.svelte` gained a
|
|
`Duplicates` item and a `CopyCheck` import when the screen was built. Delete both, and
|
|
delete the old route directory. Doing the removal and the move together is the point —
|
|
split across two commits the screen is unreachable in between.
|
|
|
|
2. **Delete the old route, do not leave it.** Two live copies of a screen that both write
|
|
is how they drift.
|
|
|
|
3. **`setBookSelectionState` is not available under `/settings`.** It is set in
|
|
`(root)/(library)/+layout.svelte`, which the settings group is not inside. This is
|
|
fine — the duplicates page uses `BookImage` directly, not `book-thumbnail.svelte`, and
|
|
`MergeBooks` takes its `libraryId` as a prop. **Verify this stays true** if you touch
|
|
either component; a `getBookSelectionState()` under settings returns `undefined` and
|
|
fails at the first access, not at import.
|
|
|
|
4. **Keep `depends('app:duplicate-books')`.** Both the dismiss action and `MergeBooks`
|
|
call `invalidate('app:duplicate-books')` to make a resolved group leave the screen.
|
|
Drop it and the page silently stops refreshing. `MergeBooks` also invalidates
|
|
`app:books`, which is a no-op under settings and should stay that way.
|
|
|
|
5. **The settings shell is height-constrained.** `settings/+layout.svelte` is
|
|
`h-[calc(100vh-var(--header-height)-2rem)]` with `overflow-auto` on the content pane.
|
|
The review screen is a long list of cards — it must scroll *inside* that pane. Its
|
|
current `mx-auto max-w-5xl` wrapper will want revisiting.
|
|
|
|
6. **Three levels of nav is option B's known cost.** Nav → library → section, and the
|
|
pane is narrower than the full-width route the screen was designed against. The group
|
|
cards are `w-36` covers in a wrapping flex row, so they reflow, but check a group of
|
|
four at a narrow window before calling it done.
|
|
|
|
7. **`resolve()` must be a direct call in markup** for `svelte/no-navigation-without-resolve`.
|
|
Where `nav-main.svelte` computes a url through a variable it carries an
|
|
`eslint-disable-next-line`; prefer the direct call over inheriting that.
|
|
|
|
8. **The loader depends on the `?ids=` fix.** `+page.server.ts` fetches full `Book`
|
|
records with `listBooks({ ids, pageSize })` because the merge workbench needs
|
|
identifiers, description and publisher, which `DuplicateBookRead` does not carry.
|
|
advanced_alchemy's stock id filter types that parameter as `list[str]` regardless of
|
|
config, which made Postgres refuse `bigint = character varying`; the override lives in
|
|
`backend/src/chitai/services/dependencies.py` (`create_book_filter_dependencies`).
|
|
If `GET /books?ids=1&ids=2` 500s, that override is missing — do not work around it in
|
|
the loader.
|
|
|
|
## Out of scope
|
|
|
|
The **General** and **Danger zone** sections (rename, path template, read-only, consume
|
|
directory, delete), and any change to the merge workbench itself. The toolbar entry point
|
|
for merge — select 2+ books in the library view — is unrelated and stays where it is.
|
|
|
|
## Verification
|
|
|
|
```bash
|
|
cd frontend
|
|
pnpm check # baseline: 30 errors, 1 warning, 8 files — none of them yours
|
|
pnpm lint # not clean either; check the files you touched, not the tree
|
|
pnpm build
|
|
```
|
|
|
|
By hand, with a library that has a duplicate group:
|
|
|
|
- Settings → Libraries lists every library beneath it; clicking one opens its pane.
|
|
- Duplicates shows the same groups the old route did, and scrolls inside the settings pane.
|
|
- **Not duplicates** removes the group and it stays gone after a reload.
|
|
- **Merge…** opens the workbench, merges, and the group leaves the screen.
|
|
- The main sidebar no longer has a Duplicates entry, and
|
|
`/library/<id>/duplicates` no longer resolves.
|
|
- A library with no duplicates shows the empty state, not a blank pane.
|