fabi/Mangalord
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(mangas): address review on sort feature — alias, index, validation, a11y

Browse Source 
Backend
- sort=recent stays a back-compat alias for created (parse_sort); invalid
  sort/order now return the structured 422 envelope instead of plain-text 400
- per-field default direction on omitted order (dates desc, text asc), matching
  the frontend so a bare ?sort=<field> reads the same in UI and API
- migration 0033: index mangas(updated_at DESC, id) backing the default sort
  and its id tie-break; NULLS LAST now applied only to the nullable author key
- tests: recent alias, per-field default (title+author), invalid-value 422,
  and a tie-break test that pins ordering by ascending id (mutation-verified)

Frontend
- pure sort helpers extracted to $lib/mangaSort with unit tests; coerceSort
  honors the recent alias so pasted/legacy URLs resolve to the same field
- SegmentedControl: roving tabindex + arrow/Home/End keyboard nav, anchored on
  focus so rapid keypresses don't stick
- UX: visible "Direction" labels (desktop + mobile), mobile sort-sheet section
  headings and a "Done" button; README sort/order contract updated

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-06-25 07:15:33 +02:00
parent 1079a0151a
commit 78edea4277
11 changed files with 650 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
backend/migrations/0033_mangas_updated_at_index.sql Normal file
View File

@@ -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);

54
backend/src/api/mangas.rs
View File

@@ -51,16 +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::SortField,
pub sort: Option<String>,
/// `asc` or `desc`. Omitted → the field's natural default
/// (dates `desc`, text `asc`). Anything else → 422.
#[serde(default)]
pub order: repo::manga::SortOrder,
pub order: Option<String>,
}
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<repo::manga::SortField> {
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<Option<repo::manga::SortOrder>> {
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<Vec<Uuid>> {
let Some(raw) = raw else { return Ok(Vec::new()) };
let mut out = Vec::new();
@@ -96,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,
@@ -106,8 +152,8 @@ async fn list(
cw_exclude: crate::api::page_tags::parse_warnings_csv(params.cw_exclude.as_deref())?,
limit,
offset,
sort: params.sort,
order: params.order,
sort,
order,
};
let (items, total) = repo::manga::list_cards(&state.db, &q).await?;
Ok(Json(PagedResponse::with_total(items, limit, offset, total)))

50
backend/src/repo/manga.rs
View File

@@ -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;
@@ -19,9 +18,10 @@ pub const STATUSES: &[&str] = &["ongoing", "completed"];
pub const DEFAULT_STATUS: &str = "ongoing";
/// Which column the listing is ordered by. The direction lives in a separate
/// [`SortOrder`] so any field can be sorted either way.
#[derive(Debug, Clone, Copy, Default, Deserialize)]
#[serde(rename_all = "snake_case")]
/// [`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,
@@ -35,10 +35,21 @@ pub enum SortField {
Author,
}
/// Sort direction. Defaults to descending so the default listing
/// (`Updated`/`Desc`) shows the most recently updated mangas first.
#[derive(Debug, Clone, Copy, Default, Deserialize)]
#[serde(rename_all = "snake_case")]
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=<field>` 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]
@@ -148,15 +159,15 @@ 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<Manga>, i64)> {
// Both `col` and `dir` are interpolated from hard-coded enums, never from
// request input, so this is not a SQL injection seam. `NULLS LAST` keeps
// authorless mangas out of the way in both directions, and the trailing
// `id` is a stable tie-break that keeps pagination deterministic across
// rows with equal sort keys.
// 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",
@@ -176,7 +187,14 @@ pub async fn list(pool: &PgPool, query: &ListQuery) -> AppResult<(Vec<Manga>, i6
SortOrder::Asc => "ASC",
SortOrder::Desc => "DESC",
};
let order_by = format!("{col} {dir} NULLS LAST, id");
// 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();

172
backend/tests/api_mangas.rs
View File

@@ -281,6 +281,178 @@ async fn list_sort_author_orders_by_author_name_nulls_last(pool: PgPool) {
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;
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<String> = vec![page(1, 0).await, page(1, 1).await, page(1, 2).await]
.iter()
.map(|b| b["items"][0]["title"].as_str().unwrap().to_string())
.collect();
assert_eq!(paged, vec!["Bravo", "Charlie", "Alpha"]);
}
#[sqlx::test(migrations = "./migrations")]
async fn search_reflects_filtered_total(pool: PgPool) {
let h = common::harness(pool);