og_moderation_node_grants() checks all groups a user belongs to, and checks all node types even though they can only belong to groups and their content. If they are a member of a lot of groups this doesn't scale well.
Proposed Solution
Instead of adding all combos let's just implement hook_node_access(), which gives us the node that we are after, it's status and it's type.
I'm not sure if this covers all scenarios but thought I should share because the query is significantly not scaling well.
This is similar to the check in node_access() function itself.
modules/node/node.module:3039
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | view_any_unpublished-2894728-12.patch | 2.32 KB | jastraat |
| #10 | view_any_unpublished-2894728-10.patch | 2.31 KB | jastraat |
| #2 | 2894728-2.patch | 2.11 KB | joelpittet |
| Add_Edit_Documentation___after.png | 123.87 KB | joelpittet | |
| Add_Edit_Documentation.png | 1.86 MB | joelpittet |


Comments
Comment #2
joelpittetHere's the patch that does the changes, lots of code deleted:)
Comment #3
joelpittetFrom an performance improvement I had ~4s queries change to ~600ms, so it was significant.
Comment #4
mark.labrecqueLooks good to me. There was a concern about the hook implementation changing to use a different hook, but as you stated in a conversation, core does a similar treatment with unpublished permissions so it should be fine.
Comment #6
shenzhuxi commentedCommitted. Thanks!
Comment #7
joelpittetSweet! thanks @mark.labrecque and @shenzhuxi!
Comment #9
jastraat commentedI'm sorry - but this patch does not actually work.
http://www.drupalcontrib.org/api/drupal/contributions%21og%21og.module/f...
og_user_access takes an argument of $gid NOT the $nid of the node being viewed.
Right now, this will only work if the 'view any unpublished' happens to be the group type.
Comment #10
jastraat commentedUpdated patch that needs review. This gets all the groups associated with the node being viewed and then checks if the user has view any permission in them.
Comment #11
shenzhuxi commented@jastraat #10 Error: corrupt patch at line 72
Comment #12
jastraat commentedComment #13
joelpittetI'm not sure what the problem you have with this is, could you be more clear or write some steps where it doesn't apply so that we could write a test case?
Comment #14
jastraat commentedYou're passing the node ID of the node that you are viewing to og_user_access() but og_user_access takes an argument of the GROUP node id. Not the id of the individual piece of content.
So group level roles that have 'view any unpublished __' will not actually be able to see unpublished nodes in their groups.
Comment #15
joelpittetSince this issue is closed, you should likely create a follow-up to this issue so that the commit credit is correct and since this is already committed the patch should be off the current -dev which at a glance it doesn't appear to be.
You sound like you are correct, but we need a test also to prove that. I'll gladly help with a test, just let us know the new issue.
Comment #16
jastraat commented@joelpittet I rolled the patch against the 7.x-2.x branch. Could you be more specific as to which branch you would like this to be rolled against?
Comment #17
jastraat commentedNew issue: #2901558: View any unpublished group permissions no longer work.
Comment #18
joelpittetThank you, I think you fixed the patch in the other issue.