Skip to content

Fix the issue with reset password when customer has address from not allowed country #18179

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

Merged

Conversation

dmytro-ch
Copy link
Contributor

@dmytro-ch dmytro-ch commented Sep 21, 2018

Description

Fix the issue with reset password when customer has an address from not allowed country.

Fixed Issues (if relevant)

  1. Unable to reset password if customer has address from not allowed country #18170: Unable to reset password if customer has address from not allowed country

Manual testing scenarios

  1. Create customer on frontend, add some customer address with Germany (just an example) and log out
  2. Go to Admin >> Stores >> Configuration >> General >> General
  3. In Country Options >> Allow Countries deselect Germany (mark as not allowed), click to Save Config (probably flushing cache also required)
  4. On frontend got to "Forgot password" form and try to reset password for customer that you created in step 1

Expected result

  1. Customer should get reset password link without any issues

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@dmytro-ch dmytro-ch requested a review from rogyar September 21, 2018 13:15
@magento-engcom-team magento-engcom-team added Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner labels Sep 21, 2018
@magento-engcom-team
Copy link
Contributor

Hi @dmytro-ch. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me $VERSION instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@rogyar
Copy link
Contributor

rogyar commented Sep 24, 2018

Please, consider fixing the failing static tests. The issues might not be related directly to your changes but to files that were changed in general. If you need help with that, feel free to connect me in Slack

@dmytro-ch
Copy link
Contributor Author

@rogyar, thank you!
I've fixed the code sniffer and mess detector issues.

@slavvka
Copy link
Member

slavvka commented Sep 26, 2018

@dmytro-ch please look at Codacy review and fix the issue.

@dmytro-ch
Copy link
Contributor Author

@slavvka, thank you! I've fixed the issue mentioned in Codacy review.

@rogyar
Copy link
Contributor

rogyar commented Sep 27, 2018

@magento-engcom-team give me 2.2.5 instance

@magento-engcom-team
Copy link
Contributor

Hi @rogyar. Thank you for your request. I'm working on Magento 2.2.5 instance for you

@magento-engcom-team
Copy link
Contributor

Hi @rogyar, here is your Magento instance.
Admin access: https://i-18179-2-2-5.engcom.dev.magento.com/admin
Login: admin Password: 123123q
Instance will be terminated in up to 3 hours.

@slavvka
Copy link
Member

slavvka commented Sep 28, 2018

@dmytro-ch I didn't notice that $visitorCollectionFactory existed before. Please could you revert the change back to maintain backward compatibility? You may ignore Codacy fail then. Thank you!

@dmytro-ch
Copy link
Contributor Author

@slavvka, sure, I reverted the changes :)
Thank you.

Copy link
Member

@slavvka slavvka left a comment

Choose a reason for hiding this comment

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

Thank you!

@magento-engcom-team
Copy link
Contributor

Hi @slavvka, thank you for the review.
ENGCOM-3056 has been created to process this Pull Request

@slavvka
Copy link
Member

slavvka commented Nov 14, 2018

Hey @dmytro-ch during testing we found the problem:

Problem: Cannot set New Password using reset password link.

Manual testing scenario:

  1. Create customer on frontend.
  2. Add customer address with Germany country
  3. Log out.
  4. Go to Admin >> Stores >> Configuration >> General >> General
  5. In Country Options >> Allow Countries deselect Germany (mark as not allowed), click to Save Config.
  6. On frontend go to "Forgot password" page.
  7. Fill form and click Reset My Password button.
  8. Click Set a New Password link from email.
  9. Enter new password and click Set a New Password
  10. ❌ Error message "Invalid value of "DE" provided for the countryId field."
    image

@slavvka
Copy link
Member

slavvka commented Nov 14, 2018

Also please resolve the conflicts with mainline

# Conflicts:
#	app/code/Magento/Customer/Model/AccountManagement.php
#	app/code/Magento/Customer/Test/Unit/Model/AccountManagementTest.php
@dmytro-ch
Copy link
Contributor Author

Hello @slavvka,
I've resolved the merge conflict and fixed the mentioned issue.

Thank you!

@magento-engcom-team
Copy link
Contributor

Hi @dmytro-ch. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Award: bug fix Component: Customer Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner Progress: accept Release Line: 2.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants