This bug prevents this module from working properly without doing a "rebuild permissions" everytime permissions are changed on a parent that is of a different content type than its child. Upon node save, eventually the function nodeaccess_nodereference_node_access_records is called with the node that was updated as the only argument. On line 188, it begins to loop through the fields on the node looking for nodereference fields.

I think what's going on here is that we're looking for changes to a parent's permissions, and cascading them down to the child. However, the nodereference field is on the child, not the parent. This code works if the parent and child are of the same content type, but will fail everytime if they belong to different content types.

I'm really in over my head here, but I think we can put together a SQL query that will find out if any nodes refer to the node being passed in. I'll see what I can come up with.

CommentFileSizeAuthor
#4 na_nr.patch6.55 KBjustintime

Comments

danielb’s picture

I don't understand what a 'parent' and a 'child' means? It is possible my post here may shed some light: http://drupal.org/node/608936#comment-2173978

justintime’s picture

Sorry, in this case, the "child" node has the cck nodereference field pointing back to the "parent" node.

I saw your other post, and in my case where I have hundreds of photos pointing to one event type, it's not feasible to save every photo when I update permissions on an event. Rebuilding all node access permissions isn't much better.

I'm actually getting a little bit closer to the solution, but I've got a bug I can't squash yet. I've found a way to find out if the node has "children", and I then push those nodes onto the nodelist variable. This part works, but for some reason I'm getting some duplicates on inserting into node_access, and I think there might be a bug in the acquire_grants function where it's dropping some grants it shouldn't.

If I can get it working, I'll post up a patch.

danielb’s picture

OK well I think the module needs to push those affected nodes into the nodelist by default. Are you saying it's only putting them in if the node type matches? Hmm..

justintime’s picture

StatusFileSize
new6.55 KB

Wow. Talk about learning to run before you crawl. Thank god for Xdebug - I now have this thing working after about 20 minutes of testing. It's fully working with content_access (and thereby ACL), and I fixed the anonymous access cascade as well.

Also fixed is the need to either save the "child" node or rebuild, it will figure out which nodes point to the node just saved, then process those on the next init.

The final problem I had had to do with the nodeaccess_nodereference table itself. It's a copy of node_access, and in my particular case, it was not being kept in sync. I honestly couldn't find a reason for it's existence, and given that node_access can get large I figured it was a performance hit to maintain a copy of an already existing table. All this is just my reasoning for blowing it away ;-) What do you know, it worked!

I'm shot, and need to go to bed. I hope to have some more time tomorrow to clean things up and test a little more, but I'm pretty confident this is good to go. Here's my patch!

danielb’s picture

You have made quite a lot of changes not relevant to this issue, including redesigning the whole fricken module, making this a massive patch to test :(
I also think those other points you raised need more discussion, rather than just changing it and seeing that it happens to still work in your case.

justintime’s picture

Heh, yeah, you should see what I have now ;-)

It's really kinda grown into a fork, serving a similar but different purpose. I was posting the patch for your review and optional inclusion (and gpl2 compliance), not trying to step on any toes.

I'll likely finish this up and release it as a different module completely, with credits to your module. Do you want me to post a patch of the finished work?

Anyways, thanks for the hard work - I never would have been able to write this from scratch as my first module!

danielb’s picture

I just don't get how you could achieve the same functionality and do away with the duplication of the node access tables and functions. I couldn't think of a way around that - the problem was that during a normal rebuild, the first thing drupal does is delete all node access records. So how on earth do you check a user's access on the nodes while updating the node access records, if they are most likely incomplete. The only nodes that you could rely on are nodes that were processed earlier than the current node being processed - which I'm worried is the behaviour you saw while testing.

danielb’s picture

Status: Active » Fixed

This module is changing significantly and this issue may no longer apply in future

Status: Fixed » Closed (fixed)

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