Pulls tab: 'open' highlight but empty-state ListPRs returns both #332
Labels
No labels
actions
bug
cli
duplicate
enhancement
fork
forum
git storage
help wanted
insights
invalid
issues
moderation
oidc
ownership transfer
packages
pr/merge protection rules
projects
pull requests
question
releases
sponsorships
tags
webhooks
wiki
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
crueber/walhub#332
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Follow-up flagged by the #323 review (PR #330 findings). The pulls tab highlight defaults to open, but empty-state
ListPRsreturns both open and closed (internal/pulls/service.go— verify exact line). Either the pulls list should resolve to open-only by default (matching #323 for issues) or the tab highlight should not claim 'open'. Frontend + backend need to agree; keep URL honest the same way #323 did (state=allrepresentation + doc decision in the pulls/UI docs).Fixed by #341 (branch fix/issue-332): pulls list now defaults to open-only, mirroring #323 — ?state=all is the explicit both-choice, backend untouched.
Review of PR #341 (fix/issue-332) — verified in scratch worktree, main untouched.
MIRROR FIDELITY (#323/PR #330): faithful. web/src/lib/pullState.js is a line-for-line mirror of web/src/lib/issueState.js (resolve/pullListState, PULL_STATE_BOTH/DEFAULT, legacy '' -> both, unknown passthrough -> server 400). pull-state.test.js mirrors issue-state.test.js exactly. Pulls.jsx query (Pulls.jsx:29) now sends state=open on a bare visit, matching Issues.jsx:39.
BACKEND AGREEMENT: verified, no backend change needed as claimed. internal/pulls/http.go:415-421 accepts only open|closed|absent; internal/pulls/service.go:782 skips the filter when State=='' so empty-state returns both by contract — exactly what the wire-omitted both-choice relies on. SDK web/sdk/src/pulls.js:89-96 qs() skips '', so 'all' never reaches the server. Pulls.jsx is the sole pulls.list consumer (only call site); no other Go caller depends on both-by-default (only the HTTP handler + tests pinning the contract).
CHECKLIST: state=all URL representation + wire omission OK; legacy ?state= reads as both OK; third 'all' tab consistent with Issues three-option select OK; tabs bind resolved value OK (one fix below); empty-title uses resolved state (Pulls.jsx:59) OK; doc decision docs/features/03_pull_requests.md:359 accurate; cache keys distinct per state (pulls:{full}:{queryJSON}) with wildcard invalidation pulls:{full}:* (collab.js) — no #318 interplay issue; deep links honored; no new deps (4 files only, no package.json/go.mod).
ONE FIX PUSHED (
2bcfe49): Pulls.jsx:77 closed tab bound raw search.state while open/all bound the resolved value — behaviorally identical today but inconsistent with the PR's own 'tabs bind the RESOLVED value' claim, so I made it resolvePullState(...)===closed. Re-tested after the fix.TESTS: go test -race ./internal/pulls/ PASS, coverage 97.7% (gate 95%). node --test pull-state + issue-state: 12/12 PASS. Full web suite: 3 unrelated files fail identically on main baseline (reaction-cache/tolerate-missing: solid-js not installed — no node_modules in this env; smoke: hangs) — pre-existing environmental, not from this PR. Vite build not runnable here (no node_modules, no network installs per task constraints); noted explicitly, no browser needed — risk nil (relative import + buttons only).
RECOMMENDATION: ready to merge.
Fixed by PR #341 (review clean + one tab-binding consistency fix by reviewer; #323 mirror faithful, frontend/backend agreement verified), merged. Closing.