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.

Sign up for GitHub

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

Jump to bottom

3.1.1 - Adding new User fails to save once Password Method is changed #16709

Open
deJaya opened this issue Feb 20, 2025 · 4 comments · May be fixed by #16710
Open

3.1.1 - Adding new User fails to save once Password Method is changed #16709

deJaya opened this issue Feb 20, 2025 · 4 comments · May be fixed by #16710
Labels
bug The issue in the code or project, which should be addressed.

Comments

@deJaya
Copy link

deJaya commented Feb 20, 2025

Issue seemingly introduced in 3.1.1-pl

When adding a new user and changing the Password Method to anything other than "Let MODX generate a password" - the radio button for "Show the new password on screen" becomes unchecked.

In this state, saving the user is unsuccessful - but no notification is presented on screen so it feels like save was successful.

Image

Checking the "Show the new password on screen" radio button solves the issue.

Since "Show the new password on screen" is the only option under "Password notification method" - it should always remain checked.

It could be argued that it could be removed, depending on what future plans there are for this section.

Environment

MODX 3.1.1-pl
Tested in 3.1.0-pl and the radio does not become unchecked.
@deJaya deJaya added the bug The issue in the code or project, which should be addressed. label Feb 20, 2025
@deJaya deJaya changed the title Adding new User fails to save once Password Method is changed 3.1.1 - Adding new User fails to save once Password Method is changed Feb 20, 2025
@deJaya
Copy link
Author

deJaya commented Feb 20, 2025

Issue is also present when editing a user.

@halftrainedharry
Copy link
Contributor

halftrainedharry commented Feb 20, 2025 •
edited
Loading

I guess this issue is related to the change introduced with #16668

It looks like when the radio button is unchecked, no value is submitted for the field "passwordnotifymethod", which then generates an error here in the validation:

revolution/core/src/Revolution/Processors/Security/User/Validation.php

Lines 89 to 92 in f279675

$passwordNotifyMethod = $this->processor->getProperty('passwordnotifymethod', null);
if (empty($passwordNotifyMethod)) {
$this->processor->addFieldError('password_notify_method', $this->modx->lexicon('user_err_not_specified_notification_method'));
}

I'm not sure, if these 4 lines of code are even necessary or could just be deleted.

@smg6511 Can you maybe take a look at this issue?

@smg6511
Copy link
Collaborator

smg6511 commented Feb 21, 2025

Taking a look...

@smg6511 smg6511 linked a pull request Feb 21, 2025 that will close this issue
Open
@smg6511
Copy link
Collaborator

smg6511 commented Feb 21, 2025

Ok, easy enough to fix. The PR I put up simply corrects the basic issue. I do agree that the "Show the new password on screen" field is of little to no use. There are some changes I have in mind that would improve this area, but that's better done in another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug The issue in the code or project, which should be addressed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix error in User validation that blocks creation of new Users smg6511/revolution
3 participants
@smg6511 @deJaya @halftrainedharry