feat: added features to credential offer workflow #114
Conversation
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
| timeout: Option<u32>, | ||
|
|
||
| #[clap(long = "self", short = 's')] | ||
| sent_to_self: bool, |
There was a problem hiding this comment.
| sent_to_self: bool, | |
| send_to_self: bool, |
WDYT about shortening to self and adding docs about the intended use via clap(...about=...)?
There was a problem hiding this comment.
self is a reserved keyword and does not enjoy me using it.
cli/src/modules/workflow.rs
Outdated
| for i in 1..=limit { | ||
| let connection = | ||
| ConnectionModule::get_by_id(&agent, connection.connection_id.to_owned()) | ||
| .await?; | ||
| if connection.state != "active" && connection.state != "response" { | ||
| trace!("Connection state is not active"); | ||
| std::thread::sleep(std::time::Duration::from_millis(1000)); | ||
| } else { | ||
| trace!("Connection state is active, continuing the flow"); | ||
| credential_offer(connection.connection_id, agent).await?; | ||
| break; | ||
| } | ||
| if i == limit { | ||
| return Err(Error::InactiveConnection.into()); | ||
| } | ||
| } |
There was a problem hiding this comment.
nit; since we ask the user for a timeout - im guessing in seconds? - this might wait longer than that amount of time since we are also waiting for a network request to complete. i don't think this is an issue particularly just that the time a user specifies will not be precise
There was a problem hiding this comment.
I wanted to provide an option to override it. (its in millis), just if anyones use case might work differently.
cli/src/modules/workflow.rs
Outdated
| ConnectionModule::get_by_id(&agent, connection.connection_id.to_owned()) | ||
| .await?; | ||
| if connection.state != "active" && connection.state != "response" { | ||
| trace!("Connection state is not active"); |
There was a problem hiding this comment.
Do these logs work when there is a spinner spinning?
cli/src/modules/workflow.rs
Outdated
| attributes.insert(String::from("Valid Until"), String::from("20251212")); | ||
| debug!("Mock credential:\n{:#?}", attributes); | ||
|
|
||
| let options = CredentialOfferWorkflow { |
There was a problem hiding this comment.
| let options = CredentialOfferWorkflow { | |
| let workflow = CredentialOfferWorkflow { |
seems like a clearer name IMO
cli/src/modules/workflow.rs
Outdated
| ); | ||
| copy!("{}", connection.invitation_url); | ||
| } | ||
| let limit = timeout.map(|t| t / 1000).unwrap_or(10); |
There was a problem hiding this comment.
I think we should increase the default to 30s, rather than have it very low and then tell the user they should increase it.
There was a problem hiding this comment.
maybe even 1 minute TBH
cli/src/modules/workflow.rs
Outdated
| agent.receive_invitation(invitation_object).await?; | ||
| } else { | ||
| qr::print_qr_code(&connection.invitation_url)?; | ||
| println!("Connection id: {}", connection.connection_id); |
There was a problem hiding this comment.
i think we should update the format of these logs, optimizing for making sure the user understands what is currently expected of them
[QR code]
\n
Created.green() invitation to connect with <id>.bold() - your env.name().cyan() agent.
\n
Scan.bold() the QR code above to accept the invitation or use this URL:
\n
[URL]
\n
Waiting.cyan() for invitation to be accepted (timeout <sec>s.bold())...
[spinner spinning away]
| @@ -20,7 +26,13 @@ pub struct WorkflowOptions { | |||
| pub enum WorkflowSubcommands { | |||
| CredentialOffer { | |||
There was a problem hiding this comment.
Would you mind adding some docs to this bad boy?
Co-authored-by: Jean-Louis Leysens <[email protected]>
Co-authored-by: Jean-Louis Leysens <[email protected]>
Co-authored-by: Jean-Louis Leysens <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
… feat/offer-workflow
Signed-off-by: Berend Sliedrecht <[email protected]>
|
@blu3beri i've pushed a few cosmetic changes to the workflow copy.
It'd be great to indent the text consistently under the heading. TUIs! |
|
I think we can consider generalizing this kind of output to other workflows as they are added, no need to get into that for this PR IMO. |
will need a cleanup when we have time
You can test it with