Enable adding E-Mail addresses to new user accounts using the CLI #29368
Conversation
philipreinken
force-pushed
the
issue-25319/dev
branch
2 times, most recently
from
3e87c16
to
b557ea6
Compare
Pytal
requested review from
a team,
PVince81,
ArtificialOwl and
come-nc
and removed request for
a team
philipreinken
force-pushed
the
issue-25319/dev
branch
from
b557ea6
to
dd54afe
Compare
Signed-off-by: Philip Gatzka <philip.gatzka@mailbox.org>
philipreinken
force-pushed
the
issue-25319/dev
branch
from
dd54afe
to
147c900
Compare
| @@ -118,17 +180,32 @@ protected function execute(InputInterface $input, OutputInterface $output): int | |||
| return 1; | |||
| } | |||
|
|
|||
| if (trim($password) === '' && $emailIsSet) { | |||
There was a problem hiding this comment.
You should not trim here, I think it will cause weird edge cases when $password is all spaces.
Below you use $password ?: $temporaryPassword, which will evaluate to $password if it is not empty and full of spaces.
| ); | ||
| } | ||
|
|
||
| protected function execute(InputInterface $input, OutputInterface $output): int { | ||
| $uid = $input->getArgument('uid'); | ||
| $emailIsSet = \is_string($input->getOption('email')) && \mb_strlen($input->getOption('email')) > 0; | ||
| $emailIsValid = $this->emailValidator->isValid($input->getOption('email') ?? '', new RFCValidation()); |
There was a problem hiding this comment.
If an invalid email is passed, it should abort with an error right there, it should not continue and do something else that what the caller asked for.
| $user->setSystemEMailAddress($input->getOption('email')); | ||
| $output->writeln(sprintf('E-Mail set to "%s"', (string) $user->getSystemEMailAddress())); | ||
|
|
||
| if (trim($password) === '' && $this->config->getAppValue('core', 'newUser.sendEmail', 'yes') === 'yes') { |
There was a problem hiding this comment.
Same thing with trimming, here you can use isset($temporaryPassword) I think.
|
The code should probably be abstracted inside a Service that is called by both the occ command and controller. This will ensure that it stays the same and doesn't diverge, as well as make it easier to unit test. |
|
That's definitely a more maintainable approach than the one I went with 👍 I'd be happy to try and refactor this accordingly in case there are no plans/PRs to do this already. |
No one is working on it I think, so you can start a PR on this. 👍 |
|
We ran into the same issue describe here. |
|
I'm sorry it took so long for me to revisit this PR. I don't think I'll be able to finish the work on this in the near future. In case anyone wants to build on the changes done here, please feel free to do so. |
|
Hi there, I have stumbled upon this conversation and I would really appreciate having the possibility to add user account with occ using an --email option. Is anybody working on this? |
|
also interested on this feature as in our organization we create users massively from command line |
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
Hi there 👋 With this PR I'm trying to address issue #25319 .
Quick manual testing & unit tests were successful so far.
It seems like the commands are instantiated without using the DIC, so I fetched the arguments needed from
\OC::$serverlike it's done for some of the other commands, I hope I didn't miss something here.