Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I'm probably doing this wrong, but I think I can't find anything for taxonomy term names. I need to do this for a customer project and I'd love to help out but I don't understand this well enough to actually be of service for now. I tried this with beta 5 the last dev as well as with both git-HEAD projects.
Steps to reproduce:
- Install search api, db defaults, autocomplete
- Add the term name to the index as a fulltext field (see tags.png)
- Configure autocomplete for this index, select the tags field in the "retrieve from server" plugin (see autocomplete-config.png)
- Go to frontend, search for on the values in the term
After .4, the expected result is to see the values of the term in the autocomplete, actually nothing shows up.
Comment | File | Size | Author |
---|---|---|---|
#32 | 2867144-32--reenable_autocomplete_test.patch | 564 bytes | drunken monkey |
#19 | 2867144-19--autocomplete_test.patch | 8.21 KB | drunken monkey |
Comments
Comment #2
borisson_Comment #3
borisson_I spent some time on this yesterday as well as some more this morning but I can't figure it out, this might belong in search_api_db as I think that the problem in there.
First of all, it looks like the min_chars configuration from autocomplete is not passed into db properly, but that seems like a non-issue for me.
The results we get back from the db modules is an empty array and that shouldn't happen. I'd like to write a test for this but I'm not sure how to start. I think this should be a kernel test in search_api_db.
Comment #4
drunken monkeyDoes the information that is passed to the DB backend look correct? Then yes, please move it to the Search API queue, DB backend component, and we'll try to come up with a test there.
Comment #5
borisson_It does look correct.
Comment #6
drunken monkeyThis adds infrastructure for testing autocomplete, plus two assertions for simple autocompletion cases.
For me, both of them pass. If they do for you, too, please try to add a failing assertion based on your specific scenario. (If necessary, you can of course also change the index (in the test code, not the export) or other config, or add more items.)
Comment #8
drunken monkeyDamn, that #$*@
@group
gets me every single time …Comment #10
drunken monkeyOh, didn't know about that annotation … Great work, Drupal testing infrastructure!
(Hm, though it remains to be seen whether this will make it download the module or skip the test …)
However, forgot to say before that I'm not sure anyways whether we should commit this patch at this point. The Autocomplete module is far from stable, I'll still surely change lots of things (e.g., most class/interface names), and I'm not sure breaking the Search API tests each time is a good idea. (It would at least ensure we keep our integration up-to-date, but it's still less than ideal, I'd say. Even if both modules are maintained by the same person.)
Comment #12
borisson_I think the @requires should be on the class, let's test that. Will need to test the actual patch still.
Comment #14
borisson_Comment #16
borisson_I tested the patch and it works, looks good. I think the tests are still failing because dependencies are calculated on commit, not just in a patch.
Comment #17
drunken monkeyYes, seems that's the problem. Then let's just go with the patch in #8.
However, I actually thought you wanted to use the test to show what doesn't work? (Even if the test bot fails in any case, we can use it locally for verifying/debugging.) Would be great if you could try to come up with a failing test based on the patch.
Also, see my notes about committing this in #10 – what's your opinion on that?
Comment #18
borisson_I think we should already add this, since autocomplete doesn't have a lot of tests yet. And this side does need tests. I did want to use this patch for a usecase that didn't work, however I did get it to work with the latest dev versions so I think it was just a case of having older modules and/or bad configuration in my original setup.
This test doesn't use a lot of internals of what the autocomplete module does, so committing it now doesn't seem like a bad idea.
Comment #19
drunken monkeyIt's not really about internals. The
SearchApiAutocompleteSearchInterface
class name, e.g., will definitely be changed.Hm, so maybe just remove the type hint there for now? (Patch attached, interdiff compared to #8.)
However, the method signature itself might also change, and with the discrepancy between dev version and the version used by the d.o test bot, it might be tricky to adapt to that without causing test fails in the mean time.
Furthermore, now that we know this is working, I'd say it's pretty unlikely to break unless we actually change that method. In which case we can also just run the tests manually, if we want to verify we didn't break anything.
But all of that considered, if you still think we should add it, then let's do it. (With or without type hint, your choice.) We'll survive if the test bot chokes at some point on this for a day or two.
Comment #21
borisson_I think we should add the test now, we can always change it later when we decide to change the class / method names. The patch here seems to be failing though, so let's fix that before commit.
Comment #22
drunken monkeyIt's working fine for me. What failures do you get?
(The test bot failing, as discussed, is just a fault in the test bot regarding new module dependencies. It should be fixed once we commit the patch – at least we'll have to hope that.)
Comment #23
borisson_Ah yes, I forgot about that. Sorry.
Comment #25
drunken monkeyAh, OK, no problem.
Committed. Let's hope for the best. ;)
Hm, should we then mark this as "Fixed" or move it back to "Active" for your original problem?
Comment #26
borisson_Let's keep it as fix for now, I can reopen / open a new issue when I get to that project again :)
Comment #27
drunken monkeyDidn't pass after all, for some reason, had to disable it again for now. Maybe after we've made another release for the Search API? (Which we probably should do soon in any case.)
Comment #28
drunken monkeyLet's try again.
(Just realize I should have probably removed it from the changelog after disabling the test. But well, no big deal.)
Comment #30
drunken monkeyAh, wait, seems we forgot to actually depend on the Search API Autocomplete module in the test module! Maybe that'll solve it?
Comment #32
drunken monkeyApparently not.
But I now committed that dependency, maybe that helps. Otherwise, let's wait for the next release (1.2), then try again and if it still doesn't work go complain in the Drupal CI issue queue.
Perhaps it won't work as long as the Autocomplete module doesn't have a stable release?
Comment #34
drunken monkey