Page MenuHomePhabricator

[lib] Introduce raw thread info validators
ClosedPublic

Authored by michal on Apr 21 2023, 8:12 AM.
Tags
None
Referenced Files
F3386816: D7569.diff
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
Unknown Object (File)
Tue, Nov 5, 4:33 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
No Lint Coverage
Unit
No Test Coverage

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