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 <noreply@anthropic.com>
quotesdb
Elijah Voigt 3 months ago
parent e5cb1fb8e2
commit dbab0f466c

@ -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")

@ -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()
}

@ -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<String>,
},
/// 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<String>,
},
/// 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<String>,
},
/// 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<String>, 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<String>, 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<String>, 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<Ticket> = list_tickets(&root)
.await?
.into_iter()
.filter(|t| filter.matches(t))
.collect();
if json {
display::print_list_json(&tickets);

@ -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<Ticket> {
/// 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<MigrateReport> {
pub async fn migrate_tickets(
root: &Path,
dry_run: bool,
filter: &TicketFilter,
) -> Result<MigrateReport> {
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<MigrateReport
};
ticket.id = stem;
// Apply caller-supplied filter; skip non-matching tickets.
if !filter.matches(&ticket) {
report.skipped += 1;
continue;
}
let new_json = match serde_json::to_string_pretty(&ticket) {
Ok(s) => s,
Err(e) => {

@ -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);

@ -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"
);
}

Loading…
Cancel
Save