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.

Sign up for GitHub

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

Jump to bottom

Password change bug fix #803

Merged
merged 3 commits into from
Oct 28, 2022
Merged

Password change bug fix #803

merged 3 commits into from
Oct 28, 2022

Conversation

yunakim714
Copy link
Collaborator

@yunakim714 yunakim714 commented Oct 27, 2022 •
edited
Loading

Description

Currently, after changing their password, customers cannot add products to their cart.

To reproduce the problem:

  1. Open https://pwa-kit.mobify-storefront.com/ and login as a registered shopper
  2. Change your password in your Account Details page
  3. Try to add a product to your basket -> Error

This fix involves restarting the "SLAS login dance" after a user changes their credentials (otherwise, both the SLAS access & refresh tokens will be invalid)

Types of Changes

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Documentation update
  • Breaking change (could cause existing functionality to not work as expected)
  • Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

  • Removing a public function or component or prop
  • Adding a required argument to a function
  • Changing the data type of a function parameter or return value
  • Adding a new peer dependency to package.json

Changes

  • Pass customer email when calling updatePassword
  • Re-login the customer with new credentials (email & updated pw)

How to Test-Drive This PR

  • Checkout this branch
  • Run npm start
  • Go to http://localhost:3000/
  • Sign in as a registered user
  • Go to "Account Details" page by clicking on Account icon
  • Change password
  • Navigate to a product and click "Add to Cart"
  • Verify that product was successfully added!🥳

Checklists

General

  • Changes are covered by test cases
  • CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

  • There are no changes to UI

or...

Localization

  • Changes include a UI text update in the Retail React App (which requires translation)

@yunakim714 yunakim714 marked this pull request as ready for review October 27, 2022 21:05
@yunakim714 yunakim714 requested a review from a team as a code owner October 27, 2022 21:05
bfeister
bfeister reviewed Oct 27, 2022
View reviewed changes
packages/template-retail-react-app/app/commerce-api/hooks/useCustomer.js Outdated
@@ -200,6 +201,14 @@ export default function useCustomer() {
throw new Error(json.detail)
}
}

// Send a new login request with updated password
Copy link
Collaborator

@bfeister bfeister Oct 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I would say "fetch a new jwt to update the invalid one in client app state"

@yunakim714 yunakim714 requested a review from a team October 28, 2022 16:19
bfeister
bfeister approved these changes Oct 28, 2022
View reviewed changes
Copy link
Collaborator

@bfeister bfeister left a comment

Choose a reason for hiding this comment

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

Great work finding that missing email param I missed!🕵️‍♀️

@yunakim714 yunakim714 merged commit 3c21c66 into develop Oct 28, 2022
@wjhsf wjhsf deleted the change-password-relogin branch March 17, 2023 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bfeister bfeister bfeister approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yunakim714 @bfeister