Warning: Parameter 1 to ldap_user_user_presave() expected to be a reference, value given in Drupal\Core\Extension\ModuleHandler->invokeAll() (line 393 of core/lib/Drupal/Core/Extension/ModuleHandler.php).
•Warning: Parameter 1 to ldap_user_user_update() expected to be a reference, value given in Drupal\Core\Extension\ModuleHandler->invokeAll() (line 393 of core/lib/Drupal/Core/Extension/ModuleHandler.php).

CommentFileSizeAuthor
#19 interdiff-2727653-19.patch8.48 KBgrahl
#10 wrong_parameters_in-2727653-10.patch17.58 KBqueenvictoria
#9 wrong_parameters_in-2727653-9.patch21 KBgrahl
#7 wrong_parameters_in-2727653-7.patch20.97 KBgrahl
#6 wrong_parameters_in-2727653-6.patch20.73 KBgrahl
#4 wrong_parameters_in-2727653-4.patch4.31 KBgrahl
#3 wrong_parameters_in-2727653-3.patch10.49 KBgrahl
#2 wrong_parameters_in-2727653-2.patch5.68 KBgrahl
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

grahl created an issue. See original summary.

grahl’s picture

grahl
he/him
Location Zürich
CreditAttribution: grahl at Namics commented
FileSize
wrong_parameters_in-2727653-2.patch5.68 KB

The patch updates the helper functions for the current format, no guarantees that the dependent functionality actually works.

grahl’s picture

grahl
he/him
Location Zürich
CreditAttribution: grahl at Namics commented
FileSize
wrong_parameters_in-2727653-3.patch10.49 KB

Next patch for further issues

grahl’s picture

grahl
he/him
Location Zürich
CreditAttribution: grahl at Namics commented
FileSize
wrong_parameters_in-2727653-4.patch4.31 KB

Attached is an additional patch which fixes issues with the insert hook. Due to the number of dependent patches I'm having trouble producing a combined patch, please apply #3 and #4 sequentially. Should you merge other issues in the meantime into -dev, I can of course produce an updated single patch.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext for Department of Justice & Community Safety, Victoria commented 
  1. +++ b/ldap_user/ldap_user.module
@@ -755,24 +755,18 @@ function ldap_user_form_register_form_submit2(&$form, FormState $form_state) {
+function ldap_user_ldap_exclude(\Drupal\user\UserInterface $account = NULL) {

@@ -945,11 +939,12 @@ function ldap_user_ldap_provision_semaphore($action, $op, $value = NULL, $reset
+function ldap_user_user_login(\Drupal\user\Entity\User $account) {

    Please import the FQDN with a use statement and use the short class name. 

  2. +++ b/ldap_user/ldap_user.module
@@ -1003,13 +998,13 @@ function ldap_user_user_login($account) {
+function ldap_user_user_insert($account) {

    Can you type-hint this too 

  3. +++ b/ldap_user/ldap_user.module
@@ -1003,13 +998,13 @@ function ldap_user_user_login($account) {
+  $new_account_request = (boolean)($user->id() == 0 && $account->access == 0 && $account->login == 0); // check for first time user

    We can use $user->isNew() here 

  4. +++ b/ldap_user/ldap_user.module
@@ -1051,19 +1046,19 @@ function ldap_user_user_insert(&$user_edit, $account, $category) {
+function ldap_user_user_update($account) {

    Can you type-hint here too 

  5. +++ b/ldap_user/ldap_user.module
@@ -1051,19 +1046,19 @@ function ldap_user_user_insert(&$user_edit, $account, $category) {
+  /* @var $ldap_user_conf \Drupal\ldap_user\LdapUserConf */

    these two are back to front 

  6. +++ b/ldap_user/ldap_user.module
@@ -1093,23 +1088,21 @@ function ldap_user_user_update(&$user_edit, $account, $category) {
+function ldap_user_user_presave($account) {

    Let's type-hint this to UserInterface 

  7. +++ b/ldap_user/ldap_user.module
@@ -1093,23 +1088,21 @@ function ldap_user_user_update(&$user_edit, $account, $category) {
+  //debug("ldap_user_user_presave, category=$category"); debug($user_edit); debug($account); // ldap_user_user_insert, category='. $category . 'account->status = ' . $account->status);

    Let's remove this while we're here

grahl’s picture

grahl
he/him
Location Zürich
CreditAttribution: grahl at Namics commented
FileSize
wrong_parameters_in-2727653-6.patch20.73 KB

Thanks for your feedback larowlan, updated combined patch is attached. I added a few more hints while I was at it plus the error from ldap_help. I presume you wanted UserInterface on all user hooks? I'm not 100% certain what the difference there is but harmonized them as such.

grahl’s picture

grahl
he/him
Location Zürich
CreditAttribution: grahl at Namics commented
FileSize
wrong_parameters_in-2727653-7.patch20.97 KB

Minor update to ldap_help, I missed an l().

grahl’s picture

grahl
he/him
Location Zürich
CreditAttribution: grahl at Namics commented
grahl’s picture

grahl
he/him
Location Zürich
CreditAttribution: grahl at Namics commented
FileSize
wrong_parameters_in-2727653-9.patch21 KB

Sorry, one last thing, runs fine for me now.

queenvictoria’s picture

queenvictoria CreditAttribution: queenvictoria at Holly commented
Status: Needs work » Needs review
FileSize
wrong_parameters_in-2727653-10.patch17.58 KB

I've rerolled this after https://www.drupal.org/node/2726479. Could someone take a look? It was a lot of manual work.

Status: Needs review » Needs work

The last submitted patch, 10: wrong_parameters_in-2727653-10.patch, failed testing.

The last submitted patch, 10: wrong_parameters_in-2727653-10.patch, failed testing.

The last submitted patch, 10: wrong_parameters_in-2727653-10.patch, failed testing.

The last submitted patch, 10: wrong_parameters_in-2727653-10.patch, failed testing.

The last submitted patch, 10: wrong_parameters_in-2727653-10.patch, failed testing.

The last submitted patch, 10: wrong_parameters_in-2727653-10.patch, failed testing.

The last submitted patch, 10: wrong_parameters_in-2727653-10.patch, failed testing.

queenvictoria’s picture

queenvictoria CreditAttribution: queenvictoria at Holly commented
Status: Needs work » Needs review
grahl’s picture

grahl
he/him
Location Zürich
CreditAttribution: grahl at Namics commented
FileSize
interdiff-2727653-19.patch8.48 KB

Thanks for the effort, the patch looks fine to me with three minor issues:
- RouteMatchInterface is declared absolutely in ldap_user_help()
- $user->id == 0 is used instead of $user->isAnonymous()
- ldap_authentication_show_reset_pwd() no longer has a hint, but I'm not sure if that even is a full user class

Attached is a diff between a) patch #9 before phpcbf was committed and phpcbf manually run afterwards and b) patch #12 applied to 8.x-3.x from today, where you can see those.

  • grahl committed 0e42322 on 8.x-3.x 
    Issue #2727653 by grahl, queenvictoria, larowlan: Wrong parameters in...
grahl’s picture

grahl
he/him
Location Zürich
CreditAttribution: grahl at Namics commented
Status: Needs review » Closed (fixed)

Committed per feedback in #2730841 based on patch #10 with the first two points I mentioned above. Also added two more type hints.