UserLoginForm::submitForm has some dead code [#3340128]

Problem/Motivation

$this->getRequest()->query->set('destination', $this->getRequest()->request->get('destination')); does nothing, let's remove it
Also we're doing some dancing around generating a URL, but in fact we can just use ->setRedirectUrl($account->toUrl())

Steps to reproduce

Proposed resolution

Remove the else clause
Change setRedirect to use ->setRedirectUrl($account->toUrl())

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comment	File	Size	Author
#13	3340128-13.patch	1.77 KB	amanshukla6158
#12	interdiff_9-12.txt	3.92 KB	akram khan
#12	3340128-12.patch	2.31 KB	akram khan

Issue fork drupal-3340128

Show commands

Start within a Git clone of the project using the version control instructions.

Add & fetch this issue fork’s repository

Or, if you do not have SSH keys set up on git.drupalcode.org:

Add & fetch this issue fork’s repository

2 hidden branches

3319184-hookconditioninfoalter-is-not

changes, plain diff MR !3410

Check out this branch for the first time

Check out existing branch, if you already have it locally

3340128-userloginformsubmitform-has-some

changes, plain diff MR !3409

Check out this branch for the first time

Check out existing branch, if you already have it locally

About issue forks

Comments

Comment #1

8 February 2023 at 04:45

larowlan created an issue. See original summary.

Comment #2

larowlan

🇦🇺🏝.au GMT+10

commented 8 February 2023 at 04:51

Issue summary:

View changes

Comment #3

8 February 2023 at 04:52

Gauravvv made their first commit to this issue’s fork.

Comment #4

royalpinto007

he/him

English

commented 8 February 2023 at 04:52

Assigned:	Unassigned	» royalpinto007
Status:	Active	» Needs work

Comment #5

8 February 2023 at 04:54

Gauravvv opened merge request !3409

Comment #6

gauravvvv commented 8 February 2023 at 04:59

Status:

Needs work

» Needs review

Comment #7

mglaman

English

WI, USA

commented 8 February 2023 at 05:19

This looks like it handles redirects set by DefaultExceptionHtmlSubscriber

See \Drupal\Core\EventSubscriber\DefaultExceptionHtmlSubscriber::makeSubrequest

      // Add to query (GET) or request (POST) parameters:
      // - 'destination' (to ensure e.g. the login form in a 403 response
      //   redirects to the original URL)
      // - '_exception_statuscode'
      $parameters = $sub_request->isMethod('GET') ? $sub_request->query : $sub_request->request;
      $parameters->add($this->redirectDestination->getAsArray() + ['_exception_statuscode' => $status_code]);

This adds destination into the request parameter bag.

The big red flag is the fact tests didn't fail! Which means this is not tested.