Details

Reviewers

varun
bartek

Commits

rCOMM3cada40de7b3: [lib] Add types for identity search websocket server

Summary

This implements the types associated with identity search messages to be used by identity search context provider

Depends on D11125

Test Plan

flow

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

will created this revision.Feb 1 2024, 10:20 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptFeb 1 2024, 10:20 PM

Harbormaster completed remote builds in B26454: Diff 36572.Feb 1 2024, 10:38 PM

will requested review of this revision.Feb 1 2024, 10:38 PM

will added inline comments.Feb 1 2024, 10:45 PM

lib/types/identity-search/search-query-types.js
8 ↗	(On Diff #36572)	SearchQuery is set equal to Prefix as I have yet to implement other search options

will retitled this revision from [lib] Add types for identity search websocket server to [lib] [1/n] Add types for identity search websocket server.Feb 1 2024, 10:45 PM

will added a child revision: D10938: [lib] Basic implementation of identity search context provider without message sending.Feb 2 2024, 8:40 PM

ashoat added inline comments.Feb 3 2024, 9:28 AM

lib/types/identity-search/messages.js
27 ↗	(On Diff #36572)	Are you sure we want to use a single enum for messages that either side sends? Or should we separate the message types between server/client?

will added inline comments.Feb 5 2024, 6:51 AM

lib/types/identity-search/messages.js
27 ↗	(On Diff #36572)	Agreed. Would definitely make this more organized. Was following Tunnelbroker conventions, but this would make type checking airtight on client end. Will implement these changes

will planned changes to this revision.Feb 5 2024, 7:44 AM

will retitled this revision from [lib] [1/n] Add types for identity search websocket server to [lib] [2/n] Add types for identity search websocket server.

will edited the summary of this revision. (Show Details)

Split messages into client and server messages

Harbormaster completed remote builds in B26499: Diff 36631.Feb 5 2024, 10:45 AM

will planned changes to this revision.Feb 5 2024, 11:38 AM

will retitled this revision from [lib] [2/n] Add types for identity search websocket server to [lib] [1/n] Add types for identity search websocket server.Feb 6 2024, 11:35 PM

will retitled this revision from [lib] [1/n] Add types for identity search websocket server to [lib] [3/n] Add types for identity search websocket server.

will edited the summary of this revision. (Show Details)

will added a parent revision: D10981: [shared] [identity] Move search result to response and add ids.

will retitled this revision from [lib] [3/n] Add types for identity search websocket server to [lib] [4/n] Add types for identity search websocket server.Feb 6 2024, 11:38 PM

will retitled this revision from [lib] [4/n] Add types for identity search websocket server to [lib] Add types for identity search websocket server.Feb 8 2024, 10:14 AM

rebase on changes to identity search message type changes

Harbormaster failed remote builds in B26661: Diff 36869!Feb 8 2024, 10:25 AM

will edited the summary of this revision. (Show Details)Feb 8 2024, 10:56 AM

will added a parent revision: D10996: [identity] Remove usage of sending raw error messages to client.

will added inline comments.Feb 8 2024, 11:48 AM

lib/types/identity-search/messages.js
27 ↗	(On Diff #36572)	Rust addressed in https://phab.comm.dev/D10945 and updated review to separate server/client message types

varun added inline comments.Feb 12 2024, 8:29 PM

lib/types/identity-search/messages.js
27–37 ↗	(On Diff #36869)	do we need this message in other files, too?

would also be great if you could rebase on master now that the CI is fixed

rebase

Harbormaster completed remote builds in B26776: Diff 37016.Feb 13 2024, 12:47 AM

will added inline comments.Feb 13 2024, 10:54 AM

lib/types/identity-search/messages.js
27–37 ↗	(On Diff #36869)	Yeah, I think that'd definitely be nice to have. Will add in next diff

varun accepted this revision.Feb 13 2024, 10:58 AM

varun added inline comments.

lib/types/identity-search/messages.js
27–37 ↗	(On Diff #36869)	might be easier to just rebase and add it in this diff. if not, can you please link a linear task or diff here before landing?

This revision is now accepted and ready to land.Feb 13 2024, 10:58 AM

will added inline comments.Feb 13 2024, 11:01 AM

lib/types/identity-search/messages.js
27–37 ↗	(On Diff #36869)	Sorry. Should have clarified. I meant I'll rebase on this diff

varun added inline comments.Feb 13 2024, 11:20 AM

lib/types/identity-search/messages.js
27–37 ↗	(On Diff #37016)	i feel like this note should come before the imports

varun mentioned this in D10945: [identity] [shared] Separate identity search messages into client and server messages.Feb 13 2024, 11:21 AM

will mentioned this in D11060: [web] Change identity search function return type to GlobalUserInfo.Feb 14 2024, 12:51 PM

rebase with change to IdentitySearchUser and add comments

typo in AuthMessage

lib/types/identity-search/messages.js
27–37 ↗	(On Diff #37016)	Moved

will added inline comments.Feb 15 2024, 12:42 PM

lib/types/identity-search/messages.js
27–37 ↗	(On Diff #36869)	Addressed in latest rebase

Harbormaster completed remote builds in B26937: Diff 37278.Feb 15 2024, 12:52 PM

Harbormaster completed remote builds in B26938: Diff 37279.Feb 15 2024, 12:56 PM

Please take the feedback about User and apply it to all of your named exports

lib/types/identity-search/auth-message-types.js
20 ↗	(On Diff #37279)	We need to be more thoughtful about names that get exported. They should be "universal" across the JS codebase. In this case, this should be something more like `IdentitySearchAuthMessage`
27 ↗	(On Diff #37279)	And this should be more like `identitySearchAuthMessageValidator`
lib/types/identity-search/search-query-types.js
22 ↗	(On Diff #37279)	Extremely generic name