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

Bugfix/multiple redirections on my account refresh #2780

Conversation

patzick
Copy link
Collaborator

@patzick patzick commented Apr 23, 2019

Related issues

closes #2713

Short description and why it's useful

Fixes problem with infinite redirections and redirections on SSR on account site. What has been done:

  • changed user/startSession to async method and awaits its invocations
  • make sure in router/beforeEach that local session data are readed before redirecting to home page
  • do not redirect to home on SSR render on auth site, using NoSSR for session-related data and no more blinking home->account (and no hydration issue with that change)
  • startSession only once with reading localStorage data, there were many requests with multiple invocations and slowed down initial render
  • user data in localstorage are kept on single key, removed prefix causing problem with reading local session after changing language

Which environment this relates to

Check your case. In case of any doubts please read about Release Cycle

  • Test version (https://test.storefrontcloud.io) - this is a new feature or improvement for Vue Storefront. I've created branch from develop branch and want to merge it back to develop
  • RC version (https://next.storefrontcloud.io) - this is a stabilisation fix for Release Candidate of Vue Storefront. I've created branch from release branch and want to merge it back to release
  • Stable version (https://demo.storefrontcloud.io) - this is an important fix for current stable version. I've created branch from hotfix or master branch and want to merge it back to hotfix

Upgrade Notes and Changelog

  • No upgrade steps required (100% backward compatibility and no breaking changes)
  • I've updated the Upgrade notes and Changelog on how to port existing VS sites with this new feature

IMPORTANT NOTICE - Remember to update CHANGELOG.md with description of your change

Contribution and currently important rules acceptance

@patzick patzick requested review from pkarw and filrak April 23, 2019 21:36
@patzick patzick added this to the 1.9.0 milestone Apr 23, 2019
Copy link
Collaborator

@pkarw pkarw left a comment

Choose a reason for hiding this comment

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

Just some naming minor issues

@@ -6,6 +6,7 @@ const getters: GetterTree<UserState, RootState> = {
isLoggedIn (state) {
return state.current !== null
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be named like “local_session_started” or “local_session_loaded”

export const USER_GROUP_CHANGED = SN_USER + 'GROUP_ID_CHANGED'
Copy link
Collaborator

Choose a reason for hiding this comment

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

/ - missing char in the mutation name

@pkarw pkarw merged commit debe392 into vuestorefront:release/1.9 Apr 24, 2019
@pkarw
Copy link
Collaborator

pkarw commented Apr 24, 2019

Cool! Merged in

@patzick patzick deleted the bugfix/multiple-redirections-on-my-account-refresh branch April 24, 2019 08:20
@michhy
Copy link

michhy commented Apr 27, 2019

Hello,
I have tested it is working (solving the refresh issue) on default theme,
but not the vuetique theme. (it will reload, with no content generated (only showing the my account navigations, and all the buttons are unabled. )

Any idea of the related file that causing the difference?
thanks
螢幕截圖 2019-04-27 下午6 11 16
螢幕截圖 2019-04-27 下午6 11 24

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.

None yet

3 participants