As a followup to #2153891: Add a Url value object, we can now convert form redirects to use the Url object.
| Comment | File | Size | Author |
|---|---|---|---|
| #26 | interdiff.txt | 2.05 KB | tim.plunkett |
| #26 | redirect_route-2189661-26.patch | 111.75 KB | tim.plunkett |
| #17 | redirect_route-2189661-17.patch | 109.16 KB | tim.plunkett |
| #16 | redirect_route-2189661-15.patch | 80.99 KB | tim.plunkett |
| #10 | form_state-redirect-2189661-10.patch | 6.45 KB | tim.plunkett |
Comments
Comment #1
tim.plunkettHere's an example patch to kick things off. This does one of each kinds of redirects in HEAD I think.
Not sending to the bot until the Url issue goes in.
Comment #2
tim.plunkettComment #5
tim.plunkettComment #6
jibranCan we change the scope of the issue to Add FormState::setRedirect()?
Comment #7
tim.plunkettWe already have setRedirect(\Drupal\Core\Url $url);
Comment #8
jibranSo perhaps replace
$form_state['redirect']and$form_state['redirect_route']withsetRedirect(\Drupal\Core\Url $url);. I think it will include conversion in it.Comment #9
tim.plunkettThere was also discussion of a method that took
($route_name, $route_parameters, $options)as params.setRedirectUrl() and setRediect()?
Comment #10
tim.plunkettI think this is what @Berdir was describing in IRC.
Comment #11
jibranI like this apporoch. Are these all the redirect we have in the core?
Minor doc improvement suggestions.
I think we should also add @see setRedirectUrl here.
I think we should also add @see setRedirect here.
Comment #12
tim.plunkettNo, there are about 225 more to convert. I just wanted to get the API right before converting them all.
Comment #13
berdirYep, that's exactly what I meant, nice.
Comment #14
tim.plunkettGoing to work on this on my flight.
Comment #15
tim.plunkettJust posting what I got through so far.
Comment #16
tim.plunkettComment #17
tim.plunkettHmm, I don't know if finishing all of the individual path-based redirects is in scope here. Those have been handled in dedicated issues.
Comment #20
tim.plunkettFixed a couple things.
Comment #21
tim.plunkettRerolled.
Comment #22
jibranThank you for the reroll. This is RTBC. Needs change notice update and a follow up for path-based redirects.
Comment #23
kim.pepperThis is looking good.
Comment #24
alexpottFormState::getRedirect() has the following code / comment:
At the very least the @todo needs updating to point to a new issue.
Comment #25
alexpott@jibran re #22 can you list the CRs you think need updating. Thanks.
Comment #26
tim.plunkettWe can actually remove that right now, the string-based stuff will be covered in this follow-up.
Comment #27
jibran#24 is addressed so back to RTBC.
re #25 https://www.drupal.org/node/2189619 and https://www.drupal.org/node/2174429
Comment #28
alexpottCommitted 627a719 and pushed to 8.0.x. Thanks!
Comment #30
jibranThank you @alexpott for the commit. I have updated the change notices.
Comment #31
tim.plunkettThanks!