From c7d2382b14161bf964ef3139f86cc65a05e26037 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 23 Jun 2026 12:45:18 +0000 Subject: [PATCH 1/2] =?UTF-8?q?feat(rm-handlers):=20D2=20=E2=80=94=20issue?= =?UTF-8?q?=20list=20filter,=20sort,=20paginate=20(17-yr=20Redmine=20UX)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the iconic Redmine issue-list chrome (the Plan §4 D2 depth track on top of the W1 width track): `?q=` subject filter, `?sort=[:asc|desc]` clickable sort headers, `?page=N&per_page=N` pagination. GET /issues?q=foo&sort=subject:desc&page=2&per_page=50 Designed once, generic across resources. The new `rm_handlers::list_chrome` module is the helper the W2 (projects), W3 (time-entries), W4 (users / members), and subsequent W-handlers reuse instead of each re-rolling its own search bar + pagination math. The exported surface is: pub struct ListQuery { q, sort, page, per_page } // axum Query pub enum SortDir { Asc, Desc } impl ListQuery { pub fn search_needle(&self) -> String; // lowercased+trimmed pub fn page(&self) -> u32; // clamps to >= 1 pub fn per_page(&self) -> u32; // Redmine default 25, cap 100 pub fn sort(&self) -> Option<(&str, SortDir)>; pub fn page_window(&self, total) -> (start, end); // never out-of-bounds pub fn render_filter_bar(&self, action, placeholder) -> String; pub fn sort_href(&self, action, column) -> String; pub fn render_pagination(&self, action, total) -> String; } The W1 /issues handler now consumes the chrome around the canonical `render_list` output: filter bar above (with Clear link when active), sort-affordance nav above the table, pagination strip below. Per-page is hard-capped at 100 so a hostile URL can't force the whole table into one render. Unknown sort columns fall back silently to insertion order (never errors the request). XSS-escaping covers the filter input value, the action URL, and the HTML-attribute hrefs (percent-encoding the q= value covers the URL boundary; html-escape the attribute boundary). Filter/sort/page math runs in-memory on `Store::list_issues()` today (deliberate first step — the Store explicitly defers WHERE + LIMIT/OFFSET push-down to a follow-on). The handler's public shape is fixed by ListQuery; moving the predicate into SurrealQL later doesn't change the URL contract. Status / priority / tracker / assignee facets are deferred — IssueRow doesn't carry those FKs yet (W2/W3 taxonomy + W4 actor land them; the chrome grows one helper per facet without re-deriving the pagination math). Scope: 4 files, +650 lines (mostly tests). - rm-handlers/Cargo.toml +3 (workspace serde for Query) - rm-handlers/src/lib.rs +3 (pub mod list_chrome; re-export) - rm-handlers/src/list_chrome.rs (NEW) 14 unit tests - rm-handlers/src/issues.rs replaced — 6 new integration tests (filter substring, sort asc/desc, paginate, per_page cap, unknown-sort silent fallback, filter-input XSS escape) cargo +1.95 test -p rm-handlers: 75 passed (was 64; +11 new). cargo fmt --check + cargo clippy clean. No new deps beyond workspace serde. --- crates/rm-handlers/Cargo.toml | 4 + crates/rm-handlers/src/issues.rs | 324 ++++++++++++--- crates/rm-handlers/src/lib.rs | 2 + crates/rm-handlers/src/list_chrome.rs | 564 ++++++++++++++++++++++++++ 4 files changed, 841 insertions(+), 53 deletions(-) create mode 100644 crates/rm-handlers/src/list_chrome.rs diff --git a/crates/rm-handlers/Cargo.toml b/crates/rm-handlers/Cargo.toml index f36d0d9..e4ef44b 100644 --- a/crates/rm-handlers/Cargo.toml +++ b/crates/rm-handlers/Cargo.toml @@ -26,6 +26,10 @@ surrealdb-types = { git = "https://github.com/AdaWorldAPI/surrealdb", branc # HTTP server primitives. axum = "0.7" +# Query-string deserialization for the D2 filter / sort / paginate extractor +# on /issues. Workspace serde — same version every other crate already pulls. +serde = { workspace = true } + # Async runtime + error glue. tokio = { version = "1", features = ["macros", "rt-multi-thread"] } thiserror = "1" diff --git a/crates/rm-handlers/src/issues.rs b/crates/rm-handlers/src/issues.rs index c93f5e5..2facd4d 100644 --- a/crates/rm-handlers/src/issues.rs +++ b/crates/rm-handlers/src/issues.rs @@ -6,22 +6,34 @@ //! //! Routes (mounted by [`router`] at the workspace router): //! -//! - `GET /issues` — list page, columns `#` + `Subject` -//! - `GET /issues/:id` — detail page for a single issue +//! - `GET /issues` — list page with **filter / sort / paginate** chrome +//! ([`ListQuery`]: `?q=`, `?sort=`, `?page=`, `?per_page=`). +//! - `GET /issues/:id` — detail page for a single issue. //! //! # Today's scope //! //! - Columns hardcoded; the `default_columns_for(&class)` factoring //! lands when W2 (Project) introduces a second caller. -//! - No filter / sort / group on the list (D2 — Plan §4). -//! - No edit form (D1 — Plan §4); the create form already exists in -//! `rm-handlers/src/issues_form.rs` (TODO, follow-up). +//! - **D2** — filter by subject substring, sort by subject (asc/desc), +//! paginate (25/page default, capped at 100). Status / priority / +//! tracker / assignee filters wait until those FKs land on the +//! `IssueRow` (W2/W3 taxonomy, W4 actor). +//! - No edit form (D1 — Plan §4). The create form will live in a +//! sibling module. //! - URL `:id` is the SurrealDB record-key segment (string ulid). //! The render-kit's u64 `record_id` parameter is filled via //! [`common::record_id_to_u64`] (stable hash); proper integer ids //! land when a Redmine-shaped `iid` column is added to the row. +//! +//! The filter / sort / pagination math runs in-memory on the result of +//! [`Store::list_issues`] today — a deliberate first step. Pushing the +//! predicate + `LIMIT/OFFSET` into the SurrealDB query is a follow-on +//! optimisation that doesn't change the handler's public shape; the +//! [`ListQuery`] extractor stays the same. +//! +//! [`Store::list_issues`]: rm_store::Store::list_issues -use axum::extract::{Path, State}; +use axum::extract::{Path, Query, State}; use axum::response::Html; use axum::routing::get; use axum::Router; @@ -32,16 +44,32 @@ use rm_store::IssueRow; use surrealdb_types::{RecordId, ToSql}; use crate::common::{html_escape, record_id_to_u64, wrap_in_doc, AppState, HandlerError}; +use crate::list_chrome::{ListQuery, SortDir}; + +/// The list URL — used by the chrome to build self-referential links +/// (filter form action, sort headers, pagination Prev/Next). +const LIST_PATH: &str = "/issues"; -/// `GET /issues` — render the issue list. -pub async fn list(State(state): State) -> Result, HandlerError> { +/// The set of column names the `?sort=` parameter accepts on the +/// issue list. Unknown columns silently fall back to insertion order so +/// a hostile URL never errors out the page. +const SORTABLE_COLUMNS: &[&str] = &["id", "subject"]; + +/// `GET /issues` — render the issue list with D2 filter / sort / page. +pub async fn list( + State(state): State, + Query(q): Query, +) -> Result, HandlerError> { let issues = state.store.list_issues().await?; + let view = apply_filter_sort(&issues, &q); + let total = view.len(); + let (start, end) = q.page_window(total); + let page_view: &[&IssueRow] = &view[start..end]; + let cols = list_columns(); - // Row construction is allocation-light: each row keeps refs to - // the issue's strings + the column slice. The askama kit takes - // a `&[RowSource<'_>]` so the lifetime ties to `issues` + `cols`, - // both held in this scope. - let hrefs: Vec = issues + // Row construction is allocation-light per the W1 doc; each row's + // strings are borrowed from the IssueRow. + let hrefs: Vec = page_view .iter() .map(|i| { i.id.as_ref() @@ -49,11 +77,11 @@ pub async fn list(State(state): State) -> Result, Handler .unwrap_or_default() }) .collect(); - let ids: Vec = issues + let ids: Vec = page_view .iter() .map(|i| i.id.as_ref().map(record_id_to_u64).unwrap_or(0)) .collect(); - let rows: Vec> = issues + let rows: Vec> = page_view .iter() .enumerate() .map(|(idx, issue)| RowSource { @@ -81,11 +109,86 @@ pub async fn list(State(state): State) -> Result, Handler block: Vec::new(), }) .collect(); - let body = render_list("Issues", 0x0102, "project_work_item", &cols, &[], &rows) + let table = render_list("Issues", 0x0102, "project_work_item", &cols, &[], &rows) .map_err(|e| HandlerError::Render(e.to_string()))?; + + // Chrome composes around the table: filter bar above (with search + + // clear), clickable sort headers (column-by-column links over the + // existing table rows — emitted as a small nav block above the + // table), and pagination strip below. + let filter_bar = q.render_filter_bar(LIST_PATH, "Filter by subject"); + let sort_nav = render_sort_nav(&q); + let pagination = q.render_pagination(LIST_PATH, total); + let body = format!("{filter_bar}\n{sort_nav}\n{table}\n{pagination}"); Ok(Html(wrap_in_doc("Issues", &body))) } +/// Filter (subject substring, case-insensitive) + sort the list of +/// issues per the query. Returns a view of references to the originals +/// so the page-window slice never moves bytes. +fn apply_filter_sort<'a>(issues: &'a [IssueRow], q: &ListQuery) -> Vec<&'a IssueRow> { + let needle = q.search_needle(); + let mut view: Vec<&IssueRow> = if needle.is_empty() { + issues.iter().collect() + } else { + issues + .iter() + .filter(|i| i.subject.to_lowercase().contains(&needle)) + .collect() + }; + if let Some((col, dir)) = q.sort() { + // Only sort on columns we recognise — guards against a hostile + // `?sort=` triggering a partial-order on a column the + // row doesn't carry today. + if SORTABLE_COLUMNS.contains(&col) { + match col { + "subject" => view.sort_by(|a, b| a.subject.cmp(&b.subject)), + "id" => view.sort_by_key(|i| { + i.id.as_ref() + .map(|rid| rid.key.to_sql()) + .unwrap_or_default() + }), + _ => {} // SORTABLE_COLUMNS guard above + } + if dir == SortDir::Desc { + view.reverse(); + } + } + } + view +} + +/// Render the clickable column-header strip. Sortable columns become +/// links that emit `?sort=:` URLs (preserving filter + page). +/// Active column gets an arrow indicator (↑ asc, ↓ desc). Kept terse — +/// the canonical render-kit list view already shows the column captions +/// in the table head; this strip is the *sort affordance* sitting above +/// the table, where Redmine users expect to click for re-ordering. +fn render_sort_nav(q: &ListQuery) -> String { + use std::fmt::Write as _; + let active = q.sort(); + let mut out = String::with_capacity(160); + out.push_str(r#""); + out +} + /// `GET /issues/:id` — render an issue's detail page. pub async fn detail( State(state): State, @@ -183,51 +286,155 @@ mod tests { router(AppState { store }) } - #[tokio::test] - async fn list_renders_empty_state_when_no_issues() { - let app = app_with_issues(&[]).await; + async fn body_of(app: Router, uri: &str) -> (StatusCode, String) { let res = app - .oneshot( - Request::builder() - .uri("/issues") - .body(Body::empty()) - .unwrap(), - ) + .oneshot(Request::builder().uri(uri).body(Body::empty()).unwrap()) .await .unwrap(); - assert_eq!(res.status(), StatusCode::OK); - let body = res.into_body().collect().await.unwrap().to_bytes(); - let s = std::str::from_utf8(&body).unwrap(); + let status = res.status(); + let bytes = res.into_body().collect().await.unwrap().to_bytes(); + let s = String::from_utf8(bytes.to_vec()).unwrap(); + (status, s) + } + + #[tokio::test] + async fn list_renders_empty_state_when_no_issues() { + let app = app_with_issues(&[]).await; + let (status, s) = body_of(app, "/issues").await; + assert_eq!(status, StatusCode::OK); assert!( s.contains("data-class-id=\"0x0102\""), "expected canonical class id in:\n{s}" ); assert!(s.contains("No data."), "expected empty-state in:\n{s}"); + // Empty result → no pagination strip (the chrome's contract). + assert!( + !s.contains("Page 1 of"), + "no pagination on empty list:\n{s}" + ); + // But the filter bar always renders (so the user can type to add data). + assert!(s.contains("list-filter"), "filter bar always renders:\n{s}"); } #[tokio::test] async fn list_renders_seeded_issues() { let app = app_with_issues(&[("Fix the foo", Some("a description")), ("Bar broken", None)]).await; - let res = app - .oneshot( - Request::builder() - .uri("/issues") - .body(Body::empty()) - .unwrap(), - ) - .await - .unwrap(); - assert_eq!(res.status(), StatusCode::OK); - let body = res.into_body().collect().await.unwrap().to_bytes(); - let s = std::str::from_utf8(&body).unwrap(); + let (status, s) = body_of(app, "/issues").await; + assert_eq!(status, StatusCode::OK); assert!(s.contains("Fix the foo"), "expected first subject in:\n{s}"); assert!(s.contains("Bar broken"), "expected second subject in:\n{s}"); - // The IdLink column emits `/issues/` hrefs. assert!( s.contains("href=\"/issues/"), "expected detail hrefs in:\n{s}" ); + // Pagination strip shows the right total. + assert!(s.contains("Page 1 of 1 (2)"), "expected pagination:\n{s}"); + } + + // ── D2: filter / sort / paginate ──────────────────────────────── + + #[tokio::test] + async fn list_filters_by_subject_substring_case_insensitive() { + let app = app_with_issues(&[ + ("Fix the foo", None), + ("Bar broken", None), + ("Another FOO bug", None), + ]) + .await; + let (status, s) = body_of(app, "/issues?q=foo").await; + assert_eq!(status, StatusCode::OK); + assert!(s.contains("Fix the foo"), "kept matching first row:\n{s}"); + assert!( + s.contains("Another FOO bug"), + "case-insensitive match must keep this row:\n{s}" + ); + assert!( + !s.contains("Bar broken"), + "non-match must be filtered:\n{s}" + ); + // The filter bar should echo the query and offer a clear link. + assert!(s.contains(r#"value="foo""#), "filter bar echoes q:\n{s}"); + assert!(s.contains("Clear"), "clear link when filter active:\n{s}"); + assert!( + s.contains("(2)"), + "pagination reflects filtered total:\n{s}" + ); + } + + #[tokio::test] + async fn list_sort_by_subject_asc_and_desc_changes_row_order() { + let app = app_with_issues(&[("Charlie", None), ("Alpha", None), ("Bravo", None)]).await; + let (_, asc) = body_of(app.clone(), "/issues?sort=subject:asc").await; + let (_, desc) = body_of(app.clone(), "/issues?sort=subject:desc").await; + let pos = |s: &str, needle: &str| s.find(needle).unwrap_or(usize::MAX); + // Asc → Alpha, Bravo, Charlie + assert!( + pos(&asc, "Alpha") < pos(&asc, "Bravo") && pos(&asc, "Bravo") < pos(&asc, "Charlie"), + "asc order broken:\n{asc}" + ); + // Desc → Charlie, Bravo, Alpha + assert!( + pos(&desc, "Charlie") < pos(&desc, "Bravo") + && pos(&desc, "Bravo") < pos(&desc, "Alpha"), + "desc order broken:\n{desc}" + ); + // Sort nav shows the indicator on the active direction. + assert!(asc.contains("subject ↑"), "asc indicator missing:\n{asc}"); + assert!( + desc.contains("subject ↓"), + "desc indicator missing:\n{desc}" + ); + } + + #[tokio::test] + async fn list_paginates_with_per_page_cap_and_page_window() { + // 5 issues, per_page=2 → 3 pages. + let app = app_with_issues(&[ + ("alpha", None), + ("bravo", None), + ("charlie", None), + ("delta", None), + ("echo", None), + ]) + .await; + let (_, p1) = body_of(app.clone(), "/issues?per_page=2&sort=subject:asc").await; + let (_, p2) = body_of(app.clone(), "/issues?per_page=2&page=2&sort=subject:asc").await; + let (_, p9) = body_of(app.clone(), "/issues?per_page=2&page=99&sort=subject:asc").await; + + // Page 1 → alpha, bravo (sorted asc); page 2 → charlie, delta; + // page 99 → no rows but no panic (empty body, pagination still + // shows position clamped to last page). + assert!(p1.contains("alpha"), "{p1}"); + assert!(p1.contains("bravo"), "{p1}"); + assert!(!p1.contains("charlie"), "page-1 must not bleed: {p1}"); + assert!(p2.contains("charlie") && p2.contains("delta"), "{p2}"); + assert!(!p2.contains("alpha"), "page-2 must not include alpha: {p2}"); + assert!(p2.contains("Page 2 of 3 (5)"), "pagination wrong:\n{p2}"); + // page 99 → page slice is empty; the kit's empty-state shows. + assert!(p9.contains("No data."), "out-of-range page → empty:\n{p9}"); + } + + #[tokio::test] + async fn list_per_page_clamps_hostile_values_to_redmine_band() { + let app = app_with_issues(&[("a", None), ("b", None), ("c", None), ("d", None)]).await; + // per_page=1000 → caps to 100; all 4 fit on one page. + let (_, all) = body_of(app, "/issues?per_page=1000").await; + assert!(all.contains("Page 1 of 1 (4)"), "expected 1 page:\n{all}"); + // No "Prev" / "Next" links on the single page. + assert!(!all.contains("« Prev") && !all.contains("Next »")); + } + + #[tokio::test] + async fn list_ignores_unknown_sort_columns_silently() { + // A hostile URL asking to sort on a column that doesn't exist on + // IssueRow today (e.g. `priority`) must NOT error the request and + // must NOT crash — it falls back to insertion order. + let app = app_with_issues(&[("Aaa", None), ("Zzz", None)]).await; + let (status, s) = body_of(app, "/issues?sort=priority:desc").await; + assert_eq!(status, StatusCode::OK); + // Both rows still rendered; the page didn't 4xx. + assert!(s.contains("Aaa") && s.contains("Zzz"), "{s}"); } #[tokio::test] @@ -245,18 +452,8 @@ mod tests { let key = rid.key.to_sql().to_string(); let app = router(AppState { store }); - let res = app - .oneshot( - Request::builder() - .uri(format!("/issues/{key}")) - .body(Body::empty()) - .unwrap(), - ) - .await - .unwrap(); - assert_eq!(res.status(), StatusCode::OK); - let body = res.into_body().collect().await.unwrap().to_bytes(); - let s = std::str::from_utf8(&body).unwrap(); + let (status, s) = body_of(app, &format!("/issues/{key}")).await; + assert_eq!(status, StatusCode::OK); assert!(s.contains("Detail target"), "expected subject in:\n{s}"); assert!( s.contains("data-class-id=\"0x0102\""), @@ -323,4 +520,25 @@ mod tests { "expected escaped form somewhere:\n{s}" ); } + + #[tokio::test] + async fn list_filter_input_value_is_xss_safe() { + // The filter bar echoes ?q= into a — must + // HTML-escape so a hostile `?q=">` + // never breaks out of the attribute. + let app = app_with_issues(&[("foo", None)]).await; + let (_, s) = body_of( + app, + r#"/issues?q=%22%3E%3Cscript%3Ealert(1)%3C%2Fscript%3E"#, + ) + .await; + assert!( + !s.contains("\">"), + "raw payload survived into filter input — XSS:\n{s}" + ); + assert!( + s.contains(""><script>alert(1)"), + "expected escaped form in filter input:\n{s}" + ); + } } diff --git a/crates/rm-handlers/src/lib.rs b/crates/rm-handlers/src/lib.rs index 5e39cfa..8445985 100644 --- a/crates/rm-handlers/src/lib.rs +++ b/crates/rm-handlers/src/lib.rs @@ -47,6 +47,7 @@ mod common; pub mod issues; +pub mod list_chrome; pub mod news; pub mod projects; pub mod queries; @@ -62,3 +63,4 @@ pub use common::{ encode_path_segment, html_escape, identifier_to_u64, record_id_to_u64, wrap_in_doc, AppState, HandlerError, }; +pub use list_chrome::{ListQuery, SortDir}; diff --git a/crates/rm-handlers/src/list_chrome.rs b/crates/rm-handlers/src/list_chrome.rs new file mode 100644 index 0000000..6f2a4da --- /dev/null +++ b/crates/rm-handlers/src/list_chrome.rs @@ -0,0 +1,564 @@ +//! **D2** — list-view chrome: filter bar, clickable sort headers, and +//! pagination strip. The "17 years of Redmine" UX layer wrapping the +//! canonical [`render_list`] output. +//! +//! [`render_list`]: ogar_render_askama::render_list +//! +//! Today's scope is the minimum honest slice on the existing data shape: +//! +//! - **`?q=`** — case-insensitive substring filter on a single configurable +//! text field (subject, name, …). Redmine's quick-search shape, the most +//! commonly-used filter across 17 years of the issue list. +//! - **`?sort=[:asc|desc]`** — sort by a named column with explicit +//! direction. Render kit's `sortable` flag is now URL-emitted: each +//! sortable header is a link that toggles direction. +//! - **`?page=N&per_page=N`** — 1-indexed page over the filtered+sorted +//! set. `per_page` defaults to 25 (Redmine's default), capped at 100 so +//! a hostile URL can't force the whole table into one render. +//! +//! Status / priority / tracker / assignee facets are NOT covered today — +//! the IssueRow doesn't carry those foreign keys yet (W4 actor + W2/W3 +//! taxonomy land next). When they do, this module grows one helper per +//! facet without re-deriving the pagination math. +//! +//! Designed as a generic helper from day one so the **W2 (projects), W3 +//! (time-entries), W4 (users / members), and later resources reuse the +//! same chrome** instead of each re-rolling its own search box. + +use std::fmt::Write as _; + +use serde::Deserialize; + +use crate::common::html_escape; + +/// Query-string parameters every D2-enabled list route accepts. Each field +/// is optional so a bare `GET /issues` still works (no query params = the +/// first page, default sort, no filter). +/// +/// Use with axum's `Query` extractor: +/// +/// ```ignore +/// pub async fn list( +/// State(state): State, +/// Query(q): Query, +/// ) -> Result, HandlerError> { ... } +/// ``` +#[derive(Debug, Clone, Default, Deserialize)] +pub struct ListQuery { + /// Substring filter (URL `?q=foo`). The handler decides which text + /// field(s) to match against — usually the row's primary text column + /// (subject for issues, name for projects, etc.). + #[serde(default)] + pub q: Option, + /// Sort spec, `""` or `":asc"` / `":desc"`. The handler + /// validates `` against its column allow-list — unknown columns + /// fall back silently to the resource's default order (insertion). + #[serde(default)] + pub sort: Option, + /// 1-indexed page. Values `< 1` clamp to `1` via [`Self::page`]. + #[serde(default)] + pub page: Option, + /// Items per page. Values `< 1` or `> 100` clamp via [`Self::per_page`] + /// to guard against a hostile URL forcing a huge render. + #[serde(default)] + pub per_page: Option, +} + +/// Sort direction parsed from a `:asc|desc` suffix. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SortDir { + /// Ascending — the default when the suffix is omitted or unrecognized. + Asc, + /// Descending — when the suffix is `:desc`. + Desc, +} + +impl SortDir { + /// Toggle direction — the column-header click semantic. + #[must_use] + pub fn toggled(self) -> Self { + match self { + Self::Asc => Self::Desc, + Self::Desc => Self::Asc, + } + } + + /// `"asc"` / `"desc"` — the URL spelling. + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::Asc => "asc", + Self::Desc => "desc", + } + } +} + +impl ListQuery { + /// The substring filter, lowercased and trimmed for case-insensitive + /// comparison. Empty string ⇒ no filter. + #[must_use] + pub fn search_needle(&self) -> String { + self.q + .as_deref() + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_lowercase) + .unwrap_or_default() + } + + /// 1-indexed page number, clamped to `>= 1`. + #[must_use] + pub fn page(&self) -> u32 { + self.page.unwrap_or(1).max(1) + } + + /// Items per page. Defaults to Redmine's `25`; clamped to `1..=100` so + /// a hostile URL can't ask for the whole table at once. + #[must_use] + pub fn per_page(&self) -> u32 { + self.per_page.unwrap_or(25).clamp(1, 100) + } + + /// Parse `?sort=` into `(column, direction)`. `None` when no sort is + /// requested or the value can't be parsed. `:asc` / `:desc` recognised; + /// anything else after the colon falls back to `Asc`. + #[must_use] + pub fn sort(&self) -> Option<(&str, SortDir)> { + let spec = self.sort.as_deref()?.trim(); + if spec.is_empty() { + return None; + } + match spec.split_once(':') { + Some((col, "desc")) => Some((col.trim(), SortDir::Desc)), + Some((col, _)) => Some((col.trim(), SortDir::Asc)), + None => Some((spec, SortDir::Asc)), + } + } + + /// Compute the page slice indices `[start, end)` over a collection of + /// `total` items already in display order. `start == total` when the + /// requested page is past the end (the slice is then empty, never out + /// of bounds). + #[must_use] + pub fn page_window(&self, total: usize) -> (usize, usize) { + let per = self.per_page() as usize; + let start = ((self.page() - 1) as usize).saturating_mul(per).min(total); + let end = start.saturating_add(per).min(total); + (start, end) + } + + /// Render the search input + hidden state inputs as one form. Submits + /// to `action` (the list URL). Hidden inputs preserve sort/per_page so + /// the user keeps their view; `page` resets to 1 on a new filter. + #[must_use] + pub fn render_filter_bar(&self, action: &str, placeholder: &str) -> String { + let action_esc = html_escape(action); + let needle = html_escape(self.q.as_deref().unwrap_or("")); + let placeholder_esc = html_escape(placeholder); + let mut out = String::with_capacity(256); + let _ = write!( + &mut out, + r#""); + out + } + + /// Build an `href` for a sortable column header. Clicking toggles the + /// direction when this column is already active; otherwise sorts asc + /// by default. Always resets to page 1 (the Redmine convention — a + /// new sort starts from the top of the result). + #[must_use] + pub fn sort_href(&self, action: &str, column: &str) -> String { + let (next_col, next_dir) = match self.sort() { + Some((active, dir)) if active == column => (column, dir.toggled()), + _ => (column, SortDir::Asc), + }; + let next = Self { + q: self.q.clone(), + sort: Some(format!("{next_col}:{}", next_dir.as_str())), + page: None, + per_page: self.per_page, + }; + next.as_query_path(action) + } + + /// Build the pagination strip: `Prev` / `Next` plus the current + /// position `"page of total_pages"`. Minimalist (no per-page jumpers) + /// — Redmine's modern shape with the same data Redmine 5.0 shows. + /// Empty string when `total_items == 0` (the empty-state message in + /// the table already conveys "no results"). + #[must_use] + pub fn render_pagination(&self, action: &str, total_items: usize) -> String { + if total_items == 0 { + return String::new(); + } + let per = self.per_page() as usize; + let total_pages = total_items.div_ceil(per).max(1); + let cur = (self.page() as usize).min(total_pages); + let mut out = String::with_capacity(160); + out.push_str(r#""); + out + } + + /// Render `action?` with only the set fields, + /// each value HTML-escaped (the href lives in an HTML attribute). + /// Used by [`Self::render_filter_bar`], [`Self::sort_href`], + /// [`Self::render_pagination`]. + fn as_query_path(&self, action: &str) -> String { + let mut parts: Vec = Vec::new(); + if let Some(q) = self.q.as_deref() { + if !q.is_empty() { + parts.push(format!("q={}", percent_encode(q))); + } + } + if let Some(s) = self.sort.as_deref() { + if !s.is_empty() { + parts.push(format!("sort={}", percent_encode(s))); + } + } + if let Some(p) = self.page { + if p > 1 { + parts.push(format!("page={p}")); + } + } + if let Some(pp) = self.per_page { + if pp != 25 { + parts.push(format!("per_page={pp}")); + } + } + if parts.is_empty() { + html_escape(action) + } else { + format!("{}?{}", html_escape(action), parts.join("&")) + } + } +} + +/// Percent-encode a single query-string value. Conservative: anything +/// outside the URL-unreserved set (`A-Z a-z 0-9 - . _ ~`) is `%HH`-escaped. +fn percent_encode(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for b in s.bytes() { + match b { + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'.' | b'_' | b'~' => { + out.push(b as char); + } + _ => { + let _ = write!(&mut out, "%{b:02X}"); + } + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn default_query_yields_first_page_no_filter_no_sort() { + let q = ListQuery::default(); + assert_eq!(q.search_needle(), ""); + assert_eq!(q.page(), 1); + assert_eq!(q.per_page(), 25); + assert_eq!(q.sort(), None); + assert_eq!(q.page_window(0), (0, 0)); + assert_eq!(q.page_window(100), (0, 25)); + } + + #[test] + fn search_needle_trims_and_lowercases_only_non_empty_values() { + let q = ListQuery { + q: Some(" Foo BAR ".into()), + ..Default::default() + }; + assert_eq!(q.search_needle(), "foo bar"); + let blank = ListQuery { + q: Some(" ".into()), + ..Default::default() + }; + assert_eq!(blank.search_needle(), ""); + } + + #[test] + fn per_page_clamps_to_redmine_band() { + // Below floor → 1; above ceiling → 100; default → 25. + assert_eq!( + ListQuery { + per_page: Some(0), + ..Default::default() + } + .per_page(), + 1 + ); + assert_eq!( + ListQuery { + per_page: Some(50_000), + ..Default::default() + } + .per_page(), + 100 + ); + assert_eq!( + ListQuery { + per_page: Some(50), + ..Default::default() + } + .per_page(), + 50 + ); + } + + #[test] + fn page_clamps_to_at_least_one() { + assert_eq!( + ListQuery { + page: Some(0), + ..Default::default() + } + .page(), + 1 + ); + assert_eq!( + ListQuery { + page: Some(7), + ..Default::default() + } + .page(), + 7 + ); + } + + #[test] + fn sort_parses_column_and_direction() { + let q = ListQuery { + sort: Some("subject".into()), + ..Default::default() + }; + assert_eq!(q.sort(), Some(("subject", SortDir::Asc))); + let q = ListQuery { + sort: Some("subject:desc".into()), + ..Default::default() + }; + assert_eq!(q.sort(), Some(("subject", SortDir::Desc))); + let q = ListQuery { + sort: Some("subject:asc".into()), + ..Default::default() + }; + assert_eq!(q.sort(), Some(("subject", SortDir::Asc))); + let q = ListQuery { + sort: Some("subject:gibberish".into()), + ..Default::default() + }; + // Unknown suffix falls back to asc — the safe default. + assert_eq!(q.sort(), Some(("subject", SortDir::Asc))); + let q = ListQuery { + sort: Some("".into()), + ..Default::default() + }; + assert_eq!(q.sort(), None); + } + + #[test] + fn page_window_is_never_out_of_bounds() { + // Page 1: [0, 25); page 2 with 30 items: [25, 30); page 9999: empty. + let q1 = ListQuery::default(); + assert_eq!(q1.page_window(30), (0, 25)); + let q2 = ListQuery { + page: Some(2), + ..Default::default() + }; + assert_eq!(q2.page_window(30), (25, 30)); + let q9 = ListQuery { + page: Some(9999), + ..Default::default() + }; + let (s, e) = q9.page_window(30); + assert!(s <= 30 && e <= 30 && s <= e, "got ({s},{e}) total=30"); + } + + #[test] + fn sort_href_toggles_direction_on_same_column() { + // First click on subject when no sort active → asc. + let q0 = ListQuery::default(); + let href = q0.sort_href("/issues", "subject"); + assert!(href.contains("sort=subject%3Aasc"), "{href}"); + // Already asc on subject → toggles to desc. + let q1 = ListQuery { + sort: Some("subject:asc".into()), + ..Default::default() + }; + let href = q1.sort_href("/issues", "subject"); + assert!(href.contains("sort=subject%3Adesc"), "{href}"); + // desc on subject → toggles to asc. + let q2 = ListQuery { + sort: Some("subject:desc".into()), + ..Default::default() + }; + let href = q2.sort_href("/issues", "subject"); + assert!(href.contains("sort=subject%3Aasc"), "{href}"); + } + + #[test] + fn sort_href_resets_page_so_a_new_sort_starts_from_the_top() { + // Operator convention — switching sort jumps back to page 1. + let q = ListQuery { + sort: Some("subject:asc".into()), + page: Some(7), + ..Default::default() + }; + let href = q.sort_href("/issues", "subject"); + assert!( + !href.contains("page="), + "page must reset on new sort: {href}" + ); + } + + #[test] + fn filter_bar_preserves_sort_and_renders_clear_link_when_filtered() { + let q = ListQuery { + q: Some("foo".into()), + sort: Some("subject:desc".into()), + ..Default::default() + }; + let bar = q.render_filter_bar("/issues", "Filter issues"); + assert!(bar.contains(r#"action="/issues""#)); + assert!(bar.contains(r#"value="foo""#)); + assert!(bar.contains(r#"placeholder="Filter issues""#)); + // Sort preserved as a hidden input across the filter submit. + assert!(bar.contains(r#"name="sort" value="subject:desc""#)); + // Clear link visible when filter is active. + assert!(bar.contains("Clear")); + } + + #[test] + fn pagination_emits_prev_next_and_position() { + let q = ListQuery { + page: Some(2), + ..Default::default() + }; + let nav = q.render_pagination("/issues", 80); + assert!(nav.contains("Prev"), "{nav}"); + assert!(nav.contains("Page 2 of 4 (80)"), "{nav}"); + assert!(nav.contains("Next"), "{nav}"); + // Prev links back to page 1 — the canonical default, so the URL + // is just the action (no `?page=1` parameter; matches Redmine's + // own URL canonicalization). + assert!(nav.contains(r#"href="/issues""#), "{nav}"); + // Next links to page 3 explicitly. + assert!(nav.contains(r#"href="/issues?page=3""#), "{nav}"); + } + + #[test] + fn pagination_preserves_filter_and_sort_state_across_pages() { + let q = ListQuery { + q: Some("foo".into()), + sort: Some("subject:desc".into()), + page: Some(2), + per_page: Some(50), + ..Default::default() + }; + let nav = q.render_pagination("/issues", 200); + // Both Prev (→ page 1, omitted since canonical) and Next (→ page 3) + // must keep q + sort + per_page in the URL so the filtered/sorted + // view survives navigation. + for marker in ["q=foo", "sort=subject%3Adesc", "per_page=50"] { + assert!( + nav.matches(marker).count() >= 2, + "expected `{marker}` to appear in BOTH prev + next hrefs:\n{nav}" + ); + } + } + + #[test] + fn pagination_is_empty_when_no_items() { + let nav = ListQuery::default().render_pagination("/issues", 0); + assert!(nav.is_empty(), "no nav for empty result, got: {nav:?}"); + } + + #[test] + fn query_path_escapes_html_in_action_and_percent_encodes_values() { + let q = ListQuery { + q: Some("a&b=c".into()), + ..Default::default() + }; + let href = q.sort_href(r#"/issues">