Breaking off from #720876: Third parameter of user_module_invoke() should be object but user_register_validate() calls it with an array instead. as requested by pwolanin on IRC.

[15:41] <pwolanin> pillarsdotnet:  can you add to the doxygen of http://api.drupal.org/api/drupal/modules--user--user.module/function/user_module_invoke/6
[15:49] <pwolanin> pillarsdotnet:  please add some doxygen for the function I linked to indicate it has to be an object
[15:49] <pwolanin> pillarsdotnet:  other than that it's rtbc
Files: 
CommentFileSizeAuthor
#18 user_module_invoke-docs-1192178-18.patch1.53 KBpillarsdotnet
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#16 user_module_invoke-docs-1192178-16.patch1.88 KBpillarsdotnet
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#8 php-test-program.txt940 bytespillarsdotnet
#8 test-program-output.txt866 bytespillarsdotnet
#6 user_module_invoke-document_parameters-1192178-6.patch1.61 KBpillarsdotnet
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch user_module_invoke-document_parameters-1192178-6.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#1 user_module_invoke-document_parameters-1192178-1.patch1.26 KBpillarsdotnet
PASSED: [[SimpleTest]]: [MySQL] 31,427 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new1.26 KB
PASSED: [[SimpleTest]]: [MySQL] 31,427 pass(es).
[ View ]

Patch adds documentation for parameters.

Status:Needs review» Needs work
Issue tags:-needs backport to D6, -needs backport to D7

The last submitted patch, user_module_invoke-document_parameters-1192178-1.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+needs backport to D6, +needs backport to D7

Status:Needs review» Needs work

To me, the description of $type is a bit confusing, since it's not really the hook name, but the hook name without 'user_'.

Other than that, it looks good.

Oh. Also, in D7/8, the paragraph above where your patch is states that it invokes hook_user(), which is not the hook name any more in D7/8. That should also be fixed.

Status:Needs work» Needs review
StatusFileSize
new1.61 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch user_module_invoke-document_parameters-1192178-6.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

Corrected as suggested, and also added an explicit list of valid choices for the $type parameter. Considered adding @see http://api.drupal.org/api/search/8/hook_user_, but that would be misleading, as there are 17 API functions whose names begin with "hook_user_" but only 5 that have signatures compatible with user_module_invoke().

So this is interesting. On another issue yesterday, I traced down module_invoke_* and realized that actually the args *are* passed by reference (see http://drupal.org/node/1193514#comment-4629398). So that line that says:

* We cannot use module_invoke() for this, because the arguments need to
* be passed by reference.

is actually incorrect. Maybe there is no good reason for user_invoke, and maybe there is a good reason, but that isn't the reason.

Any insight?

The rest of the doc in the patch looks really good to me...

StatusFileSize
new866 bytes
new940 bytes

I wrote a test program which I believe shows that the comment is indeed correct, at least for my version of PHP.

Status:Needs review» Reviewed & tested by the community

you are right. I was confused. Within user_module_invoke or module_invoke, everything is pass by reference via PHP's call_user_func functions, but module_invoke itself doesn't take its args by reference.

In which case, the patch in #6 is fine. D8/d7, and then mark as to be ported for d6 please.

Within user_module_invoke or module_invoke, everything is pass by reference via PHP's call_user_func functions

At least for some versions of PHP < 5.3.0

Whatever. In any case, the patch is fine, so let's get it in.

Version:8.x-dev» 6.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Committed to 8.x and 7.x. Thanks!

Marking to be ported to D6.

Issue tags:-needs backport to D7

Status:Patch (to be ported)» Needs review
Issue tags:-needs backport to D6

Status:Needs review» Needs work
Issue tags:+needs backport to D6

The last submitted patch, user_module_invoke-document_parameters-1192178-6.patch, failed testing.

Category:task» bug
Status:Needs work» Needs review
StatusFileSize
new1.88 KB
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

Patch backported.

Status:Needs review» Needs work

In Drupal 6, the hooks are all hook_user($op = 'whatever'), not hook_user_cancel() etc. So the patch cannot be simply copied from Drupal 7.

Status:Needs work» Needs review
StatusFileSize
new1.53 KB
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

Corrected.

Status:Needs review» Reviewed & tested by the community

That looks quite reasonable to me. Thanks!

Status:Reviewed & tested by the community» Fixed

Thanks, committed, pushed.

Automatically closed -- issue fixed for 2 weeks with no activity.