Unsetting form elements produces errors. Instead, pass as a #type value.
| Project: | Vocabulary Permissions |
| Version: | 5.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
By "limitation" I mean I don't think Drupal has a bug, it's just the way Drupal works.
OK, the "bug". It's worth noting that Taxonomy Access Control (TAC) module suffered from this to and I provided a fix last year for it. Also, note, I have not tried your module but have reviewed the code and can see where/when this will happen. Fwiw, in a recent Drupal Dojo lesson on node access control I covered this problem.
Basically the problem is this. The way Drupal works when you submit a node form is to "delete all current vocab terms" and then "re-insert terms based on the form values provided". How does this effect your module? Well, let's assume we have two users, A and B. User A has full access but user B has had some access to certain vocabs removed by your module.
User A creates a node and applies some terms from various vocabs. Now, user B comes along and edits the node. But user B has some vocabs removed from his node edit page by your module. Now when user B submits the node form Drupal removes ALL terms in ALL vocabs, then re-adds them. However, it cannot "re-add" terms that you didn't allow user B to have. So, effectively user B has managed to deleted terms in vocabs she didn't have access to. User A will be puzzled as to why certain terms in restricted vocabs "keep dropping off the node" not knowing user B has effective delete rights.
Hope that's clear. The fix I provided for TAC was to find terms on a node that user B will not see and store them in a form API #value field. On submit these terms are re-added to the node (note, this requires you to lower your modules "weight" at install time to ensure that your re-insert code runs after Drupal has done it's "DELETE FROM {term_node} WHERE nid = %d" stuff). See the TAC module for more details and two functions you may find useful in finding terms that need saving across node edits.
#1
Restoring ev. existing term-assignment form forbidden vocabularies in the validate-hook should do the trick too, no?
#2
I love open-source... that was a great catch! Thanks!
I put a big comment in the code that basically says:
So my solution is to replace the unset, with the following:
if ($form['nid']) {$form['taxonomy'][$vid] =
taxonomy_node_get_terms_by_vocabulary($form['nid'], $vid);
}
else {
unset($form['taxonomy'][$vid]);
}
#3
I think this is fixed. I was waiting for a response from AjK before marking fixed, but maybe no response was needed.
#4
#5
The fix above raised a bug because
taxonomy_node_get_terms_by_vocabulary()returns an array of term objects, which trigger an error when handled as form arrays:"Fatal error: Cannot use object of type stdClass as array in
/var/www/site/form.inc on line
779"
Instead of unsetting, we instead should be converting these elements into #type 'value', so that their values are retained. We should keep in mind that even in node creation there may be default values. The attached patch uses the #default_value where available, and othewise defaults to the first option in the select (or set of radios, if a given site has converted taxonomy selects to radios).
#6
More descriptive title.
#7
... tested and committed. Thanks!
#8
#9