-
Notifications
You must be signed in to change notification settings - Fork 66
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
Reset store state on logout #5120
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5120 +/- ##
==========================================
- Coverage 78.26% 78.26% -0.01%
==========================================
Files 339 339
Lines 15967 15985 +18
Branches 3693 3697 +4
==========================================
+ Hits 12497 12511 +14
- Misses 3470 3474 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
I'd propose to keep the platform settings in local storage, reason being that clearing the account state on logout could pose problems to the login page (rendering platform setting dependent elements like the sign-up button). There's nothing in there that could be considered sensitive and the data is not subject to change very often either |
Sorry - I'm out of time to get this reviewed. Will need to pass over to someone else. |
can you explain why we have two |
Yes, in Vuex, all state changes must pass through mutations. This ensures that state updates are synchronous and trackable (actions can contain async logic but must commit mutations to modify the state). Even though in this particular case the action doesn't contain async code, directly modifying the state outside mutations violates Vuex principles and may lead to unexpected behavior |
initialState = initialState() | ||
|
||
if (meta.persistence) { | ||
// remove any initial state keys from the initial state that should be ignored while clearing state |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
've read this line many times, and I can't process/understand it, can you clear it up please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is Skip clearing state keys that should be persisted across state resets a bit better?
Description
adds a global way of resetting store state on user logou
Related Issue(s)
closes #5115
Checklist
flowforge.yml
?FlowFuse/helm
to update ConfigMap TemplateFlowFuse/CloudProject
to update values for Staging/ProductionLabels
area:migration
label