Page MenuHomePhabricator

[web] Change identity search function return type to GlobalUserInfo
AbandonedPublic

Authored by will on Feb 13 2024, 2:12 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 24, 12:22 AM
Unknown Object (File)
Sat, Nov 23, 10:20 PM
Unknown Object (File)
Wed, Nov 13, 12:43 AM
Unknown Object (File)
Wed, Nov 13, 12:04 AM
Unknown Object (File)
Tue, Nov 12, 10:22 PM
Unknown Object (File)
Oct 10 2024, 11:01 AM
Unknown Object (File)
Oct 10 2024, 11:01 AM
Unknown Object (File)
Oct 10 2024, 11:01 AM
Subscribers

Details

Summary

This replaces the return type of the identity search function with an array of GlobalUserInfo instead of Users.

This is to replace code duplication at search function callsites that require a GlobalUserInfo type instead of a User type.

Test Plan

flow and tested on web

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

will requested review of this revision.Feb 13 2024, 2:27 PM
ashoat added inline comments.
lib/shared/search-utils.js
29

Is this type used anywhere else? Should it be deleted?

I can't find this file on master or in your stack, so I suspect you need to update your stack to accurately reflect what your local branch structure is. I should always be able to find changes like this either in master or somewhere else in your stack.

This revision is now accepted and ready to land.Feb 14 2024, 10:10 AM
lib/shared/search-utils.js
29

The User type was introduced in https://phab.comm.dev/D10928. Didn't show up on stack because I didn't put depends on D11001 on https://phab.comm.dev/D11049. Should be all fixed

lib/shared/search-utils.js
29

Thanks! How about these questions?

Is this type used anywhere else? Should it be deleted?

lib/shared/search-utils.js
29

I can't delete the type as the SearchResult I receive from my websocket server is composed of the User type. I need to keep it to properly authenticate the SearchResponse type I receive from the websocket server. It is also used in https://phab.comm.dev/D11000 for defining Promise types

lib/shared/search-utils.js
29

If you're referring to utilizing GlobalUserInfo instead of User for defining websocket responses, that's definitely an option; however, GlobalUserInfo includes an avatar field which isn't included as part of my search index. I could return null in this case, but I use User to refer to what is currently stored per User in the search index

lib/shared/search-utils.js
29

The avatar field is optional in GlobalAccountUserInfo

I think it's fine to define a new type if you want, but looking at it now, User is way too generic a name. Can you pick a better name, such as IdentitySearchUser?

lib/shared/search-utils.js
29

This feedback still needs to be addressed. User is way too generic of a name for a type that is exported. Can you please rename to something like IdentitySearchUser?

lib/shared/search-utils.js
29

Addressed Rust implementation in https://phab.comm.dev/D11096 and in JavaScript in latest revision of https://phab.comm.dev/D10928