Details

Reviewers

• abosh
tomek
marcin

Commits

rCOMM55f930b9caee: [keyserver] Modify `legacyMultimediaMessageCreationResponder` to handle new…

Summary

This is a temporary change on the keyserver side that keeps things working as they are when client hits create_multimedia_message with the new payload.

We're basically translating mediaMessageContents back to mediaIDs by pulling out the uploadID property.

This ensures that the client continues working as normal with the new payload while we're still working on implementing the "new" multimediaMessageCreationResponder.

Depends on D5061

Test Plan

Able to send image(s) on web and native as expected.

Set breakpoints at various points throughout the media message flow and observed that values were as expected at each point.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Sep 5 2022, 9:43 PM

Herald added a subscriber: ashoat. · View Herald TranscriptSep 5 2022, 9:43 PM

atul mentioned this in D5061: [lib] Update `SendMultimediaMessageRequest` type.Sep 5 2022, 9:48 PM

Harbormaster completed remote builds in B11900: Diff 16343.Sep 5 2022, 9:52 PM

atul requested review of this revision.Sep 5 2022, 9:52 PM

atul added a child revision: D5063: [lib] Introduce new `sendMultimediaMessage` and consume in `native`.Sep 5 2022, 10:52 PM

atul edited the summary of this revision. (Show Details)Sep 5 2022, 10:52 PM

atul added a parent revision: D5061: [lib] Update `SendMultimediaMessageRequest` type.

atul added a reviewer: marcin.Sep 5 2022, 11:02 PM

atul removed a reviewer: • jacek.Sep 7 2022, 5:34 PM

atul added 1 blocking reviewer(s): tomek.Sep 7 2022, 10:22 PM

Not sure if something went wrong during rebase, but this change was already included as a part of D5061

keyserver/src/responders/message-responders.js
119 ↗	(On Diff #16343)	We could modify this function to accept only "legacy" shape

This revision now requires changes to proceed.Sep 8 2022, 8:12 AM

In D5062#148425, @tomek wrote:

Not sure if something went wrong during rebase, but this change was already included as a part of D5061

Shoot yeah looks like I made a mistake while rebasing, I'll go through the stack and diff history to clean it up. Thanks for catching

atul added inline comments.Sep 8 2022, 2:25 PM

keyserver/src/responders/message-responders.js
119 ↗	(On Diff #16343)	Good point, I'll take the `tShape`s within `sendMultimediaMessageRequestInputValidator` and name them so they can be used outside of the `t.union` (eg as the type for `request` argument to `legacyMultimediaMessageCreationResponder(...)`).