HomePhabricator
Diffusion Comm 83252ba96c76

[keyserver] Update `fetchUserInfos` to include `avatar`s in `GlobalUserInfo`s

Description

[keyserver] Update fetchUserInfos to include avatars in GlobalUserInfos

Summary:
We're going to include avatars from the users table in the GlobalUserInfos returned by fetchUserInfos.

NOTE: I know it was previously discussed that we want to avoid sending avatars in *UserInfos to older clients. However, the *UserInfo types are used in so many places and included in so many of the "messages" sent between the client and the server that handling every single case with a codeVersion check would be very involved.

If it's possible for older clients to recieve avatars in *UserInfos without issue, then I think we should proceed by including avatars everywhere. My hunch (after reading through the codebase) is that it's completely fine to have superfulous fields in the Redux CurrentUserInfo and in the UserInfo entries. I only took a cursory glance, I will read through thoroughly to confirm this.

If we conclude that it would be catastrophic for old clients to recieve avatar field in *UserInfo, then the best way to handle it would be:

  1. Start from the DB queries and include avatar in all *UserInfos "downstream."
  2. Look at every payload in redux-types to see where *UserInfos from the keyserver are included.
  3. Look at the return types of every responder to see where *UserInfo might be included.
  4. Look at every serverRequest (maybe not necessary?) to see if we ever send *UserInfos as part of a request.
  5. Create some sort of filterOutAvatarsFromUserInfo[s] utility function that would be used RIGHT BEFORE we send data to native clients based on codeVersion

This approach--filtering the avatar field out right before we communicate with client--would be a lot cleaner than drilling Viewer/version check to every single query and having to handle *UserInfos potentially with/without avatar field all throughout keyserver.

This aproach would also avoid complexity with *UserInfo queries that are made by the keyserver rather than any particular client. An example of this is how fetchUserInfos is called by send.js:fetchNotifUserInfos which is called by send.js:fetchInfos which is called by send.js:sendPushNotifs which is called by message-creator:postMessageSend which is called by message-creator.js:createMessages which is called a dozen different places.

TLDR, I think if we can get away with including avatar in *UserInfos for older client, we should seriously consider it. If not, I strongly think that the filtering out of the avatar field should happen where the client and server "interface," NOT at every query that returns *UserInfos OR throughout keyserver where those objects are passed all over the place. I will, however, still consider EG state check mechanism since this approach would necessitate that the *UserInfos would be out of sync.

Test Plan:
Set breakpoint and observed that avatar was included in *UserInfos returned by fetchUserInfos:

[screenshot coming shortly]


This won't be landed until the rest of the refactoring is complete AND there's been a LOT more testing.

Reviewers: ashoat, ginsu

Reviewed By: ashoat

Subscribers: tomek

Differential Revision: https://phab.comm.dev/D7162