issue: https://linear.app/comm/issue/ENG-5284/refactor-setdevicetoken-action
We will need to communicate the device token to every keyserver we are connected to. The device token may change. A keyserver might not have our current device token. Each keyserver may realise they have the wrong token at a different time. We want to keep track of each keyservers knowledge. I am moving the logic that was used for one keyserver to multiple keyservers. This logic is flawed, but it will be updated in a different task: https://linear.app/comm/issue/ENG-5599/device-token-setting-mechanizm
Details
Checked:
The migration, the keyserver selectors, sanitizeState, logInExtraInfoSelector
that the reducer sets the device token in the keyserver store on setDeviceTokenActionTypes.success action,
that the notifs still work,
that when the ks has an incorrect device token it is able to get a new one in the same way as it was before
checked that if a user turns off and back on the notifications, they work
checked that login works. Assuming that register and siweAuth work by symmetry
ran yarn flow-all
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
native/push/push-handler.react.js | ||
---|---|---|
245–247 ↗ | (On Diff #32467) | This doesn't make much sense now because setDeviceToken action is not yet refactored for multiple keyserver. It will be done in the next diff, and this logic will be updated |
277–285 ↗ | (On Diff #32467) | The keyserver might try to send a notif and get an error from the Push service even before the client is notified about the token change. Also, the keyservers don't necesserly lear this at the same time. Thats why if any of the keyservers sets the deviceID to null we want to check if notifs are enabled |
It might make sense for @marcin to take a look at this given his experience with notifs
Don't we need a migration for web too?
deviceToken wasn't being persisted, so I don't think so (it's not on the persistWhitelist)
lib/types/account-types.js | ||
---|---|---|
51–55 ↗ | (On Diff #32638) | If you mean DeviceTokenUpdateRequest then yes |