-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Don't set email if invalid in user:add command #28426
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
Conversation
phil-davis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works
the question is now, should it work like this (create the user, but just not set any email address), or
not create the user at all
|
@PVince81 @DeepDiver1975 or whoever, what do you think should be the behavior when the email address is not valid? |
|
I'd say don't add the user at all as the whole user entity failed validation by having a wrong email address |
core/Command/User/Add.php
Outdated
| // Validate first | ||
| if(!$this->mailer->validateMailAddress($input->getOption('email'))) { | ||
| // Invalid! Error | ||
| $output->writeln('<error>Invalid email address supplied</error>'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please throw / return error code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation will have to happen before:
$this->userManager->createUser()
and throw an error there, before the user is created.
Then the code to create the email address happen around here, after the user is created.
c9cd9c0 to
85e1bd6
Compare
PVince81
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
@tomneedham please backport to stable10 |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Related Issue
#28425
Motivation and Context
Otherwise invalid emails can be set by accident
How Has This Been Tested?
OCC commands with invalid and valid email addreses
Types of changes
Checklist: