Paths

Table of Contentst

Differential D13889

[native][web] Add migration to unshim Blob-hosted multimedia
Needs ReviewPublic
Actions

Authored by bartek on Thu, Nov 7, 3:37 AM.

Tags

None

Referenced Files

	F3168809: D13889.id45656.diff
	Thu, Nov 7, 8:34 AM

	F3168135: D13889.id.diff
	Thu, Nov 7, 5:39 AM

	F3168046: D13889.id.diff
	Thu, Nov 7, 5:21 AM

	F3168003: D13889.diff
	Thu, Nov 7, 5:06 AM

	F3167625: D13889.diff
	Thu, Nov 7, 4:23 AM

	F3167333: D13889.id45656.diff
	Thu, Nov 7, 3:51 AM

	F3167328: D13889.id.diff
	Thu, Nov 7, 3:51 AM

	F3167313: D13889.diff
	Thu, Nov 7, 3:51 AM

Subscribers

Details

Reviewers

ashoat
tomek

Commits

rCOMM30f6d90c525a: [native][web] Add migration to unshim Blob-hosted multimedia

Summary

Point 3c from ENG-6459.

Depends on D13888

Test Plan

Before applying D13888 and this, I sent some encrypted multimedia messages. They appeared as robotext
Applied both diffs and ran migration
Multimedia messages appeared normally

Diff Detail

Repository

Branch

ashoat/migrations

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.Thu, Nov 7, 3:37 AM

bartek held this revision as a draft.

Herald added a subscriber: tomek. · View Herald TranscriptThu, Nov 7, 3:37 AM

bartek added a child revision: D13890: [native][web] Stop limiting media encryption to staff community.Thu, Nov 7, 3:41 AM

bartek edited the test plan for this revision. (Show Details)Thu, Nov 7, 3:54 AM

Harbormaster completed remote builds in B32569: Diff 45656.Thu, Nov 7, 3:56 AM

bartek published this revision for review.Thu, Nov 7, 4:50 AM

bartek added inline comments.

web/redux/persist.js
721–724 ↗	(On Diff #45656)	As stated in https://phab.comm.dev/D13888#inline-78351, we weren't shimming this on web so I created a no-op migration to keep state version consistent across platforms

ashoat accepted this revision.Thu, Nov 7, 5:31 AM

This revision is now accepted and ready to land.Thu, Nov 7, 5:31 AM

Closed by commit rCOMM30f6d90c525a: [native][web] Add migration to unshim Blob-hosted multimedia (authored by bartek). · Explain WhyThu, Nov 7, 8:25 AM

This revision was automatically updated to reflect the committed changes.

bartek added a commit: rCOMM30f6d90c525a: [native][web] Add migration to unshim Blob-hosted multimedia.

ashoat added a reverting change: rCOMM40822b3c7079: Revert "[native][web] Add migration to unshim Blob-hosted multimedia".Thu, Nov 7, 7:13 PM

ashoat reopened this revision.Thu, Nov 7, 7:20 PM

ashoat added inline comments.

native/redux/persist.js
1499 ↗	(On Diff #45694)	It's hard to believe this migration was tested, as it appears to me that it throws an exception on every single run

This revision is now accepted and ready to land.Thu, Nov 7, 7:20 PM

ashoat added inline comments.Thu, Nov 7, 8:26 PM

native/redux/persist.js
1499 ↗	(On Diff #45694)	There was an additional error here in not passing in `handleReduxMigrationFailure`. See D11928

Split out legacyUnshimClientDB for existing usages
Introduce new unshimClientDB
Do the same for web

I did not do any testing. @bartek will need to test this before landing, thoroughly this time.

Setting @tomek as a blocking reviewer as I made significant changes here and he is most familiar with the migrations code.

native/redux/persist.js
1315	Context in D11928 for why this is the only place on `native` where we pass `handleReduxMigrationFailure` to `legacyUnshimClientDB`

This revision now requires review to proceed.Thu, Nov 7, 8:33 PM

Harbormaster completed remote builds in B32584: Diff 45703.Thu, Nov 7, 8:51 PM

Revision Contents
Changeset List

Path

Size

lib/

utils/

migration-utils.js

12 lines

native/

redux/

24 lines

unshim-utils.js

49 lines

web/

redux/

persist-constants.js

2 lines

8 lines

unshim-utils.js

58 lines

Diff 45703

lib/utils/migration-utils.js

Loading...

native/redux/persist.js

Loading...

native/redux/unshim-utils.js

Loading...

web/redux/persist-constants.js

Loading...

web/redux/persist.js

Loading...

web/redux/unshim-utils.js

Loading...