From fcd4ccbec30493872feba137342347541b6a4e28 Mon Sep 17 00:00:00 2001 From: Michael Gattozzi Date: Mon, 2 Dec 2019 17:31:33 -0500 Subject: Upgrade ticket format from V0 to V1 to use UUIDs This is a necessary upgrade to deal with the fact that incremental ids do not work in distributed systems. For instance say we have two branches from the same commit on master and they both add a new ticket. Both will have the same incremental ID despite being completely separate tickets. In this case we want to use UUIDs, specifically version 1 as defined in IETF RFC 4122. This version of UUID uses a timestamp to generate it and as a result the UUID it generates is *sortable*. This means that the UUIDs can be created whenever on any branch, be unique, and will be sortable by time. No matter when or where our tickets can be sorted correctly by this UUID in a deterministic order. Since we are also upgrading the code we've set up migration upgrade code to handle this in case we need to do this again in the future. We also add a few more fields and make some breaking changes since we already are for the UUIDs. Resources: - https://tools.ietf.org/html/rfc4122 --- .../ticket/closed/2-create-git-hooks-tool.toml | 11 -- .../closed/3-create-a-find_root-function.toml | 12 -- .../ticket/closed/4-create-a-tui-for-ticket.toml | 10 -- .../ticket/closed/create-a-find_root-function.toml | 15 ++ .../ticket/closed/create-a-tui-for-ticket.toml | 13 ++ .../ticket/closed/create-git-hooks-tool.toml | 14 ++ .../ticket/open/1-add-ability-to-edit-tickets.toml | 9 - .../ticket/open/add-ability-to-edit-tickets.toml | 12 ++ Cargo.lock | 1 + ticket/Cargo.toml | 1 + ticket/src/actions.rs | 65 ++++++- ticket/src/main.rs | 193 ++++++++++++++------- ticket/src/tui.rs | 20 +-- 13 files changed, 247 insertions(+), 129 deletions(-) delete mode 100644 .dev-suite/ticket/closed/2-create-git-hooks-tool.toml delete mode 100644 .dev-suite/ticket/closed/3-create-a-find_root-function.toml delete mode 100644 .dev-suite/ticket/closed/4-create-a-tui-for-ticket.toml create mode 100644 .dev-suite/ticket/closed/create-a-find_root-function.toml create mode 100644 .dev-suite/ticket/closed/create-a-tui-for-ticket.toml create mode 100644 .dev-suite/ticket/closed/create-git-hooks-tool.toml delete mode 100644 .dev-suite/ticket/open/1-add-ability-to-edit-tickets.toml create mode 100644 .dev-suite/ticket/open/add-ability-to-edit-tickets.toml diff --git a/.dev-suite/ticket/closed/2-create-git-hooks-tool.toml b/.dev-suite/ticket/closed/2-create-git-hooks-tool.toml deleted file mode 100644 index c51799c..0000000 --- a/.dev-suite/ticket/closed/2-create-git-hooks-tool.toml +++ /dev/null @@ -1,11 +0,0 @@ -title = 'Create git hooks tool' -status = 'Closed' -number = 2 -description = ''' -Git hooks are great, but the problem is that it's hard to standardize -them across a team. This is because they don't travel with the repo for -security reasons. However, having a way to manage and install them in a -repo is a good thing to standardizing checks across a team to avoid -issues that just get caught in CI way later when it would be trivial to -catch with a script. -''' diff --git a/.dev-suite/ticket/closed/3-create-a-find_root-function.toml b/.dev-suite/ticket/closed/3-create-a-find_root-function.toml deleted file mode 100644 index 5057356..0000000 --- a/.dev-suite/ticket/closed/3-create-a-find_root-function.toml +++ /dev/null @@ -1,12 +0,0 @@ -title = 'Create a find_root function' -status = 'Closed' -number = 3 -description = ''' -One of the issues with ticket is that it has a bug in init where if it's -not given the path to the root of a git then it will fail. This is -frankly frustrating and can be a bit unexpected in behavior. We fixed -this in the ticket creation with a `find_root` function. This should be -a more generalized function and be shared amongst future tools as this -will be a common operation. It shouldn't matter where in a repo you are, -the tools should just work. -''' diff --git a/.dev-suite/ticket/closed/4-create-a-tui-for-ticket.toml b/.dev-suite/ticket/closed/4-create-a-tui-for-ticket.toml deleted file mode 100644 index 2d4b77a..0000000 --- a/.dev-suite/ticket/closed/4-create-a-tui-for-ticket.toml +++ /dev/null @@ -1,10 +0,0 @@ -title = 'Create a tui for ticket' -status = 'Closed' -number = 4 -description = ''' -Currently ticket is all cli driven. The experience isn't the worst, but -the end goal has always been to get the user to be able to have a nice -in terminal experience, almost as if they never left GitHub. A basic -tui that allows one to read and comment on issues will suffice to close -this ticket. -''' diff --git a/.dev-suite/ticket/closed/create-a-find_root-function.toml b/.dev-suite/ticket/closed/create-a-find_root-function.toml new file mode 100644 index 0000000..49a550c --- /dev/null +++ b/.dev-suite/ticket/closed/create-a-find_root-function.toml @@ -0,0 +1,15 @@ +title = 'Create a find_root function' +status = 'Closed' +id = '0f37b780-1553-11ea-8003-000502040903' +assignees = [] +description = ''' +One of the issues with ticket is that it has a bug in init where if it's +not given the path to the root of a git then it will fail. This is +frankly frustrating and can be a bit unexpected in behavior. We fixed +this in the ticket creation with a `find_root` function. This should be +a more generalized function and be shared amongst future tools as this +will be a common operation. It shouldn't matter where in a repo you are, +the tools should just work. +''' +comments = [] +version = 'V1' diff --git a/.dev-suite/ticket/closed/create-a-tui-for-ticket.toml b/.dev-suite/ticket/closed/create-a-tui-for-ticket.toml new file mode 100644 index 0000000..999e3c9 --- /dev/null +++ b/.dev-suite/ticket/closed/create-a-tui-for-ticket.toml @@ -0,0 +1,13 @@ +title = 'Create a tui for ticket' +status = 'Closed' +id = '0fd04e00-1553-11ea-8004-000502040903' +assignees = [] +description = ''' +Currently ticket is all cli driven. The experience isn't the worst, but +the end goal has always been to get the user to be able to have a nice +in terminal experience, almost as if they never left GitHub. A basic +tui that allows one to read and comment on issues will suffice to close +this ticket. +''' +comments = [] +version = 'V1' diff --git a/.dev-suite/ticket/closed/create-git-hooks-tool.toml b/.dev-suite/ticket/closed/create-git-hooks-tool.toml new file mode 100644 index 0000000..4b929d9 --- /dev/null +++ b/.dev-suite/ticket/closed/create-git-hooks-tool.toml @@ -0,0 +1,14 @@ +title = 'Create git hooks tool' +status = 'Closed' +id = '0e068a80-1553-11ea-8002-000502040903' +assignees = [] +description = ''' +Git hooks are great, but the problem is that it's hard to standardize +them across a team. This is because they don't travel with the repo for +security reasons. However, having a way to manage and install them in a +repo is a good thing to standardizing checks across a team to avoid +issues that just get caught in CI way later when it would be trivial to +catch with a script. +''' +comments = [] +version = 'V1' diff --git a/.dev-suite/ticket/open/1-add-ability-to-edit-tickets.toml b/.dev-suite/ticket/open/1-add-ability-to-edit-tickets.toml deleted file mode 100644 index e0279d3..0000000 --- a/.dev-suite/ticket/open/1-add-ability-to-edit-tickets.toml +++ /dev/null @@ -1,9 +0,0 @@ -title = 'Add ability to edit tickets' -status = 'Open' -number = 1 -description = ''' -Currently if you want to edit a ticket you need to directly edit it -inside of the git repo itself. While it works it's hardly ideal. The -ability to edit tickets by specifiying the id with an accompanying cli -arg to go with it would be enough to close this. -''' diff --git a/.dev-suite/ticket/open/add-ability-to-edit-tickets.toml b/.dev-suite/ticket/open/add-ability-to-edit-tickets.toml new file mode 100644 index 0000000..d668b68 --- /dev/null +++ b/.dev-suite/ticket/open/add-ability-to-edit-tickets.toml @@ -0,0 +1,12 @@ +title = 'Add ability to edit tickets' +status = 'Open' +id = '0d6df400-1553-11ea-8001-000502040903' +assignees = [] +description = ''' +Currently if you want to edit a ticket you need to directly edit it +inside of the git repo itself. While it works it's hardly ideal. The +ability to edit tickets by specifiying the id with an accompanying cli +arg to go with it would be enough to close this. +''' +comments = [] +version = 'V1' diff --git a/Cargo.lock b/Cargo.lock index 219a070..3199d7a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -944,6 +944,7 @@ name = "ticket" version = "0.1.0" dependencies = [ "anyhow 1.0.22 (registry+https://github.com/rust-lang/crates.io-index)", + "chrono 0.4.10 (registry+https://github.com/rust-lang/crates.io-index)", "colored 1.9.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "paw 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/ticket/Cargo.toml b/ticket/Cargo.toml index 6c752b5..a83f504 100644 --- a/ticket/Cargo.toml +++ b/ticket/Cargo.toml @@ -9,6 +9,7 @@ edition = "2018" [dependencies] anyhow = "1.0" colored = "1.9" +chrono = "0.4" paw = "1.0" rustyline = "5.0" serde = { version = "1.0", features = ["derive"] } diff --git a/ticket/src/actions.rs b/ticket/src/actions.rs index 381c9f7..3e237ff 100644 --- a/ticket/src/actions.rs +++ b/ticket/src/actions.rs @@ -1,5 +1,11 @@ -use crate::Ticket; -use anyhow::Result; +use crate::{ + Ticket, + TicketV0, +}; +use anyhow::{ + bail, + Result, +}; use log::*; use shared::find_root; use std::{ @@ -8,11 +14,11 @@ use std::{ }; pub fn get_open_tickets() -> Result> { - get_tickets(ticket_root()?.join("open")) + get_tickets(open_tickets()?) } pub fn get_closed_tickets() -> Result> { - get_tickets(ticket_root()?.join("closed")) + get_tickets(closed_tickets()?) } fn get_tickets(path: PathBuf) -> Result> { @@ -24,13 +30,60 @@ fn get_tickets(path: PathBuf) -> Result> { trace!("Looking at entry {}.", path.display()); if path.is_file() { trace!("Entry is a file."); - out.push(toml::from_slice::(&fs::read(&path)?)?); + match toml::from_slice::(&fs::read(&path)?) { + Ok(ticket) => out.push(ticket), + Err(e) => { + error!("Failed to parse ticket {}", path.canonicalize()?.display()); + error!("Is the file an old ticket format? You might need to run `ticket migrate`."); + bail!("Underlying error was {}", e); + } + } } } - out.sort_by(|a, b| a.number.cmp(&b.number)); + out.sort_by(|a, b| a.id.cmp(&b.id)); Ok(out) } pub fn ticket_root() -> Result { Ok(find_root()?.join(".dev-suite").join("ticket")) } + +pub fn closed_tickets() -> Result { + Ok(ticket_root()?.join("closed")) +} + +pub fn open_tickets() -> Result { + Ok(ticket_root()?.join("open")) +} + +// Old version ticket code to handle grabbing code +pub fn get_all_ticketsv0() -> Result> { + let mut tickets = get_open_ticketsv0()?; + tickets.extend(get_closed_ticketsv0()?); + Ok(tickets) +} +pub fn get_open_ticketsv0() -> Result> { + get_ticketsv0(open_tickets()?) +} + +pub fn get_closed_ticketsv0() -> Result> { + get_ticketsv0(closed_tickets()?) +} + +fn get_ticketsv0(path: PathBuf) -> Result> { + let mut out = Vec::new(); + debug!("Looking for ticket."); + for entry in fs::read_dir(&path)? { + let entry = entry?; + let path = entry.path(); + trace!("Looking at entry {}.", path.display()); + if path.is_file() { + trace!("Entry is a file."); + if let Ok(ticket) = toml::from_slice::(&fs::read(&path)?) { + out.push(ticket); + } + } + } + out.sort_by(|a, b| a.number.cmp(&b.number)); + Ok(out) +} diff --git a/ticket/src/main.rs b/ticket/src/main.rs index 4d5457d..11e27e8 100644 --- a/ticket/src/main.rs +++ b/ticket/src/main.rs @@ -6,6 +6,7 @@ use anyhow::{ bail, Result, }; +use chrono::prelude::*; use colored::*; use log::*; use rustyline::{ @@ -21,6 +22,15 @@ use std::{ fs, process, process::Command, + thread, + time, +}; +use uuid::{ + v1::{ + Context, + Timestamp, + }, + Uuid, }; #[derive(structopt::StructOpt)] @@ -33,13 +43,14 @@ struct Args { enum Cmd { /// Initialize the repo to use ticket Init, + /// Update tickets to newer formats + Migrate, + /// Create a new ticket New, - Show { - id: usize, - }, - Close { - id: usize, - }, + /// Show a ticket on the command line + Show { id: Uuid }, + /// Close a ticket on the command line + Close { id: Uuid }, } #[paw::main] @@ -53,6 +64,7 @@ fn main(args: Args) { if let Err(e) = match cmd { Cmd::Init => init(), Cmd::New => new(), + Cmd::Migrate => migrate(), Cmd::Show { id } => show(id), Cmd::Close { id } => close(id), } { @@ -81,22 +93,8 @@ fn new() -> Result<()> { debug!("Getting ticket root."); let ticket_root = ticket_root()?; trace!("Got ticket root: {}", ticket_root.display()); - let open = ticket_root.join("open"); - let closed = ticket_root.join("closed"); + let open = open_tickets()?; let description = ticket_root.join("description"); - let mut ticket_num = 1; - - // Fast enough for now but maybe not in the future - debug!("Getting number of tickets total."); - for entry in fs::read_dir(&open)?.chain(fs::read_dir(&closed)?) { - let entry = entry?; - let path = entry.path(); - if path.is_file() { - ticket_num += 1; - } - } - debug!("Ticket Total: {}", ticket_num - 1); - debug!("Next Ticket ID: {}", ticket_num); let mut rl = Editor::<()>::new(); let title = match rl.readline("Title: ") { @@ -132,16 +130,20 @@ fn new() -> Result<()> { let t = Ticket { title, status: Status::Open, - number: ticket_num, - assignee: None, + id: Uuid::new_v1( + Timestamp::from_unix(Context::new(1), Utc::now().timestamp() as u64, 0), + &[0, 5, 2, 4, 9, 3], + )?, + assignees: Vec::new(), description: description_contents, + comments: Vec::new(), + version: Version::V1, }; debug!("Converting ticket to toml and writing to disk."); fs::write( open.join(&format!( - "{}-{}.toml", - ticket_num, + "{}.toml", t.title .to_lowercase() .split_whitespace() @@ -155,7 +157,7 @@ fn new() -> Result<()> { Ok(()) } -fn show(id: usize) -> Result<()> { +fn show(id: Uuid) -> Result<()> { debug!("Getting ticket root."); let ticket_root = ticket_root()?; trace!("Ticket root at {}.", ticket_root.display()); @@ -170,33 +172,32 @@ fn show(id: usize) -> Result<()> { let path = entry.path(); trace!("Looking at entry {}.", path.display()); if path.is_file() { - if let Some(file_name) = path.file_name().and_then(|f| f.to_str()) { - trace!("Entry is a file."); - if file_name.starts_with(&id.to_string()) { - trace!("This is the expected entry."); - let ticket = toml::from_slice::(&fs::read(&path)?)?; + let ticket = toml::from_slice::(&fs::read(&path)?)?; + if ticket.id == id { + trace!("This is the expected entry."); + println!( + "{}", + format!("{} - {}\n", ticket.id, ticket.title).bold().red() + ); + if !ticket.assignees.is_empty() { println!( - "{}", - format!("{} - {}\n", ticket.number, ticket.title) - .bold() - .red() + "{}{}", + "Assignees: ".bold().purple(), + ticket.assignees.join(", ") ); - if let Some(a) = ticket.assignee { - println!("{}{}", "Assignee: ".bold().purple(), a); - } - - print!( - "{}{}\n\n{}", - "Status: ".bold().purple(), - match ticket.status { - Status::Open => "Open".bold().green(), - Status::Closed => "Closed".bold().red(), - }, - ticket.description - ); - found = true; - break; } + + print!( + "{}{}\n\n{}", + "Status: ".bold().purple(), + match ticket.status { + Status::Open => "Open".bold().green(), + Status::Closed => "Closed".bold().red(), + }, + ticket.description + ); + found = true; + break; } } } @@ -207,7 +208,7 @@ fn show(id: usize) -> Result<()> { } } -fn close(id: usize) -> Result<()> { +fn close(id: Uuid) -> Result<()> { debug!("Getting ticket root."); let ticket_root = ticket_root()?; trace!("Ticket root at {}.", ticket_root.display()); @@ -221,20 +222,20 @@ fn close(id: usize) -> Result<()> { let path = entry.path(); trace!("Looking at entry {}.", path.display()); if path.is_file() { - if let Some(file_name) = path.file_name().and_then(|f| f.to_str()) { - if file_name.starts_with(&id.to_string()) { - trace!("The ticket is open and exists."); - debug!("Reading in the ticket from disk and setting it to closed."); - let mut ticket = toml::from_slice::(&fs::read(&path)?)?; - ticket.status = Status::Closed; - debug!("Writing ticket to disk in the closed directory."); - fs::write(closed.join(file_name), toml::to_string_pretty(&ticket)?)?; - debug!("Removing old ticket."); - fs::remove_file(&path)?; - trace!("Removed the old ticket."); - found = true; - break; - } + let mut ticket = toml::from_slice::(&fs::read(&path)?)?; + if ticket.id == id { + debug!("Ticket found setting it to closed."); + ticket.status = Status::Closed; + trace!("Writing ticket to disk in the closed directory."); + fs::write( + closed.join(path.file_name().expect("Path should have a file name")), + toml::to_string_pretty(&ticket)?, + )?; + debug!("Removing old ticket."); + fs::remove_file(&path)?; + trace!("Removed the old ticket."); + found = true; + break; } } } @@ -245,8 +246,70 @@ fn close(id: usize) -> Result<()> { } } +/// Upgrade from V0 to V1 of the ticket +fn migrate() -> Result<()> { + let ctx = Context::new(1); + let tickets = get_all_ticketsv0()?; + + let open_tickets_path = open_tickets()?; + let closed_tickets_path = closed_tickets()?; + + for t in tickets.into_iter() { + let ticket = Ticket { + title: t.title, + status: t.status, + id: Uuid::new_v1( + Timestamp::from_unix(&ctx, Utc::now().timestamp() as u64, 0), + &[0, 5, 2, 4, 9, 3], + )?, + assignees: t.assignee.map(|a| vec![a]).unwrap_or_else(Vec::new), + description: t.description, + comments: Vec::new(), + version: Version::V1, + }; + + let path = match ticket.status { + Status::Open => &open_tickets_path, + Status::Closed => &closed_tickets_path, + }; + + let mut name = ticket + .title + .split_whitespace() + .collect::>() + .join("-"); + name.push_str(".toml"); + name = name.to_lowercase(); + fs::write(path.join(&name), toml::to_string_pretty(&ticket)?)?; + fs::remove_file(path.join(format!("{}-{}", t.number, name)))?; + // We need to make sure we get different times for each ticket + // Possible future migrations might not have this issue + thread::sleep(time::Duration::from_millis(1000)); + } + Ok(()) +} + #[derive(Serialize, Deserialize)] pub struct Ticket { + title: String, + status: Status, + id: Uuid, + assignees: Vec, + description: String, + comments: Vec<(User, String)>, + version: Version, +} + +#[derive(Serialize, Deserialize)] +pub enum Version { + V1, +} + +#[derive(Serialize, Deserialize)] +pub struct User(String); + +#[derive(Serialize, Deserialize)] +pub struct TicketV0 { title: String, status: Status, number: usize, diff --git a/ticket/src/tui.rs b/ticket/src/tui.rs index 90dbeb6..e1aa666 100644 --- a/ticket/src/tui.rs +++ b/ticket/src/tui.rs @@ -216,7 +216,7 @@ pub fn run() -> Result<()> { .direction(Direction::Horizontal) .vertical_margin(3) .constraints( - [Constraint::Percentage(50), Constraint::Percentage(50)].as_ref(), + [Constraint::Percentage(30), Constraint::Percentage(70)].as_ref(), ) .split(size); @@ -283,7 +283,7 @@ pub fn run() -> Result<()> { impl<'a> App<'a> { fn table(&self, tab: &'a str) -> impl Widget + '_ { Table::new( - ["Id", "Title", "Assignee"].iter(), + ["Id", "Title"].iter(), self .tickets .tickets @@ -292,15 +292,7 @@ impl<'a> App<'a> { .iter() .enumerate() .map(move |(idx, i)| { - let data = vec![ - i.number.to_string(), - i.title.to_string(), - i.assignee - .as_ref() - .cloned() - .unwrap_or_else(|| "None".into()), - ] - .into_iter(); + let data = vec![i.id.to_string(), i.title.to_string()].into_iter(); let normal_style = Style::default().fg(Color::Yellow); let selected_style = Style::default().fg(Color::White).modifier(Modifier::BOLD); @@ -313,11 +305,7 @@ impl<'a> App<'a> { ) .block(Block::default().title(tab).borders(Borders::ALL)) .header_style(Style::default().fg(Color::Yellow)) - .widths(&[ - Constraint::Percentage(10), - Constraint::Percentage(70), - Constraint::Percentage(20), - ]) + .widths(&[Constraint::Percentage(30), Constraint::Percentage(70)]) .style(Style::default().fg(Color::White)) .column_spacing(1) } -- cgit v1.2.3