Repository navigation
Conversation
Pulls the in-memory per-email attempt counter out of controllers/authController.ts into controllers/loginAttempts.ts so the upcoming /api/v1 mobile login can share the same throttling state instead of getting its own separate budget.
An admin can hide specific items/genres/types from a collection's non-admin members (settings.visibility). The listing endpoint already respected this; item detail did not, so a member could bypass it by guessing/copying an id - exactly the id-guessing bypass core/routes/itemRoutes.ts's own detail route already guards against.
Lets the mobile client discover each plugin's form fields, formats and external-id conventions without hardcoding them, so add/edit UIs don't need an app update every time a plugin's fields change.
The API search handler only forwarded type/year/country/genre/label, so a client could not narrow a games search to a ScreenScraper platform. Mirror the web handler: read the plugin's declared `searchFormFields` from the body (strings only, trimmed) and spread them into the source call before the core fields, so undeclared keys cannot reach the provider. Expose `searchFormFields` on `/plugins` too — without it a client has no way to discover that `platform` exists. Tests: new recording fixture `registerSearchFieldsPlugin`, assertions for forwarding, trimming/omitting blanks, ignoring undeclared body keys, and the `/plugins` field. Docs updated.
C1 of the shelves API. Adds two viewer reads — GET /collections/:id/shelves (the location vocabulary) and GET /collections/:id/furniture (pieces, cells and per-cell item counts) — plus ?location= and ?unshelved=true on the items listing, using the shelf view's exact-match semantics. Also closes a real invariant gap: the API's item create and edit wrote `location` verbatim, bypassing resolveShelfLocation, so a mobile client could reintroduce the duplicate shelves the boot migration merged. Both now route through the store, which canonicalises the spelling and creates the compartment for a new name. Counts are computed with a find (not an aggregate) so Mongoose casts the visibility filter's hidden-item ids; they carry the same filters as the listing, so a viewer is never told about an item hidden from them.
Review found the plan's declared C2 interface broken: furnitureToApi lived module-local inside an `export =` router, so the write routes could not import it. Moved to core/apiSerializers.ts beside toApiItem, with a unit test pinning its shape, and corrected the shelfCounts comment/spec, which implied the count read is bounded by the furniture when it is bounded by the collection's shelved items.
Review of the shelves write surface found four defects, all now covered: - PUT /furniture silently ignored `order`, though the spec and the shipped OpenAPI document promise it; saveFurniture now applies a cleanOrder. - DELETE /furniture with a malformed id returned 500 via a Mongoose CastError; it is now the 404 the rest of the API uses. - A save or cell move could place compartments below a piece's own floor when the cells outnumbered the 12-row grid; both now refuse (too_many_shelves / target_full). The web editor maps them too, where it previously wrote a piece the view could not draw. - The unique-index race path produced a 409 whose message said "undefined"; the API message now falls back to a generic one and only carries `shelf` when it knows the name.
GET /plugins now carries each type's own sortOptions (key + label), and GET /collections/:id/items and /wishlist accept ?sort=<key>_<asc|desc>. The vocabulary is the page's: added/title/year always, artist and a plugin's declared options only while `type` names that plugin. An omitted sort stays added_desc; any value outside the current type's vocabulary is a 400 `unknown sort` rather than the page's silent fallback. The key/dir mapping moves to core/itemSort.ts, shared by the API and the page so the two cannot drift. The page keeps its user-defined-field sort and its lenient fallback wrapped around the helper. Two page behaviours change with the extraction: `artist` with an unrecognised type now sorts on the normalized title (sort_title + title), the key the `all` case already used, instead of the raw title; and a value that merely starts with `artist` but carries no `_asc`/`_desc` suffix (e.g. a hand-crafted `?sort=artist`) now falls back to added_desc instead of an artist sort. Neither is reachable from the sort controls, which always send the suffix.
…n the guard Review follow-ups on the sort surface: - resolveItemSort now consults a plugin's own sortOptions before the built-in keys, so a plugin that declares a colliding key (a type naming its series sort `title`, say) gets the entry /plugins advertises instead of the built-in shadowing it. This matches what the page did before the helper existed. - A repeated `?sort=a&sort=b` parses to an array; that is now rejected as `unknown sort` instead of quietly falling back to the default order. - Two route tests the CI suite runs now pin behaviours only the core unit table covered: title sorting keys on the normalised title (a raw-title sort reverses `The Apple`/`Banana`), and year sorting is a seed order distinct from both the default and its reverse; the artist fixture no longer relies on an undefined tie order. - OpenAPI: the items 400 also documents the location/unshelved combination, and sortOptions note that the plugin must be enabled for the collection.
GET /collections/:id/items and /wishlist accept the collection page's content filters - format, decade, genre, style, platform, artist - with the page's semantics: a comma list ORs within one field, the fields AND across, blanks are ignored. filterMode=hide inverts that block; the selected type and the collection's visibility/module rules are scope and stay applied, mirroring the page's two-bucket build. GET /collections/:id/values and /wishlist/values return the distinct genres, styles and platforms to offer in the pickers. They are scoped to exactly the items the matching listing can return, so a value always corresponds to something the caller can list - deliberately tighter than the page's picker, which reads every item of the collection. The type-scope block the two handlers share moves into applyTypeScope / selectedPluginFor rather than being copied.
- OpenAPI: the comma-list examples were unquoted inside a YAML flow mapping, so `example: 1980,1990` parsed as a number plus a bogus `1990` key; quote them. - /values: drop blank entries from the returned lists instead of excluding a whole document with `$nin`, so an item whose arrays also hold an empty string still contributes its real genres/styles. The four distinct lookups now run in one Promise.all. - decade: only a 4-digit start is expanded; a malformed value is ignored rather than turned into a range (or, past 2^53, an empty `$in`). - artist: skip the condition when no plugin is enabled, rather than push an empty `$or` that Mongoose would silently drop. - Tests: the scalar `genre` clause, `filterMode=hide` with the type scope and with empty criteria, a malformed decade, and a values array that also holds an empty entry.
Closes A2 of the remaining-features handoff, which asked only to verify (and fix if broken) that a provider-sourced edition list survives the round-trip: confirm returns `details` verbatim, and a form field the client posts back reaches the stored item. It does, with no production change - `edition` is a formField, so buildApiItemUpdateData copies it. The test plugin gains an `edition` form field and a getDetails() that returns an `editions` list, standing in for books' Hardcover editions.
Owner
|
Hi @Siebe-Uy, and first of all thank you. This is a huge piece of work! far beyond what most contributors would attempt, and it's clear you put real care into it. I'm sorry to say I have to decline, and I'd like to explain why. A public, versioned API is a long-term commitment: every feature exposed through it becomes part of a contract that needs to be documented, maintained, and kept backward compatible. On top of that, adding token-based authentication to an application exposed to the internet introduces a security surface that would need to be maintained and secured over time. These are commitments I'm not able to take on for this project.. More importantly, I'm going in a different direction. I'm currently redesigning the whole mobile experience as a PWA. I see real value in it: one codebase that installs on any phone and gets every feature at the same time, instead of maintaining several clients that do the same thing in different code. Layouts are also coming soon, and they will make the PWA flexible enough that I'm confident it's the right path for DVinyl. For that reason I don't plan to support a native app. I know that's not what you hoped to hear, especially with an iOS app already built on top of it. You're of course free to keep the API in your own fork, and I'll try not to make that harder than it needs to be. Thanks again for the time and effort you put into this! And sorry for taking so long to get back to you.. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds a versioned JSON REST API under
/api/v1so native/third-party clients canauthenticate against and drive a self-hosted DVinyl instance without scraping the
server-rendered EJS UI.
Use case. The motivating client is a Jellyfin-style mobile app: point it at any
DVinyl instance and it browses/edits that collection over JSON. Today the only surface
is EJS pages (the few JSON responses are XHR edge cases), which a native app cannot
consume. This PR provides the stable, stateless, versioned surface such a client needs,
plus the docs and tests to keep it honest.
50 endpoints across auth, account, admin, collections (members, share links, QR), the
full item lifecycle, search/confirm, and plugin metadata. It ships an OpenAPI 3.1
document with a Scalar reference UI, a manual acceptance checklist, an automated HTTP
integration suite (131 tests), and CI.
Related issue
None.
Type of change
What's included
Auth —
POST /auth/login,/auth/refresh,/auth/logout,GET /auth/me.JWT Bearer access token (15 min, signed with the existing
PASSJWT), opaque refreshtoken stored only as a sha256 hash, rotation-on-use, per-device logout. Shares the
existing brute-force gate with the web login so neither endpoint doubles an attacker's
budget.
Account — unified profile update (username/theme/language/currency), password change
(purges every refresh token), avatar upload / Gravatar import / remove, OIDC unlink with
an SSO-lockout guard.
Admin — instance settings, user management (list/create/reset/delete with peer-admin
guards), blocked IPs, login-log listing/cleanup.
Collections — list/create/rename/delete, members with roles and password reset,
share links (create/update/regenerate/delete + QR PNG), paginated item listing with
type/searchfilters that honour visibility and enabled-module settings, and stats.Items — detail/edit/delete, move to collection/wishlist, metadata refresh.
Plugins —
GET /pluginsmetadata so a client can render per-plugin forms generically(no per-plugin code in the client).
Docs —
docs/openapi.yaml(OpenAPI 3.1) served with a Scalar reference UI at/api-docs;docs/api-v1-testing-checklist.md.Tests — 131 HTTP integration tests (
node:test+supertest+mongodb-memory-server, no external services), covering happy paths and the meaningfulnegatives (401/403/404/400/409/413/429/502). GitHub Actions workflow added.
Design notes
collectionIdin the path; thecaller's role is checked per request (
viewer<editor<admin; instance adminspass). No server-side "active collection" like the web session.
{ "success": false, "error": "..." }with theHTTP status appropriate to the cause (401/403/404/400/409/413/429/502). A fresh,
un-setup instance returns
503JSON rather than an HTML redirect./api/v1/now; a breaking change would ship as/api/v2/alongside.Checklist
make typecheckpasses (npx tsc --noEmitclean)npm test→ 131/131; manual checklist for the two cases the suite cannot reach)Screenshots
Not a UI change. The API reference page (
/api-docs, Scalar) is new and can bescreenshotted on request.
Anything else?
routes/api/v1/collectionsRoutes.tsand the test suite; the rest are focused routersand shared helpers.
mongodb-memory-server,supertest,@types/supertest.docs/api-v1-testing-checklist.md: the global setup gate(
app.ts503) and IP-block middleware stay manual, because the automated suite mountsthe
/api/v1router directly rather than booting the full app..envis untracked, and.env.exampleholds placeholders only.