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.
Convert the page callback "views_ajax_autocomplete_taxonomy" to a new-style Controller, using the instructions in the WSCCI Conversion Guide.
@see views_menu() in core/modules/views/views.module
Comment | File | Size | Author |
---|---|---|---|
#35 | vdc-1979038-35.patch | 11.1 KB | dawehner |
#35 | interdiff.txt | 1.93 KB | dawehner |
#32 | vdc-1979038-32.patch | 10.72 KB | dawehner |
#32 | interdiff.txt | 1.66 KB | dawehner |
#28 | interdiff.txt | 12.07 KB | dawehner |
Comments
Comment #1
xjmSince this is an AJAX callback rather than a full page callback, we should wait on #1944472: Add generic content handler for returning dialogs.
Comment #2
xjmComment #3
jibran#1944472: Add generic content handler for returning dialogs is in.
Comment #4
tstoecklerI think instead of converting it to a route, we should probably convert Views to use [#1801356], no?
Comment #5
dawehnerTheoretically #1169964: Taxonomy autocomplete: decouple from Field API and factor out a helper so that code is reusable would have allowed us to not have our own code, but well now that taxonomy uses the entity reference callback we are basically lost.
Comment #6
glennpratt CreditAttribution: glennpratt commented@tstoeckler that issue doesn't appear to exist, what were you trying to reference?
Comment #7
tstoecklerDon't know why Drupal.org hates me, but the issue does exist: http://drupal.org/node/1801356. It's the issue (that's already in) about a generic entity reference autocomplete. The problem is that views wants an autocomplete that does not rely on a field, but only filters taxonomy terms by bundle. I still think that should be a generic utility somewhere, but @dawehner is right, that Entity Reference doesn't make sense, as that is solely about fields. Maybe we can make a generic EntityAutocomplete in \Drupal\Core\Entity that Entity Reference extends?
Comment #8
amateescu CreditAttribution: amateescu commentedActually, there is a patch for ER that tries to solve exactly this use case, make the generic entity reference available as a form element that doesn't rely on fields. The code is not quite ready yet and I've been awfully slow at reviewing it, but Views is exactly the reason why I pushed my colleague (@goldorak) to submit it as a D8 patch first. Here's the issue: #1959806: Provide a generic 'entity_autocomplete' Form API element
Comment #9
tstoecklerRe #8: Thanks @amateescu, I wasn't aware of that issue. Should we re-title this "Remove views_ajax_autocomplete_taxonomy()" and postpone on that one?
Comment #10
amateescu CreditAttribution: amateescu commentedYep, that sounds like a good plan.
Comment #11
tstoecklerOK. :-)
Comment #12
effulgentsia CreditAttribution: effulgentsia commented#1959806: Provide a generic 'entity_autocomplete' Form API element is currently categorized as a feature request and has had no activity in 3 months. Meanwhile, this issue is part of a critical meta, so shouldn't be postponed on that one.
Comment #13
dawehnerIt is good to know that views already has a test for it.
In theory you could argue to just move this code to the autocomplete controller of taxonomy and reuse most of the code.
Comment #14
jibranOther then this white space RTBC.
Comment #15
tstoecklerThere we go. Leaving at RTBC.
Comment #16
effulgentsia CreditAttribution: effulgentsia commentedCNR for bot.
Comment #17
effulgentsia CreditAttribution: effulgentsia commentedTaxonomyIndexTid::valueForm() needs to change the #autocomplete_path to #autocomplete_route_name. If #16 passes tests, that also means we're missing test coverage for this autocomplete appearing.
Comment #18
jibranI was also going to remove the menu item but @dawehner said we can't remove
menu items
with theme callback.Comment #19
effulgentsia CreditAttribution: effulgentsia commentedThat's generally correct, but autocomplete controllers aren't ajax in the sense of Drupal's Ajax API, so don't require this theme callback.
Comment #20
effulgentsia CreditAttribution: effulgentsia commentedTagging per #17.
Comment #21
effulgentsia CreditAttribution: effulgentsia commentedHere's the test.
Comment #22
effulgentsia CreditAttribution: effulgentsia commentedAnd the fix.
Comment #23
dawehnerOh right I agree to remove the 'theme callback'. The request is not a classical ajax request, which pulls down potentially other CSS but it just returns the JSON result.
Comment #24
jibranIt is green it has tests and clean conversion so back to RTBC.
Comment #25
kim.pepper#22: vdc-1979038-22.patch queued for re-testing.
Comment #26
catchConversion looks fine, but while we're moving this, why not just put it in taxonomy module?
Comment #27
alexpottThis needs to use the new
Drupal\Core\Controller\ContainerInjectionInterface
as ControllerInterface no longer exists. See #2057589: [Docs clean-up] Rename ControllerInterface to ContainerInjectionInterfaceComment #28
dawehnerThis adds a new method on the taxonomy autocompletion and reuses as much as possible.
Comment #30
dawehner#28: views_taxonomy_ajax-1979038-28.patch queued for re-testing.
Comment #32
dawehnerFixed it.
Comment #33
jibranAs #26 is addressed. Back to RTBC.
Comment #34
alexpottManually tested works fine
But
This will never return a normal response.
All the respective use statements for these classes can be removed from the top of this file. They are no longer used.
Comment #35
dawehnerGood points!
Comment #36
jibranBack to RTBC as #34 is fixed.
Comment #37
webchickCommitted and pushed to 8.x. Thanks!
Comment #38.0
(not verified) CreditAttribution: commentedUpdated issue summary.