feat: Retry fetching missing user/conversation metadata every 3h (FS-1691) #15158
Conversation
| const allUnavailableUsers: User[] = []; | ||
| this.conversationState.conversations().forEach(conversation => { | ||
| allUnavailableUsers.push(...conversation.allUserEntities().filter(user => !user.isAvailable())); | ||
| }); |
| @@ -291,6 +291,7 @@ export class ConversationRepository { | |||
|
|
|||
| this.conversationRoleRepository = new ConversationRoleRepository(this.teamRepository, this.conversationService); | |||
| this.leaveCall = noop; | |||
| this.scheduleMissingUsersAndConversationsMetadataRefresh(); | |||
There was a problem hiding this comment.
if this is supposed to be federation only, why isn't it conditional at all?
There was a problem hiding this comment.
there is no missing data so we'll iterate over an empty array and no request will be fired, but still we could add a isFederated check to avoid that as well but performance wise that's micro optimization
There was a problem hiding this comment.
is there no way to tell we are in a federated environment? i dont see why we should run a bunch of async calls every 3 hours if we dont need to. (regardless of performance impact)
There was a problem hiding this comment.
maybe this.core.backendFeatures.isFederated? @thisisamir98
| @@ -100,6 +100,8 @@ describe('ConversationRepository', () => { | |||
| }; | |||
|
|
|||
| beforeAll(async () => { | |||
| jest.useFakeTimers(); | |||
There was a problem hiding this comment.
Is this somehow breaking some other test cases? The ci is breaking
There was a problem hiding this comment.
maybe, i'm checking...
|
please fill out the PR info |
Codecov Report
@@ Coverage Diff @@
## dev #15158 +/- ##
==========================================
+ Coverage 43.15% 43.20% +0.04%
==========================================
Files 644 644
Lines 21634 21647 +13
Branches 4954 4956 +2
==========================================
+ Hits 9337 9353 +16
+ Misses 11105 11102 -3
Partials 1192 1192 |
| if (!this.core.backendFeatures.isFederated) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
if we do nothing in this case, maybe let's just not call this function at all, so wrap it with the condition:
if(this.core.backendFeatures.isFederated){
scheduleMissingUsersAndConversationsMetadataRefresh()
}wdyt?
| @@ -467,6 +468,35 @@ export class ConversationRepository { | |||
| await this.userRepository.refreshUsers(unavailableUsers.map(user => user.qualifiedId)); | |||
| } | |||
|
|
|||
| public async refreshAllConversationsUnavailableParticipants(): Promise<void> { | |||
There was a problem hiding this comment.
do we need this one to be public since it's being used only in this class?
There was a problem hiding this comment.
the other one (refreshUnavailableParticipants) implemented previously by @atomrc was public so I made this public as well but we could make it private.
Scenario:
Issue:
At this point user X is unable to fetch the meta data of user Y because the data lives on the B backend and connection is offline.
Solution
@atomrc Has already implemented a try to refetch that missing meta data when opening the user details menu here:
#14975
This PR will add another solution to this issue to try to refetch all missing data every 3 hours.