[lib] Extract user ids from server socket messages
ClosedPublic
Actions

Authored by inka on May 15 2024, 7:42 AM.

Details

Reviewers

tomek
kamil

Commits

rCOMMc9dc7ed65003: [lib] Extract user ids from server socket messages

Summary

issue: ENG-7742
This handler extracts user ids from server messages that come over the socket.

Test Plan

Sent a message from user1 and checked that updates/server messages on user2 are parsed correctly - correct user ids are extracted.
Tested that no errors show up

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

inka created this revision.May 15 2024, 7:42 AM

Herald added a subscriber: ashoat. · View Herald TranscriptMay 15 2024, 7:42 AM

Harbormaster completed remote builds in B28941: Diff 40222.May 15 2024, 7:58 AM

inka requested review of this revision.May 15 2024, 7:58 AM

ashoat added inline comments.May 15 2024, 6:35 PM

lib/socket/user-infos-handler.react.js
1 ↗	(On Diff #40222)	Nit: newline below Flow declaration
3 ↗	(On Diff #40222)	We usually type `React.useEffect` directly
5–9 ↗	(On Diff #40222)	Merge imports
17 ↗	(On Diff #40222)	Would it be good to rename this to be clear it only handles `UserInfos` that come in through the socket?

tomek accepted this revision.May 16 2024, 1:20 AM

tomek added inline comments.

lib/socket/user-infos-handler.react.js
13–14 ↗	(On Diff #40222)	It's more convenient to type these as returning `mixed`

This revision is now accepted and ready to land.May 16 2024, 1:20 AM

Address review

Harbormaster completed remote builds in B28975: Diff 40278.May 16 2024, 3:59 AM

inka added a child revision: D12072: [lib] Add httpMessageUserInfosHandler.May 16 2024, 7:03 AM

ashoat added inline comments.May 16 2024, 11:47 AM

lib/socket/socket.react.js
15 ↗	(On Diff #40278)	I think it's best to use `SocketMessageUserInfosHandler` here too
lib/socket/user-infos-handler.react.js
20 ↗	(On Diff #40278)	I'd move this lower, next to where they're used