Allow various node types with category behavior to be used for free-tagging (per container)
| Project: | Category |
| Version: | 6.x-2.0-rc1 |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | JirkaRybka |
| Status: | fixed |
Hi,
I'm sorry if this is obvious or documented but I have gone through the docs and this forum and can't find an answer...
I would like to be able to add fields to what I classify my content with on my site.
- obviously Taxonomy cannot do this
- after fighting around with nodereference and CCK I found this module.
What I expect to be able to do : one container with lots of categories in it. Each of these categories is a CCK enhanced node.
When I create a category from "Create Content", I cannot edit it to create CCK field
When I create a content-type, I can set it to be a category. But then I cannot see it in admin/content/category to say which content-types it should apply to
Has anyone ever done that ? It seems to me like one of the key points of making taxonomies with nodes but I don't see how to do it
Thanks
#1
A little more specific: I can get the following setup
"MyContainer" container, applies to "Story" content-type, multiple & tag setting to allow free tagging of categories
"MyCategory" content-type with an extra CCK field, behavior is "Category", allowed container is "MyContainer"
When I create a story and give it "cat1,cat2,cat3" value in the "MyContainer" setting, it creates nodes of type "Category" and not "MyCategory"
Humm... now that I've recast the problem this way I believe I have seen it somewhere...
#2
Ok, the answer is : you can't because be default the module will create a node of type "category", see category.module line 515.
After much trial and error, I have patched the code so it looks for a content_type with "category" behavior associated with the container. I haven't made a clean patch yet but all you have to do is replace lines 513 to 528
<?phpif (!$typed_cat_cid) {
$tag_node = new stdClass();
$tag_node->title = $typed_cat;
$tag_node->type = 'category';
$tag_node->category['cnid'] = $cnid;
$tag_node->category['parents'][0] = $cnid;
$node_options = variable_get('node_options_'. $tag_node->type, array('status', 'promote'));
$tag_node->status = in_array('status', $node_options);
$tag_node->promote = in_array('promote', $node_options);
$tag_node->sticky = in_array('sticky', $node_options);
$tag_node->revision = in_array('revision', $node_options);
$tag_node->name = $user->name ? $user->name : 0;
$tag_node->date = date('j M Y H:i:s');
$tag_node = node_submit($tag_node);
node_save($tag_node);
$typed_cat_cid = $tag_node->nid;
}
?>
with the following:
<?php
if (!$typed_cat_cid) {
//watcherFR : select (one of) the category types associated with the container to provide the type of the new node to create
$WFRquery = "SELECT n.type FROM {category} c INNER JOIN {node} n ON n.nid=c.cid WHERE c.cnid=%d";
$result = db_query($WFRquery, $cnid);
$type_of_category_authorized_in_container=db_fetch_array($result);
if ($type_of_category_authorized_in_container==FALSE)
{
watchdog("category","No content_type found for categories associated with container $cnid. This means none of your Categories are associated with that container. Please do so!",array(),WATCHDOG_ERROR);
} else {
if(db_fetch_array($result))
//More than 1 result ??
watchdog("category","More than one content_type found for categories associated with container $cnid. We are using the first one returned by the database query...",array(),WATCHDOG_WARNING);
$tempnode = new stdClass();
$tempnode->title = $typed_cat;
$tempnode->uid=$user->uid;
$tempnode->name= $user->name;
$tempnode->type=$type_of_category_authorized_in_container['type'];
$tempnode->cnid= $cnid;
$tempnode->parent = $cnid;
$tempnode->language= $node->language;
$tempnode->parents = array($tempnode->parent);
$node_options = variable_get('node_options_'. $tempnode->type, array('status', 'promote'));
$tempnode->status = in_array('status', $node_options);
$tempnode->promote = in_array('promote', $node_options);
$tempnode->sticky = in_array('sticky', $node_options);
$tempnode->revision = in_array('revision', $node_options);
$tempnode->name = $user->name ? $user->name : 0;
$tempnode->date = date('j M Y H:i:s');
$tempnode->category=array(
'cnid'=>$cnid,
'parents'=> $tempnode->parents,
);
$tempnode = node_submit($tempnode);
node_save($tempnode);
$typed_cat_cid = $tempnode->nid;
//END watcherFR
}
?>
#3
Actually, let me take that back. "Behavior" is stored in a variable so this is really like class/instance.
My "hack" assumes that you have created at least one category within the container and that the container hosts only one type of categories. In that case, looking at the instance relationships as is done throught the category_node table is enough to know about the class relationships. It would be nicer to move the behavior stuff into the database.
Also change that select to SELECT DISTINCT(n.type)...
If anyone is interested, I am working on the views v2 (D6) integration of category. I am trying to do it by porting http://drupal.org/node/69861 but since I'm a newbie to Drupal I feel like I'm jumping through a lot of hoops. I've got 3 fields visible in views : wieght, category title (of its node) and category id (the nid of its node=its cid) and it can take one argument (the category name)...
#4
Hello.
I was having the same issue above [ creating a category using free-tagging only uses the default category node-type ].
This patch, reuses a lot of the same code found in the patch by bdragon located here: #127097: Freetagging modernization and updates it to work with 6.x.
It fixes the above listed problem by adding a select box with choices of category node types to create using free tagging for each individual container.
Why this patch was never added to HEAD, is beyond me as it originally fixed this issue. This is patched against 6.x-2.0-rc1.
Please review.
Thanks.
#5
No time to test today, but at least I took a look at the patch visually:
- Weird half-indentation in schema definition
- Default node type should be 'category' (6.x), not 'category_cat' (5.x) - three instances
- The new update function would be nicer if it used Schema API (db_add_field, or what the exact name is)
- What's the global $category_last_update_nid good for? Globals are ugly, and this seems completely unused.
- I would rather not use a full node_load() to just get a single setting from container - with a lot of other modules that might be a performance problem. I believe we have other ways in category.inc (didn't really look now, but I'm 99% sure)
- I would add at least a blank line (better a comment) before function _category_node_type_options() definition
- The variable $types is unused. Why call node_get_types() again in the foreach?
- From the patch I can't see what's
$freetag_callback_nid = 0;for, but I admit I didn't read the other existing code. Is it a needed change involving already existing variable, or a leftover from debugging?Otherwise, as I said, I didn't test the code, and I didn't analyze in-depth how this works, so I'm not reviewing that part yet.
#6
More thoughts:
- We should test whether the new setting persists over container previews. This is not to say that I see any specific problem, but I had a lot of trouble with this part before, and it's often overlooked. We just want to make sure, that a new setting passes cleanly through the entire workflow with container nodes creation/editing, without getting stripped off somewhere.
- It would be nice to hide the new setting through JavaScript, if freetagging is disabled. Not that I know how to do this exactly, but we already have such things in the UI, to possibly copy from.
- As for the node_load() that just retrieves a setting from container, the good replacement is category_get_container($cnid), which should be pretty snappy: With the new caching now in place, it most likely won't even touch the database at all. We'll avoid the whole node loading process (involving node table querying, all other modules hooks etc.). Even a direct query against the category_cont table might be acceptable here IMO, but category_get_container() is cleaner, and benefits from caching.
Also adjusting issue title. Nice to see someone working on category module :-)
#7
Just for record: There's a very old related issue: #64183: Determine the default node type for categories in each container
The proposal of that issue is to handle (in one way or other) some kind of a "default" node type for category additions through various shortcut-links, like "add child category" on already existing nodes, or adding a category from category administrative listing. The idea was about some automated way of guessing the node type from allowed node types for containers, but I see that as overly complicated, error prone, and unpredictable for users, possibly creating some WTF moments - so I would prefer a setting as also proposed here.
We might want to have one setting for both, as these cases are really similar to free-tagging in a way (i.e. adding a category by shortcut, without really thinking of / going through the list of all content types). I'm not sure whether it makes sense to do that in this patch here, but it should be pretty simple: Just adjust the few links with hardcoded "node/add/category" to be "node/add/[something retrieved from settings]". Plus consider this wider use case when it comes to the placement and description for the new setting.
Edit - Additionally I marked #107737: Freetagging on a container where only custom category types are allowed uses wrong type as duplicate (avoiding follow-up bloat here, otherwise not related to that other proposal).
#8
A few thoughts from me...
Currently, we have these ways to create a new container or category:
- From administrative listing.
This is the tab "Add container" (admin/content/category/add), and the link "Add category" for each existing container (admin/content/category/%/add). These are both presenting an intermediate page with list of applicable content types - generated by category_add_container_page(), category_add_category_page(), possibly falling back to category_no_node_types(). (BTW, there's a fair bit of code duplication in there). I'm unsure here, whether we want to incorporate a default here, too, somehow. IMO we want to abstract the code for fetching a list of applicable content types into a separate helper function (to be used elsewhere too), add some descriptive heading above the list, and perhaps append ' (default)' to the category node type being default for given container.
We might even want to turn that intermediate selection of content types into a form, so that we can have the default pre-selected, but I'm unsure on this one.
- Through the book-like "add child [type]" links on category/container nodes.
Now I see that we already print these links for all possible node types separately, so that the user have all the options available right there in node links of would-be parent. Although it might be questionable whether we want to allow some further limitation to be configured, generally this bit is fine as-is, IMO.
- Through the calssic Create content screen (node/add)
There's nothing to talk about, the user starts from scratch, and so naturally chooses everything from scratch too.
- Free tagging.
This case have 'category' node type hardcoded, and that's the point of this whole issue. We need to look into category_node_save(), as the previous patch already does.
- Legacy save of a new term or vocabulary through Taxonomy wrapper.
Also hardcoded 'category' and 'container' node types in this part. We most probably want to fix this case too (just for categories, most probably). For example, new forums from Forum module are saved as categories through this code, getting a hardcoded node type. That's in _taxonomy_term_into_category() called from taxonomy_save_term(), and _taxonomy_vocabulary_into_container() called from taxonomy_save_vocabulary().
------------------------------
So, I'm going to attempt a re-rolled patch with:
- Cleanup (see #5)
- Put default node type selection under "Content types" on container node form, and title/describe it as a generic default for situations where a category is created without full node form being submitted, such as free tagging and legacy Taxonomy operations. If we have only just one possible category type, hide this new selection, and replace it with a fixed value in the form structure.
- Use the default for both freetagging and Taxonomy wrapper legacy saves. (For legacy Vocabularies->Containers we don't have a default available, and I would rather not introduce one just for this purpose.)
- Improve the intermediate type selection page on administrative cat/cont additions, so that we share code, mark the default choice (at least), and have a descriptive title for the list.
#9
I spent quite a bit of time on this, but didn't reach a final patch yet (too late night, hurrying to bed...) I'm posting what I have, both for possible review of the concept [unlikely here, I know], and for myself to have things summarized. THIS IS NOT FINISHED, so leaving at "needs work" - and I mean it!
This patch already contains the following:
- All points from #5, and first&last from #6 addressed. Additionally, the update function lacked a comment (or at least a blank line) above.
- The patch was rolled with windows-style CR/LF line endings, and so failed to apply on my setup. Fixed that by manual edit.
- The new setting of default type is now changed into a generic per-container default for various uses (database/object field name changed, descriptions updated, form item moved into "Content types" fieldset, reference added to description on the free tagging option).
- To avoid code duplication (and more), the content-types-fetching loop is now abstracted into a new helper function _category_category_type_options($cnid = NULL, $local = TRUE, $distant = FALSE). This function fetches a list of applicable category-behaving content types for a given container $cid, or container-behaving types if no $cid given. Also accepts switches for fetching locally/via-distant-parenting allowed types in category-behavior mode, so that it may be easily used from category_link().
- Refactored first half of category_link() to use the new helper function. category_link() was quite a beast, it became much shorter, and IMO more readable too. I took care to keep the same logic, obviously. BTW - the old version was fetching stuff (the container corresponding to the node in question) repeatedly, for every possible link candidate out there.
- As of the new setting on container form, the new helper function replaces _category_node_type_options() from the previous patch. Note that the previous patch was flawed in that it didn't honor "allowed containers" setting on content types.
- The new helper function also includes the warning about no applicable node types being available, that was previously a part of administrative workflow only. That part was flawed, as it printed the warning to everyone doing categories administration (including lesser admins without administer content types permission), and it didn't print at all if other ways of adding categories and containers were used (and we have quite a few outside administration). Now it prints whenever this misconfiguration is encountered, but only if the current user have administer content types permission.
- Usage of the new setting in freetagging, and updates to it on node types editions/deletions, are kept from the previous patch.
- The new setting is now used in a similar way for legacy Taxonomy [wrapper] operations.
Things still TODO:
- Refactor the node-type selection on administrative category/container additions, to use the new helper function (getting rid of a bit of code), to have a proper title/description, and most probably to become a form with radios, default type being pre-selected. (BTW there's currently a category/container mismatch in a code-comment in there.)
- Hide the new setting on container forms, if there's only just one option available.
- Tweak the warning message about misconfiguration. The current way of concantenating the string is not friendly to the Drupal localization workflow, namely it may not be properly extracted to .pot template by the extractor at drupal.org.
- Check what exactly node_get_types() does, since we now call it twice on category/container nodes being viewed. If it's a performance concern, cache the result in static variable.
- Testing, a lot of testing to do.
#10
Today's updated patch adds to the list of finished changes:
- As for the node-type selection on administrative category/container additions - I reconsidered my suggestion of turning this into a form, because we generally use forms for settings or posts only. Selection of node type to be created is already handled by lists of links on various places (for example node/add, or 'Add child' links on categories/containers), so I left it as a list.
Still, I merged both the cases (category/container) into a single function that uses the new abstracted helper, added a title to the list (badly needed IMO), and incorporated the new default setting. The default just puts that node type to the top of the list, and appends '(default)'. For an administrative task, I see this as sufficient. BTW drupal_goto() passes the query argument directly to url(), so we can just re-use the array (no need to encode the string of 1-2 arguments manually).
- Additionally, there was a bogus (I believe) path argument in the 'Add categories' links generated on the admin/content/category overview: It was admin/content/category/%/add/category - the last '/category' part being unused.
- Hide the new setting on container forms, if there's only just one option available: Done.
- As for the warning message about misconfiguration (i.e. no applicable content types) - finally I just merged the dynamic part into the message as simple string, as this is really edge-case warning, and it makes sense in both the cases IMO: "for use with this container" refers either to the container where we're going to create categories, OR to the container that we're going to create. I hope it's OK, although English is not my first language. Really, the important bits are "no content types with .... behavior available", and "go create one"; the rest is pretty minor.
- Check what exactly node_get_types() does....: Checked, and it's OK. node.module caches that in a static, should be a very cheap call.
- Testing, a lot of testing to do. Still TODO!
Important: This patch needs update.php run (has a new update), or at very least modules page submit (changes menu definition) to work.
#11
This is my final patch here (I hope).
- I tested pretty much everything I can think of. Apart from running into #484660: Distant parenting of categories is flawed again (can't select a local parent at all, if distant parents are allowed), and apart from messy state of affairs with $container->hierarchy (going to file another issue), all seems to be fine with this patch.
- Hiding of 'Add child [category]' links on categories per $container->hierarchy flag makes little sense IMO, because that flag is not a setting now, it's only just a summary of current state (and so changes "magically" along with category additions, and have nothing to do with ability to add child nodes). But since this bit is wider in scope, and needs more cleanup, I left the behavior unchanged for now.
- When there are no usable content types to add a new category/container from administrative listing, we're now redirecting straight to the prepared destination, so that the warning shows above the listing where the link got clicked, rather than above an empty list of options where we're effectively stuck (until navigating away through tabs, or so).
- Also fixed the warning on administrative listing *before* anything is clicked, if no content types with container behavior exists (didn't catch that before). Also, this was yet another instance of duplicated code, now replaced by the new helper function. I adjusted the text of the warning a bit, to fit to this place too, and suppressed possible duplicates (if someone clicks 'Add container' tab, and gets redirected back with the same warning), using the third argument of drupal_set_message().
- Clarified a few code-comments that are being added in this patch. While testing, I did shoot myself in the foot, so to speak, with distant parenting and allowed containers for content types and resulting 'Add child' links on categories... So this definitely needs clear explanation in comments. (I liked the code comment "Now say that backwards")
----------------------------
So, summarizing: This patch introduces a per-container default content type setting for newly created categories, to be used in cases where no user input is available (free-tagging, taxonomy wrapper vocabulary saves). The setting is hidden if there's only just one possible option. Also upgrade path is included. The selected default is now also highlighted in the content type selection on administrative additions of categories.
Additionally, the mentioned [administrative additions] selection got a bit of cleanup (title, redirection if empty), and several instances of duplicated code dealing with a list of usable content types with a given behavior, got abstracted into a helper function - in particular the code behind 'Add child' links became cleaner I believe.
Details above - from #8 on.
This patch is based on previous work done by jvalletto (#4), which in turn is based on (a part of) bdragon's #127097: Freetagging modernization - #3.
#12
Rerolled without the 'Add child' links being hidden per 'hierarchy' setting on container - it's not a real setting anymore, it doesn't control the ability to add child nodes anymore, so it makes no sense to hide links based on that. (Also the new helper function got a bit simplified now.)
See #586100: $container->hierarchy not updated correctly, plus code clean-up. for more discussion about the 'hierarchy' field.
#13
Nice new feature, guys! Thanks for the effort on this one.
Tested, all looking good. Committed to HEAD.