Page MenuHomePhabricator

[lib] Introduce `tMediaMessage[Photo/Video/Media]` to validate requests to `textMessageCreationResponder`
ClosedPublic

Authored by atul on Aug 30 2022, 3:04 PM.
Tags
None
Referenced Files
F3362141: D4990.diff
Sun, Nov 24, 9:38 PM
Unknown Object (File)
Thu, Nov 14, 1:10 AM
Unknown Object (File)
Mon, Nov 11, 2:51 AM
Unknown Object (File)
Sat, Nov 9, 6:58 AM
Unknown Object (File)
Sat, Nov 9, 3:51 AM
Unknown Object (File)
Fri, Nov 8, 8:21 PM
Unknown Object (File)
Wed, Nov 6, 12:27 PM
Unknown Object (File)
Sat, Nov 2, 1:14 PM
Subscribers
None

Details

Summary

We're modifying textMessageCreationResponder to be able to accept a list of MediaMessageServerDBContents as one of the arguments.

Before we build the new validator for textMessageCreationResponder we introduce these basic types that we will build upon in subsequent diffs.


Depends on D4985

Test Plan

Will add tests in next diff

Diff Detail

Repository
rCOMM Comm
Branch
arcpatch-D4990
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul published this revision for review.Aug 30 2022, 3:05 PM
tomek added inline comments.
lib/utils/validation-utils.js
69 ↗(On Diff #16120)

Can we use tString from validation-utils?

This revision is now accepted and ready to land.Aug 31 2022, 4:32 AM

Read up on tcomb since I had never heard of it before, but still sort of unsure why we need these. Is it because tcomb checks types at runtime and Flow is static? This diff makes sense according to the documentation of tcomb, however, and matches the spec'd design laid out in D4982's diff summary.

atul added inline comments.
lib/utils/validation-utils.js
69 ↗(On Diff #16120)

Ah yeah you're totally right, we already have something for this

atul marked an inline comment as done.

rebase around other diffs before landing

In D4990#145149, @abosh wrote:

Read up on tcomb since I had never heard of it before, but still sort of unsure why we need these. Is it because tcomb checks types at runtime and Flow is static? This diff makes sense according to the documentation of tcomb, however, and matches the spec'd design laid out in D4982's diff summary.

Yup, for validating requests at runtime

This revision was landed with ongoing or failed builds.Aug 31 2022, 11:37 AM
This revision was automatically updated to reflect the committed changes.
This revision is now accepted and ready to land.Aug 31 2022, 12:43 PM

re-open to fix import issue

Was getting the following import issue:

[NODEM] file:///Users/atul/comm/keyserver/dist/lib/utils/validation-utils.js:1
[NODEM] import t, { TUnion } from 'tcomb';
[NODEM]             ^^^^^^
[NODEM] SyntaxError: Named export 'TUnion' not found. The requested module 'tcomb' is a CommonJS module, which may not support all module.exports as named exports.
[NODEM] CommonJS modules can always be imported via the default export, for example using:
[NODEM] 
[NODEM] import pkg from 'tcomb';
[NODEM] const { TUnion } = pkg;
[NODEM] 
[NODEM]     at ModuleJob._instantiate (node:internal/modules/esm/module_job:124:21)
[NODEM]     at async ModuleJob.run (node:internal/modules/esm/module_job:179:5)
[NODEM]     at async Loader.import (node:internal/modules/esm/loader:178:24)
[NODEM]     at async Object.loadESM (node:internal/process/esm_loader:68:5)
[NODEM]     at async handleMainPromise (node:internal/modules/run_main:63:12)
[NODEM] [nodemon] app crashed - waiting for file changes before starting...
This revision was landed with ongoing or failed builds.Aug 31 2022, 12:45 PM
This revision was automatically updated to reflect the committed changes.