This diff adds support for fetching Blob service URIs for the native and web.
Depends on D7616
Details
Details
Uploaded both encrypted and non-encrypted blob to local blob service instance.
Amended the keyserver's upload-fetchers to return these Blob URIs.
Verified media is fetched and displayed correctly both on web and native.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| web/media/media-utils.js | ||
|---|---|---|
| 206–207 ↗ | (On Diff #25695) | |
| web/media/multimedia-modal.react.js | ||
| 46–52 ↗ | (On Diff #25695) | This adds support for hosting non-encrypted media on Blob service. This is also compatible with my changes from D7618 where, in theory, blob URI can be returned regardless of media type. |
Comment Actions
Uploaded both encrypted and non-encrypted blob to local blob service instance. Amended the keyserver's upload-fetchers to return these Blob URIs. Verified media is fetched and displayed correctly both on web and native.
Would there be a good way for a reviewer to test this in their local environment?
| native/media/encryption-utils.js | ||
|---|---|---|
| 272 ↗ | (On Diff #25933) | We often use snake_cased error messages EG  and can handle them conditionally in any outer catch blocks EG  would it make sense to do something similar here? Maybe throw different errors based on status/statusText? Will leave it up to you since you have more context on what type of errors we might encounter, etc. |
| web/media/media-utils.js | ||
| 202–205 ↗ | (On Diff #25933) | Looks like we have similar logic in identifier-utils. Up to you, but might make sense to consolidate at some point in the future if we re-use further? |
| web/media/multimedia-modal.react.js | ||
| 49 ↗ | (On Diff #25933) | Could we move this above media.type === 'photo' check? |
| native/media/encryption-utils.js | ||
|---|---|---|
| 272 ↗ | (On Diff #25933) | This is a good idea, worth thinking of more thoroughly. I'll make a Linear task for this |
| web/media/media-utils.js | ||
| 202–205 ↗ | (On Diff #25933) | Yeah, good idea, not sure why I didn't do it in the first place |
| web/media/multimedia-modal.react.js | ||
| 49 ↗ | (On Diff #25933) | Already tried that, Flow complains |
| native/media/encryption-utils.js | ||
|---|---|---|
| 272 ↗ | (On Diff #25933) | |
Realizing this will change the version of ws that is imported here. In fact we're already importing a newer version (7.5.9) than our version of express-ws uses (5.2.3), but it looks like we only use it for Flow types so it's okay