From 946d6ac6755a78a0b1e7bb41b3efdb824c9e9945 Mon Sep 17 00:00:00 2001 From: Slack Coder Date: Wed, 7 Dec 2022 16:45:25 -0500 Subject: [PATCH] cli: assign and unassign issues Allow assigning an issue to one or more people via a new subcommand. rad assign To unassign: rad unassign To support testing and documentation, create an example for maintaining issues for a project. Signed-off-by: Slack Coder --- radicle-cli/examples/rad-issue.md | 34 +++++++ radicle-cli/src/commands.rs | 4 + radicle-cli/src/commands/assign.rs | 96 ++++++++++++++++++++ radicle-cli/src/commands/issue.rs | 12 ++- radicle-cli/src/commands/unassign.rs | 96 ++++++++++++++++++++ radicle-cli/src/main.rs | 16 ++++ radicle-cli/tests/commands.rs | 20 ++++- radicle/src/cob/issue.rs | 129 +++++++++++++++++++++++++-- 8 files changed, 399 insertions(+), 8 deletions(-) create mode 100644 radicle-cli/examples/rad-issue.md create mode 100644 radicle-cli/src/commands/assign.rs create mode 100644 radicle-cli/src/commands/unassign.rs diff --git a/radicle-cli/examples/rad-issue.md b/radicle-cli/examples/rad-issue.md new file mode 100644 index 00000000..06e1afa6 --- /dev/null +++ b/radicle-cli/examples/rad-issue.md @@ -0,0 +1,34 @@ +Project 'todo' items are called 'issue's. They can be inspected and modified +using the 'issue' subcommand. + +Let's say the new car you are designing with your peers has a problem with its flux capacitor. + +``` +$ rad issue new --title "flux capacitor underpowered" --description "Flux capacitor power requirements exceed current supply" +``` + +The issue is now listed under our project. + +``` +$ rad issue list +de81d97d7fe07a80bfb339200c6af862d4526b6a "flux capacitor underpowered" +``` + +Great! Now we've documented the issue for ourselves and others. + +Just like with other project management systems, the issue can be assigned to +others to work on. This is to ensure work is not duplicated. + +Let's assign ourselves to this one. + +``` +$ rad assign de81d97d7fe07a80bfb339200c6af862d4526b6a z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi +$ rad issue list +de81d97d7fe07a80bfb339200c6af862d4526b6a "flux capacitor underpowered" z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi +``` + +Note: this can always be undone with the 'unassign' subcommand. + +``` +$ rad unassign de81d97d7fe07a80bfb339200c6af862d4526b6a z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi +``` diff --git a/radicle-cli/src/commands.rs b/radicle-cli/src/commands.rs index bdeb5d52..405a5df9 100644 --- a/radicle-cli/src/commands.rs +++ b/radicle-cli/src/commands.rs @@ -1,3 +1,5 @@ +#[path = "commands/assign.rs"] +pub mod rad_assign; #[path = "commands/auth.rs"] pub mod rad_auth; #[path = "commands/checkout.rs"] @@ -32,5 +34,7 @@ pub mod rad_rm; pub mod rad_self; #[path = "commands/track.rs"] pub mod rad_track; +#[path = "commands/unassign.rs"] +pub mod rad_unassign; #[path = "commands/untrack.rs"] pub mod rad_untrack; diff --git a/radicle-cli/src/commands/assign.rs b/radicle-cli/src/commands/assign.rs new file mode 100644 index 00000000..f0a698fb --- /dev/null +++ b/radicle-cli/src/commands/assign.rs @@ -0,0 +1,96 @@ +use std::ffi::OsString; +use std::str::FromStr; + +use anyhow::anyhow; + +use crate::terminal as term; +use crate::terminal::args; +use radicle::cob; +use radicle::cob::issue; +use radicle::storage::WriteStorage; + +pub const HELP: args::Help = args::Help { + name: "assign", + description: "assign an issue", + version: env!("CARGO_PKG_VERSION"), + usage: r#" +Usage + + rad assign + +Options + + --help Print help +"#, +}; + +#[derive(Debug)] +pub struct Options { + pub id: issue::IssueId, + pub peer: cob::ActorId, +} + +impl args::Args for Options { + fn from_args(args: Vec) -> anyhow::Result<(Self, Vec)> { + use lexopt::prelude::*; + + let mut parser = lexopt::Parser::from_args(args); + let mut id: Option = None; + let mut peer: Option = None; + + while let Some(arg) = parser.next()? { + match arg { + Long("help") => { + return Err(args::Error::Help.into()); + } + Value(ref val) => { + if id.is_none() { + let val = val.to_string_lossy(); + let Ok(val) = issue::IssueId::from_str(&val) else { + return Err(anyhow!("invalid issue ID '{}'", val)); + }; + + id = Some(val); + } else if peer.is_none() { + let val = val.to_string_lossy(); + let Ok(val) = cob::ActorId::from_str(&val) else { + return Err(anyhow!("invalid peer ID '{}'", val)); + }; + + peer = Some(val); + } else { + return Err(anyhow!(arg.unexpected())); + } + } + _ => { + return Err(anyhow!(arg.unexpected())); + } + } + } + + Ok(( + Options { + id: id.unwrap(), + peer: peer.unwrap(), + }, + vec![], + )) + } +} + +pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { + let profile = ctx.profile()?; + let signer = term::signer(&profile)?; + let storage = &profile.storage; + let (_, id) = radicle::rad::cwd()?; + let repo = storage.repository(id)?; + let mut issues = issue::Issues::open(*signer.public_key(), &repo)?; + + let mut issue = issues.get_mut(&options.id).map_err(|err| match err { + cob::store::Error::NotFound(_, _) => anyhow!("issue not found '{}'", options.id), + _ => err.into(), + })?; + issue.assign(vec![options.peer], &signer)?; + + Ok(()) +} diff --git a/radicle-cli/src/commands/issue.rs b/radicle-cli/src/commands/issue.rs index 4bc90368..634f03fd 100644 --- a/radicle-cli/src/commands/issue.rs +++ b/radicle-cli/src/commands/issue.rs @@ -235,7 +235,17 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { Operation::List => { for result in issues.all()? { let (id, issue, _) = result?; - println!("{} {}", id, issue.title()); + + let assigned: String = issue + .assigned() + .map(|p| p.to_string()) + .collect::>() + .join(", "); + if assigned.is_empty() { + println!("{} \"{}\"", id, issue.title().escape_default()); + } else { + println!("{} {:?} {}", id, issue.title().escape_default(), &assigned,); + } } } Operation::Delete { id } => { diff --git a/radicle-cli/src/commands/unassign.rs b/radicle-cli/src/commands/unassign.rs new file mode 100644 index 00000000..e5f10b01 --- /dev/null +++ b/radicle-cli/src/commands/unassign.rs @@ -0,0 +1,96 @@ +use std::ffi::OsString; +use std::str::FromStr; + +use anyhow::anyhow; + +use crate::terminal as term; +use crate::terminal::args::{Args, Error, Help}; +use radicle::cob; +use radicle::cob::issue; +use radicle::storage::WriteStorage; + +pub const HELP: Help = Help { + name: "unassign", + description: "unassign an issue", + version: env!("CARGO_PKG_VERSION"), + usage: r#" +Usage + + rad unassign + +Options + + --help Print help +"#, +}; + +#[derive(Debug)] +pub struct Options { + pub id: issue::IssueId, + pub peer: cob::ActorId, +} + +impl Args for Options { + fn from_args(args: Vec) -> anyhow::Result<(Self, Vec)> { + use lexopt::prelude::*; + + let mut parser = lexopt::Parser::from_args(args); + let mut id: Option = None; + let mut peer: Option = None; + + while let Some(arg) = parser.next()? { + match arg { + Long("help") => { + return Err(Error::Help.into()); + } + Value(ref val) => { + if id.is_none() { + let val = val.to_string_lossy(); + let Ok(val) = issue::IssueId::from_str(&val) else { + return Err(anyhow!("invalid issue ID '{}'", val)); + }; + + id = Some(val); + } else if peer.is_none() { + let val = val.to_string_lossy(); + let Ok(val) = cob::ActorId::from_str(&val) else { + return Err(anyhow!("invalid peer ID '{}'", val)); + }; + + peer = Some(val); + } else { + return Err(anyhow!(arg.unexpected())); + } + } + _ => { + return Err(anyhow!(arg.unexpected())); + } + } + } + + Ok(( + Options { + id: id.unwrap(), + peer: peer.unwrap(), + }, + vec![], + )) + } +} + +pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { + let profile = ctx.profile()?; + let signer = term::signer(&profile)?; + let storage = &profile.storage; + let (_, id) = radicle::rad::cwd()?; + let repo = storage.repository(id)?; + let mut issues = issue::Issues::open(*signer.public_key(), &repo)?; + + let mut issue = issues.get_mut(&options.id).map_err(|err| match err { + cob::store::Error::NotFound(_, _) => anyhow!("issue '{}' not found", options.id), + _ => err.into(), + })?; + issue.unassign(vec![options.peer], &signer)?; + + Ok(()) +} diff --git a/radicle-cli/src/main.rs b/radicle-cli/src/main.rs index 6d357f08..b4720eb0 100644 --- a/radicle-cli/src/main.rs +++ b/radicle-cli/src/main.rs @@ -102,6 +102,14 @@ fn run(command: Command) -> Result<(), Option> { fn run_other(exe: &str, args: &[OsString]) -> Result<(), Option> { match exe { + "assign" => { + term::run_command_args::( + rad_assign::HELP, + "Assign", + rad_assign::run, + args.to_vec(), + ); + } "auth" => { term::run_command_args::( rad_auth::HELP, @@ -238,6 +246,14 @@ fn run_other(exe: &str, args: &[OsString]) -> Result<(), Option> args.to_vec(), ); } + "unassign" => { + term::run_command_args::( + rad_unassign::HELP, + "Unassign", + rad_unassign::run, + args.to_vec(), + ); + } "untrack" => { term::run_command_args::( rad_untrack::HELP, diff --git a/radicle-cli/tests/commands.rs b/radicle-cli/tests/commands.rs index 0f6ff127..d001b46d 100644 --- a/radicle-cli/tests/commands.rs +++ b/radicle-cli/tests/commands.rs @@ -10,7 +10,7 @@ use framework::TestFormula; /// Run a CLI test file. fn test( path: impl AsRef, - profile: Option, + profile: Option<&Profile>, ) -> Result<(), Box> { let base = Path::new(env!("CARGO_MANIFEST_DIR")); let tmp = tempfile::tempdir().unwrap(); @@ -43,6 +43,22 @@ fn rad_auth() { test("examples/rad-auth.md", None).unwrap(); } +#[test] +fn rad_issue() { + let home = tempfile::tempdir().unwrap(); + let working = tempfile::tempdir().unwrap(); + let profile = profile(home.path()); + + // Setup a test repository. + fixtures::repository(working.path()); + // Navigate to repository. + env::set_current_dir(working.path()).unwrap(); + env::set_var(radicle_cob::git::RAD_COMMIT_TIME, "1671125284"); + + test("examples/rad-init.md", Some(&profile)).unwrap(); + test("examples/rad-issue.md", Some(&profile)).unwrap(); +} + #[test] fn rad_init() { let home = tempfile::tempdir().unwrap(); @@ -54,5 +70,5 @@ fn rad_init() { // Navigate to repository. env::set_current_dir(working.path()).unwrap(); - test("examples/rad-init.md", Some(profile)).unwrap(); + test("examples/rad-init.md", Some(&profile)).unwrap(); } diff --git a/radicle/src/cob/issue.rs b/radicle/src/cob/issue.rs index ea54bb9d..d988817e 100644 --- a/radicle/src/cob/issue.rs +++ b/radicle/src/cob/issue.rs @@ -13,7 +13,7 @@ use crate::cob::common::{Author, Reaction, Tag}; use crate::cob::store::Transaction; use crate::cob::thread; use crate::cob::thread::{CommentId, Thread}; -use crate::cob::{store, ObjectId, OpId, TypeName}; +use crate::cob::{store, ActorId, ObjectId, OpId, TypeName}; use crate::crypto::{PublicKey, Signer}; use crate::storage::git as storage; @@ -80,6 +80,7 @@ impl State { /// Issue state. Accumulates [`Action`]. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Issue { + assignees: LWWSet, title: LWWReg, clock::Lamport>, state: LWWReg, clock::Lamport>, tags: LWWSet, @@ -88,6 +89,7 @@ pub struct Issue { impl Semilattice for Issue { fn merge(&mut self, other: Self) { + self.assignees.merge(other.assignees); self.title.merge(other.title); self.state.merge(other.state); self.thread.merge(other.thread); @@ -97,6 +99,7 @@ impl Semilattice for Issue { impl Default for Issue { fn default() -> Self { Self { + assignees: LWWSet::default(), title: Max::from(String::default()).into(), state: Max::from(State::default()).into(), tags: LWWSet::default(), @@ -132,6 +135,10 @@ impl store::FromHistory for Issue { } impl Issue { + pub fn assigned(&self) -> impl Iterator { + self.assignees.iter() + } + pub fn title(&self) -> &str { self.title.get().as_str() } @@ -162,6 +169,14 @@ impl Issue { pub fn apply(&mut self, ops: impl IntoIterator) -> Result<(), Error> { for op in ops { match op.action { + Action::Assign { add, remove } => { + for assignee in add { + self.assignees.insert(assignee, op.clock); + } + for assignee in remove { + self.assignees.remove(assignee, op.clock); + } + } Action::Edit { title } => { self.title.set(title, op.clock); } @@ -199,6 +214,13 @@ impl Deref for Issue { } impl store::Transaction { + pub fn assign(&mut self, add: Vec, remove: Vec) -> OpId { + let add = add.into_iter().collect::>(); + let remove = remove.into_iter().collect::>(); + + self.push(Action::Assign { add, remove }) + } + /// Set the issue title. pub fn edit(&mut self, title: impl ToString) -> OpId { self.push(Action::Edit { @@ -264,6 +286,15 @@ impl<'a, 'g> IssueMut<'a, 'g> { &self.clock } + /// Assign one or more actors to an issue. + pub fn assign( + &mut self, + assignees: Vec, + signer: &G, + ) -> Result { + self.transaction("Assign", signer, |tx| tx.assign(assignees, vec![])) + } + /// Lifecycle an issue. pub fn lifecycle(&mut self, state: State, signer: &G) -> Result { self.transaction("Lifecycle", signer, |tx| tx.lifecycle(state)) @@ -309,6 +340,15 @@ impl<'a, 'g> IssueMut<'a, 'g> { self.transaction("React", signer, |tx| tx.react(to, reaction)) } + /// Unassign one or more actors from an issue. + pub fn unassign( + &mut self, + assignees: Vec, + signer: &G, + ) -> Result { + self.transaction("Unassign", signer, |tx| tx.assign(vec![], assignees)) + } + pub fn transaction( &mut self, message: &str, @@ -416,10 +456,23 @@ impl<'a> Issues<'a> { #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] #[serde(tag = "type", rename_all = "camelCase")] pub enum Action { - Edit { title: String }, - Lifecycle { state: State }, - Tag { add: Vec, remove: Vec }, - Thread { action: thread::Action }, + Assign { + add: Vec, + remove: Vec, + }, + Edit { + title: String, + }, + Lifecycle { + state: State, + }, + Tag { + add: Vec, + remove: Vec, + }, + Thread { + action: thread::Action, + }, } impl From for Action { @@ -435,6 +488,7 @@ mod test { use super::*; use crate::cob::Reaction; use crate::test; + use crate::test::arbitrary; #[test] fn test_ordering() { @@ -447,6 +501,49 @@ mod test { ); } + #[test] + fn test_issue_create_and_assign() { + let tmp = tempfile::tempdir().unwrap(); + let (_, signer, project) = test::setup::context(&tmp); + let mut issues = Issues::open(*signer.public_key(), &project).unwrap(); + + let assignee: ActorId = arbitrary::gen(1); + let assignee_two: ActorId = arbitrary::gen(1); + let mut issue = issues + .create("My first issue", "Blah blah blah.", &[], &signer) + .unwrap(); + + issue.assign(vec![assignee, assignee_two], &signer).unwrap(); + + let id = issue.id; + let issue = issues.get(&id).unwrap().unwrap(); + let assignees: Vec<_> = issue.assigned().cloned().collect::>(); + assert_eq!(2, assignees.len()); + assert!(assignees.contains(&assignee)); + assert!(assignees.contains(&assignee_two)); + } + + #[test] + fn test_issue_create_and_reassign() { + let tmp = tempfile::tempdir().unwrap(); + let (_, signer, project) = test::setup::context(&tmp); + let mut issues = Issues::open(*signer.public_key(), &project).unwrap(); + + let assignee: ActorId = arbitrary::gen(1); + let mut issue = issues + .create("My first issue", "Blah blah blah.", &[], &signer) + .unwrap(); + + issue.assign(vec![assignee], &signer).unwrap(); + issue.assign(vec![assignee], &signer).unwrap(); + + let id = issue.id; + let issue = issues.get(&id).unwrap().unwrap(); + let assignees: Vec<_> = issue.assigned().cloned().collect::>(); + assert_eq!(1, assignees.len()); + assert!(assignees.contains(&assignee)); + } + #[test] fn test_issue_create_and_get() { let tmp = tempfile::tempdir().unwrap(); @@ -498,6 +595,28 @@ mod test { assert_eq!(*issue.state(), State::Open); } + #[test] + fn test_issue_create_and_unassign() { + let tmp = tempfile::tempdir().unwrap(); + let (_, signer, project) = test::setup::context(&tmp); + let mut issues = Issues::open(*signer.public_key(), &project).unwrap(); + + let assignee: ActorId = arbitrary::gen(1); + let assignee_two: ActorId = arbitrary::gen(1); + let mut issue = issues + .create("My first issue", "Blah blah blah.", &[], &signer) + .unwrap(); + + issue.assign(vec![assignee, assignee_two], &signer).unwrap(); + issue.unassign(vec![assignee], &signer).unwrap(); + + let id = issue.id; + let issue = issues.get(&id).unwrap().unwrap(); + let assignees: Vec<_> = issue.assigned().cloned().collect::>(); + assert_eq!(1, assignees.len()); + assert!(assignees.contains(&assignee_two)); + } + #[test] fn test_issue_react() { let tmp = tempfile::tempdir().unwrap();