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/src/api/mangas.rs b/backend/src/api/mangas.rs index 2127f16..93ab31d 100644 --- a/backend/src/api/mangas.rs +++ b/backend/src/api/mangas.rs @@ -52,7 +52,9 @@ pub struct ListParams { #[serde(default)] pub offset: i64, #[serde(default)] - pub sort: repo::manga::ListSort, + pub sort: repo::manga::SortField, + #[serde(default)] + pub order: repo::manga::SortOrder, } fn default_limit() -> i64 { @@ -105,6 +107,7 @@ async fn list( limit, offset, sort: params.sort, + order: params.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..1734384 100644 --- a/backend/src/repo/manga.rs +++ b/backend/src/repo/manga.rs @@ -18,14 +18,31 @@ use crate::repo; 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")] -pub enum ListSort { - /// Newest first (default). +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, +} + +/// 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")] +pub enum SortOrder { + Asc, + #[default] + Desc, } #[derive(Debug, Clone, Default)] @@ -42,7 +59,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`], @@ -134,12 +152,31 @@ const FILTER_WHERE: &str = r#" /// and 0009_manga_metadata.sql) keep both queries 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. `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. + 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", + }; + let order_by = format!("{col} {dir} NULLS LAST, 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..3c36591 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,21 +150,137 @@ 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")) + .oneshot(common::get("/api/v1/mangas?sort=title&order=asc")) .await .unwrap(); let body = common::body_json(resp).await; - let titles: Vec<&str> = body["items"] - .as_array() - .unwrap() - .iter() - .map(|m| m["title"].as_str().unwrap()) - .collect(); + 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 search_reflects_filtered_total(pool: PgPool) { let h = common::harness(pool); diff --git a/frontend/e2e/mobile-list-search.spec.ts b/frontend/e2e/mobile-list-search.spec.ts index 950047e..9e4ea2e 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,22 @@ 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'); }); }); 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/routes/+page.svelte b/frontend/src/routes/+page.svelte index 8782627..be1bf4f 100644 --- a/frontend/src/routes/+page.svelte +++ b/frontend/src/routes/+page.svelte @@ -7,6 +7,7 @@ listMangas, type MangaCard as MangaCardData, type MangaSort, + type SortOrder, type MangaStatus } from '$lib/api/mangas'; import { listGenres, type Genre } from '$lib/api/genres'; @@ -14,6 +15,7 @@ import Chip from '$lib/components/Chip.svelte'; import MangaCard from '$lib/components/MangaCard.svelte'; import Pager from '$lib/components/Pager.svelte'; + import SegmentedControl from '$lib/components/SegmentedControl.svelte'; import Sheet from '$lib/components/Sheet.svelte'; import Search from '@lucide/svelte/icons/search'; import SlidersHorizontal from '@lucide/svelte/icons/sliders-horizontal'; @@ -25,7 +27,8 @@ let mangas: MangaCardData[] = $state([]); let search = $state(''); - let sort = $state('recent'); + let sort = $state('updated'); + let order = $state('desc'); let statusFilter = $state<'' | MangaStatus>(''); let selectedGenres = $state([]); let selectedTags = $state([]); @@ -55,7 +58,37 @@ (statusFilter ? 1 : 0) + selectedGenres.length + selectedTags.length ); - const sortLabel = $derived(sort === 'title' ? 'Title (A→Z)' : 'Recent'); + // Date fields read most-useful newest-first; text fields A→Z. The + // direction stays freely togglable — this only picks the default applied + // when the field changes, and labels the toggle so "descending" reads as + // "Newest" for dates and "Z→A" for text. + const isTextField = (f: MangaSort) => f === 'title' || f === 'author'; + const defaultOrderFor = (f: MangaSort): SortOrder => (isTextField(f) ? 'asc' : 'desc'); + + const SORT_FIELD_LABELS: Record = { + updated: 'Last updated', + created: 'Date added', + title: 'Title', + author: 'Author' + }; + + const dirOptions = $derived<{ label: string; value: SortOrder }[]>( + isTextField(sort) + ? [ + { label: 'A→Z', value: 'asc' }, + { label: 'Z→A', value: 'desc' } + ] + : [ + { label: 'Newest', value: 'desc' }, + { label: 'Oldest', value: 'asc' } + ] + ); + + const dirLabel = $derived( + dirOptions.find((o) => o.value === order)?.label ?? '' + ); + + const sortLabel = $derived(`${SORT_FIELD_LABELS[sort]} · ${dirLabel}`); const totalPages = $derived( total != null && total > 0 ? Math.ceil(total / PAGE_SIZE) : 1 @@ -76,6 +109,7 @@ genreIds: selectedGenres.map((g) => g.id), tagIds: selectedTags.map((t) => t.id), sort, + order, limit: PAGE_SIZE, offset: (currentPage - 1) * PAGE_SIZE }); @@ -92,7 +126,10 @@ if (!browser) return; const params = new URLSearchParams(); if (search.trim()) params.set('q', search.trim()); - if (sort !== 'recent') params.set('sort', sort); + if (sort !== 'updated') params.set('sort', sort); + // Only persist `order` when it deviates from the field's natural + // default, so the common case keeps a clean URL. + if (order !== defaultOrderFor(sort)) params.set('order', order); if (statusFilter) params.set('status', statusFilter); if (selectedGenres.length) params.set('genres', selectedGenres.map((g) => g.id).join(',')); @@ -127,7 +164,11 @@ const url = new URL($page.url); search = url.searchParams.get('q') ?? ''; const s = url.searchParams.get('sort'); - if (s === 'title' || s === 'recent') sort = s; + if (s === 'created' || s === 'updated' || s === 'title' || s === 'author') sort = s; + const o = url.searchParams.get('order'); + // Fall back to the field's natural default when `order` is absent or + // invalid, mirroring how syncUrl omits the default. + order = o === 'asc' || o === 'desc' ? o : defaultOrderFor(sort); const st = url.searchParams.get('status'); statusFilter = st === 'ongoing' || st === 'completed' ? st : ''; const genreIds = (url.searchParams.get('genres') ?? '') @@ -166,6 +207,15 @@ } function onSortChange() { + // Changing the field snaps the direction back to that field's natural + // default (dates → Newest, text → A→Z); the user can flip afterwards. + order = defaultOrderFor(sort); + resetAndReload(); + } + + function pickOrder(next: SortOrder) { + if (next === order) return; + order = next; resetAndReload(); } @@ -249,12 +299,12 @@ } function pickSort(next: MangaSort) { - if (next === sort) { - sortSheetOpen = false; - return; - } + if (next === sort) return; sort = next; - sortSheetOpen = false; + // Match desktop: snap to the field's natural direction. The sheet + // stays open so the user can also adjust the direction before + // dismissing it. + order = defaultOrderFor(sort); resetAndReload(); } @@ -504,10 +554,19 @@ + @@ -527,28 +586,28 @@ testid="sort-sheet" >
- - + {#each Object.entries(SORT_FIELD_LABELS) as [value, label] (value)} + + {/each} +
+
+
@@ -836,6 +895,12 @@ cursor: pointer; } + .sort-direction { + margin-top: var(--space-3); + padding-top: var(--space-3); + border-top: 1px solid var(--border); + } + .icon-btn { display: inline-flex; align-items: center;