Problem/Motivation

#2189661: Replace $form_state['redirect_route'] with setRedirect() removes direct setting of redirects that use routes. However we're still left with path based redirects.

Internally, FormState tracks both 'redirect' and 'redirect_route'. The latter is used for the modern approaches, the former for legacy.

Proposed resolution

The legacy support is completely removed. Internally all redirects are tracked by 'redirect', which is a \Drupal\Core\Url object.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#8 redirect-2315807-8.patch39.88 KBtim.plunkett
#5 redirect-2315807-5.patch145.12 KBtim.plunkett
#5 interdiff.txt767 bytestim.plunkett
#3 interdiff.txt1.73 KBtim.plunkett
#3 redirect-2315807-3.patch145.02 KBtim.plunkett
#1 interdiff.txt39.2 KBtim.plunkett
#1 redirect-2315807-1.patch144.44 KBtim.plunkett

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Active » Needs review
StatusFileSize
new redirect-2315807-1.patch144.44 KB
new interdiff.txt39.2 KB

If this passes before #2189661: Replace $form_state['redirect_route'] with setRedirect() goes in, I might merge it back in.

Status: Needs review » Needs work

The last submitted patch, 1: redirect-2315807-1.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs work » Needs review
StatusFileSize
new redirect-2315807-3.patch145.02 KB
new interdiff.txt1.73 KB

Status: Needs review » Needs work

The last submitted patch, 3: redirect-2315807-3.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt767 bytes
new redirect-2315807-5.patch145.12 KB

Fixed the batch API bit I broke.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Issue tags: +Needs reroll

After #2189661: Replace $form_state['redirect_route'] with setRedirect()

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Issue tags: +Needs reroll

After #2189661: Replace $form_state['redirect_route'] with setRedirect()

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Issue tags: -Needs reroll
StatusFileSize
new redirect-2315807-8.patch39.88 KB
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

Awesome work. I have already updated https://www.drupal.org/node/2174429 change notice but we need a sperate change notice for this anyway.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Title: Remove support for string-based form redirects » Remove support for path-based form redirects

Added https://www.drupal.org/node/2316687

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community

Perfect.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Issue summary: View changes
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 18ddb00 and pushed to 8.0.x. Thanks!

Looks good. I've had a quick look for change records that need updating and surprising I could not find any.

diff --git a/core/tests/Drupal/Tests/Core/Form/FormSubmitterTest.php b/core/tests/Drupal/Tests/Core/Form/FormSubmitterTest.php
index f4cea43..e76e9a6 100644
--- a/core/tests/Drupal/Tests/Core/Form/FormSubmitterTest.php
+++ b/core/tests/Drupal/Tests/Core/Form/FormSubmitterTest.php
@@ -125,7 +125,7 @@ public function testRedirectWithNull() {
    *
    * @dataProvider providerTestRedirectWithUrl
    */
-  public function testRedirectWithUrl($redirect_value, $result, $status = 303) {
+  public function testRedirectWithUrl(Url $redirect_value, $result, $status = 303) {
     $container = new ContainerBuilder();
     $container->set('url_generator', $this->urlGenerator);
     \Drupal::setContainer($container);

Minor improvement on commit.

  • alexpott committed 18ddb00 on 8.0.x 
    Issue #2315807 by tim.plunkett: Remove support for path-based form...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

hass’s picture

hass commented
Status: Closed (fixed) » Needs work

I thought change logs are only for D7 to D8 upgrades. The documentation https://www.drupal.org/node/2316687 is missing this migration details.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs work » Closed (fixed)

Change records are not just D7-D8. See https://www.drupal.org/node/2310411 for more details about form object changes. If that's not enough, please open a new issue.