Page MenuHomePhabricator

[lib] Fix invite links validation
ClosedPublic

Authored by inka on May 23 2024, 1:00 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 13, 9:00 PM
Unknown Object (File)
Thu, Nov 7, 4:42 PM
Unknown Object (File)
Mon, Nov 4, 3:55 AM
Unknown Object (File)
Sat, Nov 2, 4:20 PM
Unknown Object (File)
Fri, Oct 18, 10:41 AM
Unknown Object (File)
Thu, Oct 17, 7:40 PM
Unknown Object (File)
Oct 16 2024, 3:32 PM
Unknown Object (File)
Oct 16 2024, 3:32 PM
Subscribers
None

Details

Summary

issue: ENG-8170
Invite links were failing validation, because union was handled incorrectly

This addresses the issue, but I still have to review other tcomb types ENG-8179

Test Plan

Tested that invite links work correclty now

Diff Detail

Repository
rCOMM Comm
Branch
inka/fixValidation
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

lib/utils/user-info-extraction-utils.js
53 ↗(On Diff #40571)

Every use of any and its equivalents should be explicitly justified.

What else did you try here? Why is this absolutely necessary?

lib/utils/user-info-extraction-utils.js
31 ↗(On Diff #40571)

* is basically any and has been deprecated in Flow for a long time. This should never have been introduced in 2024. Please replace it

inka requested review of this revision.May 23 2024, 1:17 AM

Update types, fix maybe type

lib/utils/user-info-extraction-utils.js
55

The any casts are now for the same reason as in D7488

Cannot instantiate TType because I is not a valid argument of TStructProps

Using the same approach as there

61

Fixing maybe type as well, because otherwise update_user_avatar fails

ashoat added inline comments.
lib/utils/user-info-extraction-utils.js
55

If you have a strong reason for using an any, you must still always re-cast it

60

This one is safe because it gets immediately re-cast to TType<T> upon being returned

61

Nice catch!

This revision is now accepted and ready to land.May 23 2024, 6:27 AM
lib/utils/user-info-extraction-utils.js
55

I'm having trouble re-casting it. The correct type is TInterface<T>, but then I again get

Cannot instantiate TType because I is not a valid argument of TStructProps

In D7488 we had an input field which we would use to hack this problem.
I can cast this to TInterface<Object>... which is true, since TInterface<+T> props is of type TStructProps<T> = $ObjMap<T, TypeToValidator>. So as far as I understand, that means that T has to be an Object (from definition of $ObjMap)

lib/utils/user-info-extraction-utils.js
55 ↗(On Diff #40580)

Object is basically any as well. Can you use eg. { ... }?

Fix import

lib/utils/user-info-extraction-utils.js
55 ↗(On Diff #40580)

@ashoat and I tried changing this, but couldn't come up with anything that would work. We decided to leave the Object

This revision was automatically updated to reflect the committed changes.