feat(rm-handlers): D2 — issue list filter, sort, paginate (17-yr Redmine UX)#21
Merged
Merged
Conversation
…ine UX)
Adds the iconic Redmine issue-list chrome (the Plan §4 D2 depth track on
top of the W1 width track): `?q=` subject filter, `?sort=<col>[: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<T>
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<T>)
- 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.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
CI's `cargo clippy --all-targets -- -D warnings` caught two lints my
local `cargo clippy -p rm-handlers` (no --all-targets, no -D warnings)
silently let through:
1. `collapsible_if` — six nested `if let Some(x) { if pred(x) { ... } }`
patterns in list_chrome.rs + one in issues.rs. The workspace is
edition 2021, so let-chains aren't available; collapsed via
`.filter(|p| pred)` on the Option / pattern instead — same semantics,
single `if let` per site.
2. `needless_update` — one test struct literal set all four ListQuery
fields then trailed `..Default::default()`. Dropped the spread.
CI's gate (.github/workflows/ci.yml):
- cargo fmt --all --check
- cargo clippy --all-targets -- -D warnings
- cargo test --workspace
Local reproduction now matches CI's invocation (-p rm-handlers narrows
it without changing the lint set). cargo clippy --all-targets -- -D
warnings clean; 75 tests still pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds the iconic Redmine issue-list chrome — the Plan §4 D2 depth track on top of the W1 width track. The "17 years of Redmine" UX layer, made generic from day one so the W2/W3/W4 handlers can reuse it:
New surface
A new
rm_handlers::list_chromemodule ships the helper every D2-enabled W-handler will consume:pub struct ListQuery { q, sort, page, per_page }Query<T>extractor — all fields optionalpub enum SortDir { Asc, Desc }:asc/:descsuffixListQuery::search_needle() -> StringListQuery::page() -> u32ListQuery::per_page() -> u3225; capped at 100 (hostile-URL guard)ListQuery::sort() -> Option<(col, SortDir)>(column, direction);:gibberish⇒ AscListQuery::page_window(total) -> (start, end)ListQuery::render_filter_bar(action, placeholder)<form>with search input + Clear linkListQuery::sort_href(action, column)ListQuery::render_pagination(action, total)URLs are canonical (defaults like
page=1/per_page=25omitted), values percent-encoded in URL boundaries and HTML-escaped in attribute boundaries — both pinned by tests.W1
/issuesintegrationThe list handler now wraps
render_listwith:/issues/:iddetail unchanged.Honest scope (today)
Filter/sort/paginate runs in-memory on
Store::list_issues()today — the store itself explicitly defers WHERE + LIMIT/OFFSET to a follow-on. The handler's URL contract is fixed byListQuery; pushing the predicate into SurrealQL later doesn't change the wire shape.Deferred (need data shape changes first, not chrome changes):
IssueRow(W2/W3 taxonomy, W4 actor). When they do, the chrome grows one helper per facet without re-deriving the pagination math.Robustness pins (the tests guard these)
?per_page=50000→ clamps to 100; no DoS via render.?sort=<unknown_col>→ silent fallback to insertion order; no 4xx.?page=99999→ page window empty; "No data." renders; no panic.?q="><script>alert(1)</script>→ escaped in the<input value="…">echo; the raw payload never reaches the DOM.Build
1.95.cargo test -p rm-handlers→ 75 passed (was 64; +11 new — 13list_chromeunit tests + 5 new/issuesintegration tests).cargo fmt --checkclean.cargo clippy -p rm-handlersclean.serde = { workspace = true }(axumQuery<T>deserialization).Cargo.toml(+3),lib.rs(+3), newlist_chrome.rs,issues.rsrewritten.Future W-handlers consume this
Next time a list view lands (Project, TimeEntry, Member, Forum, Comment, ...), the handler imports
ListQuery+ the chrome methods and gets the iconic Redmine UX for free. The pattern is in code; the pagination math has exactly one home.