This differential alerts: developer, staff members and users on a staff release when sensitive data (currently database and data in secure store) are removed due to change in credentials of currently logged-in user.
Details
Details
- Reviewers
atul ashoat - Commits
- rCOMMa78143ad03b0: Display an alert when sensitive data are purged
Build the app, log out, create new account then delete it. in both cases the alert should be displayed.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| native/data/sqlite-context-provider.js | ||
|---|---|---|
| 48 ↗ | (On Diff #18406) | If the user is now logged out, won't this be false on prod? I think we should also check staffUserHasBeenLoggedIn, as indicated in my comment on the task. Separately, I wonder if we should be checking staffUserHasBeenLoggedIn for the other Alerts in this file... @atul, what do you think? |
| 50–51 ↗ | (On Diff #18406) | Let's be more clear |
Comment Actions
Looks good to me. Should we maybe have a separate Alert.alert(...) before the call to commCoreModule.clearSensitiveData() just in case it throws an exception and the current Alert.alert(...) is skipped?
| native/data/sqlite-context-provider.js | ||
|---|---|---|
| 51 ↗ | (On Diff #18478) | Ah TIL you can pass two strings to Alert.alert(...) and set the title and contents separately. I've just been setting the title this whole time without thinking... that's good to know. |
Comment Actions
Should we maybe have a separate Alert.alert(...) before the call to commCoreModule.clearSensitiveData() just in case it throws an exception and the current Alert.alert(...) is skipped?
That is a good question. I was hesitating between having an alert before or after `clearSensitiveData fires, but tbh I didn't consider having both. On one hand this alert is displayed exclusively to staff members who expect clearSensitiveData to fire in the case of log out or account deletion. So when user performs one of those actions and the alert doesn't show up we know something has gone wrong. On the other something might have gone wrong before we even started clearSensitiveData. Therefore when this alert doesn't show up leaves us with two questions: 1) did clearSensitiveData failed or sth else? 2) If clearSensitiveData then why. Having two alerts: one before and the other after gives us explicit answer to 1) and leave only with 2). That said I will introduce both alerts. If my reasoning is wrong then the change is simple to revert.
Comment Actions
Re-requesting review since changes are small but important since they involve text displayed to the user.