Fails to respect core 'view own unpublished content' permission

We confirm that when this module is enabled, it overrides core's 'view own unpublished content' permission. I changed status to bug because it seems to me that this breaks core in that respect.

Attached patch checks that core permission and sets up grants to replicate it.

Encountered this. Updating status now that there's a patch.

Hopefully I'll have a chance to review soon.

It works for me too.

#1
Works fine for me too! Thank you very much

It works for me as well. Just module disable/enable was needed.

Patch worked like a charm. Hope to see this committed soon.

Works for me, however I have to rebuild permissions anytime a new node is added, otherwise the same problem applies. Anyone else having this problem?

Odd. And no, I haven't experienced that myself.

Is this patch has been committed yet ?

+1 for RTBC

Worked great for me. As mentioned in #7, I had to disable and enable the module again, then rebuild permissions.

Patch in #1 looks good. Elevating this to Critical, as it renders part of a core permission unusable. ("Critical" is borderline, but it's definitely at least "Major".)

#1 works good.

#1 Confirmed.

People are mentioning that they needed to rebuild node access permissions to get this working. We should probably include an update hook in this patch to trigger a user to rebuild permissions.

Weird spacing...

(wrong thread, ignore)

Looks good. Thank you.

any reason the install file update hook couldn't simply call node_access_rebuild(); instead of node_access_needs_rebuild(TRUE);

it is super annoying UX to have code, which is capable of detecting that something needs to be done, bug the user to actually do it.

Because that operation could potentially take a really look time if there are thousands or tens of thousands of nodes on a site. I think in such a case, a maintainer of a site like that would rather rebuild the node access permissions when they want (even if that may be right after they update the module).

So, you are saying that the default behaviour is catering to an edge case...

Besides, I can't imagine anyone actually wanting two steps for this process, even if it takes a long time. I also can't imagine a maintainer wanting to leave the site in an inconsistent state for even a second. i.e. in a state where the system knows content access needs to be rebuilt, but has not been rebuilt (ie. a broken state)

I guess my major issue is that the current behaviour makes Drupal look incompetent. There is no indication to the user that the system is trying to do them a favour by not running what it knows needs to be run.

At the very least, it should run immediately for sites with less than a 1000 nodes (or some such threshold) Maybe there should be a setting for the threshold (default to 10,000), so that the handful of maintainers who do need to split the process can choose to do so without making it a pain in the butt for the rest of us. The message displayed to the user should also say WHY access was not rebuilt automatically, so the user with a huge site can choose whether they want that behaviour.

The more I think about it, the more I think node_access_needs_rebuild() should just be removed from Drupal altogether. There are other better options. Run it in batches via cron for example.

Having it run automatically for < 1000 nodes or so seems logical to me. Looks like node_access_build() has a batch parameter as well, so perhaps that could be used regardless. A new issue should be opened for this since this issue is already closed.