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.
As part of porting search_api_autocomplete
we need to adapt \Drupal\search_api_db\Plugin\search_api\backend\Database::getAutocompleteSuggestions()
.
Comment | File | Size | Author |
---|---|---|---|
#18 | 2747283-18.patch | 1.16 KB | dawehner |
#10 | 2747283-4.patch | 831 bytes | dawehner |
#2 | 2747283-2.patch | 1.25 KB | dawehner |
Comments
Comment #2
dawehnerComment #5
borisson_We can't implement a search api autocomplete interface in search api, does the interface need to be moved to search api?
Comment #6
drunken monkeyWhat are you talking about?
Are you working on porting the Autocomplete module? Where is the issue? And what's the purpose of this issue?
Also, as Joris says, we can't just use some other module's interfaces without depending on it (and we should surely not depend on it).
Comment #7
dawehnerSorry for my quite non verbose issue :) But yes, i'm working on porting the search_api_autocomplete module, see https://www.drupal.org/node/2682949#comment-11282727
For that I just posted my current local hack. I guess the right way is to keep the function in here, but just don't add the interface, much like D7 worked?
Comment #8
drunken monkeyYes, unfortunately I don't think there is any other option.
Otherwise we wouldn't need
supportsFeature()
either, but could just use interfaces.But good to hear you're working on a port! Then I guess we can just leave this open until you have the necessary patch for the backend.
Comment #9
drunken monkeyComment #10
dawehnerSorry again for my stupid first patch. This portion though is something we should still have.
Comment #11
borisson_@dawehner, are you sure that that patch does something?
Comment #12
dawehner@borisson_
Yes! Open up your IDE without the patch and browse to
\Drupal\search_api_db\Plugin\search_api\backend\Database::getAutocompleteSuggestions
. You'll see two unknown classes. This patch fixes that.Comment #13
borisson_Oh, okay! In that case, let's get this in.
Comment #14
dawehnerI agree its funny how much this patch solves.
Comment #15
drunken monkeyWhy does it need the interface imported, when we don't use it?
Also, you should create an interface for the entity, since type hinting should be done on interfaces wherever possible. (I'd also rename the class(es) to not include the module name as prefix, since those aren't needed anymore with namespaces. But I admit that the coding standards discussion is still ongoing.)
Comment #16
dawehnerI like this suggestion!
Well that itself is also a bit in discussion at the moment, but yeah its easy to add an interface ...
Comment #17
drunken monkeyOK, then let's see what changes will be made to the autocomplete module yet and then apply them here. (Or, as discussed, maybe the implementation will get moved to the autocomplete module anyways and we just have to remove the code here.)
Comment #18
dawehnerHere is a small adaption.
Comment #19
borisson_These are very small changes that are needed, so let's commit both patches. #10 + #18.
Comment #21
borisson_Oh, this should be rtbc, because the patch fails are not related to this issue, we have the same fails in #2777483: Unmet dependencies.
Comment #22
mpp CreditAttribution: mpp as a volunteer and at AmeXio commentedSee https://www.drupal.org/node/2784849#comment-11533091
Comment #24
drunken monkeyOK, fine with me, too.
Committed.
Thanks, dawehner!
Should I leave this issue open, or will we open a new one in case further changes are needed?
Comment #25
dawehnerThank you for the commit! We are getting somewhere.
I personally don't care. Do you have a preference?
Comment #26
drunken monkeyAh, let's just mark it as "fixed".