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

ISSUE-27954: Forgot password save user only one column #27972

Merged
merged 5 commits into from
Nov 9, 2020
Merged

ISSUE-27954: Forgot password save user only one column #27972

merged 5 commits into from
Nov 9, 2020

Conversation

sheepfy
Copy link
Contributor

@sheepfy sheepfy commented Apr 24, 2020

Fixes #27954

@m2-assistant
Copy link

m2-assistant bot commented Apr 24, 2020

Hi @sheepfy. 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 give me test instance - deploy test instance based on PR changes
  • @magento give me 2.4-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

@ghost ghost added this to Pending Review in Pull Requests Dashboard Apr 24, 2020
@ghost ghost added the Severity: S1 Affects critical data or functionality and forces users to employ a workaround. label Apr 24, 2020
@ihor-sviziev ihor-sviziev self-assigned this Apr 24, 2020
@okorshenko okorshenko linked an issue Apr 24, 2020 that may be closed by this pull request
@ihor-sviziev ihor-sviziev removed their assignment Apr 25, 2020
@ghost ghost added Priority: P3 May be fixed according to the position in the backlog. Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. and removed Severity: S1 Affects critical data or functionality and forces users to employ a workaround. labels Apr 29, 2020
@aleron75 aleron75 self-assigned this Aug 29, 2020
@aleron75
Copy link
Contributor

Hello @sheepfy thanks for your contribution, it's very appreciated.

Due to Magento Definition of Done, all code must be covered by tests.

For this specific case, this fix should be covered by automated tests with the scenario which leads to an issue.
Tests should fail on the mainline and pass with this fix.

To answer the question "which kind of tests should we write", I would go with a lightweight unit test.

@aleron75 aleron75 added Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests Award: bug fix labels Aug 29, 2020
Copy link
Contributor

@ihor-sviziev ihor-sviziev left a comment

Choose a reason for hiding this comment

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

Hi @sheepfy,
Could you update your solution as it was requested here? #27972 (comment)

@ghost ghost moved this from Pending Review to Changes Requested in Pull Requests Dashboard Sep 21, 2020
@ghost ghost assigned ihor-sviziev Sep 21, 2020
@ihor-sviziev ihor-sviziev removed their assignment Sep 21, 2020
@sheepfy
Copy link
Contributor Author

sheepfy commented Sep 21, 2020

Hi @sheepfy,
Could you update your solution as it was requested here? #27972 (comment)

Hi, so, to be clear for me, you want me to implement unit tests for "ProductResource:: updateColumn" function ?
If that so, i can do it untill the end of this week

@ihor-sviziev
Copy link
Contributor

@sheepfy I think yes + fix failing unit tests should be enough.

@engcom-Charlie engcom-Charlie self-assigned this Sep 22, 2020
@engcom-Charlie
Copy link
Contributor

Hi, @sheepfy could you please fix static and unit tests?
Thank you.

@sheepfy
Copy link
Contributor Author

sheepfy commented Sep 24, 2020 via email

@engcom-Charlie
Copy link
Contributor

Hi, @sheepfy do you have any updates?
Thank you.

@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Oct 23, 2020

@engcom-Charlie thank you! now looks much better! Let's wait for test results

@ghost ghost moved this from Changes Requested to Review in Progress in Pull Requests Dashboard Oct 23, 2020
@ghost ghost removed the Progress: needs update label Oct 23, 2020
@ghost ghost moved this from Review in Progress to Ready for Testing in Pull Requests Dashboard Oct 26, 2020
@magento-engcom-team
Copy link
Contributor

Hi @ihor-sviziev, thank you for the review.
ENGCOM-8388 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@sheepfy thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@sheepfy
Copy link
Contributor Author

sheepfy commented Oct 26, 2020 via email

@engcom-Bravo engcom-Bravo self-assigned this Oct 28, 2020
@engcom-Bravo engcom-Bravo moved this from Ready for Testing to Testing in Progress in Pull Requests Dashboard Oct 29, 2020
@engcom-Bravo
Copy link
Contributor

This PR fixes an issue for Magento Enterprise Edition

✔️ QA Passed

Manual testing scenario:

  1. Go to Store->Configuration->Customer->Customer Configuration
  2. Enable 'Require Emails Confirmation'
  3. On Admin go to Customers->All customers and Add New Customer with address
  4. Add some kind of validation to address fields in the Database (limit length for city name)
    UPDATE customer_eav_attribute SET validate_rules='{"input_validation":1,"min_text_length":0,"max_text_length":2}' WHERE attribute_id=(SELECT attribute_id FROM eav_attribute WHERE entity_type_id=2 AND attribute_code='city');
  5. Go to frontend
  6. Click Sign In
  7. Click Forgot Your Password?
  8. Fill fields and click Reset My Password
  9. Open the email and click Set a New Password

Before applying changes provided in this PR, the Customer has been redirected to Forgot Your Password
Before PR

After switching to the PR, the Customer is redirected to Set a New Password
After PR

@engcom-Bravo engcom-Bravo added the QA: Added to Regression Scope Scenario was analysed and added to Regression Testing Scope label Oct 29, 2020
@ghost ghost moved this from Testing in Progress to Ready for Testing in Pull Requests Dashboard Oct 29, 2020
@engcom-Bravo engcom-Bravo moved this from Ready for Testing to Testing in Progress in Pull Requests Dashboard Oct 29, 2020
@engcom-Bravo engcom-Bravo moved this from Testing in Progress to Merge in Progress in Pull Requests Dashboard Oct 29, 2020
@magento-engcom-team magento-engcom-team merged commit 777203c into magento:2.4-develop Nov 9, 2020
@m2-assistant
Copy link

m2-assistant bot commented Nov 9, 2020

Hi @sheepfy, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@sidolov sidolov moved this from Merge in Progress to Recently Merged in Pull Requests Dashboard Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Award: bug fix Component: Customer Priority: P3 May be fixed according to the position in the backlog. Progress: accept QA: Added to Regression Scope Scenario was analysed and added to Regression Testing Scope Release Line: 2.4 Severity: S2 Major restrictions or short-term circumventions are required until a fix is available.
Projects
Pull Requests Dashboard
  
Recently Merged
Development

Successfully merging this pull request may close these issues.

Can't create/reset user password if user address is not valid
6 participants