Page MenuHomePhabricator

[lib] Introduce `FarcasterRelationshipRequest`, validators, and unit tests
ClosedPublic

Authored by atul on Apr 8 2024, 1:50 PM.
Tags
None
Referenced Files
Unknown Object (File)
Nov 3 2024, 7:07 AM
Unknown Object (File)
Oct 12 2024, 7:07 PM
Unknown Object (File)
Oct 12 2024, 7:07 PM
Unknown Object (File)
Oct 12 2024, 7:07 PM
Unknown Object (File)
Oct 12 2024, 7:07 PM
Unknown Object (File)
Oct 12 2024, 7:07 PM
Unknown Object (File)
Sep 30 2024, 1:51 AM
Unknown Object (File)
Sep 20 2024, 3:51 PM
Subscribers

Details

Summary

Introduce FarcasterRelationshipRequest which will take a dictionary of commUserID -> farcasterUserID entries instead of array of userIDs like the "Traditional" RelationshipRequests.


Depends on D11592

Test Plan

flow and unit tests for validator passes

Diff Detail

Repository
rCOMM Comm
Branch
arcpatch-D11593 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

lib/types/relationship-types.js
75–78 ↗(On Diff #38925)

With the input validator we can ensure that FarcasterRelationshipRequests that reach the responder will have exactly two entries.

However, there's no way (that I know of) to encode that requirement using flow. Another thing I considered, but scrapped, was something like:

export type FarcasterRelationshipRequest = {
  action: 'farcaster',
  user1ID: string,
  user1FID: string,
  user2ID: string,
  user2FID: string
};

which I felt was clunky and more difficult to work with, but would allow the "two user requirement" to be enforced by type system.

Don't think it really matters but CC @ashoat, @tomek

atul published this revision for review.Apr 8 2024, 1:58 PM
lib/types/relationship-types.js
75–78 ↗(On Diff #38925)

One option to consider is having a dedicated object describing the relationship:

type UserIDToFID = {
  +userID: string,
  +userFID: string,
};

export type FarcasterRelationshipRequest = {
  +action: 'farcaster',
  +user1: UserIDToFID,
  +user2: UserIDToFID,
};
76–77 ↗(On Diff #38925)
tomek added inline comments.
lib/types/relationship-types.js
71 ↗(On Diff #38925)

Should this be RelationshipActionSansFarcaster?

This revision is now accepted and ready to land.Apr 9 2024, 3:00 AM
lib/types/relationship-types.js
71 ↗(On Diff #38925)

Yeah! It'll be updated to RelationshipActionSansFarcaster later in the stack,

make RelationshipRequest and FarcasterRelationshipRequest readonly

This revision is now accepted and ready to land.Apr 9 2024, 10:14 AM
lib/types/relationship-types.js
75–78 ↗(On Diff #38925)

True, that would definitely be better than the

export type FarcasterRelationshipRequest = {
  action: 'farcaster',
  user1ID: string,
  user1FID: string,
  user2ID: string,
  user2FID: string
};

solution that I proposed.

Will go with what's in the diff for now since I think it's convenient to be able to index in via comm userID for our use case.