A few more changes needed for create_multimedia_message endpoint to properly handle encrypted multimedia. This was not needed for native, but is needed for web where message is created from pre-uploaded media.
Details
Details
Encrypted multimedia messages can be created from uploads having encryption key in upload extras.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Looks good, minor nits inline.
| keyserver/src/fetchers/upload-fetchers.js | ||
|---|---|---|
| 109–120 ↗ | (On Diff #24583) | Let's handle the "easy case first" and "exit early" to improve the readability here a bit. |
| 122–133 ↗ | (On Diff #24583) | Similar note as above. |
| 135–140 ↗ | (On Diff #24583) | Not sure your thoughts on ternaries, but I'd prefer something like: // $FlowFixMe add thumbnailID, thumbnailURI once they're in DB return loop ? { ...video, loop } : video; |
| keyserver/src/responders/message-responders.js | ||
| 203 ↗ | (On Diff #24583) | I know it's against our typical naming convention, but I wonder if something like: forceMULTIMEDIAMessageTypewould make it clearer that we're discussing messageTypes? Just thinking out loud, feel free to land as-is. |
| keyserver/src/fetchers/upload-fetchers.js | ||
|---|---|---|
| 134 | Just realized that this isn't addressed in a later diff in the stack. Wondering if there are any risks / issues with landing this to master, and whether we should be concerned about shipping the feature without this | |
| keyserver/src/fetchers/upload-fetchers.js | ||
|---|---|---|
| 134 | Oh I guess this is just moved from the old version | |