Page MenuHomePhabricator

[native] Modify `messageStoreMessagesBlockListTransform` to ensure messageStore migration completes successfully
ClosedPublic

Authored by atul on Nov 29 2022, 7:23 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 22, 7:59 AM
Unknown Object (File)
Fri, Nov 22, 7:59 AM
Unknown Object (File)
Fri, Nov 22, 7:59 AM
Unknown Object (File)
Fri, Nov 22, 7:52 AM
Unknown Object (File)
Fri, Nov 22, 7:45 AM
Unknown Object (File)
Wed, Nov 6, 7:05 PM
Unknown Object (File)
Wed, Nov 6, 6:17 AM
Unknown Object (File)
Wed, Nov 6, 6:17 AM
Subscribers
None

Details

Summary

Context: https://linear.app/comm/issue/ENG-2377/upgrading-from-build-131-public-release-to-build-=154-leads-to-empty

Basically in D5545 we made sure to set messageStore.messages to {} so we wouldn't run into issues with messageStore.messages being undefined.

It was a reasonable assumption to make that messageStore.messages would always be undefined coming into this transform since we no longer persist messages via redux-persist.

However, there is one situation where we expect messageStore.messages to exist AND where we NEED the contents... and that's in migration 31 where we switch over from persisting messages via redux-persist to storing them in the SQLite messages table. That migration depends on the contents of messageStore.messages to construct the "message ops" that--when processed--populate the SQLite messages table for the first time.

Because messageStoreMessagesBlocklistTransform modifies the messageStore before migrations are run, we were inadvertently clearing messageStore.messages for older clients before migration 31 completed... leaving them with an empty messageStore.messages even after a "successful" SET_MESSAGE_STORE_MESSAGES. This diff handles that situation by setting messageStore.messages to {} only if undefined (or falsey) via nullish coalescing.

Test Plan
  1. Install build 131 on my iPhone via App Store
  2. Make a release build that includes this change and deploy to my phone
  3. Make sure that messages continue to appear in the app as expected

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul published this revision for review.Nov 29 2022, 7:23 PM
atul added inline comments.
native/redux/persist.js
475–491 ↗(On Diff #18983)

Stared at this for too long, not sure it's as clear/concise as it could be. Happy to hear any/all suggestions

This revision is now accepted and ready to land.Nov 29 2022, 8:20 PM