taxonomy_get_tree() wrapper function's return value inconsistent to that of core function

I second that. I'm getting same set of errors.

Pretty much same set of modules (I have taxonomy wrapper installed and enabled).

The issue node is created in db, but it does not get category right. Further, when viewing the issue node, I'm getting the following:

Warning: htmlspecialchars() expects parameter 1 to be string, array given in /var/www/drupal-4.7.2/includes/bootstrap.inc on line 599

Any pointers?

no pointers. this is just a known bug. :(

project.module is doing some fairly tricky stuff with its own taxonomy (to make the project node UI on drupal.org more fool-proof). i've never tried using the category module, but i'm not surprised it's getting confused by the tricks i had to play to get the taxonomy stuff working correctly.

someone who knows a lot about the category module just has to step up, and come up with a solution that works with category (which doesn't break the regular taxonomy version). perhaps i'll get to this at some point, but these days my drupal time is overwhelming focused on the new release system. as soon as that's done, my top priority will be porting project* to 5.x. after that, maybe i'll work on this. otherwise, someone else has to step up.

perhaps if the sites i'll be building at my day job veer into wanting to use category.module, this issue will take on a new urgency for me, but i don't see that happening any time soon.

sorry,
-derek

/subscribing

No brainwave solutions have popped into my head as of now :P.

I need to get this sorted quite soon.

dww: I understand that you don't have time to look at it, but could you let me know if you have any idea what the problem is? It'll save me having to read and understand the whole module.

Even knowing if it would be better to patch category or project would be a start!

i don't know what to tell you. i've never used or looked at category module, so i don't know any of the details on that side of the equation.

i've asked jaza a few times in IRC, and he doesn't really know what to say, either. basically, project is doing some weird things with taxonomy that category isn't expecting. nothing project is doing is technically wrong or illegal, but it's unexpected, and category's attempts to insert itself into the taxonomy handling is getting confused by it.

so, you either have to make category's automatic taxonomy handling stuff smart enough to handle the curve-ball project.module is throwing at it, you have to modify what project module is doing to avoid confusing category module, or you have to disable all the taxonomy stuff regarding projects entirely.

how much do you really need project taxonomy support (i.e. classifying your projects like we do at drupal.org with modules vs. themes, categories of modules, etc)? if you remove all the terms from the project taxonomy, do things continue to blow up? just a thought...

anyway, i wish i could be more help for this, but i'm totally swamped with fall-out from the new release system, and a growing urgency to port project* and cvs.module to 5.0 so we can upgrade drupal.org. :(

good luck,
-derek

oh, a few details if someone's going to look closely at this:

project_form_alter() in project.module unsets the default taxonomy form elements for the project-specific vocabulary on project nodes and inserts its own form elements.

_project_db_save_taxonomy() in project.inc is what looks at these custom form elements and saves the right things into the DB.

_project_get_vid() is how project knows what the vocabulary id is for the vocab it creates. this function either returns the vid for the existing vocab, or if there isn't one yet, it creates it. (this in and of itself shouldn't be confusing category module at all, but its worth knowing about if you're going to dive into the murky world of how project uses taxonomy). ;)

not knowing anything about how category works, i suspect it's trying to play similar games, and that's where the collision/badness happens. sorting out the mess in a sane way is left as an excercise for the interested reader. ;)

hope that helps!
-derek

well one little bit that breaks bewtween these 2 modules...

i just started trying out the project module and sure enough cant get it to work with tax wrapper from cat module. The issue i get is i cant create a project becuase it keeps telling me i have to select a "project type".

Here's part of why.

project module has code to build a list of project types starting with the parent node. In project.inc the function project_project_form() checks the terms and tries to find parent = 0. In real taxonomy_get_tree() this will return a parent = 0 for top term - but in wrapper it returns the nid of the container not 0 (i.e. root). and from there there are just a bunch of errors from dww's code since he assumes a lot of variables are now defined - which since no "0" case they aren't.

SO... not sure if this is a feature request for project.module or a bug on the wrapper in cat module... i would vote on the latter... shouldnt wrapper return same answer as the original module??

i may try a simple hack just to try this out.. but not sure what the trickle effect is going to be.

Peter Lindstrom
LiquidCMS - Content Management Solution Experts

ok, here's my 5 minute hack to get project working with category - see 4 lines following p.lindstrom comment - and, as mentioned above... this code is in project.inc -> project_project_form()

  if (project_use_taxonomy()) {
    $tree = taxonomy_get_tree(_project_get_vid());
    $top_level = array();
    $options = array();
    foreach ($tree as $i => $term) {
      // p.lindstrom - hack to make work with category module
      if (module_exist('category')) {
        $tid = $term->parents[0];
        if (category_get_container($tid)) $term->parents[0] = 0;
      }
      if ($term->parents[0] == 0) {
        $last_top = $term->tid;
        $top_level[$term->tid] = $term->name;
      }
      else {
        $options[$last_top][$term->tid] = $term->name;
      }
    }

this won't work in the case where a container is ot in root.. but sorry.. i am very busy and all my containers at the moment are at root (so it works for me) - but real fix should trickle through any container to find a root.

so my fix is simple..

- if cat module is intalled
- for each term's parent i check to see if it is a container
- if it is i fake the terms parent as being 0 (rather than the container)

seems unlikely it could be this simple.. but works for me...

Peter Lindstrom
LiquidCMS - Content Management Solution Experts

it could very easily be that easy. ;) however, i'm not *thrilled* about adding such code to project.module, since it's a little confusing to me why category.module's taxonomy wrapper is changing the behavior of taxonomy like this. i'd rather consider this a category.module bug. but, on the other hand, i'm also pragmatic about such things, and if it's a trivial 4 lines to work-around the problems, i'd be willing to commit it. the downside is that project* runs on d.o, and i'd like to avoid bloat/hacks as much as possible.

peter, you seem to be the first person who actually knows category code and project code. ;) if you can understand how to fix this in category, that'd be a big help. however, i understand if you're too busy.

i'm reluctant to commit code to project if it's not even going to work in all the cases for category users. however, never having used category or looking at the code at all, i don't understand this stuff about 'root containers' and the like. so this issue remains active until someone posts a real patch, and some other category user can confirm it works. ;)

thanks much for the first real start on this issue, peter!
-derek

shouldnt wrapper return same answer as the original module??

Yes, it should. If what you say is true, ptalindstrom, then this is indeed a category module bug, and not a project module bug. I would strongly advocate AGAINST patching project.module to fix this, as the whole point of the wrappers is to avoid other modules having to write code that includes if (module_exist('category')) { /* ... */ }.

The taxonomy wrapper should return a parent of '0' for categories that have their own container, or any distant category/container, as their parent. This no doubt causes problems for other contrib modules too, so it should be fixed in category.

category_get_tree(), OTOH, should continue to go against the taxonomy API, and return parent containers or distant parents for categories (where defined - which is almost always), instead of '0'.

OK, fix committed to HEAD, 5, and 4.7 branches. ptalindstrom, I pretty much took your solution and applied it in the taxonomy wrapper. Thanks.

Download version 1.1 of the category module, for either Drupal 4.7 or for Drupal 5. If you're upgrading from a previous version of the category module, you will need to uninstall and then re-install the taxonomy wrapper (from the 'administer -> content management -> categories -> settings' page (for Drupal 5), or the 'administer -> settings -> category' page (for Drupal 4.7)).

so not sure this IS fixed.

When i originally made my hack to the project module it allowed me to save an issue; but i just now realized that it wasn't saving the category selection. Now that i have cmoe back to this and i see Jeremy has modd'ed the wrapper i thought i would go back and try his new version and go back to original versions of the project and project_issue modules.

I seem to get the exact same result i had when i made my changes (which almost works but cat not saved) - which i case makes sense since Jeremy suggested his change and mine did basically the same thing.

So i will play around with this a bit; but pretty sure that project module and cat module still not working together...

see http://drupal.org/node/124846 .... :(