Posted by alexpott on April 27, 2010 at 3:28pm
6 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | taxonomy.module |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
To reproduce this bug on a standard Drupal install:
1. Create an article node with 3 terms "x, y, z"
2. Using the taxonomy term edit form delete the x term.
3. Visit the node - you will see only the z term
4. Edit the node - you will get php notices Notice: Trying to get property of non-object in taxonomy_implode_tags() .
I've marked this as critical for two reasons - on step 3 the y term is missing and on step 4 you get php errors.
The issue seems to be that taxonomy_term_delete is not removing the term information from the field tables but it is removing it from the taxonomy tables.
Comments
#1
Here is a patch that adds a test for the above issue. Obviously it currently fails :)
#2
Investigations so far:
Taxonomy_term_delete calls field_attach_delete('taxonomy_term', $term) but this is doing nothing as the entity type the data is stored on is the node not taxonomy_term.
#3
There is currently no way for modules which provide reference fields to update field API about edits or deletions of the referenced entities.
The approach taken in D6 nodereference and userreference has been to account for stale data, and I think taxonomy reference field mostly follows that pattern, looks like we need to do the same in this particular case.
Actually updating field storage runs into the same issues as #556022: When a text format is deleted, some modules (like text.module) don't update their data, even though we say they do which is a minefield.
Let's watch the test fail.
#4
The last submitted patch, 783112_taxonomy_term_delete.patch, failed testing.
#5
Here's a patch that fixes both (3) and (4) from the issue post. It only includes a test for (3) so far.
It works in the way suggest by catch - basically it ignores field data that isn't a valid taxonomy term.
#6
Oops forgot to set to review... on you go testbot!
#7
Fixed some whitespace issues in the patch, and changed a variable name in the test to be more descriptive.
#8
Tested on todays drupal dev version, and works as explained
#9
Committed to CVS HEAD. Thanks.
#10
Automatically closed -- issue fixed for 2 weeks with no activity.