My apologies if this is already in the issue queue -- many functions and systems seem to be involved so possibly this is filed under another category.
I noticed that the following use case with taxonomy "Apply To" types does not work and traced it back to the DrupalDefaultEntityController::load() function.
- latest head install (7.x-dev, sept. 3), clean install
- admin/structure/taxonomy/1
- Apply to content types: choose Article and Page
- Save
- Your selections are both in the database now (as we can see in table {taxonomy_vocabulary_node_type}).
- Only the Page type is visible in the table. (We expect both types to be visible), as they are both in the database.
- Edit the vocabulary again
- Notice that the Article type, previously selected, is not selected. We expect it selected because it is in the database.
Part of the problem seems to be that the TaxonomyVocabularyController's buildQuery() uses a join that causes the query to result in a number of rows with the same vid, differing only in the type column.
When the parent class's load() function is called, it uses the database's DatabaseStatementBase::fetchAllAssoc() function which attempts to return the association by key, and *only the last row* is used.
For example,
| vid | name | ... | type |
| 1 | Tags | ... | article |
| 1 | Tags | ... | page |
| 2 | Countries | ... | article |
| 2 | Countries | ... | page |
| 2 | Countries | ... | type3 |
results in
array(
'1' => array(
...
type => 'page';
)
'2' => array(
...
type => 'type3';
)
);
within DatabaseStatementBase::fetchAllAssoc().
I've looked in the issue queue (including #460320: Standarized, pluggable entity loading (nodes, users, taxonomy, files, comments)), and I'm a bit stumped as to how to fix the problem, because I'm not sure which function is not doing what's expected/assumed.
(I'm at DrupalCon if anyone wants to discuss the problem)
Cheers,
Albert.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | Screen shot 2010-01-08 at 9.45.59 AM.png | 35.21 KB | alberto56 |
| #1 | taxonomy.test.diff | 898 bytes | nestorconde |
Comments
Comment #1
nestorconde commentedThis simpletest confirms the issue.
Comment #2
Crell commentedAs discussed at DrupalCon, this is a misuse of fetchAllAssoc(). Your key field should be one that you know definitively to be unique. If it's not, then you can't assoc-it. (That's just a fun word to say.) So the controller code needs to do something else, as that DB method is not going to work.
Comment #3
sun.core commentedNot sure whether this is really critical... HEAD seems to work just fine. Feel free to overrule me though.
Comment #4
alberto56 commentedOne symptom of the original issue was that it was impossible to apply a taxonomy vocabulary to more than one content type (say, article and page). In the latest version of HEAD, there is no apparent way to select several content types for one vocabulary (the selection checkboxes no longer appear; see image). So I can't find a way to test HEAD to make sure it works.
Cheers,
Albert.
Comment #5
Crell commentedOK, so let's fix it properly. :-) As noted above, it sounds like misuse of the DB API is the culprit.
Comment #6
sfyn commented@D7csmtl
Did a final test today at the Montreal D7 codesprint. We were able to create and apply taxonomies normally. However I think there is a usability issue with taxonomy - I suggest the inclusion of help text on the taxonomy edit page to direct people to add taxonomies to content types via content types > manage fields.
Comment #7
alberto56 commentedRegarding sfyn's suggestion, above: see #719424: Usability: on taxonomy vocabulary page, document new way of applying to content types.