Got this error in the latest dev.

In flag.module it is:
function flag_flag_access_multiple($flag, $content_ids, $account)

And in flag.inc:
function type_access_multiple($content_ids, $account = NULL)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

MiSc’s picture

Status: Active » Needs review
FileSize
1.55 KB

In the attached patch, I changed it to:
function type_access_multiple($flag, $content_ids, $account)

MiSc’s picture

Seems that the $flag variable is missing in access_multiple, added global flag to that function. Should be considered a temp fix I think...

apotek’s picture

Status: Needs review » Reviewed & tested by the community

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?

apotek’s picture

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.

caw67’s picture

Version: 7.x-2.x-dev » 7.x-2.0-beta7
Assigned: Unassigned » caw67
joachim’s picture

Version: 7.x-2.0-beta7 » 7.x-2.x-dev
Assigned: caw67 » Unassigned
Status: Reviewed & tested by the community » Needs work

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).

apotek’s picture

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?

joachim’s picture

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) {

joachim’s picture

Version: 7.x-2.x-dev » 7.x-3.x-dev
Issue tags: +Novice, +Needs backport to D7
Marlon’s picture

Sorry, still getting this error.

joachim’s picture

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

socketwench’s picture

Status: Needs work » Needs review
FileSize
495 bytes

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().

joachim’s picture

Status: Needs review » Fixed

Thanks for the patch!

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

joachim’s picture

Version: 7.x-3.x-dev » 7.x-2.x-dev
Status: Fixed » Patch (to be ported)
drupalmonkey’s picture

Status: Patch (to be ported) » Needs review
FileSize
810 bytes

Patched against 7.x-2.x.

joachim’s picture

Status: Needs review » Fixed

Thanks!

Applies to both 7x2x and 6x2x.

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