Skip to content

Commit

Permalink
Login improvement (#8306)
Browse files Browse the repository at this point in the history 
* refactor to improve requests on login

* include in team sidebar order by display name teams not present in the order preference

* Fix iOS reload while developing

* fix code causing tests to fail

* feedback review

* update network-client
  • Loading branch information
@enahum
enahum authored Nov 13, 2024
1 parent f6bc8ae commit ea2cb45
Show file tree
Hide file tree
Showing 18 changed files with 362 additions and 116 deletions.
9 changes: 7 additions & 2 deletions app/actions/remote/channel.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,13 @@ const mockClient = {
getUser: jest.fn((userId: string) => ({...user, id: userId})),
getMyChannels: jest.fn((teamId: string) => ([{id: channelId, name: 'channel1', creatorId: user.id, team_id: teamId}])),
getMyChannelMembers: jest.fn(() => ([{id: user.id + '-' + channelId, user_id: user.id, channel_id: channelId, roles: ''}])),
getCategories: jest.fn((userId: string, teamId: string) => ({categories: [{id: 'categoryid', channel_id: [channelId], team_id: teamId}], order: ['categoryid']})),
getCategories: jest.fn((userId: string, teamId: string) => ({categories: [{id: 'categoryid', channel_ids: [channelId], team_id: teamId}], order: ['categoryid']})),
viewMyChannel: jest.fn(),
getTeamByName: jest.fn((teamName: string) => ({id: teamId, name: teamName})),
getTeam: jest.fn((id: string) => ({id, name: 'teamname'})),
addToTeam: jest.fn((teamId: string, userId: string) => ({id: userId + '-' + teamId, user_id: userId, team_id: teamId, roles: ''})),
getUserByUsername: jest.fn((username: string) => ({...user, id: 'userid2', username})),
createDirectChannel: jest.fn((userId1: string, userId2: string) => ({id: userId1 + '__' + userId2, team_id: '', type: 'D', display_name: 'displayname'})),
createDirectChannel: jest.fn((userIds: string[]) => ({id: userIds[0] + '__' + userIds[1], team_id: '', type: 'D', display_name: 'displayname'})),
getChannels: jest.fn((teamId: string) => ([{id: channelId, name: 'channel1', creatorId: user.id, team_id: teamId}])),
getArchivedChannels: jest.fn((teamId: string) => ([{id: channelId + 'old', name: 'channel1old', creatorId: user.id, team_id: teamId, delete_at: 1}])),
createGroupChannel: jest.fn(() => ({id: 'groupid', team_id: '', type: 'G', display_name: 'displayname'})),
Expand All @@ -129,6 +129,11 @@ const mockClient = {
convertChannelToPrivate: jest.fn(),
getGroupMessageMembersCommonTeams: jest.fn(() => ({id: teamId, name: 'teamname'})),
convertGroupMessageToPrivateChannel: jest.fn((channelId: string) => ({id: channelId, name: 'channel1', creatorId: user.id, type: 'P'})),
getPosts: jest.fn(() => ({
order: [],
posts: [],
previousPostId: '',
})),
};

const teamId = 'teamid1';
Expand Down
3 changes: 2 additions & 1 deletion app/actions/remote/channel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import type ChannelModel from '@typings/database/models/servers/channel';
import type {IntlShape} from 'react-intl';

export type MyChannelsRequest = {
teamId?: string;
categories?: CategoryWithChannels[];
channels?: Channel[];
memberships?: ChannelMembership[];
Expand Down Expand Up @@ -441,7 +442,7 @@ export async function fetchMyChannelsForTeam(serverUrl: string, teamId: string,
setTeamLoading(serverUrl, false);
}

return {channels, memberships, categories};
return {channels, memberships, categories, teamId};
} catch (error) {
logDebug('error on fetchMyChannelsForTeam', getFullErrorMessage(error));
if (!fetchOnly) {
Expand Down
83 changes: 43 additions & 40 deletions app/actions/remote/entry/common.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {fetchPostsForUnreadChannels} from '@actions/remote/post';
import {type MyPreferencesRequest, fetchMyPreferences} from '@actions/remote/preference';
import {fetchRoles} from '@actions/remote/role';
import {fetchConfigAndLicense, fetchDataRetentionPolicy} from '@actions/remote/systems';
import {fetchMyTeams, fetchTeamsChannelsAndUnreadPosts, handleKickFromTeam, type MyTeamsRequest, updateCanJoinTeams} from '@actions/remote/team';
import {fetchMyTeams, fetchTeamsChannelsThreadsAndUnreadPosts, handleKickFromTeam, type MyTeamsRequest, updateCanJoinTeams} from '@actions/remote/team';
import {syncTeamThreads} from '@actions/remote/thread';
import {fetchMe, type MyUserRequest, updateAllUsersSince, autoUpdateTimezone} from '@actions/remote/user';
import {General, Preferences, Screens} from '@constants';
Expand Down Expand Up @@ -66,9 +66,6 @@ export type EntryResponse = {
error: unknown;
}

const FETCH_MISSING_DM_TIMEOUT = 2500;
export const FETCH_UNREADS_TIMEOUT = 2500;

export const entry = async (serverUrl: string, teamId?: string, channelId?: string, since = 0): Promise<EntryResponse> => {
const {database} = DatabaseManager.getServerDatabaseAndOperator(serverUrl);
const result = entryRest(serverUrl, teamId, channelId, since);
Expand Down Expand Up @@ -311,8 +308,7 @@ const fetchAlternateTeamData = async (
let initialTeamId = '';
let chData;

for (const teamId of availableTeamIds) {
// eslint-disable-next-line no-await-in-loop
for await (const teamId of availableTeamIds) {
chData = await fetchMyChannelsForTeam(serverUrl, teamId, includeDeleted, since, fetchOnly, false, isCRTEnabled);
const chError = chData.error;
if (isErrorWithStatusCode(chError) && chError.status_code === 403) {
Expand Down Expand Up @@ -371,49 +367,56 @@ async function entryInitialChannelId(database: Database, requestedChannelId = ''
async function restDeferredAppEntryActions(
serverUrl: string, since: number, currentUserId: string, currentUserLocale: string, preferences: PreferenceType[] | undefined,
config: ClientConfig, license: ClientLicense | undefined, teamData: MyTeamsRequest, chData: MyChannelsRequest | undefined,
initialTeamId?: string, initialChannelId?: string) {
setTimeout(async () => {
if (chData?.channels?.length && chData.memberships?.length) {
// defer fetching posts for unread channels on initial team
fetchPostsForUnreadChannels(serverUrl, chData.channels, chData.memberships, initialChannelId);
}
}, FETCH_UNREADS_TIMEOUT);

// defer fetch channels and unread posts for other teams
if (teamData.teams?.length && teamData.memberships?.length) {
fetchTeamsChannelsAndUnreadPosts(serverUrl, since, teamData.teams, teamData.memberships, initialTeamId);
initialTeamId?: string, initialChannelId?: string,
) {
const isCRTEnabled = (preferences && processIsCRTEnabled(preferences, config.CollapsedThreads, config.FeatureFlagCollapsedThreads, config.Version)) || false;
const directChannels = chData?.channels?.filter(isDMorGM);
const channelsToFetchProfiles = new Set<Channel>(directChannels);

// sidebar DM & GM profiles
if (channelsToFetchProfiles.size) {
const teammateDisplayNameSetting = getTeammateNameDisplaySetting(preferences || [], config.LockTeammateNameDisplay, config.TeammateNameDisplay, license);
fetchMissingDirectChannelsInfo(serverUrl, Array.from(channelsToFetchProfiles), currentUserLocale, teammateDisplayNameSetting, currentUserId);
}

if (preferences && processIsCRTEnabled(preferences, config.CollapsedThreads, config.FeatureFlagCollapsedThreads, config.Version)) {
if (initialTeamId) {
await syncTeamThreads(serverUrl, initialTeamId);
}
updateAllUsersSince(serverUrl, since);
updateCanJoinTeams(serverUrl);

if (teamData.teams?.length) {
for await (const team of teamData.teams) {
if (team.id !== initialTeamId) {
// need to await here since GM/DM threads in different teams overlap
await syncTeamThreads(serverUrl, team.id);
}
// defer fetch channels and unread posts for other teams
setTimeout(async () => {
if (chData?.channels?.length && chData.memberships?.length && initialTeamId) {
if (isCRTEnabled && initialTeamId) {
await syncTeamThreads(serverUrl, initialTeamId);
}
fetchPostsForUnreadChannels(serverUrl, chData.channels, chData.memberships, initialChannelId);
}
}

updateCanJoinTeams(serverUrl);
await updateAllUsersSince(serverUrl, since);
if (teamData.teams?.length && teamData.memberships?.length) {
const teamsOrder = preferences?.find((p) => p.category === Preferences.CATEGORIES.TEAMS_ORDER);
const sortedTeamIds = new Set(teamsOrder?.value.split(','));
const membershipSet = new Set(teamData.memberships.map((m) => m.team_id));
const teamMap = new Map(teamData.teams.map((t) => [t.id, t]));

let myTeams: Team[];
if (sortedTeamIds.size) {
const mySortedTeams = [...sortedTeamIds].
filter((id) => membershipSet.has(id) && teamMap.has(id)).
map((id) => teamMap.get(id)!);
const extraTeams = [...membershipSet].
filter((id) => !sortedTeamIds.has(id) && teamMap.get(id)).
map((id) => teamMap.get(id)!).
sort((a, b) => a.display_name.toLocaleLowerCase().localeCompare(b.display_name.toLocaleLowerCase()));
myTeams = [...mySortedTeams, ...extraTeams];
} else {
myTeams = teamData.teams.
sort((a, b) => a.display_name.toLocaleLowerCase().localeCompare(b.display_name.toLocaleLowerCase()));
}
fetchTeamsChannelsThreadsAndUnreadPosts(serverUrl, since, myTeams, isCRTEnabled);
}
});

// Fetch groups for current user
fetchGroupsForMember(serverUrl, currentUserId);

// defer sidebar DM & GM profiles
setTimeout(async () => {
const directChannels = chData?.channels?.filter(isDMorGM);
const channelsToFetchProfiles = new Set<Channel>(directChannels);
if (channelsToFetchProfiles.size) {
const teammateDisplayNameSetting = getTeammateNameDisplaySetting(preferences || [], config.LockTeammateNameDisplay, config.TeammateNameDisplay, license);
fetchMissingDirectChannelsInfo(serverUrl, Array.from(channelsToFetchProfiles), currentUserLocale, teammateDisplayNameSetting, currentUserId);
}
}, FETCH_MISSING_DM_TIMEOUT);
}

export const setExtraSessionProps = async (serverUrl: string) => {
Expand Down
44 changes: 36 additions & 8 deletions app/actions/remote/post.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

/* eslint-disable max-lines */

import {chunk} from 'lodash';

import {markChannelAsUnread, updateLastPostAt} from '@actions/local/channel';
import {addPostAcknowledgement, removePost, removePostAcknowledgement, storePostsForChannel} from '@actions/local/post';
import {addRecentReaction} from '@actions/local/reactions';
Expand Down Expand Up @@ -40,6 +42,13 @@ type PostsRequest = {
previousPostId?: string;
}

export type PostsForChannel = PostsRequest & {
actionType?: string;
authors?: UserProfile[];
channelId?: string;
error?: unknown;
};

type PostsObjectsRequest = {
error?: unknown;
order?: string[];
Expand Down Expand Up @@ -275,7 +284,7 @@ export const retryFailedPost = async (serverUrl: string, post: PostModel) => {
return {};
};

export async function fetchPostsForChannel(serverUrl: string, channelId: string, fetchOnly = false) {
export async function fetchPostsForChannel(serverUrl: string, channelId: string, fetchOnly = false): Promise<PostsForChannel> {
try {
if (!fetchOnly) {
EphemeralStore.addLoadingMessagesForChannel(serverUrl, channelId);
Expand Down Expand Up @@ -312,7 +321,7 @@ export async function fetchPostsForChannel(serverUrl: string, channelId: string,
}
}

return {posts: data.posts, order: data.order, authors, actionType, previousPostId: data.previousPostId};
return {posts: data.posts, order: data.order, authors, actionType, previousPostId: data.previousPostId, channelId};
} catch (error) {
logDebug('error on fetchPostsForChannel', getFullErrorMessage(error));
return {error};
Expand All @@ -323,15 +332,34 @@ export async function fetchPostsForChannel(serverUrl: string, channelId: string,
}
}

export const fetchPostsForUnreadChannels = async (serverUrl: string, channels: Channel[], memberships: ChannelMembership[], excludeChannelId?: string) => {
const promises = [];
export const fetchPostsForUnreadChannels = async (serverUrl: string, channels: Channel[], memberships: ChannelMembership[], excludeChannelId?: string, fetchOnly = false): Promise<PostsForChannel[]> => {
const membersMap = new Map<string, ChannelMembership>();
for (const member of memberships) {
const channel = channels.find((c) => c.id === member.channel_id);
if (channel && !channel.delete_at && (channel.total_msg_count - member.msg_count) > 0 && channel.id !== excludeChannelId) {
promises.push(fetchPostsForChannel(serverUrl, channel.id));
membersMap.set(member.channel_id, member);
}

const unreadChannelIds = channels.reduce<string[]>((result, channel) => {
const member = membersMap.get(channel.id);
if (member && !channel.delete_at && (channel.total_msg_count - member.msg_count) > 0 && channel.id !== excludeChannelId) {
result.push(channel.id);
}
return result;
}, []);

const postsForChannel: PostsForChannel[] = [];

// process 10 unread channels at a time
const chunks = chunk(unreadChannelIds, 10);
for await (const channelIds of chunks) {
const promises = [];
for (const channelId of channelIds) {
promises.push(fetchPostsForChannel(serverUrl, channelId, fetchOnly));
}

const results = await Promise.all(promises);
postsForChannel.push(...results);
}
return Promise.all(promises);
return postsForChannel;
};

export async function fetchPosts(serverUrl: string, channelId: string, page = 0, perPage = General.POST_CHUNK_SIZE, fetchOnly = false): Promise<PostsRequest> {
Expand Down
51 changes: 43 additions & 8 deletions app/actions/remote/team.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ import {
fetchAllTeams,
fetchTeamsForComponent,
updateCanJoinTeams,
fetchTeamsChannelsAndUnreadPosts,
fetchTeamByName,
removeCurrentUserFromTeam,
removeUserFromTeam,
handleTeamChange,
handleKickFromTeam,
getTeamMembersByIds,
buildTeamIconUrl,
fetchTeamsChannelsThreadsAndUnreadPosts,
} from './team';

import type ServerDataOperator from '@database/operator/server_data_operator';
Expand Down Expand Up @@ -81,12 +81,38 @@ const mockClient = {
getMyTeams: jest.fn(() => ([{id: teamId, name: 'team1'}])),
getMyTeamMembers: jest.fn(() => ([{id: 'userid1-' + teamId, user_id: 'userid1', team_id: teamId, roles: ''}])),
getTeams: jest.fn(() => ([{id: teamId, name: 'team1'}])),
getMyChannels: jest.fn((id: string) => ([{id: channelId, name: 'channel1', creatorId: user.id, team_id: id}])),
getMyChannelMembers: jest.fn(() => ([{id: user.id + '-' + channelId, user_id: user.id, channel_id: channelId, roles: ''}])),
getCategories: jest.fn((userId: string, id: string) => ({categories: [{id: 'categoryid', channel_id: [channelId], team_id: id}], order: ['categoryid']})),
getMyChannels: jest.fn((id: string) => ([{id: channelId, name: 'channel1', creatorId: user.id, team_id: id, total_msg_count: 1}])),
getMyChannelMembers: jest.fn(() => ([{id: user.id + '-' + channelId, user_id: user.id, channel_id: channelId, roles: '', msg_count: 0}])),
getCategories: jest.fn((userId: string, id: string) => ({categories: [{id: 'categoryid', channel_ids: [channelId], team_id: id}], order: ['categoryid']})),
getTeamByName: jest.fn((name: string) => ({id: teamId, name})),
removeFromTeam: jest.fn(),
getTeamMembersByIds: jest.fn((id: string, userIds: string[]) => (userIds.map((userId) => ({id: userId + '-' + id, user_id: userId, team_id: id, roles: ''})))),
getPosts: jest.fn(() => ({
order: ['yocj9xgkh78exk1uhx9yny1zxy', 'ad6yoisgh385fy7rkph49zpfqa', 'o5qqa4ntdigp7rbnf75f8hgeaw'],
posts: [{
channel_id: channelId,
create_at: 1726107604522,
delete_at: 0,
edit_at: 0,
hashtags: '',
id: 'yocj9xgkh78exk1uhx9yny1zxy',
is_pinned: false,
last_reply_at: 0,
message: 'Message ead863',
metadata: {},
original_id: '',
participants: null,
pending_post_id: '',
props: {},
remote_id: '',
reply_count: 0,
root_id: '',
type: '',
update_at: 1726107604522,
user_id: 'k479wsypafgjddz8cerqx4m1ha',
}],
previousPostId: '',
})),
};

beforeAll(() => {
Expand Down Expand Up @@ -245,16 +271,25 @@ describe('teams', () => {
expect(result).toBeDefined();
});

it('fetchTeamsChannelsAndUnreadPosts - handle not found database', async () => {
const result = await fetchTeamsChannelsAndUnreadPosts('foo', 0, [], []) as {error: unknown};
it('fetchTeamsChannelsThreadsAndUnreadPosts - handle not found database', async () => {
const result = await fetchTeamsChannelsThreadsAndUnreadPosts('foo', 0, []) as {error: unknown};
expect(result?.error).toBeDefined();
});

it('fetchTeamsChannelsAndUnreadPosts - base case', async () => {
const result = await fetchTeamsChannelsAndUnreadPosts(serverUrl, 0, [team], [{team_id: teamId, user_id: 'userid1'} as TeamMembership]);
it('fetchTeamsChannelsThreadsAndUnreadPosts - base case', async () => {
const result = await fetchTeamsChannelsThreadsAndUnreadPosts(
serverUrl, 0,
[team], true, false);
expect(result).toBeDefined();
});

it('fetchTeamsChannelsThreadsAndUnreadPosts - fetch only case', async () => {
const result = await fetchTeamsChannelsThreadsAndUnreadPosts(
serverUrl, 0,
[team], true, true);
expect(result.models).toBeDefined();
});

it('fetchTeamByName - handle not found database', async () => {
const result = await fetchTeamByName('foo', '') as {error: unknown};
expect(result?.error).toBeDefined();
Expand Down
Loading

0 comments on commit ea2cb45

Please sign in to comment.