Page MenuHomePhabricator

[lib] Introduce raw thread info validators
ClosedPublic

Authored by michal on Apr 21 2023, 8:12 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 29, 8:27 AM
Unknown Object (File)
Fri, Nov 29, 6:20 AM
Unknown Object (File)
Mon, Nov 25, 8:48 PM
Unknown Object (File)
Mon, Nov 25, 8:44 PM
Unknown Object (File)
Mon, Nov 25, 8:43 PM
Unknown Object (File)
Mon, Nov 25, 8:28 PM
Unknown Object (File)
Fri, Nov 15, 12:19 AM
Unknown Object (File)
Sun, Nov 10, 12:35 AM
Subscribers

Details

Summary

Adds validators for RawThreadInfo and nested types. The example thread info in tests is taken from the app but it's cut a bit so it's shorter.

Depends D7567

Test Plan

Run yarn jest

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Could we add a test where avatar is not defined (RawThreadInfo doesn't have this field at all, not just it is set to null or undefined).

lib/types/thread-types.js
265 ↗(On Diff #25547)

This prop is optional and nullable in RawThreadInfo but is described by tComb types as other props that are only nullable. Is there a way to describe it more precisely?

This revision is now accepted and ready to land.Apr 25 2023, 3:25 PM

Could we add a test where avatar is not defined (RawThreadInfo doesn't have this field at all, not just it is set to null or undefined).

The existing thread example already doesn't have avatar defined.

lib/types/thread-types.js
265 ↗(On Diff #25547)

We would have to add custom tComb validators for only-undefined-maybe and only-null-maybe. We already use t.maybe for fields that are both optional and nullable in a few places. If we add these validators it would probably make sense to also update existing validators.

lib/types/thread-types.js
265 ↗(On Diff #25547)

Personally I think it's fine to leave as-is... I can't think of a security threat that this input validation would catch, and it won't help for the IDs migration either