Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: The new password policy is not present in the reset password page #35410 #35418

Merged
merged 2 commits into from Feb 26, 2024

Conversation

Codencode
Copy link
Contributor

@Codencode Codencode commented Feb 19, 2024

Questions Answers
Branch? 8.1.x
Description? if you try to do the password reset steps, you will get the reset page where to set a new password two times
from this page is not present the new password verification, in fact it is possible to set weak password
Type? bug fix
Category? FO
BC breaks? no
Deprecations? no
How to test? 1. go at login
2. push on the link "I don't remember the password"
3. set an email to get the reset link
4. push on the reset link
5 set a new password, but weak like "casetta"
6. Password verification will not appear
Fixed issue or discussion? Fixes #35410
Related PRs Template modification is required PrestaShop/classic-theme#140
Sponsor company Codencode

@Codencode Codencode requested a review from a team as a code owner February 19, 2024 18:17
@prestonBot prestonBot added 8.1.x Branch Bug fix Type: Bug fix labels Feb 19, 2024
Copy link
Contributor

@Hlavtox Hlavtox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving for now, but maybe we should unify the logic, because we have basically the same code in OrderConfirmationController and GuestTrackingController. ;-)

@ps-jarvis ps-jarvis added the Waiting for QA Status: action required, waiting for test feedback label Feb 20, 2024
@PrestaShop PrestaShop deleted a comment from prestonBot Feb 20, 2024
@Hlavtox Hlavtox added this to the 8.1.5 milestone Feb 20, 2024
@Codencode
Copy link
Contributor Author

Approving for now, but maybe we should unify the logic, because we have basically the same code in OrderConfirmationController and GuestTrackingController. ;-)

Hi @Hlavtox,
I did not notice that.
How could we unify the 3 parts?

@paulnoelcholot
Copy link

Hi @Hlavtox and @Codencode,

Do you think it is better to unify everything in a single PR or would you prefer that we test this one and create another PR to unify the rest?

Thanks for your feedback

@Codencode
Copy link
Contributor Author

@Hlavtox,

Hi @paulnoelcholot,

We are waiting for the opinion of @Hlavtox who has more experience than me!

Thank you.

@paulnoelcholot
Copy link

The future of the world is in your hands @Hlavtox 😄

@kpodemski
Copy link
Contributor

@PrestaShop/qa-functional you can proceed with the tests, there's always a room for improvement later ;-)

@Hlavtox
Copy link
Contributor

Hlavtox commented Feb 25, 2024

@paulnoelcholot @Codencode I forgot about this PR sorry guys 🤣

Lets merge this, we can improve later!

@AureRita AureRita self-assigned this Feb 26, 2024
Copy link
Contributor

@AureRita AureRita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Codencode

Thank you for your PR, I tested it and it seems to works as you can see :

recording.112.webm

Link to the auto test : https://github.com/AureRita/testing_pr/actions/runs/8048090042

Because the auto test is 🟢 and the PR seems to works as expected, It's QA ✔️

Thank you

@AureRita AureRita added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Feb 26, 2024
@AureRita AureRita removed their assignment Feb 26, 2024
@kpodemski kpodemski merged commit a72cc0a into PrestaShop:8.1.x Feb 26, 2024
38 checks passed
@kpodemski
Copy link
Contributor

Great! Thank you @Codencode @AureRita :)

@Codencode
Copy link
Contributor Author

However, this PR also requires the modification to the template that I made using the PR PrestaShop/classic-theme#140 of the classic theme.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.1.x Branch Bug fix Type: Bug fix QA ✔️ Status: check done, code approved
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

8 participants