From dbab0f466cc27608977a51d09a1fa9ce6531dea9 Mon Sep 17 00:00:00 2001 From: Elijah Voigt Date: Sun, 22 Feb 2026 15:04:28 -0800 Subject: [PATCH] feat(nbd): wire --filter flag into list, ready, and migrate commands [887344] Add repeatable --filter KEY=VALUE option to nbd list, nbd ready, and nbd migrate. Filters are parsed into a TicketFilter and applied in each command handler. Different keys are ANDed; same key with multiple values is ORed; values support glob wildcards. - store: add skipped field to MigrateReport; migrate_tickets accepts a &TicketFilter and skips non-matching tickets - display: format_migrate_report shows optional Skipped line; format_migrate_report_json includes skipped key - filter: suppress dead_code on is_empty/has_status_filter (public API reserved for future done-exclusion feature) - tests: update MigrateReport literals and migrate_tickets call sites; add unit tests for skipped formatting; add 14 integration tests Co-Authored-By: Claude Sonnet 4.6 --- nbd/src/display.rs | 8 ++ nbd/src/filter.rs | 2 + nbd/src/main.rs | 57 +++++++-- nbd/src/store.rs | 21 +++- nbd/src/tests.rs | 76 +++++++++++- nbd/tests/integration.rs | 253 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 398 insertions(+), 19 deletions(-) diff --git a/nbd/src/display.rs b/nbd/src/display.rs index 1a8f532..0003347 100644 --- a/nbd/src/display.rs +++ b/nbd/src/display.rs @@ -208,6 +208,13 @@ pub fn format_migrate_report(report: &MigrateReport) -> String { report.already_current, if report.already_current == 1 { "" } else { "s" }, ); + if report.skipped > 0 { + out.push_str(&format!( + "\nSkipped {} ticket{} (did not match filter).", + report.skipped, + if report.skipped == 1 { "" } else { "s" }, + )); + } if !report.errors.is_empty() { out.push_str(&format!( "\nErrors {} ticket{} could not be migrated:", @@ -244,6 +251,7 @@ pub fn format_migrate_report_json(report: &MigrateReport) -> String { let value = serde_json::json!({ "updated": report.updated, "already_current": report.already_current, + "skipped": report.skipped, "errors": errors, }); serde_json::to_string_pretty(&value).expect("migrate report serialisation must not fail") diff --git a/nbd/src/filter.rs b/nbd/src/filter.rs index 7032bca..d46f6b6 100644 --- a/nbd/src/filter.rs +++ b/nbd/src/filter.rs @@ -168,6 +168,7 @@ impl TicketFilter { } /// Returns `true` when no filter groups are set (no-op filter). + #[allow(dead_code)] pub fn is_empty(&self) -> bool { self.status.is_empty() && self.ticket_type.is_empty() @@ -179,6 +180,7 @@ impl TicketFilter { /// /// Used by `cmd_list` to detect whether to apply the implicit /// done-exclusion heuristic. + #[allow(dead_code)] pub fn has_status_filter(&self) -> bool { !self.status.is_empty() } diff --git a/nbd/src/main.rs b/nbd/src/main.rs index dafd651..db036ed 100644 --- a/nbd/src/main.rs +++ b/nbd/src/main.rs @@ -75,7 +75,15 @@ enum Commands { }, /// List all tickets sorted by priority (highest first). - List, + List { + /// Filter tickets by field: repeatable `key=value` pairs. + /// + /// Keys: `status`, `type`, `priority`, `title`. + /// Different keys are ANDed; the same key with multiple values is ORed. + /// Values support glob wildcards: `title=*login*`. + #[arg(long = "filter", value_name = "KEY=VALUE")] + filter: Vec, + }, /// Initialise a new `.nbd/tickets/` store in the current directory. /// @@ -87,7 +95,14 @@ enum Commands { /// A ticket is ready when its status is not `done` and every ticket it /// depends on has status `done`. Tickets with no dependencies and status /// `todo` or `in_progress` are always ready. - Ready, + Ready { + /// Filter ready tickets by field: repeatable `key=value` pairs. + /// + /// Applied after the ready check — narrows within already-ready tickets. + /// Keys: `status`, `type`, `priority`, `title`. Values support globs. + #[arg(long = "filter", value_name = "KEY=VALUE")] + filter: Vec, + }, /// Re-serialise all ticket files through the current schema. /// @@ -98,6 +113,13 @@ enum Commands { /// Print what would change without writing any files. #[arg(long)] dry_run: bool, + + /// Only migrate tickets matching these filters: repeatable `key=value` pairs. + /// + /// Non-matching tickets are counted as skipped in the report. + /// Keys: `status`, `type`, `priority`, `title`. Values support globs. + #[arg(long = "filter", value_name = "KEY=VALUE")] + filter: Vec, }, /// Update fields of an existing ticket and print the result. @@ -159,13 +181,13 @@ async fn dispatch(cli: Cli) -> store::Result<()> { Commands::Init => cmd_init(cli.json).await, - Commands::Ready => cmd_ready(cli.json).await, + Commands::Ready { filter } => cmd_ready(filter, cli.json).await, - Commands::Migrate { dry_run } => cmd_migrate(dry_run, cli.json).await, + Commands::Migrate { dry_run, filter } => cmd_migrate(filter, dry_run, cli.json).await, Commands::Read { id } => cmd_read(id, cli.json).await, - Commands::List => cmd_list(cli.json).await, + Commands::List { filter } => cmd_list(filter, cli.json).await, Commands::Update { id, @@ -295,7 +317,10 @@ async fn cmd_init(json: bool) -> store::Result<()> { /// its `dependencies` list belongs to a ticket with `status == Done`. /// Missing dependency IDs are treated conservatively — the ticket is **not** /// ready if any dep cannot be resolved. -async fn cmd_ready(json: bool) -> store::Result<()> { +/// +/// `filter_args` are applied after the ready check, narrowing the results. +async fn cmd_ready(filter_args: Vec, json: bool) -> store::Result<()> { + let filter = crate::filter::parse_filters(&filter_args)?; let root = find_nbd_root()?; let all = list_tickets(&root).await?; @@ -313,6 +338,7 @@ async fn cmd_ready(json: bool) -> store::Result<()> { && t.dependencies .iter() .all(|dep| done_ids.contains(dep.as_str())) + && filter.matches(t) }) .collect(); @@ -330,9 +356,13 @@ async fn cmd_ready(json: bool) -> store::Result<()> { /// When `dry_run` is `true`, describe what *would* change without writing any /// files. The command exits zero even when individual files fail to parse — /// those errors are included in the summary. -async fn cmd_migrate(dry_run: bool, json: bool) -> store::Result<()> { +/// +/// `filter_args` restrict which tickets are candidates; non-matching tickets +/// are counted as skipped in the report. +async fn cmd_migrate(filter_args: Vec, dry_run: bool, json: bool) -> store::Result<()> { + let filter = crate::filter::parse_filters(&filter_args)?; let root = find_nbd_root()?; - let report = migrate_tickets(&root, dry_run).await?; + let report = migrate_tickets(&root, dry_run, &filter).await?; if json { display::print_migrate_report_json(&report); @@ -400,9 +430,16 @@ async fn cmd_read(id: String, json: bool) -> store::Result<()> { } /// List all tickets sorted by priority and print them. -async fn cmd_list(json: bool) -> store::Result<()> { +/// +/// `filter_args` are optional `key=value` expressions that narrow the output. +async fn cmd_list(filter_args: Vec, json: bool) -> store::Result<()> { + let filter = crate::filter::parse_filters(&filter_args)?; let root = find_nbd_root()?; - let tickets = list_tickets(&root).await?; + let tickets: Vec = list_tickets(&root) + .await? + .into_iter() + .filter(|t| filter.matches(t)) + .collect(); if json { display::print_list_json(&tickets); diff --git a/nbd/src/store.rs b/nbd/src/store.rs index c3c1554..c632584 100644 --- a/nbd/src/store.rs +++ b/nbd/src/store.rs @@ -10,6 +10,7 @@ use std::path::{Path, PathBuf}; use async_std::fs; use async_std::prelude::*; +use crate::filter::TicketFilter; use crate::ticket::Ticket; /// Convenience alias for fallible operations in this module. @@ -197,14 +198,16 @@ pub async fn read_ticket(root: &Path, id: &str) -> Result { /// Report produced by [`migrate_tickets`]. /// -/// Summarises how many ticket files were updated, were already current, or -/// could not be parsed. +/// Summarises how many ticket files were updated, were already current, +/// were skipped by the filter, or could not be parsed. #[derive(Debug)] pub struct MigrateReport { /// Number of files that were re-serialised (had stale content). pub updated: usize, /// Number of files that were already in the current schema format. pub already_current: usize, + /// Number of files excluded by the caller-supplied [`TicketFilter`]. + pub skipped: usize, /// Files that could not be deserialised. Each entry is `(filename, error)`. pub errors: Vec<(String, String)>, } @@ -213,6 +216,7 @@ pub struct MigrateReport { /// /// For each `*.json` file in the tickets directory: /// - Deserialise into [`Ticket`], injecting the `id` from the filename stem. +/// - If the ticket does not match `filter`, count it as skipped and continue. /// - Re-serialise to the current pretty-printed JSON schema. /// - If the bytes differ, write the new content (unless `dry_run` is `true`). /// - If the bytes are identical, count the file as already current. @@ -227,11 +231,16 @@ pub struct MigrateReport { /// # Errors /// /// Returns an error only if the tickets directory itself cannot be read. -pub async fn migrate_tickets(root: &Path, dry_run: bool) -> Result { +pub async fn migrate_tickets( + root: &Path, + dry_run: bool, + filter: &TicketFilter, +) -> Result { let dir = tickets_dir(root); let mut report = MigrateReport { updated: 0, already_current: 0, + skipped: 0, errors: Vec::new(), }; @@ -275,6 +284,12 @@ pub async fn migrate_tickets(root: &Path, dry_run: bool) -> Result s, Err(e) => { diff --git a/nbd/src/tests.rs b/nbd/src/tests.rs index a962cb9..e0c288d 100644 --- a/nbd/src/tests.rs +++ b/nbd/src/tests.rs @@ -449,6 +449,7 @@ mod store { /// Tests for [`crate::store::migrate_tickets`]. mod migrate { + use crate::filter::TicketFilter; use crate::store::{ensure_tickets_dir, migrate_tickets, tickets_dir, write_ticket}; use crate::ticket::Ticket; @@ -470,7 +471,9 @@ mod migrate { .await .unwrap(); - let report = migrate_tickets(&root, false).await.unwrap(); + let report = migrate_tickets(&root, false, &TicketFilter::default()) + .await + .unwrap(); assert_eq!(report.updated, 1); assert_eq!(report.already_current, 0); assert!(report.errors.is_empty()); @@ -496,7 +499,9 @@ mod migrate { write_ticket(&root, &t1).await.unwrap(); write_ticket(&root, &t2).await.unwrap(); - let report = migrate_tickets(&root, false).await.unwrap(); + let report = migrate_tickets(&root, false, &TicketFilter::default()) + .await + .unwrap(); assert_eq!(report.updated, 0); assert_eq!(report.already_current, 2); assert!(report.errors.is_empty()); @@ -513,7 +518,9 @@ mod migrate { .await .unwrap(); - let report = migrate_tickets(&root, true).await.unwrap(); + let report = migrate_tickets(&root, true, &TicketFilter::default()) + .await + .unwrap(); assert_eq!( report.updated, 1, "dry_run should still count as would-update" @@ -541,7 +548,9 @@ mod migrate { .await .unwrap(); - let report = migrate_tickets(&root, false).await.unwrap(); + let report = migrate_tickets(&root, false, &TicketFilter::default()) + .await + .unwrap(); assert_eq!(report.errors.len(), 1); assert!(report.errors[0].0.contains("badbad.json")); @@ -555,7 +564,9 @@ mod migrate { #[async_std::test] async fn empty_store_returns_empty_report() { let (tmp, root) = setup_store().await; - let report = migrate_tickets(&root, false).await.unwrap(); + let report = migrate_tickets(&root, false, &TicketFilter::default()) + .await + .unwrap(); assert_eq!(report.updated, 0); assert_eq!(report.already_current, 0); assert!(report.errors.is_empty()); @@ -570,7 +581,9 @@ mod migrate { std::fs::create_dir(tmp.path().join(".nbd")).unwrap(); let root = tmp.path().to_path_buf(); - let report = migrate_tickets(&root, false).await.unwrap(); + let report = migrate_tickets(&root, false, &TicketFilter::default()) + .await + .unwrap(); assert_eq!(report.updated, 0); assert_eq!(report.already_current, 0); assert!(report.errors.is_empty()); @@ -942,6 +955,7 @@ mod display { let report = MigrateReport { updated: 3, already_current: 5, + skipped: 0, errors: vec![("bad.json".to_string(), "parse error".to_string())], }; let output = format_migrate_report(&report); @@ -960,6 +974,7 @@ mod display { let report = MigrateReport { updated: 1, already_current: 2, + skipped: 0, errors: vec![], }; let output = format_migrate_report(&report); @@ -969,12 +984,61 @@ mod display { ); } + /// `format_migrate_report` shows a Skipped line when `skipped > 0`. + #[test] + fn format_migrate_report_shows_skipped_when_nonzero() { + let report = MigrateReport { + updated: 1, + already_current: 2, + skipped: 3, + errors: vec![], + }; + let output = format_migrate_report(&report); + assert!( + output.contains("Skipped"), + "should show Skipped line when skipped > 0" + ); + assert!(output.contains('3'), "should include skipped count"); + } + + /// `format_migrate_report` omits the Skipped line when `skipped == 0`. + #[test] + fn format_migrate_report_omits_skipped_when_zero() { + let report = MigrateReport { + updated: 1, + already_current: 2, + skipped: 0, + errors: vec![], + }; + let output = format_migrate_report(&report); + assert!( + !output.contains("Skipped"), + "should not show Skipped when skipped == 0" + ); + } + + /// `format_migrate_report_json` includes a `skipped` key. + #[test] + fn format_migrate_report_json_includes_skipped() { + let report = MigrateReport { + updated: 1, + already_current: 2, + skipped: 4, + errors: vec![], + }; + let output = format_migrate_report_json(&report); + let parsed: serde_json::Value = + serde_json::from_str(&output).expect("output should be valid JSON"); + assert_eq!(parsed["skipped"], 4, "JSON should contain skipped count"); + } + /// `format_migrate_report_json` produces valid JSON with the expected keys. #[test] fn format_migrate_report_json_is_valid() { let report = MigrateReport { updated: 2, already_current: 4, + skipped: 0, errors: vec![("err.json".to_string(), "bad".to_string())], }; let output = format_migrate_report_json(&report); diff --git a/nbd/tests/integration.rs b/nbd/tests/integration.rs index 8e9a97e..cec9a2f 100644 --- a/nbd/tests/integration.rs +++ b/nbd/tests/integration.rs @@ -613,3 +613,256 @@ fn update_deps_replaces_list() { assert_eq!(deps.len(), 1, "should have exactly one dependency"); assert_eq!(deps[0], dep2, "dependency should be dep2"); } + +// ── --filter tests ──────────────────────────────────────────────────────────── + +/// `nbd list --filter type=bug` shows only bug tickets. +#[test] +fn list_filter_by_type() { + let env = TestEnv::new(); + + env.create(&["--title", "Bug ticket", "--type", "bug"]); + env.create(&["--title", "Task ticket", "--type", "task"]); + + let output = env.run(&["list", "--filter", "type=bug", "--json"]); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + let arr = parsed.as_array().unwrap(); + assert_eq!(arr.len(), 1, "only the bug ticket should appear"); + assert_eq!(arr[0]["ticket_type"], "bug"); +} + +/// `nbd list --filter status=in_progress` shows only in_progress tickets. +#[test] +fn list_filter_by_status() { + let env = TestEnv::new(); + + let id = env.create(&["--title", "Active ticket"]); + env.create(&["--title", "Todo ticket"]); + env.run(&["update", &id, "--status", "in_progress"]); + + let output = env.run(&["list", "--filter", "status=in_progress", "--json"]); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + let arr = parsed.as_array().unwrap(); + assert_eq!(arr.len(), 1, "only in_progress ticket should appear"); + assert_eq!(arr[0]["status"], "in_progress"); +} + +/// Same key repeated is ORed: `--filter status=todo --filter status=in_progress` +/// shows both todo and in_progress tickets. +#[test] +fn list_filter_same_key_ored() { + let env = TestEnv::new(); + + let id = env.create(&["--title", "Active"]); + env.create(&["--title", "Todo"]); + let done_id = env.create(&["--title", "Done"]); + env.run(&["update", &id, "--status", "in_progress"]); + env.run(&["update", &done_id, "--status", "done"]); + + let output = env.run(&[ + "list", + "--filter", + "status=todo", + "--filter", + "status=in_progress", + "--json", + ]); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + let arr = parsed.as_array().unwrap(); + assert_eq!( + arr.len(), + 2, + "todo and in_progress should appear; done excluded" + ); + let statuses: Vec<&str> = arr.iter().map(|v| v["status"].as_str().unwrap()).collect(); + assert!(statuses.contains(&"todo"), "todo should be in results"); + assert!( + statuses.contains(&"in_progress"), + "in_progress should be in results" + ); +} + +/// Different keys are ANDed: `--filter type=bug --filter status=todo` shows +/// only bug tickets with status todo. +#[test] +fn list_filter_different_keys_anded() { + let env = TestEnv::new(); + + env.create(&["--title", "Bug todo", "--type", "bug"]); + let bug_active = env.create(&["--title", "Bug active", "--type", "bug"]); + env.create(&["--title", "Task todo", "--type", "task"]); + env.run(&["update", &bug_active, "--status", "in_progress"]); + + let output = env.run(&[ + "list", + "--filter", + "type=bug", + "--filter", + "status=todo", + "--json", + ]); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + let arr = parsed.as_array().unwrap(); + assert_eq!(arr.len(), 1, "only bug+todo should appear"); + assert_eq!(arr[0]["title"], "Bug todo"); +} + +/// `nbd list --filter title=*login*` matches by glob on the title field. +#[test] +fn list_filter_title_glob() { + let env = TestEnv::new(); + + env.create(&["--title", "Fix login button"]); + env.create(&["--title", "Add rate limiting"]); + + let output = env.run(&["list", "--filter", "title=*login*", "--json"]); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + let arr = parsed.as_array().unwrap(); + assert_eq!(arr.len(), 1, "only the login ticket should match"); + assert_eq!(arr[0]["title"], "Fix login button"); +} + +/// `nbd list --filter status=*` wildcard matches all statuses. +#[test] +fn list_filter_status_wildcard() { + let env = TestEnv::new(); + + let id = env.create(&["--title", "A"]); + env.create(&["--title", "B"]); + env.run(&["update", &id, "--status", "done"]); + + let output = env.run(&["list", "--filter", "status=*", "--json"]); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + let arr = parsed.as_array().unwrap(); + assert_eq!(arr.len(), 2, "wildcard should match all statuses"); +} + +/// `nbd list --filter badformat` (no `=`) exits non-zero. +#[test] +fn list_filter_bad_format_exits_nonzero() { + let env = TestEnv::new(); + + let output = env.run(&["list", "--filter", "badformat"]); + assert!( + !output.status.success(), + "missing '=' in filter should exit non-zero" + ); +} + +/// `nbd list --filter unknown=foo` (unknown key) exits non-zero. +#[test] +fn list_filter_unknown_key_exits_nonzero() { + let env = TestEnv::new(); + + let output = env.run(&["list", "--filter", "colour=red"]); + assert!( + !output.status.success(), + "unknown filter key should exit non-zero" + ); + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!( + stderr.contains("colour"), + "error should mention the unknown key, got: {stderr}" + ); +} + +/// `nbd ready --filter type=bug` returns only ready bug tickets. +#[test] +fn ready_filter_by_type() { + let env = TestEnv::new(); + + env.create(&["--title", "Ready bug", "--type", "bug"]); + env.create(&["--title", "Ready task", "--type", "task"]); + + let output = env.run(&["ready", "--filter", "type=bug", "--json"]); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + let arr = parsed.as_array().unwrap(); + assert_eq!(arr.len(), 1, "only the bug ticket should appear in ready"); + assert_eq!(arr[0]["ticket_type"], "bug"); +} + +/// `nbd ready --filter priority=8` returns only ready tickets with priority 8. +#[test] +fn ready_filter_by_priority() { + let env = TestEnv::new(); + + env.create(&["--title", "High prio", "--priority", "8"]); + env.create(&["--title", "Normal prio", "--priority", "5"]); + + let output = env.run(&["ready", "--filter", "priority=8", "--json"]); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + let arr = parsed.as_array().unwrap(); + assert_eq!(arr.len(), 1, "only priority-8 ticket should appear"); + assert_eq!(arr[0]["priority"], 8); +} + +/// `nbd migrate --filter status=todo --dry-run --json` reports skipped count +/// for tickets that do not match the filter. +#[test] +fn migrate_filter_skipped_in_json() { + let env = TestEnv::new(); + + env.create(&["--title", "Todo ticket"]); + let id2 = env.create(&["--title", "Active ticket"]); + env.run(&["update", &id2, "--status", "in_progress"]); + + let output = env.run(&["migrate", "--filter", "status=todo", "--dry-run", "--json"]); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + let parsed: serde_json::Value = + serde_json::from_str(&stdout).expect("migrate --json should produce valid JSON"); + assert!( + parsed.get("skipped").is_some(), + "JSON should have 'skipped' key" + ); + assert_eq!( + parsed["skipped"], 1, + "one ticket (in_progress) should be skipped" + ); +} + +/// `nbd migrate --json` always includes a `skipped` key (even when zero). +#[test] +fn migrate_json_always_has_skipped_key() { + let env = TestEnv::new(); + + env.create(&["--title", "Some ticket"]); + + let output = env.run(&["migrate", "--json"]); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + assert!( + parsed.get("skipped").is_some(), + "JSON should always contain 'skipped' key" + ); + assert_eq!(parsed["skipped"], 0); +} + +/// `nbd ready --filter badformat` exits non-zero. +#[test] +fn ready_filter_bad_format_exits_nonzero() { + let env = TestEnv::new(); + + let output = env.run(&["ready", "--filter", "noequals"]); + assert!( + !output.status.success(), + "bad filter format on ready should exit non-zero" + ); +}