Problem/Motivation

ControllerBase has a convenience method for generating a RedirectResponse.
This would be useful for FormBase and others, which are already using UrlGeneratorTrait.

Proposed resolution

Move redirect() to UrlGeneratorTrait

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#3 interdiff.txt1.38 KBtim.plunkett
#3 2344491-url-redirect-3.patch7.66 KBtim.plunkett
#1 2344491-url-redirect-1.patch6.3 KBtim.plunkett

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Active » Needs review
Related issues: +#2340251: Remove most remaining url() calls
StatusFileSize
new 2344491-url-redirect-1.patch6.3 KB

I've converted the cases that already used $this->url(), leaving the url() calls for #2340251: Remove most remaining url() calls.

dawehner’s picture

dawehner
Primary language German
commented

I can haz tests?

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
StatusFileSize
new 2344491-url-redirect-3.patch7.66 KB
new interdiff.txt1.38 KB

Ask and ye shall receive.

znerol’s picture

znerol commented

see #2282161-10: Split off link/url generation trait

jibran’s picture

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

Looks great.

znerol’s picture

znerol commented

From the original issue I linked in #4.

redirect() belongs only in the Controller, as you have to return a redirect object; it's not just for formatting a string. Triggering a redirect in most non-Controller places is a code smell.

Did something change in the meantime such that it is now justifiable to add redirect() to the trait?

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

We still need them in Forms.

alexpott’s picture

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

Committed 4014b74 and pushed to 8.0.x. Thanks!

Yep forms need this.

  • alexpott committed 4014b74 on 8.0.x 
    Issue #2344491 by tim.plunkett: Move ControllerBase::redirect() to...

Status: Fixed » Closed (fixed)

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