Page MenuHomePhabricator

[lib] Rename `sendMultimediaMessage` => `legacySendMultimediaMessage` in `message-actions.js`
ClosedPublic

Authored by atul on Sep 5 2022, 8:44 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 11, 2:51 AM
Unknown Object (File)
Tue, Nov 5, 11:28 AM
Unknown Object (File)
Sat, Nov 2, 1:14 PM
Unknown Object (File)
Sat, Nov 2, 1:14 PM
Unknown Object (File)
Sat, Nov 2, 1:14 PM
Unknown Object (File)
Sat, Nov 2, 1:11 PM
Unknown Object (File)
Thu, Oct 24, 5:12 PM
Unknown Object (File)
Thu, Oct 24, 9:42 AM
Subscribers

Details

Summary

This is just a simple rename.

Why are we doing this?

We've modified multimediaMessageCreationResponder in the last diff (D5058) to accept a t.list(tMediaMessageMedia) while continuing to support t.list(t.String) (via legacyMultimediaMessageCreationResponder).

We needed to make this change so the client could communicate to the keyserver the relationship between each piece of media (photo or video) and corresponding uploads for video messages.

We want to update native to use the updated payload when hitting the create_multimedia_message endpoint. However, we don't want to break web while we're doing that.

So, we're keeping the existing sendMultimediaMessage around and renaming to legacy... so everything continues to work. Then

  1. Introducing a new version of sendMultimediaMessage to be consumed by native that sends over the new payload
  2. Implementing "new" multimediaMessageCreationResponder to handle the new payload

3-?: Making whatever changes on native needed to get video messages working end-to-end
?+1: Updating input-state-container on web to use the new sendMultimediaMessage
?+2: Removing legacySendMultimediaMessage altogether

Basically this lets us defer updating input-state-container on web while we're figuring things out on native.


Depends on D5058

Test Plan
  1. Able to send/recieve photo(s) on native as expected
  2. Able to send/recieve photo(s) on web as expected

Diff Detail

Repository
rCOMM Comm
Branch
arcpatch-D5059 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul requested review of this revision.Sep 5 2022, 8:54 PM

Thanks for the explanation. My first thought was that clients should start using the new API immediately, but I do agree that postponing it for web could simplify it.

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