Skip to content

Commit

Permalink
feat: Do not load all team members on app load [WPB-4553 WPB-4548]
Browse files Browse the repository at this point in the history
  • Loading branch information
otto-the-bot authored Jan 29, 2024
1 parent 27b289f commit c43835f
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 76 deletions.
2 changes: 1 addition & 1 deletion src/script/components/UserSearchableList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export const UserSearchableList: React.FC<UserListProps> = ({
*/
const fetchMembersFromBackend = useCallback(
debounce(async (query: string, ignoreMembers: User[]) => {
const resultUsers = await searchRepository.searchByName(query);
const resultUsers = await searchRepository.searchByName(query, selfUser.teamId);
const selfTeamId = selfUser.teamId;
const foundMembers = resultUsers.filter(user => user.teamId === selfTeamId);
const ignoreIds = ignoreMembers.map(member => member.id);
Expand Down
26 changes: 15 additions & 11 deletions src/script/main/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {container} from 'tsyringe';
import {Runtime} from '@wireapp/commons';
import {WebAppEvents} from '@wireapp/webapp-events';

import {E2EIHandler} from 'src/script/E2EIdentity';
import {initializeDataDog} from 'Util/DataDog';
import {DebugUtil} from 'Util/DebugUtil';
import {Environment} from 'Util/Environment';
Expand Down Expand Up @@ -424,24 +425,27 @@ export class App {
onProgress(10);
telemetry.timeStep(AppInitTimingsStep.INITIALIZED_CRYPTOGRAPHY);

const {members: teamMembers, features: teamFeatures} = await teamRepository.initTeam(selfUser.teamId);
const e2eiHandler = await configureE2EI(this.logger, teamFeatures);
if (e2eiHandler) {
/* We first try to do the initial enrollment (if the user has not yet enrolled)
* We need to enroll before anything else (in particular joining MLS conversations)
* Until the user is enrolled, we need to pause loading the app
*/
await e2eiHandler.attemptEnrollment();
}
const connections = await connectionRepository.getConnections();

telemetry.timeStep(AppInitTimingsStep.RECEIVED_USER_DATA);

const connections = await connectionRepository.getConnections();
telemetry.addStatistic(AppInitStatisticsValue.CONNECTIONS, connections.length, 50);

const conversations = await conversationRepository.loadConversations();

await userRepository.loadUsers(selfUser, connections, conversations, teamMembers);
const contacts = await userRepository.loadUsers(selfUser, connections, conversations);
let e2eiHandler: E2EIHandler | undefined;
if (selfUser.teamId) {
const {features: teamFeatures} = await teamRepository.initTeam(selfUser.teamId, contacts);
e2eiHandler = await configureE2EI(this.logger, teamFeatures);
if (e2eiHandler) {
/* We first try to do the initial enrollment (if the user has not yet enrolled)
* We need to enroll before anything else (in particular joining MLS conversations)
* Until the user is enrolled, we need to pause loading the app
*/
await e2eiHandler.attemptEnrollment();
}
}

if (supportsMLS()) {
//if mls is supported, we need to initialize the callbacks (they are used when decrypting messages)
Expand Down
2 changes: 1 addition & 1 deletion src/script/page/LeftSidebar/panels/StartUI/PeopleTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ export const PeopleTab = ({
onSearchResults(localSearchResults);
if (canSearchUnconnectedUsers) {
try {
const userEntities = await searchRepository.searchByName(searchQuery);
const userEntities = await searchRepository.searchByName(searchQuery, selfUser.teamId);
const localUserIds = localSearchResults.contacts.map(({id}) => id);
const onlyRemoteUsers = userEntities.filter(user => !localUserIds.includes(user.id));
const results = inTeam
Expand Down
22 changes: 22 additions & 0 deletions src/script/search/SearchRepository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import {User} from 'src/script/entity/User';
import {generateUser} from 'test/helper/UserGenerator';
import {createUuid} from 'Util/uuid';

import {SearchRepository} from './SearchRepository';

Expand Down Expand Up @@ -233,6 +234,27 @@ describe('SearchRepository', () => {

expect(suggestions.length).toEqual(localUsers.length - 1);
});

it('returns team users first', async () => {
const [searchRepository, {apiClient, userRepository}] = buildSearchRepository();
const teamId = createUuid();
const teamUsers = [generateUser(undefined, {team: teamId}), generateUser(undefined, {team: teamId})];
const otherTeamUsers = [generateUser(undefined, {team: createUuid()})];
const localUsers = [generateUser(), generateUser(), generateUser()];
const allUsers = [...localUsers, ...otherTeamUsers, ...teamUsers];
userRepository.getUsersById.mockResolvedValue(allUsers);

const searchResults = allUsers.map(({qualifiedId}) => qualifiedId);
jest
.spyOn(apiClient.api.user, 'getSearchContacts')
.mockResolvedValue({response: {documents: searchResults}} as any);

const suggestions = await searchRepository.searchByName('term', teamId);

expect(suggestions.length).toEqual(allUsers.length);
expect(suggestions[0].teamId).toEqual(teamId);
expect(suggestions[1].teamId).toEqual(teamId);
});
});
});

Expand Down
8 changes: 6 additions & 2 deletions src/script/search/SearchRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,11 @@ export class SearchRepository {
* @note We skip a few results as connection changes need a while to reflect on the backend.
*
* @param query Search query
* @param isHandle Is query a user handle
* @param teamId Current team ID the selfUser is in (will help prioritize results)
* @param maxResults Maximum number of results
* @returns Resolves with the search results
*/
async searchByName(term: string, maxResults = CONFIG.MAX_SEARCH_RESULTS): Promise<User[]> {
async searchByName(term: string, teamId = '', maxResults = CONFIG.MAX_SEARCH_RESULTS): Promise<User[]> {
const {query, isHandleQuery} = this.normalizeQuery(term);
const [rawName, rawDomain] = this.core.backendFeatures.isFederated ? query.split('@') : [query];
const [name, domain] = validateHandle(rawName, rawDomain) ? [rawName, rawDomain] : [query];
Expand All @@ -199,6 +199,10 @@ export class SearchRepository {
.filter(user => !user.isMe)
.filter(user => !isHandleQuery || startsWith(user.username(), query))
.sort((userA, userB) => {
if (userA.teamId === teamId && userB.teamId !== teamId) {
// put team members first
return -1;
}
return isHandleQuery
? sortByPriority(userA.username(), userB.username(), query)
: sortByPriority(userA.name(), userB.name(), query);
Expand Down
18 changes: 0 additions & 18 deletions src/script/team/TeamRepository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import {ConversationProtocol} from '@wireapp/api-client/lib/conversation';
import {FeatureList, FeatureStatus} from '@wireapp/api-client/lib/team/feature/';
import {Permissions} from '@wireapp/api-client/lib/team/member';

import {randomUUID} from 'crypto';

Expand Down Expand Up @@ -71,12 +70,6 @@ describe('TeamRepository', () => {
has_more: false,
};
const team_metadata = teams_data.teams[0];
const team_members = {
members: [
{user: randomUUID(), permissions: {copy: Permissions.DEFAULT, self: Permissions.DEFAULT}},
{user: randomUUID(), permissions: {copy: Permissions.DEFAULT, self: Permissions.DEFAULT}},
],
};

describe('getTeam()', () => {
it('returns the team entity', async () => {
Expand All @@ -93,17 +86,6 @@ describe('TeamRepository', () => {
});
});

describe('getAllTeamMembers()', () => {
it('returns team member entities', async () => {
const [teamRepo, {teamService}] = buildConnectionRepository();
jest.spyOn(teamService, 'getAllTeamMembers').mockResolvedValue({hasMore: false, members: team_members.members});
const entities = await teamRepo['getAllTeamMembers'](team_metadata.id);
expect(entities.length).toEqual(team_members.members.length);
expect(entities[0].userId).toEqual(team_members.members[0].user);
expect(entities[0].permissions).toEqual(team_members.members[0].permissions);
});
});

describe('sendAccountInfo', () => {
it('does not crash when there is no team logo', async () => {
const [teamRepo] = buildConnectionRepository();
Expand Down
44 changes: 19 additions & 25 deletions src/script/team/TeamRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import {TEAM_EVENT} from '@wireapp/api-client/lib/event/TeamEvent';
import {FeatureStatus, FeatureList} from '@wireapp/api-client/lib/team/feature/';
import type {PermissionsData} from '@wireapp/api-client/lib/team/member/PermissionsData';
import type {TeamData} from '@wireapp/api-client/lib/team/team/TeamData';
import {QualifiedId} from '@wireapp/api-client/lib/user';
import {amplify} from 'amplify';
import {container} from 'tsyringe';

Expand Down Expand Up @@ -120,15 +119,29 @@ export class TeamRepository extends TypedEventEmitter<Events> {
);
}

async initTeam(teamId?: string): Promise<{members: QualifiedId[]; features: FeatureList}> {
/**
* Will init the team configuration and all the team members from the contact list.
* @param teamId the Id of the team to init
* @param contacts all the contacts the self user has, team members will be deduced from it.
*/
async initTeam(
teamId: string,
contacts: User[] = [],
): Promise<{team: TeamEntity | undefined; features: FeatureList}> {
// async initTeam(teamId?: string): Promise<{members: QualifiedId[]; features: FeatureList}> {
const team = await this.getTeam();
// get the fresh feature config from backend
const {newFeatureList} = await this.updateFeatureConfig();
if (!teamId) {
return {members: [], features: {}};
return {team: undefined, features: {}};
}
await this.updateTeamMembersByIds(
team,
contacts.filter(user => user.teamId === teamId).map(({id}) => id),
);

// Subscribe to team members change and update the user role and guest status
this.teamState.teamMembers.subscribe(members => {
// Subscribe to team members change and update the user role and guest status
this.userRepository.mapGuestStatus(members);
const roles = this.teamState.memberRoles();
members.forEach(user => {
Expand All @@ -137,9 +150,9 @@ export class TeamRepository extends TypedEventEmitter<Events> {
}
});
});
const members = await this.loadTeamMembers(team);

this.scheduleTeamRefresh();
return {members, features: newFeatureList};
return {team, features: newFeatureList};
}

private async updateFeatureConfig(): Promise<{newFeatureList: FeatureList; prevFeatureList?: FeatureList}> {
Expand Down Expand Up @@ -190,14 +203,6 @@ export class TeamRepository extends TypedEventEmitter<Events> {
return memberEntity;
}

private async getAllTeamMembers(teamId: string): Promise<TeamMemberEntity[]> {
const {members, hasMore} = await this.teamService.getAllTeamMembers(teamId);
if (!hasMore && members.length) {
return this.teamMapper.mapMembers(members);
}
return [];
}

async conversationHasGuestLinkEnabled(conversationId: string): Promise<boolean> {
return this.teamService.conversationHasGuestLink(conversationId);
}
Expand Down Expand Up @@ -342,17 +347,6 @@ export class TeamRepository extends TypedEventEmitter<Events> {
this.updateMemberRoles(mappedMembers);
}

private async loadTeamMembers(teamEntity: TeamEntity): Promise<QualifiedId[]> {
const teamMembers = await this.getAllTeamMembers(teamEntity.id);
this.teamState.memberRoles({});
this.teamState.memberInviters({});

this.updateMemberRoles(teamMembers);
return teamMembers
.filter(({userId}) => userId !== this.userState.self().id)
.map(memberEntity => ({domain: this.teamState.teamDomain() ?? '', id: memberEntity.userId}));
}

private getTeamById(teamId: string): Promise<TeamData> {
return this.teamService.getTeamById(teamId);
}
Expand Down
8 changes: 4 additions & 4 deletions src/script/user/UserRepository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ describe('UserRepository', () => {
const connections = createConnections(users);
const fetchUserSpy = jest.spyOn(userService, 'getUsers').mockResolvedValue({found: users});

await userRepository.loadUsers(new User('self'), connections, [], []);
await userRepository.loadUsers(new User('self'), connections, []);

expect(userState.users()).toHaveLength(users.length + 1);
expect(fetchUserSpy).toHaveBeenCalledWith(users.map(user => user.qualified_id!));
Expand All @@ -277,7 +277,7 @@ describe('UserRepository', () => {
const connections = createConnections(users);
jest.spyOn(userService, 'getUsers').mockResolvedValue({found: users});

await userRepository.loadUsers(new User('self'), connections, [], []);
await userRepository.loadUsers(new User('self'), connections, []);

expect(userState.users()).toHaveLength(users.length + 1);
users.forEach(user => {
Expand All @@ -297,7 +297,7 @@ describe('UserRepository', () => {
jest.spyOn(userRepository['userService'], 'loadUserFromDb').mockResolvedValue(partialUsers as any);
const fetchUserSpy = jest.spyOn(userService, 'getUsers').mockResolvedValue({found: localUsers});

await userRepository.loadUsers(new User('self'), connections, [], []);
await userRepository.loadUsers(new User('self'), connections, []);

expect(userState.users()).toHaveLength(localUsers.length + 1);
expect(fetchUserSpy).toHaveBeenCalledWith(userIds);
Expand All @@ -312,7 +312,7 @@ describe('UserRepository', () => {
const removeUserSpy = jest.spyOn(userService, 'removeUserFromDb').mockResolvedValue();
jest.spyOn(userService, 'getUsers').mockResolvedValue({found: newUsers});

await userRepository.loadUsers(new User(), connections, [], []);
await userRepository.loadUsers(new User(), connections, []);

expect(userState.users()).toHaveLength(newUsers.length + 1);
expect(removeUserSpy).toHaveBeenCalledTimes(localUsers.length);
Expand Down
13 changes: 2 additions & 11 deletions src/script/user/UserRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,19 +196,10 @@ export class UserRepository extends TypedEventEmitter<Events> {
* @param selfUser the user currently logged in (will be excluded from fetch)
* @param connections the connection to other users
* @param conversations the conversation the user is part of (used to compute extra users that are part of those conversations but not directly connected to the user)
* @param extraUsers other users that would need to be loaded (team users usually that are not direct connections)
*/
async loadUsers(
selfUser: User,
connections: ConnectionEntity[],
conversations: Conversation[],
extraUsers: QualifiedId[],
): Promise<User[]> {
async loadUsers(selfUser: User, connections: ConnectionEntity[], conversations: Conversation[]): Promise<User[]> {
const conversationMembers = flatten(conversations.map(conversation => conversation.participating_user_ids()));
const allUserIds = connections
.map(connectionEntity => connectionEntity.userId)
.concat(conversationMembers)
.concat(extraUsers);
const allUserIds = connections.map(connectionEntity => connectionEntity.userId).concat(conversationMembers);
const users = uniq(allUserIds, false, (userId: QualifiedId) => userId.id);

// Remove all users that have non-qualified Ids in DB (there could be duplicated entries one qualified and one non-qualified)
Expand Down
6 changes: 3 additions & 3 deletions test/helper/UserGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import {faker} from '@faker-js/faker';
import {QualifiedId, UserAssetType} from '@wireapp/api-client/lib/user';
import type {User as APIClientUser} from '@wireapp/api-client/lib/user/';
import type {User as APIClientUser} from '@wireapp/api-client/lib/user';

import {createUuid} from 'Util/uuid';

Expand Down Expand Up @@ -61,7 +61,7 @@ export function generateAPIUser(
};
}

export function generateUser(id?: QualifiedId): User {
const apiUser = generateAPIUser(id);
export function generateUser(id?: QualifiedId, overwites?: Partial<APIClientUser>): User {
const apiUser = generateAPIUser(id, overwites);
return new UserMapper(serverTimeHandler).mapUserFromJson(apiUser, '');
}

0 comments on commit c43835f

Please sign in to comment.