issue: ENG-6424
We want to reset the state to default on identity login / logout / delete account. We want to do it in one place, to reduce the risk of accidentally not clearing some state, and leaking data.
We want to have a "white list" of fields that are not supposed to be wiped. For example commServicesAccessToken, loadingStatuses, notifPermissionAlertInfo, lifecycleState etc.
Details
Details
ran the provided test
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- inka/login_reducers
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/utils/reducers-utils.js | ||
|---|---|---|
| 15–23 ↗ | (On Diff #36094) | Can you explain what's going on here? I'm a little confused... it seems like we're starting with defaultState, and then going through every key in state, and adding the value for that key only if it's supposed to be excluded Is that backwards? |
Comment Actions
Unrelatedly, the term "sanitize" is already used in our codebase, with the specific meaning of cleaning up personally-identifiable user information from reports. Can you maybe pick a different verb here?
| lib/utils/reducers-utils.js | ||
|---|---|---|
| 15–23 ↗ | (On Diff #36094) | Oh, I see... the fields to exclude are the ones NOT to sanitize. Not sure this reduce is the most readable... I would probably do something more like this: const newState = { ...defaultState }; for (const field of fieldsToExcludeFromSanitization) { newState[field] = state[field]; } This also has perf benefits... we don't need to bother iterating through every key, or creating a Set |
Comment Actions
Simplify, update names
| lib/utils/reducers-utils.js | ||
|---|---|---|
| 10 ↗ | (On Diff #36127) | This is necessary |
| lib/utils/reducers-utils.js | ||
|---|---|---|
| 10 ↗ | (On Diff #36127) | I played around with this because I felt that the way you're typing it might be compromising the typechecker (it doesn't seem to be formally correct). I was able to get things to work with this... I like the types better, but because all of the enumerated properties in BaseAppState are read-only, it prevents us from mutating newState directly. Let me know what you think |
| lib/utils/reducers-utils.js | ||
|---|---|---|
| 10 ↗ | (On Diff #36127) | This seems better, thank you! |