From ea5aa6993d4c906c1703563ddc304c324c4ae079 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Wed, 21 Feb 2024 15:01:11 +0000 Subject: feat(send): in-reply-to arg for revised proposal send a revised version of a proposal using the new in-replyto argument suppliments existing 'root' tag with 'root-revision' e 'reply' tag to the original proposal --- src/git.rs | 2 + src/sub_commands/push.rs | 1 + src/sub_commands/send.rs | 96 ++++++++- tests/send.rs | 521 +++++++++++++++++++++++++++++++++++------------ 4 files changed, 478 insertions(+), 142 deletions(-) diff --git a/src/git.rs b/src/git.rs index d0eaf03..0a06ab5 100644 --- a/src/git.rs +++ b/src/git.rs @@ -1313,6 +1313,7 @@ mod tests { None, None, None, + &None, ) } fn test_patch_applies_to_repository(patch_event: nostr::Event) -> Result<()> { @@ -1473,6 +1474,7 @@ mod tests { &vec![oid_to_sha1(&oid1), oid_to_sha1(&oid2), oid_to_sha1(&oid3)], &TEST_KEY_1_KEYS, &RepoRef::try_from(generate_repo_ref_event()).unwrap(), + &None, )?; events.reverse(); diff --git a/src/sub_commands/push.rs b/src/sub_commands/push.rs index 2500e9f..73bdb38 100644 --- a/src/sub_commands/push.rs +++ b/src/sub_commands/push.rs @@ -119,6 +119,7 @@ pub async fn launch(cli_args: &Cli) -> Result<()> { patch_events.last().map(nostr::Event::id), None, None, + &None, ) .context("cannot make patch event from commit")?, ); diff --git a/src/sub_commands/send.rs b/src/sub_commands/send.rs index 105f87a..c9c81ee 100644 --- a/src/sub_commands/send.rs +++ b/src/sub_commands/send.rs @@ -1,9 +1,12 @@ -use std::time::Duration; +use std::{str::FromStr, time::Duration}; use anyhow::{bail, Context, Result}; use futures::future::join_all; use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; -use nostr::{prelude::sha1::Hash as Sha1Hash, EventBuilder, Marker, Tag, TagKind}; +use nostr::{ + nips::nip19::Nip19, prelude::sha1::Hash as Sha1Hash, EventBuilder, FromBech32, Marker, Tag, + TagKind, UncheckedUrl, +}; use super::list::tag_value; #[cfg(not(test))] @@ -25,6 +28,10 @@ pub struct SubCommandArgs { /// starting commit (commits since in current branch) or commit range, like /// in `git format-patch` starting_commit: String, + #[clap(long)] + /// nevent or event id of an existing proposal for which this is a new + /// version + in_reply_to: Option, /// optional cover letter title #[clap(short, long)] title: Option, @@ -161,6 +168,7 @@ pub async fn launch(cli_args: &Cli, args: &SubCommandArgs) -> Result<()> { &commits, &keys, &repo_ref, + &args.in_reply_to, )?; println!( @@ -390,6 +398,7 @@ pub fn generate_cover_letter_and_patch_events( commits: &Vec, keys: &nostr::Keys, repo_ref: &RepoRef, + in_reply_to: &Option, ) -> Result> { let root_commit = git_repo .get_root_commit() @@ -418,8 +427,19 @@ pub fn generate_cover_letter_and_patch_events( }, Tag::Reference(format!("{root_commit}")), Tag::Hashtag("cover-letter".to_string()), - Tag::Hashtag("root".to_string()), ], + if let Some(event_ref) = in_reply_to.clone() { + vec![ + Tag::Hashtag("root".to_string()), + Tag::Hashtag("revision-root".to_string()), + // TODO check if id is for a root proposal (perhaps its for an issue?) + e_tag_from_nip19(&event_ref,"proposal",nostr::Marker::Reply)?, + ] + } else { + vec![ + Tag::Hashtag("root".to_string()), + ] + }, // this is not strictly needed but makes for prettier branch names // eventually a prefix will be needed of the event id to stop 2 proposals with the same name colliding // a change like this, or the removal of this tag will require the actual branch name to be tracked @@ -466,6 +486,7 @@ pub fn generate_cover_letter_and_patch_events( } else { None }, + in_reply_to, ) .context("failed to generate patch event")?, ); @@ -473,6 +494,51 @@ pub fn generate_cover_letter_and_patch_events( Ok(events) } +fn e_tag_from_nip19( + reference: &str, + reference_name: &str, + marker: nostr::Marker, +) -> Result { + let mut bech32 = reference.to_string(); + loop { + if bech32.is_empty() { + bech32 = Interactor::default().input( + PromptInputParms::default().with_prompt(&format!("{reference_name} nevent")), + )?; + } + + if let Ok(nip19) = Nip19::from_bech32(bech32.clone()) { + match nip19 { + Nip19::Event(n) => { + break Ok(nostr::Tag::Event { + event_id: n.event_id, + relay_url: n.relays.first().map(UncheckedUrl::new), + marker: Some(marker), + }); + } + Nip19::EventId(id) => { + break Ok(nostr::Tag::Event { + event_id: id, + relay_url: None, + marker: Some(marker), + }); + } + _ => {} + } + } + if let Ok(id) = nostr::EventId::from_str(&bech32) { + break Ok(nostr::Tag::Event { + event_id: id, + relay_url: None, + marker: Some(marker), + }); + } + println!("not a valid {reference_name} event reference"); + + bech32 = String::new(); + } +} + pub struct CoverLetter { pub title: String, pub description: String, @@ -565,6 +631,7 @@ pub fn generate_patch_event( parent_patch_event_id: Option, series_count: Option<(u64, u64)>, branch_name: Option, + in_reply_to: &Option, ) -> Result { let commit_parent = git_repo .get_commit_parent(commit) @@ -592,16 +659,27 @@ pub fn generate_patch_event( // code that makes it into the main branch, assuming // the commit id is correct Tag::Reference(commit.to_string()), + ], - if let Some(thread_event_id) = thread_event_id { Tag::Event { + if let Some(thread_event_id) = thread_event_id { + vec![Tag::Event { event_id: thread_event_id, relay_url: relay_hint.clone(), marker: Some(Marker::Root), - } } - else { - Tag::Hashtag("root".to_string()) - }, - ], + }] + } else if let Some(event_ref) = in_reply_to.clone() { + vec![ + Tag::Hashtag("root".to_string()), + Tag::Hashtag("revision-root".to_string()), + // TODO check if id is for a root proposal (perhaps its for an issue?) + e_tag_from_nip19(&event_ref,"proposal",nostr::Marker::Reply)?, + ] + } else { + vec![ + Tag::Hashtag("root".to_string()), + ] + }, + if let Some(id) = parent_patch_event_id { vec![Tag::Event { event_id: id, diff --git a/tests/send.rs b/tests/send.rs index d8186bd..b5c803d 100644 --- a/tests/send.rs +++ b/tests/send.rs @@ -1130,38 +1130,99 @@ mod sends_2_patches_without_cover_letter { } Ok(()) } - mod specify_starting_commits { - use super::*; - fn cli_tester_create_proposal(git_repo: &GitTestRepo) -> CliTester { - let args = vec![ - "--nsec", - TEST_KEY_1_NSEC, - "--password", - TEST_PASSWORD, - "--disable-cli-spinners", - "send", - "HEAD~3", - "--no-cover-letter", - ]; - CliTester::new_from_dir(&git_repo.dir, args) - } - fn expect_msgs_first(p: &mut CliTester) -> Result<()> { - p.expect("creating patch for 3 commits\r\n")?; - p.expect("searching for profile and relay updates...\r\n")?; - p.expect("\r")?; - p.expect("logged in as fred\r\n")?; - p.expect("posting 3 patches without a covering letter...\r\n")?; +} +mod specify_starting_commits { + use super::*; + fn cli_tester_create_proposal(git_repo: &GitTestRepo) -> CliTester { + let args = vec![ + "--nsec", + TEST_KEY_1_NSEC, + "--password", + TEST_PASSWORD, + "--disable-cli-spinners", + "send", + "HEAD~3", + "--no-cover-letter", + ]; + CliTester::new_from_dir(&git_repo.dir, args) + } + fn expect_msgs_first(p: &mut CliTester) -> Result<()> { + p.expect("creating patch for 3 commits\r\n")?; + p.expect("searching for profile and relay updates...\r\n")?; + p.expect("\r")?; + p.expect("logged in as fred\r\n")?; + p.expect("posting 3 patches without a covering letter...\r\n")?; + Ok(()) + } + async fn prep_run_create_proposal() -> Result<( + Relay<'static>, + Relay<'static>, + Relay<'static>, + Relay<'static>, + Relay<'static>, + )> { + let git_repo = prep_git_repo()?; + // fallback (51,52) user write (53, 55) repo (55, 56) + let (mut r51, mut r52, mut r53, mut r55, mut r56) = ( + Relay::new( + 8051, + None, + Some(&|relay, client_id, subscription_id, _| -> Result<()> { + relay.respond_events( + client_id, + &subscription_id, + &vec![ + generate_test_key_1_metadata_event("fred"), + generate_test_key_1_relay_list_event(), + ], + )?; + Ok(()) + }), + ), + Relay::new(8052, None, None), + Relay::new(8053, None, None), + Relay::new( + 8055, + None, + Some(&|relay, client_id, subscription_id, _| -> Result<()> { + relay.respond_events( + client_id, + &subscription_id, + &vec![generate_repo_ref_event()], + )?; + Ok(()) + }), + ), + Relay::new(8056, None, None), + ); + + // // check relay had the right number of events + let cli_tester_handle = std::thread::spawn(move || -> Result<()> { + let mut p = cli_tester_create_proposal(&git_repo); + p.expect_end_eventually()?; + for p in [51, 52, 53, 55, 56] { + relay::shutdown_relay(8000 + p)?; + } Ok(()) - } - async fn prep_run_create_proposal() -> Result<( - Relay<'static>, - Relay<'static>, - Relay<'static>, - Relay<'static>, - Relay<'static>, - )> { + }); + + // launch relay + let _ = join!( + r51.listen_until_close(), + r52.listen_until_close(), + r53.listen_until_close(), + r55.listen_until_close(), + r56.listen_until_close(), + ); + cli_tester_handle.join().unwrap()?; + Ok((r51, r52, r53, r55, r56)) + } + mod cli_ouput { + use super::*; + + async fn run_test_async() -> Result<()> { let git_repo = prep_git_repo()?; - // fallback (51,52) user write (53, 55) repo (55, 56) + let (mut r51, mut r52, mut r53, mut r55, mut r56) = ( Relay::new( 8051, @@ -1198,7 +1259,20 @@ mod sends_2_patches_without_cover_letter { // // check relay had the right number of events let cli_tester_handle = std::thread::spawn(move || -> Result<()> { let mut p = cli_tester_create_proposal(&git_repo); - p.expect_end_eventually()?; + + expect_msgs_first(&mut p)?; + relay::expect_send_with_progress( + &mut p, + vec![ + (" [my-relay] [repo-relay] ws://localhost:8055", true, ""), + (" [my-relay] ws://localhost:8053", true, ""), + (" [repo-relay] ws://localhost:8056", true, ""), + (" [default] ws://localhost:8051", true, ""), + (" [default] ws://localhost:8052", true, ""), + ], + 3, + )?; + p.expect_end_with_whitespace()?; for p in [51, 52, 53, 55, 56] { relay::shutdown_relay(8000 + p)?; } @@ -1214,146 +1288,327 @@ mod sends_2_patches_without_cover_letter { r56.listen_until_close(), ); cli_tester_handle.join().unwrap()?; - Ok((r51, r52, r53, r55, r56)) + Ok(()) } - mod cli_ouput { - use super::*; - async fn run_test_async() -> Result<()> { - let git_repo = prep_git_repo()?; + #[tokio::test] + #[serial] + async fn check_cli_output() -> Result<()> { + run_test_async().await?; + Ok(()) + } + } - let (mut r51, mut r52, mut r53, mut r55, mut r56) = ( - Relay::new( - 8051, - None, - Some(&|relay, client_id, subscription_id, _| -> Result<()> { - relay.respond_events( - client_id, - &subscription_id, - &vec![ - generate_test_key_1_metadata_event("fred"), - generate_test_key_1_relay_list_event(), - ], - )?; - Ok(()) - }), - ), - Relay::new(8052, None, None), - Relay::new(8053, None, None), - Relay::new( - 8055, - None, - Some(&|relay, client_id, subscription_id, _| -> Result<()> { - relay.respond_events( - client_id, - &subscription_id, - &vec![generate_repo_ref_event()], - )?; - Ok(()) - }), - ), - Relay::new(8056, None, None), - ); + #[tokio::test] + #[serial] + async fn three_patch_events() -> Result<()> { + let (_, _, r53, r55, r56) = prep_run_create_proposal().await?; + for relay in [&r53, &r55, &r56] { + assert_eq!(relay.events.iter().filter(|e| is_patch(e)).count(), 3); + } + Ok(()) + } - // // check relay had the right number of events - let cli_tester_handle = std::thread::spawn(move || -> Result<()> { - let mut p = cli_tester_create_proposal(&git_repo); + #[tokio::test] + #[serial] + async fn first_patch_is_ancestor_and_root_others_in_correct_order() -> Result<()> { + let (_, _, r53, r55, r56) = prep_run_create_proposal().await?; + for relay in [&r53, &r55, &r56] { + let patch_events = relay + .events + .iter() + .filter(|e| is_patch(e)) + .collect::>(); - expect_msgs_first(&mut p)?; - relay::expect_send_with_progress( - &mut p, - vec![ - (" [my-relay] [repo-relay] ws://localhost:8055", true, ""), - (" [my-relay] ws://localhost:8053", true, ""), - (" [repo-relay] ws://localhost:8056", true, ""), - (" [default] ws://localhost:8051", true, ""), - (" [default] ws://localhost:8052", true, ""), + // first patch tagged as root + assert!( + patch_events[0] + .iter_tags() + .any(|t| t.as_vec()[0].eq("t") && t.as_vec()[1].eq("root")) + ); + // first patch is ancestor + assert_eq!( + patch_events[0] + .iter_tags() + .find(|t| t.as_vec()[0].eq("commit")) + .unwrap() + .as_vec()[1], + "431b84edc0d2fa118d63faa3c2db9c73d630a5ae" + ); + // second patch not tagged as root + assert_eq!( + patch_events[1] + .iter_tags() + .find(|t| t.as_vec()[0].eq("commit")) + .unwrap() + .as_vec()[1], + "232efb37ebc67692c9e9ff58b83c0d3d63971a0a" + ); + // second patch not tagged as root + assert_eq!( + patch_events[2] + .iter_tags() + .find(|t| t.as_vec()[0].eq("commit")) + .unwrap() + .as_vec()[1], + "fe973a840fba2a8ab37dd505c154854a69a6505c" + ); + } + Ok(()) + } +} + +mod specify_in_reply_to { + use super::*; + fn cli_tester_create_proposal(git_repo: &GitTestRepo) -> CliTester { + let args = vec![ + "--nsec", + TEST_KEY_1_NSEC, + "--password", + TEST_PASSWORD, + "--disable-cli-spinners", + "send", + "--in-reply-to", + "nevent1qqsypm62fzw7qynvlc4gjl3tr0jw4vmh659nvr2cc5qyhdg92a5yy0qzypumuen7l8wthtz45p3ftn58pvrs9xlumvkuu2xet8egzkcklqtesxygzam", + "--title", + "exampletitle", + "--description", + "exampledescription", + ]; + CliTester::new_from_dir(&git_repo.dir, args) + } + + async fn prep_run_create_proposal() -> Result<( + Relay<'static>, + Relay<'static>, + Relay<'static>, + Relay<'static>, + Relay<'static>, + )> { + let git_repo = prep_git_repo()?; + // fallback (51,52) user write (53, 55) repo (55, 56) + let (mut r51, mut r52, mut r53, mut r55, mut r56) = ( + Relay::new( + 8051, + None, + Some(&|relay, client_id, subscription_id, _| -> Result<()> { + relay.respond_events( + client_id, + &subscription_id, + &vec![ + generate_test_key_1_metadata_event("fred"), + generate_test_key_1_relay_list_event(), ], - 3, )?; - p.expect_end_with_whitespace()?; - for p in [51, 52, 53, 55, 56] { - relay::shutdown_relay(8000 + p)?; - } Ok(()) - }); - - // launch relay - let _ = join!( - r51.listen_until_close(), - r52.listen_until_close(), - r53.listen_until_close(), - r55.listen_until_close(), - r56.listen_until_close(), - ); - cli_tester_handle.join().unwrap()?; - Ok(()) + }), + ), + Relay::new(8052, None, None), + Relay::new(8053, None, None), + Relay::new( + 8055, + None, + Some(&|relay, client_id, subscription_id, _| -> Result<()> { + relay.respond_events( + client_id, + &subscription_id, + &vec![generate_repo_ref_event()], + )?; + Ok(()) + }), + ), + Relay::new(8056, None, None), + ); + + // // check relay had the right number of events + let cli_tester_handle = std::thread::spawn(move || -> Result<()> { + let mut p = cli_tester_create_proposal(&git_repo); + p.expect_end_eventually()?; + for p in [51, 52, 53, 55, 56] { + relay::shutdown_relay(8000 + p)?; } + Ok(()) + }); + + // launch relay + let _ = join!( + r51.listen_until_close(), + r52.listen_until_close(), + r53.listen_until_close(), + r55.listen_until_close(), + r56.listen_until_close(), + ); + cli_tester_handle.join().unwrap()?; + Ok((r51, r52, r53, r55, r56)) + } + mod cli_ouput { + use super::*; - #[tokio::test] - #[serial] - async fn check_cli_output() -> Result<()> { - run_test_async().await?; + async fn run_test_async() -> Result<()> { + let git_repo = prep_git_repo()?; + + let (mut r51, mut r52, mut r53, mut r55, mut r56) = ( + Relay::new( + 8051, + None, + Some(&|relay, client_id, subscription_id, _| -> Result<()> { + relay.respond_events( + client_id, + &subscription_id, + &vec![ + generate_test_key_1_metadata_event("fred"), + generate_test_key_1_relay_list_event(), + ], + )?; + Ok(()) + }), + ), + Relay::new(8052, None, None), + Relay::new(8053, None, None), + Relay::new( + 8055, + None, + Some(&|relay, client_id, subscription_id, _| -> Result<()> { + relay.respond_events( + client_id, + &subscription_id, + &vec![generate_repo_ref_event()], + )?; + Ok(()) + }), + ), + Relay::new(8056, None, None), + ); + + // // check relay had the right number of events + let cli_tester_handle = std::thread::spawn(move || -> Result<()> { + let mut p = cli_tester_create_proposal(&git_repo); + + expect_msgs_first(&mut p, true)?; + relay::expect_send_with_progress( + &mut p, + vec![ + (" [my-relay] [repo-relay] ws://localhost:8055", true, ""), + (" [my-relay] ws://localhost:8053", true, ""), + (" [repo-relay] ws://localhost:8056", true, ""), + (" [default] ws://localhost:8051", true, ""), + (" [default] ws://localhost:8052", true, ""), + ], + 3, + )?; + p.expect_end_with_whitespace()?; + for p in [51, 52, 53, 55, 56] { + relay::shutdown_relay(8000 + p)?; + } Ok(()) - } + }); + + // launch relay + let _ = join!( + r51.listen_until_close(), + r52.listen_until_close(), + r53.listen_until_close(), + r55.listen_until_close(), + r56.listen_until_close(), + ); + cli_tester_handle.join().unwrap()?; + Ok(()) + } + + #[tokio::test] + #[serial] + async fn check_cli_output() -> Result<()> { + run_test_async().await?; + Ok(()) } + } + + mod cover_letter_tags { + use super::*; #[tokio::test] #[serial] - async fn three_patch_events() -> Result<()> { + async fn t_tag_root() -> Result<()> { let (_, _, r53, r55, r56) = prep_run_create_proposal().await?; for relay in [&r53, &r55, &r56] { - assert_eq!(relay.events.iter().filter(|e| is_patch(e)).count(), 3); + let cover_letter_event: &nostr::Event = + relay.events.iter().find(|e| is_cover_letter(e)).unwrap(); + assert!( + cover_letter_event + .iter_tags() + .any(|t| { t.as_vec()[0].eq("t") && t.as_vec()[1].eq(&"root") }) + ); } Ok(()) } #[tokio::test] #[serial] - async fn first_patch_is_ancestor_and_root_others_in_correct_order() -> Result<()> { + async fn t_tag_revision_root() -> Result<()> { let (_, _, r53, r55, r56) = prep_run_create_proposal().await?; for relay in [&r53, &r55, &r56] { - let patch_events = relay - .events - .iter() - .filter(|e| is_patch(e)) - .collect::>(); - - // first patch tagged as root + let cover_letter_event: &nostr::Event = + relay.events.iter().find(|e| is_cover_letter(e)).unwrap(); assert!( - patch_events[0] - .iter_tags() - .any(|t| t.as_vec()[0].eq("t") && t.as_vec()[1].eq("root")) - ); - // first patch is ancestor - assert_eq!( - patch_events[0] + cover_letter_event .iter_tags() - .find(|t| t.as_vec()[0].eq("commit")) - .unwrap() - .as_vec()[1], - "431b84edc0d2fa118d63faa3c2db9c73d630a5ae" + .any(|t| { t.as_vec()[0].eq("t") && t.as_vec()[1].eq(&"revision-root") }) ); - // second patch not tagged as root + } + Ok(()) + } + + #[tokio::test] + #[serial] + async fn e_tag_in_reply_to_event_as_reply() -> Result<()> { + let (_, _, r53, r55, r56) = prep_run_create_proposal().await?; + for relay in [&r53, &r55, &r56] { + let cover_letter_event: &nostr::Event = + relay.events.iter().find(|e| is_cover_letter(e)).unwrap(); assert_eq!( - patch_events[1] + cover_letter_event .iter_tags() - .find(|t| t.as_vec()[0].eq("commit")) + .find(|t| { + t.as_vec()[0].eq("e") + && t.as_vec().len().eq(&4) + && t.as_vec()[3].eq("reply") + }) .unwrap() .as_vec()[1], - "232efb37ebc67692c9e9ff58b83c0d3d63971a0a" + // id of state nevent + "40ef4a489de0126cfe2a897e2b1be4eab377d50b360d58c5004bb5055768423c", ); - // second patch not tagged as root + } + Ok(()) + } + } + + #[tokio::test] + #[serial] + async fn patch_tags_cover_letter_event_as_root() -> Result<()> { + let (_, _, r53, r55, r56) = prep_run_create_proposal().await?; + for relay in [&r53, &r55, &r56] { + let patch_events: Vec<&nostr::Event> = + relay.events.iter().filter(|e| is_patch(e)).collect(); + + let cover_letter_event = relay.events.iter().find(|e| is_cover_letter(e)).unwrap(); + + for patch in patch_events { assert_eq!( - patch_events[2] - .iter_tags() - .find(|t| t.as_vec()[0].eq("commit")) + patch + .tags + .iter() + .find(|t| { + t.as_vec()[0].eq("e") + && t.as_vec().len().eq(&4) + && t.as_vec()[3].eq("root") + }) .unwrap() .as_vec()[1], - "fe973a840fba2a8ab37dd505c154854a69a6505c" + cover_letter_event.id.to_string() ); } - Ok(()) } + Ok(()) } } -- cgit v1.2.3