[lib] Update `rawMessageInfoFromClientDB` to handle both `IMAGES` and `MULTIMEDIA` message types
ClosedPublic
Actions

Authored by atul on Nov 15 2022, 7:03 PM.

Details

Reviewers

tomek
marcin
ginsu
rohan
kamil
ashoat

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rCOMM23cf15da298b: [lib] Update `rawMessageInfoFromClientDB` to handle both `IMAGES` and…

Summary

Update rawMessageInfoFromClientDB to handle MULTIMEDIA messages in addition to IMAGES messages.

Depends on D5636

Test Plan

Set breakpoints and observed expected behavior.

Will also be tested implicitly by subsequent diffs/video work.

Finally, will re-introduce Redux/SQLite equality checks for myself to make sure that IMAGES/MULTIMEDIA messages are persisted/unpersisted correctly.

Diff Detail

Repository

rCOMM Comm

Branch

arcpatch-D5637_1 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Nov 15 2022, 7:03 PM

Herald added subscribers: • abosh, ashoat. · View Herald TranscriptNov 15 2022, 7:03 PM

Will add some unit tests for this function later in the stack

Harbormaster completed remote builds in B13452: Diff 18451.Nov 15 2022, 7:17 PM

atul requested review of this revision.Nov 15 2022, 7:17 PM

atul added a child revision: D5638: [native] Introduce migration to unshim `MULTIMEDIA` messages.Nov 15 2022, 10:00 PM

atul added a reviewer: Restricted Owners Package.Nov 16 2022, 2:55 PM

ashoat added inline comments.Nov 16 2022, 8:13 PM

lib/shared/messages/multimedia-message-spec.js
90–94 ↗	(On Diff #18451)	I'm confused, why'd we get rid of the `invariant`?

atul added inline comments.Nov 16 2022, 8:16 PM

lib/shared/messages/multimedia-message-spec.js
90–94 ↗	(On Diff #18451)	Based on feedback here: https://phab.comm.dev/D5313?id=17390#inline-35170 regarding `translateClientDBMediaInfosToMedia`

atul added inline comments.Nov 16 2022, 8:17 PM

lib/shared/messages/multimedia-message-spec.js
90–94 ↗	(On Diff #18451)	Actually that's a different situation because In this case it is completely valid for `clientDBMessageInfo` to have these fields empty isn't accurate/relevant here. I'll revert and bring back invariant.