Problem/Motivation
In LocaleController::checkTranslation() we are returning a ResponseRedirect object hardcoding a uri.
We should use $this->redirect with a route name instead.
Proposed resolution
Use $this->redirect()
Remaining tasks
Patch!
See if this is covered by tests.
Write tests if not.
Review!
User interface changes
None.
API changes
None.
Task | Novice task? | Contributor instructions | Complete? |
---|---|---|---|
Create a patch | Novice | Instructions | Yes |
Reroll the patch if it no longer applies. | Novice | Instructions |
Comment | File | Size | Author |
---|---|---|---|
#10 | use_this_redirect-2470980-10.patch | 684 bytes | PieterJanPut |
Comments
Comment #1
fran seva CreditAttribution: fran seva commentedComment #2
fran seva CreditAttribution: fran seva commentedComment #3
PieterJanPut CreditAttribution: PieterJanPut commentedComment #4
PieterJanPut CreditAttribution: PieterJanPut at iO commentedComment #5
PieterJanPut CreditAttribution: PieterJanPut at iO commentedComment #7
PieterJanPut CreditAttribution: PieterJanPut at iO commentedComment #8
PieterJanPut CreditAttribution: PieterJanPut at iO commentedComment #9
mitrpaka CreditAttribution: mitrpaka commented$this->redirect() requires route name instead of url.
Use
return $this->redirect('locale.translate_status');
to correct it (See locale.routing.yml for details).No additional test needed, covered by Drupal\locale\Tests\LocaleTranslationUiTest
Comment #10
PieterJanPut CreditAttribution: PieterJanPut at iO commentedComment #12
fran seva CreditAttribution: fran seva commentedThe patch is good for me
Comment #13
fran seva CreditAttribution: fran seva commentedComment #14
fran seva CreditAttribution: fran seva commentedComment #15
penyaskito+1, thanks for working on it!
Comment #16
alexpottClassifying this cleanup as a bug. This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 3d93230 and pushed to 8.0.x. Thanks!
Comment #19
Gábor HojtsyThanks!