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

feat: Email option in occ user:add command #40726

Merged
merged 4 commits into from
Feb 26, 2024
Merged

Conversation

kyteinsky
Copy link
Contributor

@kyteinsky kyteinsky commented Sep 30, 2023

Summary

Builds upon #29368

This adds an occ command option (email) during the addition of a user to auto-generate a password and send a link to the email address to set a new password if:

  1. password-from-env option was not passed
  2. email is given and valid

Docs

Checklist

core/register_command.php Fixed Show fixed Hide fixed
core/register_command.php Fixed Show fixed Hide fixed
core/Command/User/Add.php Fixed Show fixed Hide fixed
@szaimen szaimen added enhancement 3. to review Waiting for reviews labels Sep 30, 2023
@szaimen szaimen added this to the Nextcloud 28 milestone Sep 30, 2023
@szaimen szaimen requested review from nickvergessen, a team, icewind1991 and Fenn-CS and removed request for a team September 30, 2023 07:35
core/Command/User/Add.php Outdated Show resolved Hide resolved
tests/Core/Command/User/AddTest.php Outdated Show resolved Hide resolved
core/register_command.php Fixed Show fixed Hide fixed
@kyteinsky kyteinsky force-pushed the feat/new-user-email branch 2 times, most recently from fd4822d to b9e4d89 Compare September 30, 2023 21:10
core/Command/User/Add.php Outdated Show resolved Hide resolved
core/register_command.php Outdated Show resolved Hide resolved
core/Command/User/Add.php Outdated Show resolved Hide resolved
@solracsf solracsf changed the title feat: Email option in occ use:add command feat: Email option in occ user:add command Oct 2, 2023
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Not blocking, but with the comments the code would be simpler/easier to read in the future

core/Command/User/Add.php Outdated Show resolved Hide resolved
core/Command/User/Add.php Outdated Show resolved Hide resolved
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.

This is still not good for when newUser.sendEmail is off, this will generate a random password but not send it.
Using --email should not change how password is handled, unless newUser.sendEmail is on and password-from-env was not given.

This needs to work and set email:
occ user:add --password-from-env --email bob@example.com bob
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

@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Feb 23, 2024
philipreinken and others added 4 commits February 24, 2024 04:56
Signed-off-by: Philip Gatzka <philip.gatzka@mailbox.org>
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
Signed-off-by: Herman van Rink <rink@initfour.nl>
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
Copy link
Contributor

Possible performance regression detected

Show Output
564 queries added

= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
= /remote.php/dav/files/test/many_files
= /remote.php/dav/files/test/new_file.txt
= /remote.php/dav/files/test/new_file.txt
= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
= /remote.php/dav/files/test/many_files
= /remote.php/dav/files/test/new_file.txt
= /remote.php/dav/files/test/new_file.txt
+ /remote.php/dav/files/test added with 45 queries
+ /remote.php/dav/files/test/test.txt added with 33 queries
+ /remote.php/dav/files/test/many_files added with 46 queries
+ /remote.php/dav/files/test/new_file.txt added with 67 queries
+ /remote.php/dav/files/test/new_file.txt added with 91 queries
+ /remote.php/dav/files/test added with 45 queries
+ /remote.php/dav/files/test/test.txt added with 33 queries
+ /remote.php/dav/files/test/many_files added with 46 queries
+ /remote.php/dav/files/test/new_file.txt added with 67 queries
+ /remote.php/dav/files/test/new_file.txt added with 91 queries

@kyteinsky
Copy link
Contributor Author

is performance regression something to worry about? Seems unrelated to this PR.

@come-nc come-nc merged commit 102773a into master Feb 26, 2024
158 of 159 checks passed
@come-nc come-nc deleted the feat/new-user-email branch February 26, 2024 10:50
@come-nc
Copy link
Contributor

come-nc commented Feb 26, 2024

is performance regression something to worry about? Seems unrelated to this PR.

It is unrelated.

@b3nks
Copy link

b3nks commented Feb 26, 2024

Great! Thanks a lot for that! 💙

@blizzz blizzz mentioned this pull request Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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