issue: ENG-9633
Details
Tested that when a user is missing a device list the accountMissingStatus is set. Checked that lastChecked is updated with next requests but missingSince stays the same.
checked that if non-empty a device list is returned by identity, accountMissingStatus is removed.
checked that further requests for the same user are stopped for 20s.
checked that if a user that has a recent lastChecked is picked in search (triggering onSelectUserFromSearch which calls useUsersSupportThickThreads) they are not queried for
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
When I first proposed this approach, I had in mind that we would modify useGetAndUpdateDeviceListsForUsers:
We could modify useGetAndUpdateDeviceListsForUsers so that before we query for a user's device lists, we first check lastChecked, and skip the check if it's sufficiently recent.
Can you explain it your approach here a bit more? I'm curious:
- How you selected these callsites in particular
- Why we're not modifying useGetAndUpdateDeviceListsForUsers
Initially I modified useGetAndUpdateDeviceListsForUsers, but I realised that it is used in many places. Some of those places seem like we don't want to use this mechanism.
- In UserInfosHandler we want to request all missing device lists as the app starts. There is a special mechanism there to make sure we request them exactly once.
- In SecondaryDeviceQRCodeScanner we request current users device list, so it can never be empty as far as I understand.
- In usePeerToPeerMessageHandler we request the device list when we receive a DEVICE_LIST_UPDATED or IDENTITY_DEVICE_LIST_UPDATED. I feel like we really want to request the device list in this case, and ignore the timeout.
As for the change in useUsersSupportThickThreads I followed @kamil's suggestion from the issue's description and checked all places that call userHasDeviceList.
The only place that uses userHasDeviceList that I didn't update, is usePotentialMemberItems. But in there we don't request the device lists, we only check if the user can be displayed in search results (D13405)
On the other hand, in useUsersSupportThickThreads we request user identities, for the purpose of verifying if they "support thick threads". It is used to check if users can be added to thick threads, and update pending thread types based on this information. If we are unable to get user's device list, they cannot be supporting thick threads.
This also prompted me to look at other usages of findUserIdentities. My conclusion was that:
- when updating relationships we don't want to respect this timeout
- UserIdentityCacheProvider has its own timeout system
- UserInfosHandler, as mentioned before has a different mechanism that wants to request every user exactly once and there is no point respecting this timeout
- FarcasterDataHandler I am not so sure about, but it requests current user id and ALL user ids, and uses that to get fids. This looks like some kind of synchronisation mechanism that also shouldn't be respecting this timeout.
Sorry for not explaining it in the summary, I forgot that you proposed changing specifically useGetAndUpdateDeviceListsForUsers
Thanks for explaining, @inka! Some more questions inline
lib/hooks/user-identities-hooks.js | ||
---|---|---|
26 ↗ | (On Diff #45228) | In your last comment, you said:
|
lib/shared/dm-ops/dm-op-utils.js | ||
142–146 ↗ | (On Diff #45228) | Would appreciate @kamil's perspective on the code here, as it's rather complex and he is most familiar |
187 ↗ | (On Diff #45228) |
This check is basically userHasDeviceList... are any changes necessary? |
lib/hooks/user-identities-hooks.js | ||
---|---|---|
26 ↗ | (On Diff #45228) |
Yes, I mean that it has it's own caching mechanism which includes not querying for the same users again until some time has passed (cacheTimeout) |
26 ↗ | (On Diff #45228) |
Ah, you are right. I will undo this change. |
lib/shared/dm-ops/dm-op-utils.js | ||
187 ↗ | (On Diff #45228) | No, this place doesn't need any changes. We want to check deviceListCanBeRequestedForUser when we are about to request users device list. This place is right after we queried for device lists, and just creates data of the right shape from the result. |