Page MenuHomePhabricator

[lib] Fetch missing usernames
ClosedPublic

Authored by inka on May 27 2024, 6:05 AM.
Tags
None
Referenced Files
F2114602: D12219.id40957.diff
Wed, Jun 26, 5:56 AM
Unknown Object (File)
Tue, Jun 18, 9:32 AM
Unknown Object (File)
Sat, Jun 15, 3:24 PM
Unknown Object (File)
Tue, Jun 11, 7:56 PM
Unknown Object (File)
Mon, Jun 10, 10:08 AM
Unknown Object (File)
Sun, Jun 9, 2:59 PM
Unknown Object (File)
Sat, Jun 8, 2:42 PM
Unknown Object (File)
Sat, Jun 8, 12:22 PM
Subscribers

Details

Summary

issue: ENG-7743
We can now fetch the usernames using the identity RPC

Test Plan

tested that when new user infos without usernames are added to the store, the effect is run and the usernames are fetched from identity, and inserted into user infos in the user store.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.May 27 2024, 6:09 AM
Harbormaster failed remote builds in B29255: Diff 40673!
inka requested review of this revision.May 27 2024, 6:22 AM
tomek requested changes to this revision.Tue, May 28, 5:09 AM

This solution has one issue: we always send requests for all the users even if we've just asked for some of them. The following scenario shows the issue:

  1. New unknown users appear in the store with IDs e.g. [1,2,3]
  2. We send a request to identity asking about [1,2,3]
  3. A new unknown user 4 appears
  4. We send a request to identity asking about [1,2,3,4]
  5. Identity responds with [1,2,3]
  6. Identity responds with [1,2,3,4]

This is a slight performance issue, so it's probably not worth spending too much time on it, but it can be fixed by maintaining a set of users for which a request to identity is pending and only send a request for the users without pending requests.

lib/handlers/user-infos-handler.react.js
35–53 ↗(On Diff #40673)

This isn't accurate, because on line 35 we're creating a function instead of a promise

This revision now requires changes to proceed.Tue, May 28, 5:09 AM

Address review
I created a scenario as descibed by @tomek, by dispatching two processNewUserIDsActionType actions and adding a sleep in findUserIdentities to make sure the second action is parsed before the id from the first action gets its username in redux.
Then I added this code and checked that the problem is fixed

This revision is now accepted and ready to land.Wed, May 29, 7:07 AM
This revision was automatically updated to reflect the committed changes.