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

Refresh Page on Sign-In / Refresh Page & Redirect back to Storefront on Sign-Out #442

Merged
merged 7 commits into from
Nov 16, 2020

Conversation

alkim91
Copy link
Contributor

@alkim91 alkim91 commented Nov 15, 2020

Description

For Signing Out :
I added the baseUrl(Storefront URL) in the App configs, and in the event handler for signing out, we redirect the user to the main storefront once they click 'Sign Out'.

The baseUrl is configured based on the main storefront url(Storefront can be any language/country, not always US and English), and implemented in StoreConfigExporter.

For Signing In :
Added a simple refresh after all the sign-in flow is complete.

Related Issue

adobe/aem-cif-guides-venia#59

Motivation and Context

If users are in any admin pages(Address book, account management), once they sign out we want them to be redirected back to the main storefront. Currently, we revoke the user token and reset cookies as well as the contexts, but no refresh/redirect takes place.

How Has This Been Tested?

This has been tested in the two sign out user cases.

  1. Signing out from the main navigation. Once we click 'sign out', no matter which page we're in(Catalog, address book, etc), we're redirected back to the storefront.

  2. Same thing happens when signing out from the account panel in responsive mode.

Also tested in the core components examples page as well.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes and the overall coverage did not decrease.
  • All unit tests pass on CircleCi.
  • I ran all tests locally and they pass.

@alkim91 alkim91 changed the title Task/cif 1552 final Refresh Page on Sign-In / Refresh Page & Redirect back to Storefront on Sign-Out Nov 15, 2020
@alkim91
Copy link
Contributor Author

alkim91 commented Nov 15, 2020

I opened a new PR on a new branch - addressed all comments from previous PR. With Christophe's help, I added a couple unit tests & integration tests but the integration tests actually required making a PR to the WCM core components so I took them out for now.

@codecov
Copy link

codecov bot commented Nov 15, 2020

Codecov Report

Merging #442 (a88dc96) into master (2fdc35a) will decrease coverage by 0.01%.
The diff coverage is 78.57%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #442      +/-   ##
============================================
- Coverage     85.30%   85.28%   -0.02%     
- Complexity     1087     1089       +2     
============================================
  Files           221      221              
  Lines          5519     5533      +14     
  Branches        800      802       +2     
============================================
+ Hits           4708     4719      +11     
- Misses          658      659       +1     
- Partials        153      155       +2     
Flag Coverage Δ Complexity Δ
integration 66.22% <57.14%> (+0.11%) 0.00 <2.00> (ø)
jest 79.83% <85.71%> (+0.02%) 0.00 <0.00> (ø)
karma 93.65% <ø> (ø) 0.00 <ø> (ø)
unittests 85.90% <42.85%> (-0.11%) 0.00 <2.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
react-components/src/context/ConfigContext.js 100.00% <ø> (ø) 0.00 <0.00> (ø)
...1/storeconfigexporter/StoreConfigExporterImpl.java 92.00% <71.42%> (-8.00%) 9.00 <2.00> (+2.00) ⬇️
...t-components/src/components/MyAccount/myAccount.js 85.71% <75.00%> (-2.53%) 0.00 <0.00> (ø)
...eact-components/src/components/SignIn/useSignin.js 97.05% <100.00%> (+0.28%) 0.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2fdc35a...a88dc96. Read the comment docs.

@herzog31
Copy link
Member

Looks good. Just as you mentioned, tests are missing.

@alkim91
Copy link
Contributor Author

alkim91 commented Nov 16, 2020

Looks good. Just as you mentioned, tests are missing.

Unit test added now :)

@alkim91 alkim91 merged commit 9c4519d into master Nov 16, 2020
@alkim91 alkim91 deleted the task/CIF-1552-final branch November 16, 2020 09:05
alkim91 added a commit that referenced this pull request Nov 16, 2020
alkim91 added a commit that referenced this pull request Nov 16, 2020
@alkim91 alkim91 restored the task/CIF-1552-final branch November 16, 2020 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants