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.
Files that need converting are:
- core/modules/taxonomy/lib/Drupal/taxonomy/TermFormController.php
- core/modules/taxonomy/taxonomy.admin.inc
Comment | File | Size | Author |
---|---|---|---|
#35 | 1999436-taxonomy-request-35.patch | 807 bytes | kim.pepper |
#33 | taxonomy-1999436-33.patch | 1.98 KB | tim.plunkett |
#29 | 1999436-taxonomy-request-29.patch | 4.35 KB | effulgentsia |
#29 | interdiff.txt | 1.48 KB | effulgentsia |
#25 | 1999436-taxonomy-request-25.patch | 5.15 KB | kim.pepper |
Comments
Comment #1
bojanz CreditAttribution: bojanz commentedComment #2
bojanz CreditAttribution: bojanz commentedThe formatting here is wrong (we don't do the indentation of the equals sign thingy), but it's not the job of this issue to fix that.
The TermFormController changes follow what ViewsEditFormController is doing.
Comment #3
kim.pepperThis needs to be removed if we are doing $query->remove() above.
Comment #4
bojanz CreditAttribution: bojanz commentedNo, see the added code comment. Plus, this is the same todo that ViewsEditFormController has.
Comment #5
bojanz CreditAttribution: bojanz commentedNo, see the added code comment. Plus, this is the same todo that ViewsEditFormController has.
Comment #6
kim.pepperThat sucks. It also means there are a lot of other conversions that are going to fail for the same reason.
Comment #7
kim.pepper#1668866: Replace drupal_goto() with RedirectResponse went in so removed the manual destination handling cruft. Tested manually ok.
Comment #8
larowlanRequest can be injected, form controllers support injection
if ($page = Drupal::request()->query->has('page')) {
then you can just use $page
Comment #9
kim.pepperThanks for the review. This includes fixes for #8.
Comment #11
kim.pepperMake the $request param have a NULL default so it still respects the buildForm() interface method.
Comment #13
sidharthapassigning it to me.
Comment #14
sidharthapI tried to apply the patch mentioned on #7 and #11. Both displaying error.
error: patch failed: core/modules/taxonomy/taxonomy.admin.inc:44
error: core/modules/taxonomy/taxonomy.admin.inc: patch does not apply
is i am doing something wrong here ?
Comment #15
sidharthapcorrected patch file.
Thanks
Comment #17
kim.pepperThis is a reroll of the code at #11. I mistakenly thought you could pass in $request via build form.
This uses the new EntityControllerInterface to inject the request via the constructor.
Comment #18
tim.plunkettThis should be in buildForm() instead, as an optional param.
Comment #19
kim.pepperPuts the request back into buildForm() by making it a proper route rather than a page callback as per #18
Comment #20
kim.pepperand the patch... :-)
Comment #21
kim.pepperForgot to remove the unused EntityControllerInterface
Comment #22
tim.plunkettThis looks prefect. RTBC if green...
Comment #23
kim.pepperI put the $request param on form() instead of buildForm().
Comment #25
kim.pepperThis removes a test for checking that additional request paths are not used. As we have converted to a symfony route, and this is not possible, I have removed the test.
Comment #26
kim.pepperdouble-post
Comment #27
effulgentsia CreditAttribution: effulgentsia commentedLooks great.
Comment #28
effulgentsia CreditAttribution: effulgentsia commentedComment #29
effulgentsia CreditAttribution: effulgentsia commentedUpdated for #2059245: Add a FormBase class containing useful methods.
Comment #30
tim.plunkettLooks good, thanks!
Comment #31
alexpottPatch no longer applies
Comment #32
alexpottComment #33
tim.plunkettOnly leftovers now.
Comment #34
alexpottPatch no longer applies
Comment #35
kim.peppertaxonomy.admin.inc was removed in #1974492: Convert taxonomy_overview_terms to a Form Controller
Only TermFormController left now.
Comment #36
kim.pepperAssuming green, moving back to RTBC as #5 is a re-roll, and Tim was happy in #33
Comment #37
webchickCommitted and pushed to 8.x. Thanks!