Posted by David Lesieur on January 3, 2009 at 6:41am
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | taxonomy.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
The vocabulary edit form hardcodes the value of the 'hierarchy' flag to '0', therefore resetting it when the form is submitted and the vocabulary saved.
This flag is normally assigned though taxonomy_check_vocabulary_hierarchy() which is called when terms are modified.
I think it should be '0' when first creating a vocabulary, but preserved whenever an existing vocabulary is edited—which is what this tiny patch does.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| taxonomy-hierarchy.patch | 932 bytes | Idle | FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy-hierarchy_1.patch. | View details | Re-test |
Comments
#1
I'm surprised that no one seems to have stumbled onto this bug.
The patch still applies...
#2
Yep, I had noticed this before but thought nothing of it, until I installed Faceted Search and ran into the bug. Patch applies and solves the issue. Thanks David!
#3
I confirm that the patch works. Also ran into it using faceted search.
#4
To others who might wonder: This is really a (trivial) bug in Drupal core. Faceted Search only springs up because only a few modules rely on this flag.
#5
Re-rolled patch for D7.
#6
Neat, but could we fix the similarly hard-coded value of $form['relations'] just below it as well?
There's another slight WTF there: The defaults added to the $edit array at the top of the function make $edit['relations'] default to 0 - but the hard-coded form value forces it to 1. If we continue to enable relations by default, then this value should be changed in the defaults array.
#7
Implemented the suggested fixes.
#8
Looks good now.
#9
Can we write a test for this please, or adjust the existing tests? Thanks!
#10
Are there any plans to release this patch in the next 6.x version of Drupal? I too am using the faceted search module and found this issue via #420354: Subcategories / Terms not heirachical in Guided Search
#11
yes, please... include it also in D6...
It's really a big issue to have to go back to database each time someone updates the hierarchy parameters...
#12
taxonomy-hierarchy.patch queued for re-testing.
#13
taxonomy-hierarchy.patch queued for re-testing.
#14
Appears to me like the d6 version is tested against d7 and fails, does it make any sense ?
#15
The patch does not apply anymore
#16
This patch is still recommanded on http://drupal.org/project/faceted_search , I assume for d6. If it is no more the case the project page should be modified, in my opinion.
#17
taxonomy-hierarchy.patch queued for re-testing.
#18
Subscribing.
#19
subscribing...
#20
Nope. It doesn't make any sense to try and apply the d6 patch to d7.
If you are like me and stumble across this patch from the Faceted Search project page - it does work in d6! It will prevent the hierarchy bugginess from recurring whenever you edit a vocabulary, whereas just modifying the database values for 'hierarchy' would be a one-time fix.
#21
The confusion came because #0 does not tell it is the version for D6, but #5 and #7 explicitely tell they are for D7. #5 also tells ''Version: 6.x-dev » 7.x-dev'' from that it can be deduced that #0 is for D6. It was just not obvious to me at first glance.
#22
I just updated core and ran into this issue again until I modified my database and reapplied the patch.
I'm changing the version on this to 6.x because, while D7 patches have been submitted, #0 is for D6 and the mismatched version is making it fail testing.
I'm changing the status on this to RTBC because this patch has been in use for over a year. Additionally it is very minor, has (in the D7 versions posted, which appear easily backported) met community suggestions for improvements, and fixes a problem that otherwise requires database edits.
I hope to see this in core some time before next year!
#23
taxonomy-hierarchy.patch queued for re-testing.
#24
The next step for getting this patch committed is clear: We have to write a test case (see #9).
#25
The usual process at this point would also require to check whether the problem is still present in Drupal 7, then write a patch and test for D7, and then backport to D6.
#26
taxonomy-hierarchy.patch queued for re-testing.
#27
Are we absolutely sure this has 'normal' priority?
#28
looks like we are still missing the test Dries wanted - let me try to write it
#29
The relations field is no longer used:
/*** Remove the related terms setting from vocabularies.
*
* This setting has not been used since Drupal 6. The {taxonomy_relations} table
* itself is retained to allow for data to be upgraded.
*/
function taxonomy_update_7003() {
db_drop_field('taxonomy_vocabulary', 'relations');
}
#30
Here's a 1st pass at a regression test - should have 1 fail which demonstrates that it detects the bug.
#31
Here's a new patch with the test.
#32
The last submitted patch, 353775-preserve-hierarchy-31.patch, failed testing.
#33
#31: 353775-preserve-hierarchy-31.patch queued for re-testing.
#34
Will this patch also list subcategories by default on the blocks?
#35
@BeaPower - that is totally unrelated to this patch.
#36
Then can you please help me, the posts Ive read similar to what I want directed me to this post. How can this be achieved? http://drupal.org/node/964970
#37
Here is a backport of patch @31 to D6.
#38
I want to help with this. Can someone please provide steps to reproduce the issue so I can test if patches resolve it?
#39
klonos for D6 you just need to:
#40
I am trying to test/reproduce this in 7.0, and here are my findings so far:
Behavior before patch:
a. After setting multiple parents for a term, I get this:
Notice: Undefined index: description in taxonomy_form_term_submit_build_taxonomy_term() (line 840 of /var/www/d7test/modules/taxonomy/taxonomy.admin.inc).b. The list page is always draggable in my case, but while the field is set to '2' it simply has no 'Save' button available. Is this a known issue? If so, please point me to it, otherwise I'll file a new Usability/UX issue for that.
c. Editing and saving the vocabulary does bring back the 'Save' button (+ the 'Reset to alphabetical' button)
and the hierarchy field of the taxonomy_vocabulary table changes to '0' (not '1' as mentioned in the 2nd step in Peter's repro instructions above). I don't know if this is important or not, but I thought I'd report it anyways./ note-to-self: read issue title better next time ;) /d. Dragging a term to another place and saving the list throws this:
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '5-1' for key 'PRIMARY': UPDATE {taxonomy_term_hierarchy} SET parent=:db_update_placeholder_0 WHERE (tid = :db_condition_placeholder_0) ; Array ( [:db_update_placeholder_0] => 1 [:db_condition_placeholder_0] => 5 ) in taxonomy_overview_terms_submit() (line 495 of /var/www/d7test/modules/taxonomy/taxonomy.admin.inc).Behavior after the patch from #31 is applied* (also reset the vocab to alphabetical and no multiple parents):
a. When setting multiple parents, I still get the notice I get without the patch applied. I've filed #1015572: Notice: Undefined index description taxonomy_form_term_submit_build_taxonomy_term(), line 842 of taxonomy.admin.inc for that.
b. Still the same behavior. Be it a known/accepted behavior or not, I still think that drag handlers should not be there at all. But that's a different issue as I've said. Let me know if it is already reported.
c. Editing and saving the vocabulary does NOT bring back the 'Save' button (NOR the 'Reset to alphabetical' button) and the hierarchy field of the taxonomy_vocabulary table DOES NOT change to '0' remaining set to '2'.
d. I cannot reproduce this (since there is no 'Save' button in the list page), but if this is considered a kind of 'locked' state, then no dragging should be enabled whatsoever.
*Note: I manually applied the patch as it seems that code has shifted a few line down. Needs re-roll(?).
PS: I was not able to locate the 'relations' field in order to include this in my tests. In which table am I supposed to be looking for it?
PPS: Does this count as a proper review?
[edit: patch from #31 was used - I'll see if I can find some time to test #37 in D6 too]
#41
...never mind, I just realized that this field is available only in D6 vocabulary table. I'll test it in D6.
#42
...minor title change to avoid confusion.
#43
Wow lots of information here. Can someone point me to the one which definitely works?
I am not very familiar with patches, and patch jargon.
#44
None of the patches works for me. I can add terms fine, but any time I try to edit or delete one it gives me the WSOD and the Hierarchy value changes back to 1.
My workaround was this:
I created 3 dummy categories and set them up like so:
X
-XZ
Z
-XZ
XZ is a child of both X and Z, so it somehow makes the vocabulary know that it shouldn't revert to a hierarchy of 1 because XZ has 2 parents, even if I change other things.
Is there a better solution? I'd like for this not to be any issue at all :-/