diff --git a/README.md b/README.md index 6bf231e..cf8f5ea 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ Everything is namespaced under `/api/v1/`. `/api/*` outside the version prefix i | Method | Path | Description | | ------ | ------------------------------------------------------- | ------------------------------------------ | | GET | `/api/v1/health` | Liveness probe. | -| GET | `/api/v1/mangas?search=&sort=recent|title&limit=&offset=` | List/search mangas. Trigram fuzzy search. | +| GET | `/api/v1/mangas?search=&sort=created\|updated\|title\|author&order=asc\|desc&limit=&offset=` | List/search mangas. Trigram fuzzy search. `sort` defaults to `updated`; `order` defaults per field (dates `desc`, text `asc`). `sort=recent` is a back-compat alias for `created`. | | GET | `/api/v1/mangas/{id}` | Single manga. | | GET | `/api/v1/mangas/{id}/chapters` | Paginated chapter list, ordered by number. | | GET | `/api/v1/mangas/{id}/chapters/{n}` | Single chapter. | diff --git a/backend/Cargo.lock b/backend/Cargo.lock index e455b7b..332ac16 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1517,7 +1517,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.87.26" +version = "0.88.0" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 61d4af0..88ade25 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.87.26" +version = "0.88.0" edition = "2021" default-run = "mangalord" diff --git a/backend/migrations/0033_mangas_updated_at_index.sql b/backend/migrations/0033_mangas_updated_at_index.sql new file mode 100644 index 0000000..f60da8b --- /dev/null +++ b/backend/migrations/0033_mangas_updated_at_index.sql @@ -0,0 +1,16 @@ +-- Back the default catalog ordering. Since 0.88.0 the catalog list defaults to +-- `ORDER BY updated_at DESC, id` (the no-filter default path), but the only +-- existing updated_at index on mangas is the PARTIAL `mangas_missing_cover_idx` +-- (WHERE cover_image_path IS NULL), which Postgres can't use for the unfiltered +-- ordering. `created_at` and `lower(title)` already have full indexes from +-- 0001; this gives the new default sort column the same treatment. +-- +-- `id` is included as a trailing key so the index also covers the stable +-- `, id` tie-break the listing appends for deterministic pagination. +-- +-- Not CONCURRENTLY: sqlx::migrate! wraps each migration in a transaction, and +-- CREATE INDEX CONCURRENTLY can't run inside one. The table is small at our +-- scale, so a brief lock on an online deploy is acceptable. + +CREATE INDEX IF NOT EXISTS mangas_updated_at_idx + ON mangas (updated_at DESC, id); diff --git a/backend/src/api/mangas.rs b/backend/src/api/mangas.rs index 2127f16..2533773 100644 --- a/backend/src/api/mangas.rs +++ b/backend/src/api/mangas.rs @@ -51,14 +51,58 @@ pub struct ListParams { pub limit: i64, #[serde(default)] pub offset: i64, + /// Sort field: `created`, `updated` (default), `title`, or `author`. + /// Also accepts the legacy `recent` alias (== `created`). Anything + /// else → 422. #[serde(default)] - pub sort: repo::manga::ListSort, + pub sort: Option, + /// `asc` or `desc`. Omitted → the field's natural default + /// (dates `desc`, text `asc`). Anything else → 422. + #[serde(default)] + pub order: Option, } fn default_limit() -> i64 { 50 } +/// Parse the `sort` wire value into a [`repo::manga::SortField`]. Validation +/// and the legacy `recent` alias live here (not in serde) so unknown values +/// return the structured 422 envelope rather than axum's plain-text +/// `QueryRejection`, matching the rest of this handler's filters. +fn parse_sort(raw: Option<&str>) -> AppResult { + use repo::manga::SortField; + match raw.map(str::trim) { + None | Some("") | Some("updated") => Ok(SortField::Updated), + // `recent` was the pre-0.88 name for "newest created first"; keep it + // as an alias so existing bots/scripts don't break. + Some("created") | Some("recent") => Ok(SortField::Created), + Some("title") => Ok(SortField::Title), + Some("author") => Ok(SortField::Author), + Some(other) => Err(AppError::ValidationFailed { + message: format!( + "sort must be one of created, updated, title, author (got {other:?})" + ), + details: json!({ "sort": "invalid" }), + }), + } +} + +/// Parse the `order` wire value. `None` means the client omitted it, so the +/// caller applies the field's natural default. An explicit unknown value → 422. +fn parse_order(raw: Option<&str>) -> AppResult> { + use repo::manga::SortOrder; + match raw.map(str::trim) { + None | Some("") => Ok(None), + Some("asc") => Ok(Some(SortOrder::Asc)), + Some("desc") => Ok(Some(SortOrder::Desc)), + Some(other) => Err(AppError::ValidationFailed { + message: format!("order must be 'asc' or 'desc' (got {other:?})"), + details: json!({ "order": "invalid" }), + }), + } +} + fn parse_uuid_csv(field: &str, raw: Option<&str>) -> AppResult> { let Some(raw) = raw else { return Ok(Vec::new()) }; let mut out = Vec::new(); @@ -94,6 +138,10 @@ async fn list( Some(s.to_string()) } }; + let sort = parse_sort(params.sort.as_deref())?; + // Omitted `order` falls back to the field's natural direction, so a bare + // `?sort=title` reads A→Z just like the UI shows it. + let order = parse_order(params.order.as_deref())?.unwrap_or_else(|| sort.default_order()); let q = repo::manga::ListQuery { search: params.search.filter(|s| !s.trim().is_empty()), status, @@ -104,7 +152,8 @@ async fn list( cw_exclude: crate::api::page_tags::parse_warnings_csv(params.cw_exclude.as_deref())?, limit, offset, - sort: params.sort, + sort, + order, }; let (items, total) = repo::manga::list_cards(&state.db, &q).await?; Ok(Json(PagedResponse::with_total(items, limit, offset, total))) diff --git a/backend/src/repo/manga.rs b/backend/src/repo/manga.rs index 628e397..fd259c9 100644 --- a/backend/src/repo/manga.rs +++ b/backend/src/repo/manga.rs @@ -5,7 +5,6 @@ //! handlers depend only on `sqlx::PgPool`, not on a trait object. Swap to //! a trait + impl if a second backend ever becomes necessary. -use serde::Deserialize; use sqlx::{PgConnection, PgExecutor, PgPool}; use uuid::Uuid; @@ -18,14 +17,43 @@ use crate::repo; pub const STATUSES: &[&str] = &["ongoing", "completed"]; pub const DEFAULT_STATUS: &str = "ongoing"; -#[derive(Debug, Clone, Copy, Default, Deserialize)] -#[serde(rename_all = "snake_case")] -pub enum ListSort { - /// Newest first (default). +/// Which column the listing is ordered by. The direction lives in a separate +/// [`SortOrder`] so any field can be sorted either way. Wire values are parsed +/// (with validation and the legacy `recent` alias) in `api::mangas`, not via +/// serde, so this stays a plain domain enum. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum SortField { + /// Creation time. + Created, + /// Last-modified time (default — "last updated first" pairs with `Desc`). #[default] - Recent, - /// A→Z by title (case-insensitive). + Updated, + /// Title (case-insensitive). Title, + /// Alphabetically-first attached author (case-insensitive); authorless + /// mangas sort last regardless of direction. + Author, +} + +impl SortField { + /// The direction applied when the client omits `order`. Dates read + /// newest-first (`Desc`); text reads A→Z (`Asc`). This mirrors the + /// frontend's `defaultOrderFor` (see `frontend/src/lib/mangaSort.ts`) so a + /// bare `?sort=` link means the same thing to the UI and the API. + pub fn default_order(self) -> SortOrder { + match self { + SortField::Created | SortField::Updated => SortOrder::Desc, + SortField::Title | SortField::Author => SortOrder::Asc, + } + } +} + +/// Sort direction. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum SortOrder { + Asc, + #[default] + Desc, } #[derive(Debug, Clone, Default)] @@ -42,7 +70,8 @@ pub struct ListQuery { pub cw_exclude: Vec, pub limit: i64, pub offset: i64, - pub sort: ListSort, + pub sort: SortField, + pub order: SortOrder, } /// Single source of truth for the `mangas` columns that hydrate a [`Manga`], @@ -130,16 +159,42 @@ const FILTER_WHERE: &str = r#" "#; /// Returns the page of mangas matching `query` plus the unfiltered total -/// count for the same filter. The trigram GIN indexes (see 0005_search.sql -/// and 0009_manga_metadata.sql) keep both queries cheap as the library -/// grows. +/// count for the same filter. The date/title sort columns are index-backed +/// (`mangas_created_at_idx`, `mangas_updated_at_idx`, `mangas_title_lower_idx`) +/// and the trigram GIN indexes (0005_search.sql, 0009_manga_metadata.sql) keep +/// the search filter cheap as the library grows. pub async fn list(pool: &PgPool, query: &ListQuery) -> AppResult<(Vec, i64)> { - // `order_by` is interpolated from a hard-coded enum, never from request - // input, so this is not a SQL injection seam. - let order_by = match query.sort { - ListSort::Recent => "created_at DESC, id", - ListSort::Title => "lower(title) ASC, id", + // Both `col` and `dir` are interpolated from hard-coded enums, never from + // request input, so this is not a SQL injection seam. The trailing `id` is + // a stable tie-break that keeps pagination deterministic across rows with + // equal sort keys. + let col = match query.sort { + SortField::Created => "created_at", + SortField::Updated => "updated_at", + SortField::Title => "lower(title)", + // Sorts on the alphabetically-first attached author. As an ORDER BY + // key this correlated subquery is evaluated per filter-matching row + // before LIMIT applies, so it scales worse than the indexed date/title + // sorts — revisit with a LATERAL join or precomputed sort-name column + // if the library grows large. + SortField::Author => { + "(SELECT min(lower(a.name)) \ + FROM manga_authors ma JOIN authors a ON a.id = ma.author_id \ + WHERE ma.manga_id = mangas.id)" + } }; + let dir = match query.order { + SortOrder::Asc => "ASC", + SortOrder::Desc => "DESC", + }; + // Only the author key can be NULL (authorless mangas); keep those last in + // both directions. The date/title columns are NOT NULL, so they need no + // NULLS clause — and omitting it lets a reverse index scan satisfy `DESC`. + let nulls = match query.sort { + SortField::Author => " NULLS LAST", + _ => "", + }; + let order_by = format!("{col} {dir}{nulls}, id"); let search = query.search.as_deref(); let status = query.status.as_deref(); diff --git a/backend/tests/api_mangas.rs b/backend/tests/api_mangas.rs index e454aa0..2892ba1 100644 --- a/backend/tests/api_mangas.rs +++ b/backend/tests/api_mangas.rs @@ -11,6 +11,53 @@ fn metadata(title: &str) -> serde_json::Value { json!({ "title": title }) } +/// Collect the `title` field of every item in a list response, in order. +fn title_list(body: &serde_json::Value) -> Vec<&str> { + body["items"] + .as_array() + .unwrap() + .iter() + .map(|m| m["title"].as_str().unwrap()) + .collect() +} + +/// Create a manga via the upload API, asserting it succeeded. +async fn seed(app: &axum::Router, cookie: &str, title: &str) { + let resp = app + .clone() + .oneshot(common::post_multipart_with_cookie( + "/api/v1/mangas", + MultipartBuilder::new().add_json("metadata", json!({ "title": title })), + cookie, + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::CREATED, "seed({title}) failed"); +} + +/// Pin a manga's `created_at` to an explicit RFC3339 instant so time-based +/// ordering assertions don't depend on insert timing. +async fn set_created_at(pool: &PgPool, title: &str, when: &str) { + let ts: chrono::DateTime = when.parse().unwrap(); + sqlx::query("UPDATE mangas SET created_at = $1 WHERE title = $2") + .bind(ts) + .bind(title) + .execute(pool) + .await + .unwrap(); +} + +/// Pin a manga's `updated_at` to an explicit RFC3339 instant. +async fn set_updated_at(pool: &PgPool, title: &str, when: &str) { + let ts: chrono::DateTime = when.parse().unwrap(); + sqlx::query("UPDATE mangas SET updated_at = $1 WHERE title = $2") + .bind(ts) + .bind(title) + .execute(pool) + .await + .unwrap(); +} + #[sqlx::test(migrations = "./migrations")] async fn list_is_empty_initially(pool: PgPool) { let h = common::harness(pool); @@ -103,19 +150,307 @@ async fn list_sort_title_orders_alphabetically(pool: PgPool) { .unwrap(); } + // Title sort now requires an explicit direction; the wire default is + // `desc`, so A→Z needs `order=asc`. + let resp = h + .app + .oneshot(common::get("/api/v1/mangas?sort=title&order=asc")) + .await + .unwrap(); + let body = common::body_json(resp).await; + let titles = title_list(&body); + assert_eq!(titles, vec!["Berserk", "One Piece", "Vinland Saga"]); +} + +#[sqlx::test(migrations = "./migrations")] +async fn list_sort_title_desc_orders_reverse_alphabetically(pool: PgPool) { + let h = common::harness(pool); + let (_, cookie) = common::register_user(&h.app).await; + + for title in ["Vinland Saga", "Berserk", "One Piece"] { + seed(&h.app, &cookie, title).await; + } + + let resp = h + .app + .oneshot(common::get("/api/v1/mangas?sort=title&order=desc")) + .await + .unwrap(); + let body = common::body_json(resp).await; + let titles = title_list(&body); + assert_eq!(titles, vec!["Vinland Saga", "One Piece", "Berserk"]); +} + +#[sqlx::test(migrations = "./migrations")] +async fn list_default_sort_is_updated_desc(pool: PgPool) { + let h = common::harness(pool.clone()); + let (_, cookie) = common::register_user(&h.app).await; + + for title in ["Alpha", "Bravo", "Charlie"] { + seed(&h.app, &cookie, title).await; + } + // Pin explicit, distinct update times so the default ordering is + // deterministic regardless of insert timing. Bravo is the most recently + // updated and must appear first under the default (updated DESC). + set_updated_at(&pool, "Alpha", "2020-01-01T00:00:00Z").await; + set_updated_at(&pool, "Bravo", "2023-01-01T00:00:00Z").await; + set_updated_at(&pool, "Charlie", "2021-01-01T00:00:00Z").await; + + // No sort/order params — exercises the default. + let resp = h.app.oneshot(common::get("/api/v1/mangas")).await.unwrap(); + let body = common::body_json(resp).await; + let titles = title_list(&body); + assert_eq!(titles, vec!["Bravo", "Charlie", "Alpha"]); +} + +#[sqlx::test(migrations = "./migrations")] +async fn list_sort_created_orders_by_creation_time(pool: PgPool) { + let h = common::harness(pool.clone()); + let (_, cookie) = common::register_user(&h.app).await; + + for title in ["Alpha", "Bravo", "Charlie"] { + seed(&h.app, &cookie, title).await; + } + set_created_at(&pool, "Alpha", "2020-01-01T00:00:00Z").await; + set_created_at(&pool, "Bravo", "2023-01-01T00:00:00Z").await; + set_created_at(&pool, "Charlie", "2021-01-01T00:00:00Z").await; + + // Newest-created first. + let resp = h + .app + .clone() + .oneshot(common::get("/api/v1/mangas?sort=created&order=desc")) + .await + .unwrap(); + let body = common::body_json(resp).await; + assert_eq!(title_list(&body), vec!["Bravo", "Charlie", "Alpha"]); + + // Ascending is the exact reverse. + let resp = h + .app + .oneshot(common::get("/api/v1/mangas?sort=created&order=asc")) + .await + .unwrap(); + let body = common::body_json(resp).await; + assert_eq!(title_list(&body), vec!["Alpha", "Charlie", "Bravo"]); +} + +#[sqlx::test(migrations = "./migrations")] +async fn list_sort_author_orders_by_author_name_nulls_last(pool: PgPool) { + let h = common::harness(pool); + let (_, cookie) = common::register_user(&h.app).await; + + // Author order is deliberately scrambled relative to title order so the + // assertion can't pass by accidentally sorting on title. + for (title, authors) in [ + ("Akira", json!(["Beta Author"])), + ("Boku", json!(["Alpha Author"])), + ("Chi", json!(["Gamma Author"])), + ("NoAuthor", json!([])), + ] { + let _ = h + .app + .clone() + .oneshot(common::post_multipart_with_cookie( + "/api/v1/mangas", + MultipartBuilder::new() + .add_json("metadata", json!({ "title": title, "authors": authors })), + &cookie, + )) + .await + .unwrap(); + } + + // Ascending by author name; the authorless manga sorts last (NULLS LAST). + let resp = h + .app + .clone() + .oneshot(common::get("/api/v1/mangas?sort=author&order=asc")) + .await + .unwrap(); + let body = common::body_json(resp).await; + assert_eq!(title_list(&body), vec!["Boku", "Akira", "Chi", "NoAuthor"]); + + // Descending flips the authored mangas but keeps the authorless one last. + let resp = h + .app + .oneshot(common::get("/api/v1/mangas?sort=author&order=desc")) + .await + .unwrap(); + let body = common::body_json(resp).await; + assert_eq!(title_list(&body), vec!["Chi", "Akira", "Boku", "NoAuthor"]); +} + +#[sqlx::test(migrations = "./migrations")] +async fn list_sort_recent_alias_maps_to_created_desc(pool: PgPool) { + // `recent` was the pre-0.88 sort value (newest created first). It is kept + // as an alias for `created` so existing bots/scripts don't break. + let h = common::harness(pool.clone()); + let (_, cookie) = common::register_user(&h.app).await; + + for title in ["Alpha", "Bravo", "Charlie"] { + seed(&h.app, &cookie, title).await; + } + set_created_at(&pool, "Alpha", "2020-01-01T00:00:00Z").await; + set_created_at(&pool, "Bravo", "2023-01-01T00:00:00Z").await; + set_created_at(&pool, "Charlie", "2021-01-01T00:00:00Z").await; + + let resp = h + .app + .oneshot(common::get("/api/v1/mangas?sort=recent")) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + let body = common::body_json(resp).await; + // Same ordering as `sort=created&order=desc`: newest-created first. + assert_eq!(title_list(&body), vec!["Bravo", "Charlie", "Alpha"]); +} + +#[sqlx::test(migrations = "./migrations")] +async fn list_sort_field_defaults_direction_per_field(pool: PgPool) { + // When `order` is omitted, the API applies the field's natural direction: + // text fields ascend (A→Z), matching the frontend's `defaultOrderFor`, so + // a bare `?sort=title` link reads the same in the UI and over the wire. + let h = common::harness(pool); + let (_, cookie) = common::register_user(&h.app).await; + + for title in ["Vinland Saga", "Berserk", "One Piece"] { + seed(&h.app, &cookie, title).await; + } + let resp = h .app .oneshot(common::get("/api/v1/mangas?sort=title")) .await .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); let body = common::body_json(resp).await; - let titles: Vec<&str> = body["items"] - .as_array() - .unwrap() + assert_eq!(title_list(&body), vec!["Berserk", "One Piece", "Vinland Saga"]); +} + +#[sqlx::test(migrations = "./migrations")] +async fn list_sort_author_default_is_ascending_nulls_last(pool: PgPool) { + // The other text field: a bare `?sort=author` must default to ascending and + // keep authorless mangas last (NULLS LAST), exercising the omitted-order + // path through the author subquery — not just `title`. + let h = common::harness(pool); + let (_, cookie) = common::register_user(&h.app).await; + + for (title, authors) in [ + ("Akira", json!(["Beta Author"])), + ("Boku", json!(["Alpha Author"])), + ("NoAuthor", json!([])), + ] { + let _ = h + .app + .clone() + .oneshot(common::post_multipart_with_cookie( + "/api/v1/mangas", + MultipartBuilder::new() + .add_json("metadata", json!({ "title": title, "authors": authors })), + &cookie, + )) + .await + .unwrap(); + } + + let resp = h + .app + .oneshot(common::get("/api/v1/mangas?sort=author")) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + let body = common::body_json(resp).await; + // Alpha Author (Boku) < Beta Author (Akira); authorless sorts last. + assert_eq!(title_list(&body), vec!["Boku", "Akira", "NoAuthor"]); +} + +#[sqlx::test(migrations = "./migrations")] +async fn list_rejects_invalid_sort_and_order_with_envelope(pool: PgPool) { + let h = common::harness(pool); + + // Unknown sort field → structured 422, not axum's plain-text 400. + let resp = h + .app + .clone() + .oneshot(common::get("/api/v1/mangas?sort=bogus")) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::UNPROCESSABLE_ENTITY); + let body = common::body_json(resp).await; + assert_eq!(body["error"]["details"]["sort"], "invalid"); + + // Unknown order → same treatment. + let resp = h + .app + .oneshot(common::get("/api/v1/mangas?order=sideways")) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::UNPROCESSABLE_ENTITY); + let body = common::body_json(resp).await; + assert_eq!(body["error"]["details"]["order"], "invalid"); +} + +/// Pin a manga's primary key to an explicit UUID. Safe only for freshly +/// seeded mangas with no chapters/authors/tags referencing them. +async fn set_id(pool: &PgPool, title: &str, id: &str) { + let uuid: uuid::Uuid = id.parse().unwrap(); + sqlx::query("UPDATE mangas SET id = $1 WHERE title = $2") + .bind(uuid) + .bind(title) + .execute(pool) + .await + .unwrap(); +} + +#[sqlx::test(migrations = "./migrations")] +async fn list_tie_break_orders_equal_keys_by_ascending_id(pool: PgPool) { + // The trailing `, id` in ORDER BY is what keeps pagination stable when the + // primary sort key ties. Pin three mangas to the SAME created_at and assign + // ids whose ascending order (Bravo < Charlie < Alpha) deliberately differs + // from insertion/physical order, so the expected ordering can ONLY come + // from the id tie-break. + // + // We sort by `created` on purpose: `mangas_created_at_idx` is `(created_at + // DESC)` with no id, so it does NOT encode the secondary order — unlike the + // default `updated` path, whose `mangas_updated_at_idx (updated_at DESC, + // id)` would mask a missing clause. Drop `, id` from the query and this + // test fails. + let h = common::harness(pool.clone()); + let (_, cookie) = common::register_user(&h.app).await; + + for title in ["Alpha", "Bravo", "Charlie"] { + seed(&h.app, &cookie, title).await; + set_created_at(&pool, title, "2022-01-01T00:00:00Z").await; + } + set_id(&pool, "Alpha", "33333333-3333-4333-8333-333333333333").await; + set_id(&pool, "Bravo", "11111111-1111-4111-8111-111111111111").await; + set_id(&pool, "Charlie", "22222222-2222-4222-8222-222222222222").await; + + let page = |limit: i64, offset: i64| { + let app = h.app.clone(); + async move { + let resp = app + .oneshot(common::get(&format!( + "/api/v1/mangas?sort=created&limit={limit}&offset={offset}" + ))) + .await + .unwrap(); + common::body_json(resp).await + } + }; + + // Single page: tied rows come back in ascending-id order. + let full = page(50, 0).await; + assert_eq!(title_list(&full), vec!["Bravo", "Charlie", "Alpha"]); + + // Paginating one row at a time reproduces that exact sequence — no overlap, + // no gap — which only holds because the id tie-break makes the order total. + let paged: Vec = vec![page(1, 0).await, page(1, 1).await, page(1, 2).await] .iter() - .map(|m| m["title"].as_str().unwrap()) + .map(|b| b["items"][0]["title"].as_str().unwrap().to_string()) .collect(); - assert_eq!(titles, vec!["Berserk", "One Piece", "Vinland Saga"]); + assert_eq!(paged, vec!["Bravo", "Charlie", "Alpha"]); } #[sqlx::test(migrations = "./migrations")] diff --git a/frontend/e2e/mobile-list-search.spec.ts b/frontend/e2e/mobile-list-search.spec.ts index 950047e..3638d11 100644 --- a/frontend/e2e/mobile-list-search.spec.ts +++ b/frontend/e2e/mobile-list-search.spec.ts @@ -149,14 +149,16 @@ test.describe('mobile catalog chrome', () => { await expect(page).toHaveURL((url) => !url.search.includes('genres=')); }); - test('phone viewport: Sort sheet swaps the sort and dismisses on pick', async ({ + test('phone viewport: Sort sheet swaps field + direction, staying open', async ({ page }) => { let lastSortParam: string | null = null; + let lastOrderParam: string | null = null; await mockAnonymous(page); await mockCatalog(page, { capture: (url) => { lastSortParam = url.searchParams.get('sort'); + lastOrderParam = url.searchParams.get('order'); } }); await page.setViewportSize(MOBILE); @@ -166,13 +168,27 @@ test.describe('mobile catalog chrome', () => { await page.getByTestId('sort-chip').click(); await expect(page.getByTestId('sort-sheet')).toBeVisible(); - // Use click(), not check(): the onchange handler closes the sheet - // synchronously so the radio is gone before check() can verify the - // `checked` state. - await page.getByTestId('sort-sheet').getByRole('radio', { name: /Title/ }).click(); - - await expect(page.getByTestId('sort-sheet')).toBeHidden(); + // Picking a field reloads but keeps the sheet open so the direction + // can be adjusted in the same interaction. + await page.getByTestId('sort-sheet').getByRole('radio', { name: /^Title$/ }).click(); + await expect(page.getByTestId('sort-sheet')).toBeVisible(); await expect(page).toHaveURL(/sort=title/); expect(lastSortParam).toBe('title'); + // The API call always carries an explicit direction; Title's natural + // default is A→Z (asc)... + expect(lastOrderParam).toBe('asc'); + // ...but the browser URL omits it since it's the default. + await expect(page).not.toHaveURL(/order=/); + + // Flipping the direction to Z→A surfaces an explicit order param both + // on the wire and in the URL. + await page.getByTestId('sort-order-mobile-desc').click(); + await expect(page).toHaveURL(/order=desc/); + expect(lastOrderParam).toBe('desc'); + + // The Done button gives an explicit dismissal affordance once the + // field + direction are set. + await page.getByTestId('sort-done').click(); + await expect(page.getByTestId('sort-sheet')).toBeHidden(); }); }); diff --git a/frontend/package.json b/frontend/package.json index 16bf29e..18e80b3 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.87.26", + "version": "0.88.0", "private": true, "type": "module", "scripts": { diff --git a/frontend/src/lib/api/mangas.test.ts b/frontend/src/lib/api/mangas.test.ts index ea7f21f..0958ab8 100644 --- a/frontend/src/lib/api/mangas.test.ts +++ b/frontend/src/lib/api/mangas.test.ts @@ -89,7 +89,7 @@ describe('mangas api client', () => { expect(result.page).toEqual({ limit: 50, offset: 0, total: 1 }); }); - it('listMangas encodes search, status, ids (csv), limit, offset, sort', async () => { + it('listMangas encodes search, status, ids (csv), limit, offset, sort, order', async () => { fetchSpy.mockResolvedValueOnce(ok(emptyPage())); await listMangas({ search: 'one piece', @@ -99,7 +99,8 @@ describe('mangas api client', () => { tagIds: ['t1', 't2'], limit: 10, offset: 20, - sort: 'title' + sort: 'title', + order: 'asc' }); const url = fetchSpy.mock.calls[0][0] as string; expect(url).toMatch(/\/v1\/mangas\?/); @@ -113,6 +114,7 @@ describe('mangas api client', () => { expect(url).toContain('limit=10'); expect(url).toContain('offset=20'); expect(url).toContain('sort=title'); + expect(url).toContain('order=asc'); }); it('getManga returns the enriched detail shape', async () => { diff --git a/frontend/src/lib/api/mangas.ts b/frontend/src/lib/api/mangas.ts index d00a99d..a531db6 100644 --- a/frontend/src/lib/api/mangas.ts +++ b/frontend/src/lib/api/mangas.ts @@ -1,7 +1,8 @@ import { request, type Manga, type MangaStatus, type Page } from './client'; import type { ContentWarning } from './page_tags'; -export type MangaSort = 'recent' | 'title'; +export type MangaSort = 'created' | 'updated' | 'title' | 'author'; +export type SortOrder = 'asc' | 'desc'; export type AuthorRef = { id: string; name: string }; export type GenreRef = { id: string; name: string }; @@ -39,6 +40,7 @@ export type ListOptions = { limit?: number; offset?: number; sort?: MangaSort; + order?: SortOrder; }; export type MangasPage = { @@ -68,6 +70,7 @@ export async function listMangas(opts: ListOptions = {}): Promise { if (opts.limit != null) params.set('limit', String(opts.limit)); if (opts.offset != null) params.set('offset', String(opts.offset)); if (opts.sort) params.set('sort', opts.sort); + if (opts.order) params.set('order', opts.order); const qs = params.toString(); return request(`/v1/mangas${qs ? `?${qs}` : ''}`); } diff --git a/frontend/src/lib/components/SegmentedControl.svelte b/frontend/src/lib/components/SegmentedControl.svelte index 7c36015..a75571d 100644 --- a/frontend/src/lib/components/SegmentedControl.svelte +++ b/frontend/src/lib/components/SegmentedControl.svelte @@ -17,14 +17,69 @@ ariaLabel: string; testid?: string; } = $props(); + + // Roving-tabindex refs so arrow keys can move focus across the group. + let buttons = $state([]); + + function select(index: number) { + const opt = options[index]; + if (!opt) return; + if (opt.value !== value) onchange(opt.value); + // Selection follows focus, per the WAI-ARIA radiogroup pattern. + buttons[index]?.focus(); + } + + // ArrowLeft/Up and ArrowRight/Down move (and wrap) through the options; + // Home/End jump to the ends. Without this a control announced as a + // `radiogroup` would not respond to the keys AT users expect. + function onkeydown(event: KeyboardEvent) { + // Navigate relative to the focused button, not the selected `value`: + // this control is parent-controlled, so `value` only updates after a + // re-render — anchoring on focus keeps rapid keypresses from sticking. + // Falls back to `value` when focus is elsewhere (e.g. event delegated + // from outside a child, as in unit tests). + const focused = buttons.findIndex((b) => b === document.activeElement); + const current = focused >= 0 ? focused : options.findIndex((o) => o.value === value); + switch (event.key) { + case 'ArrowRight': + case 'ArrowDown': + event.preventDefault(); + select((current + 1) % options.length); + break; + case 'ArrowLeft': + case 'ArrowUp': + event.preventDefault(); + select((current - 1 + options.length) % options.length); + break; + case 'Home': + event.preventDefault(); + select(0); + break; + case 'End': + event.preventDefault(); + select(options.length - 1); + break; + } + } -
- {#each options as opt (opt.value)} + + +
+ {#each options as opt, i (opt.value)}
@@ -526,30 +572,44 @@ onClose={() => (sortSheetOpen = false)} testid="sort-sheet" > -
- - +

Sort by

+
+ {#each Object.entries(SORT_FIELD_LABELS) as [value, label] (value)} + + {/each}
+
+

Direction

+ +
+ {#snippet footer()} + + {/snippet} {#if loading} @@ -836,6 +896,32 @@ cursor: pointer; } + .sort-direction { + margin-top: var(--space-3); + padding-top: var(--space-3); + border-top: 1px solid var(--border); + } + + /* Section headings inside the mobile sort sheet ("Sort by" / "Direction") + so each group is visibly labelled, mirroring the desktop inline labels. */ + .sort-section-label { + margin: 0 0 var(--space-2); + font-size: var(--font-sm); + font-weight: var(--weight-medium); + color: var(--text-muted); + } + + .primary { + border: 1px solid var(--primary); + background: var(--primary); + color: var(--primary-contrast); + } + + .primary:hover { + background: var(--primary-hover); + border-color: var(--primary-hover); + } + .icon-btn { display: inline-flex; align-items: center;