Details

Reviewers

kamil
tomek

Commits

rCOMM12b056878a97: [web] Migrate to new id schema
rCOMM3293f7d5a7b5: [web] Migrate to new id schema

Summary

https://linear.app/comm/issue/ENG-3795/migrate-the-sqlite-database
https://linear.app/comm/issue/ENG-3796/migrate-redux-state

ID schema migrations for the web. Also changes the website-responder.js to always respond with converted ids. This is because when the new web app version releases it expects that the redux store will contain converted ids (with the exception of persisted values, which are migrated).

Depends on D8382

Test Plan

Run the old version of the web app, create some drafts, close the web app
Run the new version of the web app, check if drafts still exists
Explore the app, check if there are any problems
Compare redux and sqlite from before and after, and check if all ids were converted

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

michal created this revision.Jun 30 2023, 2:50 AM

Herald added a subscriber: ashoat. · View Herald TranscriptJun 30 2023, 2:50 AM

I've renamed NEXT_CODE_VERSION to NEXT_NATIVE_CODE_VERSION and added NEXT_WEB_CODE_VERSION so there is no confusion what should be replaced with what. @atul @ashoat

Harbormaster completed remote builds in B20632: Diff 28284.Jun 30 2023, 3:07 AM

michal edited the test plan for this revision. (Show Details)Jun 30 2023, 3:22 AM

Harbormaster completed remote builds in B20633: Diff 28285.Jun 30 2023, 3:22 AM

michal requested review of this revision.Jun 30 2023, 3:22 AM

michal added a child revision: D8386: [lib] Enable new id schema.Jun 30 2023, 3:25 AM

Migration looks good, but comments regarding versioning

keyserver/src/responders/website-responders.js
597 ↗	(On Diff #28285)	looks like this should be `true` in this case or keyserver will return old ids to clients with migrated state
keyserver/src/utils/validation-utils.js
76–81 ↗	(On Diff #28285)	This should be something like: `convertToNewIDSchema && everything else...` in this case
web/redux/persist.js
140 ↗	(On Diff #28285)	You should either land the entire stack at once or, move this bump to D8386, otherwise, diffs between this and D8386 should fail in the test plan

This revision now requires changes to proceed.Jul 3 2023, 4:26 AM

Respond to review

Harbormaster completed remote builds in B20697: Diff 28359.Jul 3 2023, 8:39 AM

kamil accepted this revision.Jul 4 2023, 2:55 AM

kamil added inline comments.

keyserver/src/utils/validation-utils.js
43 ↗	(On Diff #28359)	If I understand this correctly this should `NEXT_WEB_CODE_VERSION` in D8386 after `redux-persist` version bump but not a huge deal here (also line 78)
web/redux/persist.js
73 ↗	(On Diff #28359)	nit: seems more natural