Create User Record now hides the Password field and uses the check isUserRegistrationPermitted - which prevents Administrators from setting passwords for new Users

[agileware-justin]

Overview

Create User Record now hides the Password field and uses the check isUserRegistrationPermitted - which prevents Administrators from setting passwords for new Users.

See Gitlab for more deets, https://lab.civicrm.org/dev/core/-/issues/2605

Before

Password fields no longer shown. This change was introduced by #18982 and
633d512

After

Password fields are again shown.

Technical Details

Comments

The comment was "Also removes the possibility to set a user-defined password for WordPress users." - this change affects all CMS not just WordPress.

Agileware Ref: CIVICRM-1737

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[kcristiano]

As I commented on the Issue, we should respect the WP settings which send magic links. I do not think we should revert and regress.

This change affects both admin forms and front end forms and the user experience should match what WP current convetion is.

[christianwach]

I agree with @kcristiano - better to query $config->userFramework to refine this.

[christianwach]

Also, might as well wrap 'ERROR: Password mismatch' and 'Password is required' in ts() while you're at it @agileware-justin 😁

[agileware-justin]

Just re-iterating that the change in the original PR
PR #18982 the intention was to change the behaviour for WordPress only, "Also removes the possibility to set a user-defined password for WordPress users."

However, the change affected all CMS not just WordPress.

With this change, only WordPress has the changed behaviour and other CMS are no longer affected.

Other feedback incorporated.

@kcristiano still a thumbs down mate?

[agileware-justin]

The split between Gitlab and PRs is a tad annoying sometimes. Gitlab, https://lab.civicrm.org/dev/core/-/issues/2605

[kcristiano]

@agileware-justin I fixed the emoji :)

I'll test with WP. I'll also add to d7master and see what happens there. Can you confirm the expected behavior with Drupal is to ask a user for a password on a form that has registration required? Same for back end of D7?

[agileware-justin]

Can you confirm the expected behavior with Drupal is to ask a user for a password on a form that has registration required? Same for back end of D7?

The expected behaviour is how it used to work in CiviCRM for Drupal. 😃

[christianwach]

Looks good to me based on reading alone. Thanks for wrapping the text in ts() @agileware-justin.

[kcristiano]

I have done r-run on Drupal 7 and WP.

This restores behavior in Drupal prior to 5.37.0 and WP is working as intended.

Thanks @agileware-justin

This is merge-ready. @eileenmcnaughton I do think this is a regression and if we can target 5.37 it would be in order.

[demeritcowboy]

If it's a regression then this should be rebased against 5.38 first, then would be backported to 5.37. @agileware-justin are you able to rebase it? It might work to just edit the title and change the target branch but sometimes it then adds 100 bogus commits.

I have some r-code comments since it could be more OO-ey, but since this is already tested that could be a followup later.

[eileenmcnaughton]

@jusfreeman - did you spot the rebase against the 5.38 rc branch? So we can port & do a 5.37 release with this

[eileenmcnaughton]

thanks @jusfreeman

[agileware-justin]

@eileenmcnaughton please use @agileware-justin for pings - I don't use or monitor the other one you are using there

Can't maintainers just cherry pick a commit into any branch? Seems would be much easier.

[eileenmcnaughton]

@agileware-justin ok sure & no - the github UI doesn't offer us any quick & easy way to cherry-pick from one PR to another branch

[agileware-justin]

@agileware-justin ok sure & no - the github UI doesn't offer us any quick & easy way to cherry-pick from one PR to another branch

GitHub Desktop supports cherry picking from branches https://desktop.github.com/
And you can also do this in PHP Storm if you login to Github.

Here's the announcement, GitHub Desktop now supports cherry-picking
https://github.blog/2021-03-30-github-desktop-now-supports-cherry-picking/
desktop/desktop#1685

Alternately, having a FAQ for rebasing against different CiviCRM version would be useful and you could get CiviBot to refer people to it. Would save everyone's time too.

Forgot the @eileenmcnaughton

[eileenmcnaughton]

Yeah - there are tonne of different ways to do it - I would probably in this case - if it had been my branch - have done a get reset --hard against 5.38 & then cherry-picked it on & then done a force push. If I was creating a new branch / pr instead I would have checked out 5.38, cherry-picked the commit & then pushed & opened a PR

If you take a look at the links that gitbot adds above you can add whatever docs you would have found helpful as a PR.