Posted by hefox on June 27, 2012 at 1:24am
4 followers
| Project: | Organic groups |
| Version: | 6.x-2.x-dev |
| Component: | og.module |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
While it adds flexibility, other areas don't check it before displaying links, so it negativally effects performance for little gain.
Comments
#1
#2
This patch breaks compatibility with http://drupal.org/project/og_perm.
#3
og_perm should likely be overriding the views handlers or og should have some sort of access system. menu_get_item is a hack :(
#4
Since I'm using the version 6.x-2.1, I'm not sure this will apply to your patch.
But I'm getting the following notice "Notice: Trying to get property of non-object in og_menu_access_unsubscribe() (line 294 of sites/dev/modules/contrib/og/og.module)."
I figure we should pass a object containing the property uid since the function og_menu_access_unsubscribe expect a object.
$account = new stdClass();$account->uid = $uid;
if (og_menu_access_unsubscribe($node, $account)) {
EDIT:
FYI. Drupal core issue for menu_get_item. http://drupal.org/node/1167626
#5
Core uses menu_get_item in situations like getting the current menu item where it's a perfectly fine thing for that, but for something like this, it's really unnecessary and actually caching is not the best fix -- it'll be caching per path, e.g. per user per og path o.o!; that's a helluva lot to cache when you got a lot of users.
I have patch on top of patch atm; could you paste what line is 294 is?
#6
Here is the line 294.
// Unsubscribee must already be a member or pending member.if (!og_is_group_member($group_node, FALSE, $account->uid)) {
It also happen at line 309.
// Only admins can remove another memberif ($account->uid != $user->uid && !og_is_group_admin($group_node)) {
And at line 319.
// Group manager may not unsubscribeif ($group_node->uid == $account->uid) {
#7
Oh f*ck, forgot that I was running/counting on another patch. See #1661622: Avoid user_loads for og_menu_access_unsubscribe; this prevents uneeded user_loads for best performance.
#8
Here what the patch should be until the patch for issue #1661622 is apply to dev.
#9
The last submitted patch, og_views-remove_menu_get_item-1661704-8.patch, failed testing.
#10
Re-submit patch.
#11
#12
After more testing, I realize the patches in this issue are not an improvement since user_load get call instead of menu_get_item. Until your other issue is fixed (#1661622: Avoid user_loads for og_menu_access_unsubscribe) this patch is useless.
EDIT:
I think the issue #1003604: Avoid repetitive calls to user_load() in og_is_group_member would be more useful here then the issue #1661622: Avoid user_loads for og_menu_access_unsubscribe.
#13
I'd suggest using all 3 patches; that's what we're doing.
#14
After installing both patches I realized my main problem is that some users have to many groups. When calling og_get_subscriptions php exceed the memory limit.
#15
Yea, we have our memory limit up high due to reasons like that. Og is not the most scalable of modules, and users with too many groups is defiantly a problem.
#16
Quick update to patch 0; was missing a $token for approve url
#17
Update of my patch with the missing $token.
#18
The last submitted patch, og_views-remove_menu_get_item-1661704-17.patch, failed testing.
#19
Sorry, my mistake. When I edit the patch with notepad ++ for minor change like this, the patch always fails to apply.
This one should work.
EDIT: Ignore this patch it is the wrong patch.
#20
Thsi is the patch I should have submit in post #19.
#21
#22
Update for #1661622: Avoid user_loads for og_menu_access_unsubscribe update