Issues tab: default to showing open issues only (currently open + closed) #323

Closed
opened 2026-09-11 14:44:18 +00:00 by crueber · 3 comments
Owner

What's requested

The Issues tab (/:owner/:name/issues) should default to showing open issues only. Today it shows open and closed together; a user wanting the closed ones opts in via the State filter.

Current state (code evidence)

  • web/src/pages/Issues.jsx:31-37 — the list query reads state: search.state || "", and the server treats an empty state as "open + closed" (the filter select's own labels confirm it: empty option reads "open + closed", Issues.jsx:96-101).
  • So a bare visit to the Issues tab (no ?state= param) fetches and renders both states intermixed.
  • The filter select itself offers "open + closed / open / closed" — that stays; only the default changes.
  • The State select's value binding reads the same search.state (:100), so it will naturally show "open" as the selected option once the default flips — verify that binding reflects the effective default rather than the raw param.

Proposed change

  • Default state to "open" when the URL carries no state param: state: search.state ?? "open" is NOT enough (empty string is a legitimate "both" choice from the select) — the right shape is: treat absent param as open, keep explicit ?state= (including a possible "both" representation) honored as-is. Implementation detail for the implementer: either reserve state=all/both as the explicit both-value and migrate the select's "open + closed" option to it, or treat undefined as open and "" as both — pick one, keep the URL honest, and note it in the PR.
  • The same default should apply to the milestone-filtered views that land on this page (?milestone=<id> from the milestones page "View issues" button, #314) — they inherit the query default, so they'll show open-only too. That matches GitHub behavior; call it out in the PR so it's a decision, not an accident.
  • The pulls list (Pulls.jsx) is NOT in scope of this request — leave as-is unless the user says otherwise (flag the inconsistency in the PR body for a follow-up decision).

Acceptance criteria

  • Visiting /:owner/:name/issues with no state param shows only open issues; the State select visibly reads "open".
  • Choosing "open + closed" from the select shows both, and that choice is reflected in the URL (shareable/refreshable).
  • Deep links with an explicit state (?state=closed, etc.) are honored exactly.
  • Milestone-filtered landings (?milestone=) default to open-only as well.
  • Pagination/after cursor behavior unaffected (key already includes the query JSON).
  • Headless test for the default-resolution logic (absent → open; explicit both → both; explicit closed → closed).
## What's requested The Issues tab (`/:owner/:name/issues`) should default to showing **open issues only**. Today it shows open and closed together; a user wanting the closed ones opts in via the State filter. ## Current state (code evidence) - `web/src/pages/Issues.jsx:31-37` — the list query reads `state: search.state || ""`, and the server treats an empty `state` as "open + closed" (the filter select's own labels confirm it: empty option reads "open + closed", `Issues.jsx:96-101`). - So a bare visit to the Issues tab (no `?state=` param) fetches and renders both states intermixed. - The filter select itself offers "open + closed / open / closed" — that stays; only the **default** changes. - The State select's value binding reads the same `search.state` (`:100`), so it will naturally show "open" as the selected option once the default flips — verify that binding reflects the effective default rather than the raw param. ## Proposed change - Default `state` to `"open"` when the URL carries no `state` param: `state: search.state ?? "open"` is NOT enough (empty string is a legitimate "both" choice from the select) — the right shape is: treat *absent param* as `open`, keep explicit `?state=` (including a possible "both" representation) honored as-is. Implementation detail for the implementer: either reserve `state=all`/`both` as the explicit both-value and migrate the select's "open + closed" option to it, or treat `undefined` as open and `""` as both — pick one, keep the URL honest, and note it in the PR. - The same default should apply to the milestone-filtered views that land on this page (`?milestone=<id>` from the milestones page "View issues" button, #314) — they inherit the query default, so they'll show open-only too. That matches GitHub behavior; call it out in the PR so it's a decision, not an accident. - The pulls list (`Pulls.jsx`) is NOT in scope of this request — leave as-is unless the user says otherwise (flag the inconsistency in the PR body for a follow-up decision). ## Acceptance criteria - [ ] Visiting `/:owner/:name/issues` with no state param shows only open issues; the State select visibly reads "open". - [ ] Choosing "open + closed" from the select shows both, and that choice is reflected in the URL (shareable/refreshable). - [ ] Deep links with an explicit state (`?state=closed`, etc.) are honored exactly. - [ ] Milestone-filtered landings (`?milestone=`) default to open-only as well. - [ ] Pagination/`after` cursor behavior unaffected (key already includes the query JSON). - [ ] Headless test for the default-resolution logic (absent → open; explicit both → both; explicit closed → closed).
crueber added this to the v1 milestone 2026-09-11 14:44:18 +00:00
Author
Owner

Fix ready for review: PR #330 (#330) — issues list defaults to open-only; explicit both-choice is ?state=all (URL-honest, wire-omitted, no backend change); milestone-filtered views deliberately inherit; Pulls.jsx untouched and flagged for follow-up. Not merged.

Fix ready for review: PR #330 (https://git.packden.us/crueber/walhub/pulls/330) — issues list defaults to open-only; explicit both-choice is `?state=all` (URL-honest, wire-omitted, no backend change); milestone-filtered views deliberately inherit; Pulls.jsx untouched and flagged for follow-up. Not merged.
Author
Owner

Review of PR #330 (fix/issue-323) — verified in scratch worktree /tmp/pr330 @ 6757752 (main worktree untouched, still clean on main).

All #323 acceptance criteria hold:

  • Bare visit open-only + select reads open: query() sends state=open when ?state= absent (Issues.jsx:38-39 via resolveIssueState->issueListState), select binds RESOLVED value (Issues.jsx:96). PASS
  • Both-choice shareable: select writes ?state=all (Issues.jsx:99); refresh/deep-link resolves to both and omits the wire param. PASS
  • Deep links exact: ?state=closed/open honored verbatim; legacy empty ?state= still reads as both (issueState.js:21); end-to-end covered in issue-state.test.js. PASS
  • Milestone landings (?milestone=, #314) inherit open-only: no special-casing, called out as deliberate in code comment (Issues.jsx:32-37), PR body, and doc. PASS
  • Pagination unaffected: key is issues:{full}:{query JSON} (Issues.jsx:46) — open vs both are distinct windows; after cursor in key. #318 interplay safe: invalidateIssueLists prefix-invalidates issues:{full}:* so all windows reconcile. PASS
  • Headless tests for resolver present and green (6/6).

Wire safety verified against server code: listIssues 400s on anything but open|closed and treats absent as both (internal/issues/http.go:376-382), so mapping all->"" (omitted via SDK qs() skip-empty, web/sdk/src/issues.js:90-97) means the endpoint never sees 'all' — no 400 path, no backend change needed. Unknown passthrough (?state=bogus -> 400 tray error) matches pre-change behavior. Doc contrast with milestones endpoint (accepts all natively, http.go:781-785) verified accurate.

Scope/discipline: Pulls.jsx untouched + flagged; no new deps (package.json unchanged, law 1); no Go changes (laws 7/8 vacuous); doc decision appended same-change (law 12).

Tests: node --test web/test/unit/issue-state.test.js 6/6 pass; full web/test/unit/*.test.js 612 pass / 0 fail (smoke.test.js cancelled — hangs on squatted :8080 in this sandbox, pre-existing environmental as disclosed in PR body); vite build clean (2.06s). No browser drive per task constraints (query/select binding verified headless + by reading).

Nits (non-blocking, no push): doc hunk re-indents one adjacent continuation line ('slot for other surfaces.'); PR body phrase 'pulls already defaults to open' describes the tab highlight only — strictly, Pulls.jsx empty-state sends omitted state and ListPRs with empty State returns BOTH (service.go:782), so the open-highlighted pulls tab may show both. Pre-existing, out of scope, but worth a follow-up issue.

MERGE RECOMMENDATION: ready to merge.

Review of PR #330 (fix/issue-323) — verified in scratch worktree /tmp/pr330 @ 6757752 (main worktree untouched, still clean on main). All #323 acceptance criteria hold: - Bare visit open-only + select reads open: query() sends state=open when ?state= absent (Issues.jsx:38-39 via resolveIssueState->issueListState), select binds RESOLVED value (Issues.jsx:96). PASS - Both-choice shareable: select writes ?state=all (Issues.jsx:99); refresh/deep-link resolves to both and omits the wire param. PASS - Deep links exact: ?state=closed/open honored verbatim; legacy empty ?state= still reads as both (issueState.js:21); end-to-end covered in issue-state.test.js. PASS - Milestone landings (?milestone=, #314) inherit open-only: no special-casing, called out as deliberate in code comment (Issues.jsx:32-37), PR body, and doc. PASS - Pagination unaffected: key is issues:{full}:{query JSON} (Issues.jsx:46) — open vs both are distinct windows; after cursor in key. #318 interplay safe: invalidateIssueLists prefix-invalidates issues:{full}:* so all windows reconcile. PASS - Headless tests for resolver present and green (6/6). Wire safety verified against server code: listIssues 400s on anything but open|closed and treats absent as both (internal/issues/http.go:376-382), so mapping all->"" (omitted via SDK qs() skip-empty, web/sdk/src/issues.js:90-97) means the endpoint never sees 'all' — no 400 path, no backend change needed. Unknown passthrough (?state=bogus -> 400 tray error) matches pre-change behavior. Doc contrast with milestones endpoint (accepts all natively, http.go:781-785) verified accurate. Scope/discipline: Pulls.jsx untouched + flagged; no new deps (package.json unchanged, law 1); no Go changes (laws 7/8 vacuous); doc decision appended same-change (law 12). Tests: node --test web/test/unit/issue-state.test.js 6/6 pass; full web/test/unit/*.test.js 612 pass / 0 fail (smoke.test.js cancelled — hangs on squatted :8080 in this sandbox, pre-existing environmental as disclosed in PR body); vite build clean (2.06s). No browser drive per task constraints (query/select binding verified headless + by reading). Nits (non-blocking, no push): doc hunk re-indents one adjacent continuation line ('slot for other surfaces.'); PR body phrase 'pulls already defaults to open' describes the tab highlight only — strictly, Pulls.jsx empty-state sends omitted state and ListPRs with empty State returns BOTH (service.go:782), so the open-highlighted pulls tab may show both. Pre-existing, out of scope, but worth a follow-up issue. MERGE RECOMMENDATION: ready to merge.
Author
Owner

Fixed by PR #330 (review clean; resolver + wire mapping + cache interplay verified; 612/612), merged. Closing.

Fixed by PR #330 (review clean; resolver + wire mapping + cache interplay verified; 612/612), merged. Closing.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
crueber/walhub#323
No description provided.