a73x

a226d673

Default web issue/patch lists to open, matching the nav badge

a73x   2026-08-09 16:50

Commit message
Default web issue/patch lists to open, matching the nav badge

The issues and patches list pages pulled in every closed and archived
item with no status filter, while the nav badge only ever counts open
ones — so the badge and the list it links to disagreed. Default both
lists to open items and add an explicit ?filter= query param (open,
closed, all — plus merged for patches) with a visible link-based
control in the template, so closed/archived items stay reachable
without being shown by default.

Merged patches get their own filter value rather than being folded
into "closed": merging is a successful, non-terminal-in-the-close-sense
outcome (the TUI dashboard already treats it this way when cycling
status filters), so conflating it with a rejected/abandoned closed
patch would be misleading.
src/server/http/repo/issues.rs
Old New
@@ -1,10 +1,43 @@
1 use std::sync::Arc; 1 use std::sync::Arc;
2 2
3 use axum::extract::{Path, State}; 3 use axum::extract::{Path, Query, State};
4 use axum::response::{IntoResponse, Response}; 4 use axum::response::{IntoResponse, Response};
5 5
6 use super::{collab_counts, internal_error, not_found, open_repo, AppState, CommentView}; 6 use super::{collab_counts, internal_error, not_found, open_repo, AppState, CommentView};
7 7
8 /// Which issues to include in the list view. Defaults to `Open` so the list
9 /// agrees with the nav badge, which only ever counts open issues. Closed and
10 /// archived issues stay reachable behind the `?filter=` query param.
11 #[derive(Debug, PartialEq, Clone, Copy)]
12 pub enum IssueListFilter {
13 Open,
14 Closed,
15 All,
16 }
17
18 impl IssueListFilter {
19 fn from_query(filter: Option<&str>) -> Self {
20 match filter {
21 Some("closed") => IssueListFilter::Closed,
22 Some("all") => IssueListFilter::All,
23 _ => IssueListFilter::Open,
24 }
25 }
26
27 fn as_str(self) -> &'static str {
28 match self {
29 IssueListFilter::Open => "open",
30 IssueListFilter::Closed => "closed",
31 IssueListFilter::All => "all",
32 }
33 }
34 }
35
36 #[derive(serde::Deserialize)]
37 pub struct IssuesQuery {
38 pub filter: Option<String>,
39 }
40
8 #[derive(Debug)] 41 #[derive(Debug)]
9 pub struct IssueListItem { 42 pub struct IssueListItem {
10 pub id: String, 43 pub id: String,
@@ -36,6 +69,7 @@ pub struct IssuesTemplate {
36 pub active_section: String, 69 pub active_section: String,
37 pub open_patches: usize, 70 pub open_patches: usize,
38 pub open_issues: usize, 71 pub open_issues: usize,
72 pub filter: &'static str,
39 pub issues: Vec<IssueListItem>, 73 pub issues: Vec<IssueListItem>,
40 } 74 }
41 75
@@ -50,7 +84,11 @@ pub struct IssueDetailTemplate {
50 pub issue: IssueDetailView, 84 pub issue: IssueDetailView,
51 } 85 }
52 86
53 pub async fn issues(Path(repo_name): Path<String>, State(state): State<Arc<AppState>>) -> Response { 87 pub async fn issues(
88 Path(repo_name): Path<String>,
89 Query(query): Query<IssuesQuery>,
90 State(state): State<Arc<AppState>>,
91 ) -> Response {
54 let (_entry, repo) = match open_repo(&state, &repo_name) { 92 let (_entry, repo) = match open_repo(&state, &repo_name) {
55 Ok(r) => r, 93 Ok(r) => r,
56 Err(resp) => return resp, 94 Err(resp) => return resp,
@@ -58,9 +96,28 @@ pub async fn issues(Path(repo_name): Path<String>, State(state): State<Arc<AppSt
58 96
59 let (open_patches, open_issues) = collab_counts(&repo); 97 let (open_patches, open_issues) = collab_counts(&repo);
60 98
61 let all_issues = git_collab::state::list_issues_with_archived(&repo).unwrap_or_default(); 99 let filter = IssueListFilter::from_query(query.filter.as_deref());
100
101 // "Open" is the common case and matches the badge count, so it's served
102 // from the cheaper non-archived listing. Closed/all need the archived
103 // namespace too, since closing an issue archives it.
104 let filtered_issues = match filter {
105 IssueListFilter::Open => git_collab::state::list_issues(&repo)
106 .unwrap_or_default()
107 .into_iter()
108 .filter(|i| i.status == git_collab::state::IssueStatus::Open)
109 .collect(),
110 IssueListFilter::Closed => git_collab::state::list_issues_with_archived(&repo)
111 .unwrap_or_default()
112 .into_iter()
113 .filter(|i| i.status == git_collab::state::IssueStatus::Closed)
114 .collect(),
115 IssueListFilter::All => {
116 git_collab::state::list_issues_with_archived(&repo).unwrap_or_default()
117 }
118 };
62 119
63 let issues = all_issues 120 let issues = filtered_issues
64 .into_iter() 121 .into_iter()
65 .map(|i| { 122 .map(|i| {
66 let id = i.id.clone(); 123 let id = i.id.clone();
@@ -83,6 +140,7 @@ pub async fn issues(Path(repo_name): Path<String>, State(state): State<Arc<AppSt
83 active_section: "issues".to_string(), 140 active_section: "issues".to_string(),
84 open_patches, 141 open_patches,
85 open_issues, 142 open_issues,
143 filter: filter.as_str(),
86 issues, 144 issues,
87 } 145 }
88 .into_response() 146 .into_response()
src/server/http/repo/patches.rs
Old New
@@ -1,10 +1,48 @@
1 use std::sync::Arc; 1 use std::sync::Arc;
2 2
3 use axum::extract::{Path, State}; 3 use axum::extract::{Path, Query, State};
4 use axum::response::{IntoResponse, Response}; 4 use axum::response::{IntoResponse, Response};
5 5
6 use super::{collab_counts, internal_error, not_found, open_repo, AppState, CommentView}; 6 use super::{collab_counts, internal_error, not_found, open_repo, AppState, CommentView};
7 7
8 /// Which patches to include in the list view. Defaults to `Open` so the list
9 /// agrees with the nav badge, which only ever counts open patches. Merged
10 /// isn't treated as a kind of "closed" here: a rejected/abandoned patch and
11 /// a landed one are different outcomes, so each gets its own filter value
12 /// rather than being folded into a single non-open bucket.
13 #[derive(Debug, PartialEq, Clone, Copy)]
14 pub enum PatchListFilter {
15 Open,
16 Closed,
17 Merged,
18 All,
19 }
20
21 impl PatchListFilter {
22 fn from_query(filter: Option<&str>) -> Self {
23 match filter {
24 Some("closed") => PatchListFilter::Closed,
25 Some("merged") => PatchListFilter::Merged,
26 Some("all") => PatchListFilter::All,
27 _ => PatchListFilter::Open,
28 }
29 }
30
31 fn as_str(self) -> &'static str {
32 match self {
33 PatchListFilter::Open => "open",
34 PatchListFilter::Closed => "closed",
35 PatchListFilter::Merged => "merged",
36 PatchListFilter::All => "all",
37 }
38 }
39 }
40
41 #[derive(serde::Deserialize)]
42 pub struct PatchesQuery {
43 pub filter: Option<String>,
44 }
45
8 #[derive(Debug)] 46 #[derive(Debug)]
9 pub struct PatchListItem { 47 pub struct PatchListItem {
10 pub id: String, 48 pub id: String,
@@ -24,6 +62,7 @@ pub struct PatchesTemplate {
24 pub active_section: String, 62 pub active_section: String,
25 pub open_patches: usize, 63 pub open_patches: usize,
26 pub open_issues: usize, 64 pub open_issues: usize,
65 pub filter: &'static str,
27 pub patches: Vec<PatchListItem>, 66 pub patches: Vec<PatchListItem>,
28 } 67 }
29 68
@@ -87,6 +126,7 @@ pub struct PatchDetailTemplate {
87 126
88 pub async fn patches( 127 pub async fn patches(
89 Path(repo_name): Path<String>, 128 Path(repo_name): Path<String>,
129 Query(query): Query<PatchesQuery>,
90 State(state): State<Arc<AppState>>, 130 State(state): State<Arc<AppState>>,
91 ) -> Response { 131 ) -> Response {
92 let (_entry, repo) = match open_repo(&state, &repo_name) { 132 let (_entry, repo) = match open_repo(&state, &repo_name) {
@@ -96,9 +136,34 @@ pub async fn patches(
96 136
97 let (open_patches, open_issues) = collab_counts(&repo); 137 let (open_patches, open_issues) = collab_counts(&repo);
98 138
99 let all_patches = git_collab::state::list_patches_with_archived(&repo).unwrap_or_default(); 139 let filter = PatchListFilter::from_query(query.filter.as_deref());
140
141 // "Open" is the common case and matches the badge count, so it's served
142 // from the cheaper non-archived listing. Closed/merged/all need the
143 // archived namespace too, since closing a patch archives it (merging
144 // does not — it's auto-detected from ref state on read).
145 let filtered_patches = match filter {
146 PatchListFilter::Open => git_collab::state::list_patches(&repo)
147 .unwrap_or_default()
148 .into_iter()
149 .filter(|p| p.status == git_collab::state::PatchStatus::Open)
150 .collect(),
151 PatchListFilter::Closed => git_collab::state::list_patches_with_archived(&repo)
152 .unwrap_or_default()
153 .into_iter()
154 .filter(|p| p.status == git_collab::state::PatchStatus::Closed)
155 .collect(),
156 PatchListFilter::Merged => git_collab::state::list_patches_with_archived(&repo)
157 .unwrap_or_default()
158 .into_iter()
159 .filter(|p| p.status == git_collab::state::PatchStatus::Merged)
160 .collect(),
161 PatchListFilter::All => {
162 git_collab::state::list_patches_with_archived(&repo).unwrap_or_default()
163 }
164 };
100 165
101 let patches = all_patches 166 let patches = filtered_patches
102 .into_iter() 167 .into_iter()
103 .map(|p| { 168 .map(|p| {
104 let id = p.id.clone(); 169 let id = p.id.clone();
@@ -121,6 +186,7 @@ pub async fn patches(
121 active_section: "patches".to_string(), 186 active_section: "patches".to_string(),
122 open_patches, 187 open_patches,
123 open_issues, 188 open_issues,
189 filter: filter.as_str(),
124 patches, 190 patches,
125 } 191 }
126 .into_response() 192 .into_response()
src/server/http/templates/base.html
Old New
@@ -27,6 +27,9 @@
27 .header .site { font-weight: bold; } 27 .header .site { font-weight: bold; }
28 .header nav { display: flex; gap: 12px; } 28 .header nav { display: flex; gap: 12px; }
29 .header nav a.active { font-weight: bold; } 29 .header nav a.active { font-weight: bold; }
30 .filter-bar { color: #666; margin-bottom: 12px; }
31 .filter-bar a { margin-left: 8px; }
32 .filter-bar a.active { font-weight: bold; }
30 .content { padding: 16px; max-width: 1500px; margin: 0 auto; } 33 .content { padding: 16px; max-width: 1500px; margin: 0 auto; }
31 .commit-message { padding: 12px; border: 1px solid #d0d7de; border-radius: 6px; white-space: pre-wrap; } 34 .commit-message { padding: 12px; border: 1px solid #d0d7de; border-radius: 6px; white-space: pre-wrap; }
32 .diff-file-block { border: 1px solid #d0d7de; border-radius: 6px; overflow: hidden; } 35 .diff-file-block { border: 1px solid #d0d7de; border-radius: 6px; overflow: hidden; }
src/server/http/templates/issues.html
Old New
@@ -4,6 +4,12 @@
4 4
5 {% block content %} 5 {% block content %}
6 <h2>Issues</h2> 6 <h2>Issues</h2>
7 <p class="filter-bar">
8 Showing:
9 <a href="/{{ repo_name }}/issues?filter=open"{% if filter == "open" %} class="active"{% endif %}>open</a>
10 <a href="/{{ repo_name }}/issues?filter=closed"{% if filter == "closed" %} class="active"{% endif %}>closed</a>
11 <a href="/{{ repo_name }}/issues?filter=all"{% if filter == "all" %} class="active"{% endif %}>all</a>
12 </p>
7 {% if issues.is_empty() %} 13 {% if issues.is_empty() %}
8 <p style="color: #666;">No issues.</p> 14 <p style="color: #666;">No issues.</p>
9 {% else %} 15 {% else %}
src/server/http/templates/patches.html
Old New
@@ -4,6 +4,13 @@
4 4
5 {% block content %} 5 {% block content %}
6 <h2>Patches</h2> 6 <h2>Patches</h2>
7 <p class="filter-bar">
8 Showing:
9 <a href="/{{ repo_name }}/patches?filter=open"{% if filter == "open" %} class="active"{% endif %}>open</a>
10 <a href="/{{ repo_name }}/patches?filter=closed"{% if filter == "closed" %} class="active"{% endif %}>closed</a>
11 <a href="/{{ repo_name }}/patches?filter=merged"{% if filter == "merged" %} class="active"{% endif %}>merged</a>
12 <a href="/{{ repo_name }}/patches?filter=all"{% if filter == "all" %} class="active"{% endif %}>all</a>
13 </p>
7 {% if patches.is_empty() %} 14 {% if patches.is_empty() %}
8 <p style="color: #666;">No patches.</p> 15 <p style="color: #666;">No patches.</p>
9 {% else %} 16 {% else %}
tests/common/mod.rs
Old New
@@ -524,6 +524,16 @@ impl TestRepo {
524 .to_string() 524 .to_string()
525 } 525 }
526 526
527 /// Close an issue by ID prefix.
528 pub fn issue_close(&self, id: &str) {
529 self.run_ok(&["issue", "close", id]);
530 }
531
532 /// Close a patch by ID prefix.
533 pub fn patch_close(&self, id: &str) {
534 self.run_ok(&["patch", "close", id]);
535 }
536
527 /// Create a patch from a new branch. Returns the 8-char short ID. 537 /// Create a patch from a new branch. Returns the 8-char short ID.
528 pub fn patch_create(&self, title: &str) -> String { 538 pub fn patch_create(&self, title: &str) -> String {
529 // Create a unique branch for this patch 539 // Create a unique branch for this patch
tests/server_behavior_test.rs
Old New
@@ -189,6 +189,114 @@ fn missing_readme_does_not_break_overview_page() {
189 assert!(overview.body.contains("Open Patches") || overview.body.contains("Recent Commits")); 189 assert!(overview.body.contains("Open Patches") || overview.body.contains("Recent Commits"));
190 } 190 }
191 191
192 /// The issue list defaults to open items only, so the count printed on the
193 /// page agrees with the nav badge (which only ever counts open issues).
194 /// Closed issues stay reachable behind an explicit `?filter=` query param
195 /// rather than disappearing outright.
196 #[test]
197 fn issues_list_defaults_to_open_and_offers_closed_filter() {
198 let harness = ServerHarness::new("behavior-issue-filter");
199 harness.push_head();
200
201 harness.work_repo().issue_open("Open issue");
202 let closed_id = harness.work_repo().issue_open("Closed issue");
203 harness.work_repo().issue_close(&closed_id);
204 harness.push_collab_refs();
205
206 let repo = harness.repo_name();
207
208 // Default view: open only, matching the "issues (1)" nav badge.
209 let default_page = harness.get_ok(&format!("/{repo}/issues"));
210 assert!(default_page.body.contains("Open issue"));
211 assert!(!default_page.body.contains("Closed issue"));
212 assert!(
213 default_page.body.contains("issues (1)"),
214 "badge should read issues (1):\n{}",
215 default_page.body
216 );
217
218 // Explicit filter reaches the closed issue.
219 let closed_page = harness.get_ok(&format!("/{repo}/issues?filter=closed"));
220 assert!(closed_page.body.contains("Closed issue"));
221 assert!(!closed_page.body.contains("Open issue"));
222
223 // "all" reaches both.
224 let all_page = harness.get_ok(&format!("/{repo}/issues?filter=all"));
225 assert!(all_page.body.contains("Open issue"));
226 assert!(all_page.body.contains("Closed issue"));
227 }
228
229 /// The patches list defaults to open items only, matching the nav badge.
230 /// Merged and closed patches are distinct states — merging isn't the same
231 /// kind of terminal state as an explicit close — so each gets its own
232 /// filter value, plus "all" to see everything.
233 #[test]
234 fn patches_list_defaults_to_open_and_offers_closed_and_merged_filters() {
235 let harness = ServerHarness::new("behavior-patch-filter");
236 harness.push_head();
237
238 harness.work_repo().patch_create("Open patch");
239
240 let closed_id = harness.work_repo().patch_create("Closed patch");
241 harness.work_repo().patch_close(&closed_id);
242
243 harness
244 .work_repo()
245 .git(&["checkout", "-b", "feature-merged"]);
246 harness.work_repo().commit_file(
247 "merged.txt",
248 "content for merged patch",
249 "add file for merged patch",
250 );
251 harness.work_repo().run_ok(&[
252 "patch",
253 "create",
254 "-t",
255 "Merged patch",
256 "-B",
257 "feature-merged",
258 ]);
259 harness.work_repo().git(&["checkout", "main"]);
260 harness.work_repo().git(&["merge", "feature-merged"]);
261
262 // Auto-merge detection resolves the patch's source branch on the server
263 // side too (`refs/heads/feature-merged`), so it needs to be pushed
264 // alongside the fast-forwarded main.
265 harness
266 .work_repo()
267 .git(&["push", "origin", "feature-merged"]);
268 harness.push_head();
269 harness.push_collab_refs();
270
271 let repo = harness.repo_name();
272
273 // Default view: open only, matching the "patches (1)" nav badge.
274 let default_page = harness.get_ok(&format!("/{repo}/patches"));
275 assert!(default_page.body.contains("Open patch"));
276 assert!(!default_page.body.contains("Closed patch"));
277 assert!(!default_page.body.contains("Merged patch"));
278 assert!(
279 default_page.body.contains("patches (1)"),
280 "badge should read patches (1):\n{}",
281 default_page.body
282 );
283
284 let closed_page = harness.get_ok(&format!("/{repo}/patches?filter=closed"));
285 assert!(closed_page.body.contains("Closed patch"));
286 assert!(!closed_page.body.contains("Open patch"));
287 assert!(!closed_page.body.contains("Merged patch"));
288
289 let merged_page = harness.get_ok(&format!("/{repo}/patches?filter=merged"));
290 assert!(merged_page.body.contains("Merged patch"));
291 assert!(!merged_page.body.contains("Open patch"));
292 assert!(!merged_page.body.contains("Closed patch"));
293
294 let all_page = harness.get_ok(&format!("/{repo}/patches?filter=all"));
295 assert!(all_page.body.contains("Open patch"));
296 assert!(all_page.body.contains("Closed patch"));
297 assert!(all_page.body.contains("Merged patch"));
298 }
299
192 /// Every `onchange="..."` attribute value in `body`. 300 /// Every `onchange="..."` attribute value in `body`.
193 /// 301 ///
194 /// An inline event-handler attribute is HTML-decoded *before* its contents are 302 /// An inline event-handler attribute is HTML-decoded *before* its contents are