We've had a problem whereby the autocomplete fields on some taxonomy subqueues were never finding the nodes we wanted to add.
On digging into the code, I've found that this is because smartqueue_taxonomy_nodequeue_autocomplete is calling taxonomy_select_nodes to fetch a list of node ids with the relevant taxonomy term assigned to them, within which the actual autocomplete search is done:
/**
* Implementation of hook_nodequeue_autocomplete().
*/
function smartqueue_taxonomy_nodequeue_autocomplete($queue, $subqueue, $string, $where, $where_args) {
$matches = array();
$result = taxonomy_select_nodes(array($subqueue->reference), 'or', 'all', TRUE);
$nids = array();
while ($node = db_fetch_object($result)) {
$nids[] = $node->nid;
}
if (!empty($nids)) {
// Disable language selection temporarily, enable it again later.
if (module_exists('i18n')) {
i18n_selection_mode('off');
}
$result = db_query_range(db_rewrite_sql("SELECT n.nid, n.title, n.tnid FROM {node} n WHERE n.nid IN (" . implode(',', $nids) . ") AND " . $where), $where_args, 0, variable_get('nodequeue_autocomplete_limit', 10));
if (module_exists('i18n')) {
i18n_selection_mode('reset');
}
while ($node = db_fetch_object($result)) {
$id = nodequeue_get_content_id($queue, $node);
$matches[$id] = check_plain($node->title) . " [nid: $id]";
}
}
return $matches;
}
...where $subqueue->reference is the tid of the term in question.
The problem with this is that taxonomy_select_nodes is being asked for a pager query ($pager = TRUE is the 4th param), and will therefore only return a list of variable_get('default_nodes_main', 10) nids.
Even if this were changed to $pager = FALSE, the number of nids is determined instead by variable_get('feed_default_items', 10):
if ($pager) {
$result = pager_query($sql, variable_get('default_nodes_main', 10), 0, $sql_count, $args);
}
else {
$result = db_query_range($sql, $args, 0, variable_get('feed_default_items', 10));
}
The default values of 10 happen to match variable_get('nodequeue_autocomplete_limit', 10), but in our case only 10 nids results in never finding the right nodes.
AFAICS the only way of changing this without hacking the code is to set a higher value for default_nodes_main - which may obviously have undesirable consequences elsewhere in the site.
As a temporary workaround, I've added a copy of taxonomy_select_nodes to the smartqueue module, and increased the number of nids that get returned to smartqueue_taxonomy_nodequeue_autocomplete in there.
I can provide a patch, but I'm not so sure this is a long term solution?
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | smartqueue-optional-autocomplete-limit.patch | 2.92 KB | gnindl |
| #3 | remove_taxonomy_select_nodes_from_smartqueue_autocomplete-1336776.patch | 2.25 KB | mcdruid |
| #1 | 1336776.patch | 1.56 KB | amateescu |
Comments
Comment #1
amateescu commentedOr we can temporarily override the
default_nodes_mainvariable :) Can you try this patch?Comment #2
mcdruid commentedOk yes, that could work.
Couple of problems with this approach though (which don't really relate to temporarily hacking default_nodes_main):
1) I was thinking of nodequeue_autocomplete_limit as the number of potential matches the user is shown in the UI - so 10 is quite sensible.
If you use the same limit to restrict the number of results you get back from taxonomy_select_nodes you're likely to have the same problem I described originally; at this point it's not searching for the text the user has entered, and they will only find the node they're looking for if it happens to be in the first 10 items associated with the taxonomy term returned by taxonomy_select_nodes.
You could increase nodequeue_autocomplete_limit, but in our case (which I don't think will be all that unusual), we need to increase it to > 1000 in order for the functionality to work, as we've a lot of nodes associated with the taxonomy terms. We probably don't want 1000+ suggestions shown to us in the UI though.
2) on a related note, once we've set the limit being used in taxonomy_select_nodes high enough for it to return all of the nodes associated with the taxonomy term (otherwise it doesn't really work), the next query will be horrible:
...so potentially thousands of nids get imploded and put into the SQL query.
I would suggest that it would be better to combine both steps into the one query, and let the db do the heavy lifting? The only problem I can see with this is if you need the traversing of the taxonomy tree that taxonomy_select_nodes does... which we don't on the site I'm working on, but if that has to be part of the functionality...
I would have thought it likely that assembling the tids of any children of the tid the code is trying to filter on and using those tids in a join would be more efficient than getting a (potentially very long) list of nids and imploding them?
On the other hand, this is probably happening seldom and only for admin users, so perhaps it's not that big a problem? in which case, I think all that would be needed for your solution to work is to set a sensibly huge limit as the temporary value for default_nodes_main to trick taxonomy_select_nodes into returning the full list of nids?
I'd be happy to work on the other option I've outlined above, but can't do so right now. If you agree it's a worthwhile approach, I'll try and get a patch put together as soon as I can.
Comment #3
mcdruid commentedHere's a patch removing the call to taxonomy_select_nodes, but using the same approach to get a list of the descendent term ids which are then used in a join on term_node in the main db query that actually does the search.
I've tested this with a simple taxonomy tree, and it seems to work as intended.
Comment #4
mcdruid commentedWe've had my patch from #3 in deployed in production for about 2 months now with no problems.
Comment #5
amateescu commentedCommitted #3 to 6.x-2.x. Thanks for the patch and for testing!
http://drupalcode.org/project/nodequeue.git/commit/9dd62b5
Comment #7
gnindl commentedIn some exceptional cases you may want to add nodes to a nodequeue which don't have this restrictive limitation by subqueue reference. I am voting for making the hook implementation smartqueue_taxonomy_nodequeue_autocomplete($queue, $subqueue, $string, $where, $where_args) entirely optional as it prevents us from doing our work.
Better having too much choice, than too little and breaking functionality. This is actually a widget scalabilty issue and integration with advanced widgets, like for nodereference fields, would be nice, e. g. Nodereference Explorer.
Comment #8
amateescu commentedYou have a good point, but putting an if statement around a hook implementation is a big no-no for Drupal :)