Reset notice is showing up twice (from core and legal module) and don't disappear after password change

Patch #2 works well.
Matching string check allows the removal of the duplication notification messages.

I can confirm this, coming here as I was also searching for the reason for these weird duplicate messages.

These are the two messages shown:

• You have just used your one-time login link. It is no longer necessary to use this link to log in. Please change your password.
• You have just used your one-time login link. It is no longer necessary to use this link to log in. It is recommended that you set your password.

grepping for them returns:

• modules/contrib/legal/legal.module: $reminder = t('You have just used your one-time login link. It is no longer necessary to use this link to log in. Please change your password.');
• core/modules/user/src/Controller/UserController.php: $this->messenger()->addStatus($this->t('You have just used your one-time login link. It is no longer necessary to use this link to log in. It is recommended that you set your password.'));

This message appears twice after clicking the one time login link after registration and as it seems, that's even a different message?

Grepping for the message from the patch ("You have just used your one-time login link. It is no longer necessary to use this link to log in. Please set your password.") in core (10.3.x) returns no more results!
So it seems this patch needs to be updated again?

@3li could you check that please?

I prepared an updated MR.

Works for me!

Still MAJOR as it affects all modern installations and all their registered users.

MR!16 as static patch attached. Hopefully the maintainer will fix this major issue soon ... :/

Okay after testing the MR!16 for a few days in production, we received feedback that the UX is still bad.

The reason is, that even after changing the password (after reset) the message is still shown.
This is because

$_REQUEST['pass-reset-token']

is always present in the URL even after changing the password. Feel free to try yourself. The URL parameters are kept in Drupal 8+ after form submit!

Looking at the blame you can see that this is legacy code from the Drupal 7 port and I'm pretty sure it should be removed, as core already handles everything we need.

That also safes us from having to update the hard-coded string from MR!16 / core again and again!

For that reason I'd suggest to hide MR!16 and instead merge MR!17

Back to needs review for the new approach. @Grevil will take a look tomorrow and try to align the tests accordingly.

Changes are looking good and everything is working as expected! I'll provide a test to avoid regression in the future.

Ok tests need a bit of refactoring. Since the user in the tests never accepted the legal terms, they will appear, once we try to reset our password. BUT this leads to the message not even appearing on the reset password page, because the "password-reset-token" is removed. (That's also why resetting a password without having the legal terms accepted before, does not currently work. See 🐛 Password can not be reset, when user hasn't accepted the legal terms yet Needs work ).

Alright, finished refactoring the tests! Please review!

Anybody → changed the visibility of the branch 3414370-reset-notice-is-shown-twice to hidden.

Great work @Grevil! Thank you for reworking the tests and adding GitLab CI to show it works as expected now!

RTBC!

Message is not shown at all when user needs to accept T&C.

Steps:

1. Create new T&C version
2. Use one time login link
3. User is asked to accept T&C - accept
4. User is shown password reset page - no message is shown

@Robert Castelo, my apologies, we should have properly adjusted the steps to reproduce before waiting for your review. The user needs to have already accepted the T&C, before resetting their password.
Here are the steps to reproduce (I will also add them in the issue summary):

• Install the module.
• Create new T&C and make sure "Ask to accept T&Cs on every login" is NOT set.
• Login as a non-admin user and accept the T&C.
• Logout.
• Reset the user password and login using the one time login link.
• The T&C won't show up, as the user has already accepted them, and he will be directly forwarded to the user edit page instead.
• The reset notice is showing up twice and after defining another password, it is still shown once.

The reset notice showing up twice:

The reset notice still showing once, after defining a new password:

I hope that helps!

@Grevil: Thanks! We should also check the case @Robert Castelo described and have tests for all possible cases, so we can be sure

1. The message appears as expected
2. The message does not appear multiple times

Both should be checked in the tests.

As a first step the case described by @Robert Castelo should also be checked manually.

That are exactly the tests that are done here. ;)

One is currently commented out and will be implemented in the follow-up issue.

Thanks @Robert Castelo!

Just a tiny heads up, the phpunit pipeline currently fails: https://git.drupalcode.org/project/legal/-/jobs/1593395

But not related to any tests done here, just some legacy tests defining a deprecated theme as their test theme.

This issue caused regression: 🐛 The one-time login message is not displayed anymore, when the user is first redirected to the T&C page. Active (Although minor).

Regression issue fixed.

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