Incorrect typehint of LOGGED_IN and LOGGED_OUT consts in UserAuthenticationController

Hi , i have fixed and added a patch for this , please review

Can we please have an MR for this?

Generated MR for the same, please review.

@viren18febS this was tagged for novice intended for newer users to Drupal contribution.. Looking at your post history you have 14 pages of issues and should avoid novice issues in the future.

Hi smustgrave,

Sure i'll take care of that.
Thank you.

The comments above also mention strings, but the values are not strings.

Feedback appears to be addressed.

Just realised actually where this is used:

    if ($this->currentUser()->isAuthenticated()) {
      $response = new Response(self::LOGGED_IN);
    }
    else {
      $response = new Response(self::LOGGED_OUT);
    }

The Response constructor takes a string!

    public function __construct(?string $content = '', int $status = 200, array $headers = [])

So this is being cast back to string anyway; should we actually be changing them to e.g.

  const LOGGED_IN = '1';

Agree with #14, the doc suggests this should indeed be a string. I suspect this may be misused in userland and might break if people are checking $foo === UserAuthenticationController::LOGGED_IN.

Sounds like this needs to be re-scoped.

Have updated the MR with the new approach to use strings. Added a CR just in case anyone is using these constants with strict comparisons.

Apologies for picking this up with the Novice tag, it is blocking 📌 [PP-1] Fix strict type errors in Functional tests: Add casts and fix types where needed Postponed so I was motivated to work on it.

@mstrelen thanks for noting that, think if it's a blocker for a larger issue that's perfectly valid.

Issue summary was updated by @larowlan in #17 and consts turned to strings.

larowlan → closed merge request !6037

Committed to 11.x and published the change record, thanks

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