Node access not updated on default change

This bug also exists with changes to the global default.

Marked #350204: Forum taxonomy denied access, users can still see nodes as duplicate of this issue.

Attached is just a whitespace cleanup of the submit handler to make troubleshooting easier. (Edit: already tested and committed to CVS).

One issue I can see is that a vid of 0 (representing the global default) gets passed to _taxonomy_access_get_nodes_for_vocabulary(), but that function does not handle a vid of zero. If the global default is changed, presumably nodes not controlled by the configurations that follow it should be flagged for update?

Also, the submit handler doesn't check which values have changed that I can tell; it just updates everything for every row (including nodes for each term), which will probably result in far more queries than needed. It would be better to check against the initial state of the form, and possibly to refactor so that we don't update the same node multiple times.

This issue has turned out to be a lot more complicated than I expected. Here's what I suggest:

• Refactor the submit handler to only run updates for rows the user has changed.
• Instead of processing node updates for each row, collect affected nids and run one update at the end.
• A change to the global default should set node_access_needs_rebuild(). (The other way was just too complicated.) (When this flag is set, the new and improved _taxonomy_access_node_access_update() will not bother to update individual nodes.)

This should reduce the overhead as much as possible while still ensuring that changes to defaults are processed correctly.

This patch does the following:

• Changes the admin form submit handler so that when "save all" is clicked, it checks to see if a row has changed before processing updates for that row.
• Fixes broken query in _taxonomy_access_get_nodes_for_vocabulary() so that results are actually returned when a role id is specified.
• Modifies taxonomy_access_defaults_update() so that it flags node_access_needs_rebuild() when passed the global default and only runs _taxonomy_access_node_access_update() for real vocabularies.

I did not add the refactoring to collect affected node ids and process them all at once since it is not directly related to this issue; I will open a separate issue for that. Edit: #732844: Refactor submit handler to process nodes all at once (rather than per row).

I tested the patch above, and it seems to work fine:

• When the global default is changed from its initial value, I get the appropriate message indicating that node access needs to be rebuilt, but not when it is not changed.
• Permissions for both vocabulary defaults and specific terms are saved properly on submission.
• The appropriate nodes are updated for each vocabulary default.
• When I follow the "Steps to reproduce" above, a node access rebuild is no longer necessary to get the correct behavior.

Committed to 6.x-1.x-dev:
http://drupal.org/cvs?commit=337404

This patch introduces a bug that prevents the global default row from being inserted when a role is first enabled. Unlike other defaults, the global default is not added to the database for the first time until "Save all" is clicked. I'll come up with a fix shortly.

Attached patch inserts the row as soon as the user clicks 'enable' (which is more consistent anyway).

Patch applies fine and fixes the issue.

Fix committed to dev:
http://drupal.org/cvs?commit=340250