Page MenuHomePhabricator

[web] Persist messageStore in db
ClosedPublic

Authored by michal on Wed, Apr 3, 6:21 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 14, 10:53 AM
Unknown Object (File)
Fri, Apr 12, 6:48 AM
Unknown Object (File)
Thu, Apr 11, 3:48 PM
Unknown Object (File)
Thu, Apr 11, 2:55 PM
Unknown Object (File)
Thu, Apr 11, 5:20 AM
Unknown Object (File)
Wed, Apr 10, 2:17 PM
Unknown Object (File)
Wed, Apr 10, 12:14 PM
Unknown Object (File)
Wed, Apr 10, 8:10 AM
Subscribers

Details

Summary
Test Plan
  • Open web app, check that the "migration" runs and:
    • Ops with messageStore from keyserver are dispatched to the db
    • Redux contains the data
  • Reload the web app and check that:
    • Keyserver doesn't send the message info
    • Message info is loaded from db and dispatched in setInitialReduxState action
  • Play around with the app check if there are any errors

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

michal requested review of this revision.Wed, Apr 3, 6:40 AM
michal added a subscriber: inka.

Noticed an issue where the message sometimes wouldn't send. It turns out that's because we don't persist nextLocalID on web. So after reloading the webapp new messages had local ids starting from 0, and mesasge reducer started ignoring them.

For now, I've added nextLocalID to persistWhitelist. @inka will be working on changing the local ids to uuids instead of consecutive numbers. These should be pretty fast changes so if they are landed first, I will just remove the additino to whitelist before landing. If I will have to land my stack first than this should just be 1. Remove entry in whitelist 2. Add a redux migration that removes nextLocalID from persisted data (this will need to be done on native anyway).

Like in previous diff, realized that user infos are used by navinfo in redux state responder so separated the new promise.

This will need a major refactor anyway in ENG-5179 : Handle initial state for multiple keyservers.

D11552 was landed so removed the change that persisted the nextLocalID as it's now a random uuid and there won't be conflicts after app reload.

kamil added inline comments.
web/shared-worker/types/entities.js
133 ↗(On Diff #38841)

nit: I think I've seen this more often in codebase (and using default at the end of expression

164–173 ↗(On Diff #38841)

can you move this above and simplify it?

This revision is now accepted and ready to land.Fri, Apr 5, 3:50 AM