Its impossible to re-use the Taxonomy term autocomplete without creating fields. I'm opening this thread to push to rewrite the taxonomy_autocomplete() function such that it will accept multiple vocabularies, rather than accept a single field to function from.
The current design is too closely tied with other core functionality and doesn't appear to sport Drupal's modular fashion.
For the sake of reference
line 79 and below of modules/taxonomy/taxonomy.pages.inc:
/**
* Helper function for autocompletion
*/
function taxonomy_autocomplete($field_name, $tags_typed = '') {
$field = field_info_field($field_name);
// The user enters a comma-separated list of tags. We only autocomplete the last tag.
$tags_typed = drupal_explode_tags($tags_typed);
$tag_last = drupal_strtolower(array_pop($tags_typed));
$matches = array();
if ($tag_last != '') {
// Part of the criteria for the query come from the field's own settings.
$vids = array();
$vocabularies = taxonomy_vocabulary_get_names();
foreach ($field['settings']['allowed_values'] as $tree) {
$vids[] = $vocabularies[$tree['vocabulary']]->vid;
}
$query = db_select('taxonomy_term_data', 't');
$query->addTag('translatable');
$query->addTag('term_access');
// Do not select already entered terms.
if (!empty($tags_typed)) {
$query->condition('t.name', $tags_typed, 'NOT IN');
}
// Select rows that match by term name.
$tags_return = $query
->fields('t', array('tid', 'name'))
->condition('t.vid', $vids)
->condition('t.name', '%' . db_like($tag_last) . '%', 'LIKE')
->range(0, 10)
->execute()
->fetchAllKeyed();
$prefix = count($tags_typed) ? implode(', ', $tags_typed) . ', ' : '';
$term_matches = array();
foreach ($tags_return as $tid => $name) {
$n = $name;
// Term names containing commas or quotes must be wrapped in quotes.
if (strpos($name, ',') !== FALSE || strpos($name, '"') !== FALSE) {
$n = '"' . str_replace('"', '""', $name) . '"';
}
else {
$term_matches[$prefix . $n] = check_plain($name);
}
}
}
drupal_json_output($term_matches);
}
How to improve it
Rather than try to capture which field we want, why not make the autocomplete callback accept vocabulary id's from the field generating the autocomplete URL? Then, just train the function to split up the vocabulary ID's by some common character.
Why on earth am I whining about this?
Well, discussing how to overcome the use of a Taxonomy term autocomplete started around here: http://drupal.org/node/1101432#comment-4436730 , http://drupal.org/node/1101432#comment-4520422
And after reading up on the code, I strongly urge it to be de-coupled from the field implementation.
Comment | File | Size | Author |
---|---|---|---|
#43 | drupal-1169964-43.patch | 8.66 KB | dawehner |
#43 | interdiff.txt | 2.75 KB | dawehner |
#40 | interdiff.txt | 2.49 KB | dawehner |
#39 | drupal-1169964-39.patch | 6.59 KB | dawehner |
#37 | drupal-1169964-37.patch | 6.55 KB | dawehner |
Comments
Comment #1
wjaspers CreditAttribution: wjaspers commentedComment #2
RobLoachComment #4
joachim CreditAttribution: joachim commented+1
This sounds like a more sensible way to do it, and more reusable as the OP says. taxonomy_autocomplete() should work on a vocabulary/ies, rather than a field.
Comment #5
xjmWas just about to create this exact issue.
Comment #6
joachim CreditAttribution: joachim commentedHere's a patch.
Some things to note:
> // Part of the criteria for the query come from the field's own settings.
This is true, but only to the extent that you select a vocab. The $field['settings'] array looks like this:
... but the 'parent' key does nothing at all.
At first glance I've removed something here -- but I haven't, as term reference fields only support a single vocabulary -- the $vids array was completely pointless.
It *might* be nice to allow this autocomplete to operate on more than one vocab, but that's out of the scope of this patch I think. It's kind of beyond how taxonomy currently works, too, and it would also require a rethink of the path callback arguments.
Comment #7
xjmI think taxonomy_autocomplete_validate() should probably be refactored in the same way.
Edit: Just realized this patch is not actually what I thought. My plan was to start by taking the guts out of
taxonomy_autocomplete()
andtaxonomy_autocomplete_validate()
(which calldrupal_json_output()
andform_set_value()
after logic) so that the logic could be reused by other modules wishing to alter the autocomplete behavior. That would address the problem in a different (and backportable) way.Comment #8
joachim CreditAttribution: joachim commentedI don't see how it can be, without providing a dedicated term autocomplete form element:
You can't pass parameters to the validator function. So it's dependent entirely on the form $element and the user values. So it has to do this to get valid vocabs:
If we take FieldAPI out of this, how does it know which vocabs are allowed? How would a custom module using the taxonomy autocomplete tell taxonomy_autocomplete_validate() which vocabs are allowed? This takes us to the point of needing a form element that has '#allowed_vocabs' = array($machine_name, ...) and that's a custom form element.
Definitely out of the scope of this particular issue I think.
Comment #9
joachim CreditAttribution: joachim commentedGiven this is a purely internal change, can we backport this to D7 to help contrib modules?
Comment #10
xjmI am confused; I didn't say anything about taking Field API out of anything? It could be as simple as:
And:
That way other modules can override form validation by using taxonomy.module's logic and just tweaking the results, rather than copying the whole thing. The problem is that these functions both contain logic and then do something at the end instead of returning something.
Comment #11
xjmMore of a task than a feature really, in both #6 and #10. No new functionality is being added, just reusability.
Comment #13
joachim CreditAttribution: joachim commentedDoing that wouldn't remove the tight coupling with FieldAPI which is what this issue is primarily about.
I can see what you're saying though -- split these functions so there is a pure API function that returns raw data, and then a wrapper function that handles the output.
Is that needed though? When would you want a list of term matches to a partial string in an array, rather than ready for an autocompleting form element? And as for taxonomy_autocomplete_validate, that's an #element_validate callback by definition -- it's meant to do form_set_error().
Comment #14
xjmThere is a good use case. TAC for example (the reason I am following this) needs to modify the autocomplete field in the following ways:
I am currently calling these functions directly within TAC's callbacks, but that adds the need for a lot of monkey business. If I could instead act on the results before doing
form_set_value
anddrupal_json_output()
in my own callbacks, it would save a ton of overhead and ugly workarounds.I think I thought the original issue included this as part of the fix. :) But the two fit together.
Comment #15
wjaspers CreditAttribution: wjaspers commented@joachim, in response to #8 you'll know which Vocabs are allowed by passing the Vocab ID to the URL which is firing the autocomplete activity.
For instance,
/taxonomy/autocomplete/1,5,23
Would request terms from only vocabs 1, 5, and 23.
The Field making the autocomplete request would assemble the URI properly based in its settings at that time. The reasoning behind taknig the FieldAPI out of the autcomplete is that fields, and ONLY fields can use it now--which limits the ability of modules to use it.
Comment #16
joachim CreditAttribution: joachim commented> 4. Prevent certain entries from being returned by autocomplete
I don't think that letting core find some terms and then removing the ones the user has no access to is the right way to go -- surely you'd then need your API _taxonomy_autocomplete() function to return tids rather than term names.
I think you're doing something sufficiently different from core that you warrant your own autocomplete callback :)
Comment #17
joachim CreditAttribution: joachim commented@wjaspers: you're saying pick the vocab machine name out of the $element's #autocomplete_path? Doable... bit ugly but definitely doable.
Comment #18
joachim CreditAttribution: joachim commentedAt any rate, here's new version of my patch but where the tests should pass :)
(See notes about changes in this patch in #6.)
Comment #19
xjmI have my own autocomplete callback that calls taxonomy.module's. Writing my own from scratch would be duplicating it exactly, line for line, and then adding one additional function at the end. The module's specific internals are beside the point, though. The problem is that the logic of "what stuff" is not decoupled from "do stuff," and that is, inherently, not good. Simply adding wrappers to separate the "do stuff" part out makes the logic reusable.
Comment #20
wjaspers CreditAttribution: wjaspers commentedRT #17, I would just use the Vocab ID to keep things short, but the machine name should be ok.
RT #18
$tag_last isn't running through check_plain (unless drupal_strtolower is doing it for you).
The original implementation allowed the field to request terms from multiple vocabs.People might get frustrated that it only works with one vocab.
Comment #21
joachim CreditAttribution: joachim commented> The original implementation allowed the field to request terms from multiple vocabs.
No, it didn't -- see #6 above.
Comment #22
wjaspers CreditAttribution: wjaspers commentedWhoops. my bad.
Comment #23
joachim CreditAttribution: joachim commented> $tag_last isn't running through check_plain (unless drupal_strtolower is doing it for you).
I'm not changing anything to do with $tag_last, so if it should be, it's another issue :)
Comment #24
xjmNote that #15 actually is a feature request. However, #10 coupled with the working version of joachim's patch (coupling the decouplings!) would I believe provide a back-portable workaround for contrib modules to make multi-vocabulary autocomplete fields. Contrib would override the autocomplete path (as I have done), and then in its own callback, call the helper function I suggest for each vocabulary, merge the results, and then send it to
drupal_json_output($term_matches)
. It wouldn't be optimized, but the number of vocabularies would presumably small, so it would provide the functionality when someone needed it in custom code.Comment #25
joachim CreditAttribution: joachim commented> Note that #15 actually is a feature request.
Yup. And I'm not sure on the kitten status of this.
On the one hand, adding it to the current issue kills kittens. On the other hand, it could be argued that the existing code in the function, because is uses a $vids array, is *designed* for multiple vocabs even though there aren't ever any. So... at a push it could fly.
Comment #26
xjmAttached patch incorporates both the refactor of
taxonomy_autocomplete()
and the decoupling from FAPI to provide fully reusable autocomplete functionality. (As joachim has pointed out, my earlier question about the validator refactor is still related to fields and therefore a separate issue. Similarly, the multiple vocabularies question is a separate issue.).Comment #27
xjmSorry for noise; setting a more accurate title.
Comment #28
joachim CreditAttribution: joachim commentedIf you're really uncoupling, 'typed' doesn't really come into it... and the function name shouldn't mention autocomplete. It's just taxonomy_get_matching_terms(), maybe?
Powered by Dreditor.
Comment #29
xjmHm, I am not sure I agree.
taxonomy_get_matching_terms()
is pretty vague, and the matching behavior is still designed for autocomplete. For example, the data format is autocomplete-specific--the keys include the full field content with previous tags, and the names of the matched terms are the values. So, it is still for taxonomy autocompletion; it is just available to be altered.Comment #30
joachim CreditAttribution: joachim commented> the keys include the full field content with previous tags, and the names of the matched terms are the values
Ah good point.
Comment #31
xjmMaybe
taxonomy_autocomplete_get_matches()
is a better name? I think the rest (about the data format) is clear in the doxygen comment in #26.Comment #32
xjmAttached is identical to #26 except with the better function name
taxonomy_autocomplete_get_matches()
.Comment #33
andypostIt looks like API change, maybe better to fix with parameter 'taxonomy_' . $this->vocabulary->machine_name
auto complete just needs to remove a prefix
-12 days to next Drupal core point release.
Comment #34
dawehnerThis is sort of helpful to replace the custom hack in views.
Comment #35
andypostPatch needs re-roll, having standard auto-complete in core for terms could be very helpful for contrib
Suppose no reason to backport this to D7 because it probably breaks contrib
Comment #36
xjm@andypost, the idea was to provide a helper, and not change the existing API, which would be backportable.
Comment #37
dawehnerRerolled against HEAD, hopefully not that many failed tests.
Comment #38
Wim LeersPatch bingo. Patch looks fine, I just have nitpicks and comments + naming suggestions.
Delete "to autocomplete on"?
s/json/JSON/
Generates taxonomy autocomplete suggestions.
Delete "to autocomplete on"?
"We only …" -> "Only the last tag is autocompleted."
s/possible matches/suggestions/
s/matches/suggestions/
?
Comment #39
dawehnerOh nice!
I'm not 100% sure about the suggestions changes, but yeah here is a patch.
Comment #40
dawehnerForgot the interdiff
Comment #41
Wim Leers"Matches" is fine too, as long as it's consistent everywhere. And if you use "matches", don't say "possible matches", but "matches".
Comment #43
dawehnerI'm not absolute sure about the last change, but I don't think it is really useful to print out some debug messages once someone calls it with a wrong vocabulary-id?
Comment #44
xjmComment #45
dawehnerSo everyone who subscribed to that issue: How do we want to solve it, once we have entity reference using that?
Maybe we should still define our own controller, but pass in some field for views?
We could also help with a additional wrapper for the entity reference one, so we can get the feature required in this issue.
Comment #46
dawehnerI guess this will not happen, so we could move it directly to d7?
Comment #47
jibran#43: drupal-1169964-43.patch queued for re-testing.
Comment #49
tkoleary CreditAttribution: tkoleary commentedPlease follow progress on: #2346973: Improve usability, accessibility, and scalability of long select lists
Changing to Select2 may render this issue obsolete
Comment #50
amateescu CreditAttribution: amateescu commentedWe now have a generic 'entity_autocomplete' form api element: https://www.drupal.org/node/2418529