Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.taxonomy_get_tree() returns all the terms in an array, but you have no way to find the data for a given term inside that array without iterating through the whole thing. This is silly (I'm tempted to call this a bug). It's been bothering me for a while, and I've had to work-around it by wastefully iterating through the tree once to make a new array indexed by tid, which I can then actually use. We had a very similar problem with node_revision_list() over at #114703: minor node revisions fixes.
Here's a patch for D7's taxonomy.module. I can't see how there'd be objections to this.
However, meanwhile, I'd like to see if there's any chance this will be committed to D6 and/or D5. This patch applies (with minor offsets) to D6, and would be trivial to backport to D5 if that'd fly. Given the outcome of #114703, I doubt this will go into D5, but maybe D6? ;)
Comments
Comment #1
Wim LeersThis has bothered me too for a long time. This, among other things, is necessary to clean up the Taxonomy module.
Comment #2
catchThis is a great idea, but it breaks drag and drop in taxonomy administration.
Comment #3
eojthebraveWas playing patch bingo tonight and came across this one and agree that it would be a good improvement.
This should fix the problem.
Steps to test.
1. Navigate to the "list terms" page for a Vocabulary that has terms.
2. Reorder the terms.
3. Save
Comment #4
catchPatch was reversed, here's a re-roll which should apply cleanly.
We need at least a basic test for taxonomy_get_tree really. I don't think the terms overview page is tested either.
Comment #5
cburschka1.) Applies
2.) Patched site installs.
3.) Created basic tag hierarchy
4.) Shuffled around, works.
5.) Saved with the new order, works.
I encountered a nasty PDO error while creating one of the terms, but that's probably unrelated and I'll file a separate issue for it.
Specifically, when creating a term and selecting the
<root>item as a parent (which should be the same as selecting nothing at all):Comment #6
Wim Leers[:db_insert_placeholder_1] should equal 0. So that's probably an error in the #submit callback.
Comment #7
catchdamn, the term objects page also converted a bunch of queries to dbtng, and that doesn't do things like silently case empty strings to 0/NULL for you or whatever this code was relying on. Patch and test posted at #332145: UNSTABLE-3 blocker: taxonomy_form_term_submit passes empty string as parent [dbtng conversion regression]
Comment #9
lilou CreditAttribution: lilou commentedSee: #335122: Test clean HEAD after every commit and http://pastebin.ca/1258476
Comment #11
eojthebravere-rolled against current head. Passes current set of taxonomy tests.
Comment #12
eojthebraveMarking as needs review
Comment #13
catchNote that taxonomy_get_tree gets a bit more coverage than it does already in #144969: taxonomy_term_count_nodes returns wrong count (+ tests). I'll see about adding an explicit test to this issue later on though if no-one beats me to it.
Comment #15
eojthebravebugger. Still learning how to use git, looks like my patch didn't actually apply. Should work now.
Comment #16
catchWe can't do this. taxonomy_get_tree() returns the entire taxonomy tree - in which a term might appear more than once when multiple parents are enabled, with different depth for example.
However, in a lot of cases you don't need all that information, so we might consider something like taxonomy_get_descendants() which indexed by tid, or indexing by tid for vocabularies without multiple parents inside taxonomy_get_tree() itself.
Comment #17
XanoAlso, indexing terms means we lose the order in which we fetch them from the DB.
Comment #18
dww@catch #16: "We can't do this. taxonomy_get_tree() returns the entire taxonomy tree - in which a term might appear more than once when multiple parents are enabled, with different depth for example."
Argh. :( What a pain.
@xano #17: "indexing terms means we lose the order in which we fetch them from the DB"
Ugh.
We lose that ORDER BY. I suppose there are cases where you need that.
Guess this is a feature request for a brand new API function that gives you all terms in a vocab indexed by tid or something.
Comment #19
catchYou mean like taxonomy_term_load_multiple(array(), array('vid' => $vid)) ?
Having said that though, we could definitely add an API function like taxonomy_get_descendents() returning an array of all children indexed by tid - since we just want the terms in that case, not a representation of their depth in the hierarchy. So leaving this open.
Comment #20
XanoMoments after my last reply I realised that although the query used by taxonomy_get_tree() sorts terms by weight and name, this order gets lost later on in the function. We might want to figure out a way to make sure this order remains, so contrib can use Taxonomy's function to display terms in the same order at all times.
Comment #21
XanoWe need this for #295395: DX: convert Taxonomy selectors to FAPI widgets.
Comment #22
catchBetween taxonomy_get_tree() and taxonomy_load_multiple() we have most of this, I don't think this'll get done in the next day, so moving to D8
Comment #23
xjmCrossposting #1207326: Refactor taxonomy hierarchy API for performance, consistency, and convenience.
Comment #24
Jody Lynnhttp://api.drupal.org/api/drupal/core%21modules%21taxonomy%21taxonomy.mo...
Looks like this has already been added.
Comment #25
PanchoRe #24:
Nope, taxonomy_term_load_children() has existed before and is not the same as taxonomy_term_load_descendents().
Therefore reopening.
Also, adapting title after the #1377628-9: taxonomy_get_term_by_name() should be taxonomy_term_load_multiple_by_name() rename.
Comment #26
PanchoPostponed on #1976298: Move taxonomy_get_tree() and associated functions to Taxonomy storage, deprecate procedural wrappers..
Comment #27
mgifford