[PM-3436] Username generator #285
Conversation
|
No New Or Fixed Issues Found |
# Conflicts: # crates/bitwarden/src/tool/mod.rs
# Conflicts: # crates/bitwarden/Cargo.toml
# Conflicts: # crates/bitwarden/src/tool/generators/client_generator.rs # crates/bitwarden/src/tool/generators/mod.rs
There was a problem hiding this comment.
Looks good, a few comments. Would be nice if we either in this PR or a followup write some integration tests for the external forwarders, i.e. mock the response.
We should consider if there is an easy way to have e2e tests for this to, and call the external services.
| let description = website | ||
| .as_ref() | ||
| .map(|w| format!("{w} - ")) | ||
| .unwrap_or_default(); | ||
| let description = format!("{description}Generated by Bitwarden."); |
There was a problem hiding this comment.
We seem to be duplicating this in many of the handlers. Should we pre-process this as part of the input?
There was a problem hiding this comment.
I've extracted it to a function at least, as not all the services use it. I've noticed that at the moment Firefox is using a slightly different message. I don't know if that matters, but I defined a separate function for it.
# Conflicts: # crates/bitwarden/src/tool/generators/passphrase.rs
crates/bitwarden/src/tool/generators/username_forwarders/forwardemail.rs
Show resolved
Hide resolved
| fn format_description_ff(website: &Option<String>) -> String { | ||
| let description = website | ||
| .as_ref() | ||
| .map(|w| format!("{w} - ")) | ||
| .unwrap_or_default(); | ||
| format!("{description}Generated by Bitwarden.") | ||
| } |
There was a problem hiding this comment.
@kspearrin do you remember why Firefox forwarder uses a different description than the other formatters?
There was a problem hiding this comment.
👍, let's keep it for now but with the knowledge we can revisit this in the future.
Type of change
Objective
Implement username generation.
Some of the methods are shared with password and passphrase generation (like
capitalize_first_letter,random_lowercase_stringand the username word generation). To avoid dependencies between the PRs, I included them here again, once we're done with all the generators I'll unify them.