Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enh(users): send welcome email when using occ #43250

Closed

Conversation

helmo
Copy link
Contributor

@helmo helmo commented Feb 1, 2024

Enhance PR #40726

Summary

This PR tries to solve the remaining issues in #40726
Specifically from the comment #40726 (review)

Checklist

philipreinken and others added 2 commits October 4, 2023 15:29
Signed-off-by: Philip Gatzka <philip.gatzka@mailbox.org>
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
@come-nc
Copy link
Contributor

come-nc commented Feb 1, 2024

Changed target to master to see the whole diff, my comment on original PR still applies:

This needs to work and set email:
occ user:add --password-from-env --email bob@example.com bob

-> This you fixed, I think

This needs to work and set email, if newUser.sendEmail is off it should ask for password same as without --email:
occ user:add --email bob@example.com bob

-> This you did not, if newUser.sendEmail is off, a temporary password is printed to the output instead of asking for the password in interactive mode. Passing --email option when newUser.sendEmail is off should not prevent entering a password.

I’m not sure what the correct behavior is here, maybe simply add an other option --generate-password?
Because people may want the welcome email to be sent while still setting the password.

Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above

@miaulalala miaulalala changed the title Enhance PR #40726 enh(users): send welcome email when using occ Feb 2, 2024
@kyteinsky
Copy link
Contributor

I’m not sure what the correct behavior is here, maybe simply add an other option --generate-password?

Yeah it would be better to set a generated password when explicitly asked rather than implicitly based on other options.

dropped password printing commit and pushed the requested changes in feat/new-user-email branch. Maybe I will add more tests later.

@kyteinsky
Copy link
Contributor

merged here: #40726

@kyteinsky kyteinsky closed this Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No "Welcome" email sent when using occ.
5 participants