Declaration of flag_node::type_access_multiple() should be compatible with that of flag_flag::type_access_multiple()

I applied this patch cleanly to flag.inc, and my notices went away. this looks good to me, though, as MiSc notes, having to declare $flag as global is less than ideal, but I think that would be work best left to another issue?

I'm thinking about this issue about the global $flag, and it looks more and more to me like this code

The more I look at it, it seems there is no way to resolve this issue with the $global. flag_flag_access_multiple() actually uses the $flag that is passed (although it's evidently broken in current dev since whatever would be passed from flag_* classes would be missing the flag param (until this patch is applied).

The alternative to MiSc's patch would be to change this in flag.module

function flag_flag_access_multiple($flag, $content_ids, $account) {

to this:

function flag_flag_access_multiple($content_ids, $account) {
global $flag;

Looks like some thought might need to be put into how global $flag is intended to be used across this module. Until that can be done, I think this patch is the best step forward for the moment. I think it's ready to commit.

patch #2 works for me http://drupal.org/node/1475040#comment-5709920

Neither of these approaches are correct, I'm afraid!

Let's review what we have:

- flag_flag_access_multiple($flag, $content_ids, $account) is a hook implementation.

- type_access_multiple($content_ids, $account) is a method on a flag class. It doesn't need the $flag -- because it *is* the current flag. Nor does it need the global $flag, for the same reason. The current flag is $this.

So these two functions don't have anything to do with each other in terms of the PHP's perception of them and both of these definitions are fine.

The problem is that the different implementations of the method don't match, as grep shows:

523:  function type_access_multiple($content_ids, $account) {
1324:  function type_access_multiple($content_ids, $account = NULL) {
1488:  function type_access_multiple($content_ids, $account) {
1623:  function type_access_multiple($content_ids, $account) {

The second one puts the second parameter as optional -- that's what PHP complains about, I imagine (though I'm not getting that error but it could be because my error level is different).

Thank you. You might be on the right track, but you're working off a later codebase. None of my type_access_multiple() signatures include a NULL as a default $account value.

Maybe this ticket is out-of-date with latest devs?

Can you try with the latest dev please?

I'd say that this is wrong and needs fixing anyway:

> 1324: function type_access_multiple($content_ids, $account = NULL) {

Sorry, still getting this error.

Yes, that's not surprising. Someone needs to take the time to write a patch to fix this.

If I understood the thread correctly, the signature of type_multiple_access() needs to be synced up across implementations. The only one I found that was different was in flag_node:type_multiple_access().

Thanks for the patch!

(Dunno where all the novices are though! :/)

Patched against 7.x-2.x.

Thanks!

Applies to both 7x2x and 6x2x.