model: Issues with `LOGIN_SUCCESS`, `REGISTER_COMPLETE`, `DEAD_QUEUE`

[chrisbobbe]

In the office last week, @gnprice and I found some bundles of problems with how our reducers handle LOGIN_SUCCESS, REGISTER_COMPLETE, and DEAD_QUEUE. Here's a summary I put together just now, made a bit more specific with help from git grep:

• Consistently clear old data on all ways of leaving an account #4446: A bunch of server data, and some per-account client-side data (outbox and drafts) isn't being cleared on LOGIN_SUCCESS, but it should be
• (done in model: Make REGISTER_COMPLETE reset some state we forgot about #5613) REGISTER_COMPLETE should reset or replace all server data and server-data metadata, but it isn't in some cases:
  • server data: topics, typing (should reset)
  • server-data metadata: fetching (should reset)
• (Open PR: model [nfc]: Comment on rest of REGISTER_COMPLETE handlers that reset state #5630) Some reducers don't have a comment saying why they reset state on REGISTER_COMPLETE instead of taking data from its payload:
  • For example, flagsReducer should point out that messagesReducer doesn't store messages on REGISTER_COMPLETE, and messagesReducer should say that it doesn't do that because we fetch messages with GET /messages.
  • (Probably plenty of other instances.)
• DEAD_QUEUE should clear out server-data metadata, but it isn't in some cases:
  • caughtUp
  • fetching
• (Open PR: typing model: Don't reset on DEAD_QUEUE #5631) DEAD_QUEUE is clearing out typing but probably doesn't need to; we don't clear other server data on DEAD_QUEUE, plus there's already a client-side component that makes this feature fail gracefully in the presence of stale server data (a timeout to clear typing indicators)

[chrisbobbe]

@gnprice, feel free to edit the above directly. 🙂

[chrisbobbe]

On REGISTER_COMPLETE, we set state.session.eventQueueId with the new queue ID. Should REGISTER_COMPLETE reset all the other things in PerAccountSessionState to their initial values? We do that with loading already, but don't currently touch the other two: outboxSending and hasDismissedServerCompatNotice.

[gnprice]

Good question.

outboxSending

This basically tracks whether a sendOutbox call is currently running. Nothing in a dead queue, or a fresh /register , interrupts that. Nor should it — if the sendOutbox's own request is able to complete, more power to it, and if not then it has its own retry (with backoff). So this should remain untouched.

(OTOH it's right that a RESET_ACCOUNT_DATA should clear it, as it does in #5612; the old sendOutbox call shouldn't preclude a new one for the new account's outbox. Though separately it has a case of #5009.)

hasDismissedServerCompatNotice

This should remain untouched. If the user has dismissed the notice for this server in this session, it shouldn't pop back up because we lost the queue and recreated it.

---

To describe the situation a bit more abstractly:

• Both eventQueueId and loading are about our relationship to the event system itself: loading says we have a register request in progress, and eventQueueId tells us part of what we got from that. So it's natural that registering an event queue would update them.
• The other two aren't about the event system. And they're here in state.session because they're also not part of the server data, the data that the event system maintains. So it's natural that registering an event queue doesn't affect them.