-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
(dev/backdrop#77) Fix fatal error with password validation #25371
Conversation
(Standard links)
|
Test-failures for |
@demeritcowboy seems like the failures are not related, but not 100% sure. |
The test fails are everywhere since last week. I had posted in chat and will follow up. |
jenkins retest this please |
1 similar comment
jenkins retest this please |
Looks good now. |
@@ -198,10 +198,16 @@ public static function formRule($fields, $files, $form) { | |||
$params = [ | |||
'name' => $fields['cms_name'], | |||
'mail' => $fields[$emailName], | |||
'pass' => $fields['cms_pass'], |
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.
This gives an undefined key error if the cms doesn't allow the user-entered password. But I'd be ok to fix that in the other PR after. Since either way you'll need to fix merge conflicts in one of the PRs, I'll let you decide which one you'd rather do first.
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.
@demeritcowboy okay I'll make a note of this and fix this in the other PR.
Fixes https://lab.civicrm.org/dev/backdrop/-/issues/77
It ensures that Backdrop's password validation (if enabled) will get the password field it expects: a string not an array.
And it also adds the ability for Backdrop (and other CMSes) to validate the password field if it appears on a CiviCRM profile form. (Otherwise CMS validation happens too late).