This is, I think, the last of the patches to come out of the development of the new Deploy module and the Foreign Affairs magazine project. It provides an interface via services to taxonomy_save_term() and taxonomy_save_vocabulary(). I also added some comments to the existing code to round things out a bit. Pretty straightforward patch.
If someone could take a look at this one sooner rather than later I'd be in your debt as my release of Deploy module hinges on this one.
Thanks
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 401586.patch | 4.19 KB | gdd |
| #3 | 401586.patch | 4.93 KB | gdd |
| #1 | taxonomy_service.patch | 5.27 KB | gdd |
Comments
Comment #1
gddHm I guess an actual patch would be nice
Comment #2
nonsieI don't have a site setup to run services from outside code so the review is based on examining the patch + using services browser.
1. taxonomy.selectNodes/taxonomy_service_select_nodes
Not sure why you changed node_load($node->nid) to node_load($node). This should not work since $node at this point is an object, not an array and node_load expects the parameter to be either the nid of the node or an array of conditions
2. taxonomy.saveTerm/taxonomy_service_save_term - required field description indicates this should be an object, while taxonomy_service_save_term() expects an array
3. taxonomy.saveVocabulary/taxonomy_service_save_vocabulary - required field description indicates this should be an object, while taxonomy_service_save_vocabulary() expects an array
4. taxonomy.getTree/taxonomy_service_get_tree - I'd really like to see the rest of taxonomy_get_tree() parameters implemented as well, I can see endless use cases where you want only one part of the tree. Definitely not a show stopper but a suggestion.
General note - return an error instead of an empty array if there are no matching results.
Comment #3
gddFinally got back to this tonight.
1) Fixed, no idea what happened there.
2) Done.
3) Done.
4) Done with these caveats. a) since $depth is an internal-only parameter I just hardcoded it b) I set $max_depth to also allow 0 for "all levels" since you can't pass NULL through the browser and there's no reason why you would pass 0 for that parameter during normal operations.
As far as the empty array vs error issue, my feeling is the services should return whatever Drupal returns, and since Drupal returns an empty array in these cases, that's what the service should return as well.
New patch attached.
As an aside, if you're testing a lot through the browser, you should also take a look at #394654: Make services browser easier to use for objects and arrrays since it makes dealing with the browser easier when passing objects and arrays as parameters.
Comment #4
gddI realized earlier that this patch should be re-rolled. Because Services does not currently deal with optional parameters properly, I have essentially made an API change by adding two new parameters to taxonomy_get_tree(), which already exists in the released version of Services. This is bad, or at least not something I want to do without more disucssion, so I'm going to back that change out and open up a new issue for it specifically. I'll re-roll in a bit.
Comment #5
gddOK here's the updated patch which reverts the taxonomy_get_tree() changes back to where they were (although I still updated the comments.) I think this is ready to go now.
Comment #6
gddComment #7
aasarava commentedNice work, heyrocker. I tested the patch from comment #5 and it worked for me. I tested by:
- Creating a new term in an existing vocabulary and modifying an existing node so that it used the new term. Upon deployment of the node, the receiving site successfully showed the new term and the updated node on that site was set to use the correct term.
- Creating a new free tag vocabulary and modifying an existing content type to use the new vocabulary. I then updated an existing node and added three tags to it. Upon deployment of the node, the receiving site successfully showed the new vocab and terms, and the updated node had the free tag field and it was populated with the three tags.
Tests were done on Drupal 6.10.
Comment #8
gddOK I think this is ready. Committed. Thanks all.