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.

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/core#4074) Make CRM_Core_BAO_CMSUser CMS agnostic #25323

Merged
merged 1 commit into from
Jan 30, 2023
Merged

(dev/core#4074) Make CRM_Core_BAO_CMSUser CMS agnostic #25323

merged 1 commit into from
Jan 30, 2023

Conversation

herbdool
Copy link
Contributor

@herbdool herbdool commented Jan 11, 2023

Fixes https://lab.civicrm.org/dev/core/-/issues/4074. Splits up the logic into each CMS in the userSystem. This functionality is used when using a profile to create a user account.

I've tested it with creating a user account and also checking the username which happens via ajax on the profile (on Drupal 9). Both still work.

@civibot
Copy link

civibot bot commented Jan 11, 2023

(Standard links)

@civibot civibot bot added the master label Jan 11, 2023
@herbdool
Copy link
Contributor Author

jenkins retest this please

@civicrm-builder
Copy link

Can one of the admins verify this patch?

@herbdool
Copy link
Contributor Author

Turns out I've got a Backdrop-related bug to fix as well, which would benefit from this being merged first and built on top. (I may have to make it a separate PR and rework it again if/when this PR gets merged).

Any interest in reviewing? @demeritcowboy @eileenmcnaughton

@demeritcowboy
Copy link
Contributor

Sure I can take a look just probably won't be until the weekend.

@herbdool
Copy link
Contributor Author

Thank you @demeritcowboy!

@herbdool
Copy link
Contributor Author

@demeritcowboy This is my follow-up to this one: #25371 (or could be done first if this one gets stuck).

@herbdool herbdool closed this Jan 27, 2023
@herbdool
Copy link
Contributor Author

Oops. I accidentally closed this PR. I'll see if I can update/reopen it or I'll create a replacement.

@demeritcowboy
Copy link
Contributor

Ah git. The old state of the tree should still be at cd46160, e.g. https://github.com/civicrm/civicrm-core/tree/cd46160, if needed.

@herbdool herbdool reopened this Jan 27, 2023
@herbdool
Copy link
Contributor Author

@demeritcowboy thank you. Tests are now running again.

@eileenmcnaughton
Copy link
Contributor

@demeritcowboy this has passed now - it looks like your review has been addressed?

@demeritcowboy
Copy link
Contributor

I didn't fully review it the first time and haven't re-reviewed this yet after being combined with the other PR.

@demeritcowboy demeritcowboy merged commit 5e400dd into civicrm:master Jan 30, 2023
@demeritcowboy
Copy link
Contributor

  • General standards
    • [r-explain] PASS
    • [r-user] PASS
    • [r-doc] PASS
    • [r-run] PASS
      • if ($is_drupal || $is_other_drupal) {
        PASS
        } elseif ($is_wordpress && (!is_drupal)) {
        PASS
        } elseif ($is_drupress) {
        ??
        }
  • Developer standards
    • [r-tech] PASS
    • [r-code] PASS
    • [r-maint] N/A
    • [r-test] PASS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants