Since 2.8 version, we can no more set manually the context.

For example:
og_context('node', node_load(1));

Before, og_context() was returning the node 1, now, it is returning FALSE.

Github issue opened too: https://github.com/Gizra/og/issues/123

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

SylvainM created an issue. See original summary.

amitaibu’s picture

Priority: Major » Normal

Does the user have access to that node?

SylvainM’s picture

Yes, all users (anonymous or logged in) have access to the node.

Thanks to defr, I tried to remove the id of cache keys in og_context() function and it worked better for me.

DeFr’s picture

Not sure why $id was added to the $cache_keys that should be used when doing a lookup in the static $context array, but it breaks the functionnality of manually forcing a group kinda badly.

Call flaw looks like:

  1. Code forces the context: og_context('node', $node)$context['node:NID:UID:TRUE'] = $node
  2. Code tries to determine the context og_context('node') → Look is done for $context['node::UID:TRUE'] ; that didn't get set previoulsy, so context is determined through the context handlers, and the forced context set before is ignored.

Getting rid of the $id part of the cache key solves the problem for the usual case of og_context being called without an explicit account, and thus defaulting to the currently logged in user. At that point, the cache key used for both the lookup and the retrieval is 'node:UID:TRUE' and things start to work.

DeFr’s picture

Status: Active » Needs review
FileSize
4.4 KB

Something along the lines of the patch attached seems to work for us.

Not sure what to do with the TODO though. For the second one, fixing it is easy but might break backward compatibility, so didn't fix it yet. For the first one though, fixing it properly is tricky ; the least disruptive would probably be to add an optional $account parameter at the end, check the various groups instead of the first one, and return the first group the user has access too ; ideally, I think all the provider should be made aware of the account the context is being determined for, but that's would need a lot of changes.

Status: Needs review » Needs work

The last submitted patch, 5: 2724643-5-og_forced_context.patch, failed testing.

DeFr’s picture

Status: Needs work » Needs review
FileSize
4.42 KB

array_key_exists() doesn't like being passed a boolean, so we need to convert $check_access to an integer in the cache key. Attached patch should be green.

SylvainM’s picture

Status: Needs review » Reviewed & tested by the community

Patch is working fine for me, thanks

johnennew’s picture

Priority: Normal » Major

I'm raising this to major, 7.x-2.9 completely broke my site and needed the patch in #8 to fix!

johnennew’s picture

I'm getting stuck in a recursive loop occasionally with the patch #7 which I've confirmed stops by removing it.

Trying to see if its my code or this patch thats at fault, as far as I can see entity_access() is called in og_context() and a hook_node_access() call asks for the context via a call to og_context() which in turn triggers the hook_node_access() and og_context() etc ...

johnennew’s picture

probably me, I've passed not to do the access check in during the hook_node_access - makes me feel like I'm doing something wrong somewhere ... anyways, the patch works as far as I can tell.

amitaibu’s picture

Status: Reviewed & tested by the community » Needs work

Thanks for working on this, but this requires a simpleTest to confirm the functionality and that there's no regression.

Chalk’s picture

On first look, the #7 works for me. Thanks!

rbrandon’s picture

The patch should remove the og_context_is_init function as well if it is going to remove all references to it. I am not sure how it is currently used now anyway. It appears that this version changes the structure of the returned context, is that correct?

Poindexterous’s picture

@DeFr thank you for this patch, it helped us tremendously in upgrading to 2.9. Once we upgraded we did some testing and noticed all of our views that listed content by group didn't seem to be picking up their groups audience contextual filter any longer (ex: /group-abc/articles no longer filtered content accordingly), and it also caused our og menus (that were placed on group content pages by the context module) disappear- even for admin account roles with with the "Bypass content access control" permission. Your patch resolved both of these problems with 2.9, and now all seems back to normal.

rooby’s picture

+1 for this update broke my site and this patch fixed it.

alesr’s picture

This is probably related.
OG 7.x-2.8 broke the default behaviour of og_context() function and a lot of sites broke causing numerous issues about this created in the issue queue.

https://www.drupal.org/node/2720403
https://www.drupal.org/project/og/issues/2720337
https://www.drupal.org/node/1952980
etc

og_context() should behave as it did before $check_access = TRUE was added to og_context($group_type = 'node', $group = NULL, $account = NULL, $check_access = TRUE)
It was a good addition, but the default should stay FALSE not to break the API.

It's also all in Github https://github.com/Gizra/og/issues/121 and Pull request here https://github.com/Gizra/og/pull/122.

The patch to fix that in OG Views is already merged: https://github.com/Gizra/og/pull/131

Anonymous’s picture

Thanks for the patch.

I applied it against 7x2.9 and this fixes the problem of setting the og_context('node', $group) was fixed.

Anonymous’s picture

Unfortunately the patch broke something and I started getting internal server errors while accessing forms where I have entity reference fields with select options coming from a view with contextual filter: current group from og_context

rooby’s picture

@pintoflager:

What are the error messages for your internal server errors? They should help to identify the problem.

Anonymous’s picture

Hi @rooby,

It's the '500 - Internal Server Error' without any more specific log entry. Not at least in my siteground cpanel error log.
I tried clearing caches from the site + browser but the same result.

@alesr,
I tried setting access to false and I'm still unable to set: og_context('node', node_load(100))

alesr’s picture

We agreed with @amitaibu to keep the strict approach in OG context.
That means you need to change your code where you are using og_context() function.

Try with:

og_context('node', node_load(100), NULL, FALSE)

$check_access is FALSE which should behave as the "old" og_context() function used up to OG 7.x-2.7.

Anonymous’s picture

@alesr

I tried setting $check_access = false in function itself and also as you suggested in your comment, unfortunately both fail.
when I print $context from the part of the function where new context is set:

if (!empty($group)) {
.
.
.
$context[$cache_key] = array('group_type' => $group_type, 'gid' => $id);
    print_r($context[$cache_key]);

I get the right context in an array that looks like this:
Array ( [group_type] => node [gid] => 2600 )

When I print_r $context[$cache_key] from the end of the function array looks like this:
Array ( ) Array ( [group_type] => node [gid] => 2600 )

And context is still not set to group with nid 2600 even though function returns correct array.

Finally when I manually visit the group the array printed from the end of og_context() function returns this:
Array ( [group_type] => node [gid] => 2600 )
Which is the same output as from the part of the function where new context can be set.

I commented out

// Set the context to array, so we can know this function has been already
// executed.
$context[$cache_key] = array();

that created empty array in front of the returned array but that didn't have any impact on returning the context.

Any thoughts?

alesr’s picture

If you try it with OG 7.x-2.7, does it work?

Anonymous’s picture

@alesr, yep, with 2.7 works like a glove

alesr’s picture

Ok, I would suggest you step through with a debugger to find where your breaking point is.
You are getting "500 - Internal Server Error" so it check your web server logs too.

Anonymous’s picture

I took something from patch in comment #7 and added that to 7x2.9 og_context.module

Now it works for me. I don't know if it breaks something for somebody else.

Majority of the patch changes are comments as I tried to figure out how the system works.

Anonymous’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 27: og_context_cache_problem.patch, failed testing. View results

joseph.olstad’s picture

Status: Needs work » Needs review

Patch #7 still applies cleanly against head.

Patch #27 is garbage, removing it.

Anonymous’s picture

@joseph.olstad please test the problem in #19 in your install.

This part of the garbage patch:

  // Clear the static cache for this group type when forcing a group,
  // and then store it for later use.
  if (!empty($group) && og_is_group($group_type, $group)) {
    unset($context[$group_type]);
 
    foreach (range(0, 1) as $access) {
      $context[$group_type]['detected_context'][$access] = $group;
    }
  }
  
  // If a group was found previously for this group_type and access checking
  // type, use it.
  if (!empty($context[$group_type]['detected_context'][$check_access])) {
    $group = $context[$group_type]['detected_context'][$check_access];
  }

fixed the manual setting of og_context and also prevented the errors with forms where entity reference field options come from a view.

If you find the errors (#19) in forms using the patch from #7 you may clean up the garbage patch, create it against the latest dev and see if it works for you.

Anonymous’s picture

Try this. I created the patch against the latest dev but didn't test it as I'm using patched version of 7x2.8.