Details

Reviewers

• abosh
tomek
marcin

Commits

rCOMMf5be8adfb117: [lib] Update `SendMultimediaMessageRequest` type
rCOMMdbb173686a65: [lib] Update `SendMultimediaMessageRequest` type

Summary

Update SendMultimediaMessageRequest to handle "new" payload option which takes list of MediaMessageServerDBContent instead of a list of strings.

Depends on D5060

Test Plan

NA, no functional changes but addresses a flow issue in subsequent diff

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

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

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

Harbormaster returned this revision to the author for changes because remote builds failed.Sep 5 2022, 9:44 PM

Harbormaster failed remote builds in B11899: Diff 16342!

add invariant to appease flow

keyserver/src/responders/message-responders.js
121 ↗	(On Diff #16344)	This is just to appease flow, gets handled properly in D5062 where the `invariant` is removed.

update diff to remove temporary invariant introduced in D5061

Harbormaster completed remote builds in B11901: Diff 16344.Sep 5 2022, 9:57 PM

Harbormaster completed remote builds in B11902: Diff 16345.

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

atul added a child revision: D5062: [keyserver] Modify `legacyMultimediaMessageCreationResponder` to handle new payload.Sep 5 2022, 10:52 PM

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

tomek accepted this revision.Sep 8 2022, 8:10 AM

tomek added inline comments.

keyserver/src/responders/message-responders.js
121 ↗	(On Diff #16344)	Not sure which `invariant` you are referring to.

This revision is now accepted and ready to land.Sep 8 2022, 8:10 AM

tomek mentioned this in D5062: [keyserver] Modify `legacyMultimediaMessageCreationResponder` to handle new payload.Sep 8 2022, 8:12 AM

Ah I see what happened, I introduced an invariant in this diff to appease flow... and was going to handle things correctly in the next diff. But it looks like I missed the git rebase --continue and amended the same diff.

Modified this diff to reflect Diff 2: 16344 which is what I intended this diff to be.

fixup diff

cut newline so it's identical to revision 2 of the diff

Harbormaster completed remote builds in B12025: Diff 16521.Sep 8 2022, 2:14 PM

Harbormaster completed remote builds in B12026: Diff 16522.

Closed by commit rCOMMdbb173686a65: [lib] Update `SendMultimediaMessageRequest` type (authored by atul). · Explain WhySep 8 2022, 2:18 PM

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMMdbb173686a65: [lib] Update `SendMultimediaMessageRequest` type.

tomek added a subscriber: inka.Sep 9 2022, 1:59 AM

tomek added inline comments.

keyserver/src/responders/message-responders.js
121	The change, that was introduced after the diff was accepted, breaks the master - `invariant` requires two arguments and throws an error if less is provided. @inka discovered that the lib is incorrectly typed in Flow - the types don't match the actual behavior. (that's why this diff passes CI)