Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add some improvements to thread handling #8407

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 43 additions & 12 deletions app/actions/remote/thread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ export const syncThreadsIfNeeded = async (serverUrl: string, isCRTEnabled: boole

if (teams?.length) {
for (const team of teams) {
promises.push(syncTeamThreads(serverUrl, team.id, true, true));
promises.push(syncTeamThreads(serverUrl, team.id, {excludeDirect: true, fetchOnly: true}));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious - where do we sync the DM/GM threads?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other calls to syncTeamThreads, where we don't pass the excludeDirect option. Mainly on startup / reconnect, if I am not mistaken.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exclude direct is false also for the initial team, this will fetch the threads on DM's and GM's that are returned for every team if you don't set the flag as true

}
}

Expand All @@ -317,7 +317,21 @@ export const syncThreadsIfNeeded = async (serverUrl: string, isCRTEnabled: boole
}
};

export const syncTeamThreads = async (serverUrl: string, teamId: string, excludeDirect = false, fetchOnly = false) => {
type SyncThreadOptions = {
excludeDirect?: boolean;
fetchOnly?: boolean;
refresh?: boolean;
}

export const syncTeamThreads = async (
serverUrl: string,
teamId: string,
{
excludeDirect = false,
fetchOnly = false,
refresh = false,
}: SyncThreadOptions = {},
) => {
try {
const {database, operator} = DatabaseManager.getServerDatabaseAndOperator(serverUrl);
const syncData = await getTeamThreadsSyncData(database, teamId);
Expand Down Expand Up @@ -354,36 +368,53 @@ export const syncTeamThreads = async (serverUrl: string, teamId: string, exclude
return {error: allUnreadThreads.error || latestThreads.error};
}

const dedupe = new Set(latestThreads.threads?.map((t) => t.id));

if (latestThreads.threads?.length) {
// We are fetching the threads for the first time. We get "latest" and "earliest" values.
// At this point we may receive threads without replies, so we also check the post.create_at timestamp.
const {earliestThread, latestThread} = getThreadsListEdges(latestThreads.threads);
syncDataUpdate.latest = latestThread.last_reply_at;
syncDataUpdate.earliest = earliestThread.last_reply_at;
syncDataUpdate.latest = latestThread.last_reply_at || latestThread.post.create_at;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not sync data on post edits? Or is that done per post?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thread information in the database only includes the post id, so updates in the post should not affect this piece of data (it affects the post itself, which is done somewhere else).

syncDataUpdate.earliest = earliestThread.last_reply_at || earliestThread.post.create_at;

threads.push(...latestThreads.threads);
}
if (allUnreadThreads.threads?.length) {
const dedupe = new Set(latestThreads.threads?.map((t) => t.id));
const unread = allUnreadThreads.threads.filter((u) => !dedupe.has(u.id));
threads.push(...unread);
}
} else {
const allNewThreads = await fetchThreads(
serverUrl,
teamId,
{deleted: true, since: syncData.latest + 1, excludeDirect},
);
const [allUnreadThreads, allNewThreads] = await Promise.all([
fetchThreads(
serverUrl,
teamId,
{unread: true, excludeDirect},
Direction.Down,
),
fetchThreads(
serverUrl,
teamId,
{deleted: true, since: refresh ? undefined : syncData.latest + 1, excludeDirect},
undefined,
1,
),
]);

if (allNewThreads.error) {
return {error: allNewThreads.error};
}
if (allNewThreads.threads?.length) {
// As we are syncing, we get all new threads and we will update the "latest" value.
const {latestThread} = getThreadsListEdges(allNewThreads.threads);
syncDataUpdate.latest = latestThread.last_reply_at;
const latestDate = latestThread.last_reply_at || latestThread.post.create_at;
syncDataUpdate.latest = Math.max(syncData.latest, latestDate);

threads.push(...allNewThreads.threads);
}
if (allUnreadThreads.threads?.length) {
const dedupe = new Set(allNewThreads.threads?.map((t) => t.id));
const unread = allUnreadThreads.threads.filter((u) => !dedupe.has(u.id));
threads.push(...unread);
}
}

const models: Model[] = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ export const transformThreadRecord = ({action, database, value}: TransformerArgs
const fieldsMapper = (thread: ThreadModel) => {
thread._raw.id = isCreateAction ? (raw?.id ?? thread.id) : record.id;

// When post is individually fetched, we get last_reply_at as 0, so we use the record's value
thread.lastReplyAt = raw.last_reply_at || record?.lastReplyAt;
// When post is individually fetched, we get last_reply_at as 0, so we use the record's value.
// If there is no reply at, we default to the post's create_at
thread.lastReplyAt = raw.last_reply_at || record?.lastReplyAt || raw.post.create_at;

thread.lastViewedAt = raw.last_viewed_at ?? record?.lastViewedAt ?? 0;
thread.replyCount = raw.reply_count;
Expand Down
2 changes: 1 addition & 1 deletion app/queries/servers/thread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export const prepareThreadsFromReceivedPosts = async (operator: ServerDataOperat
id: post.id,
participants: post.participants,
reply_count: post.reply_count,
last_reply_at: post.last_reply_at,
last_reply_at: post.last_reply_at || post.create_at,
is_following: post.is_following,
lastFetchedAt: post.create_at,
} as ThreadWithLastFetchedAt);
Expand Down
2 changes: 1 addition & 1 deletion app/screens/global_threads/threads_list/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const enhanced = withObservables(['tab', 'teamId'], ({database, tab, teamId}: Pr
threads: teamThreadsSyncObserver.pipe(
switchMap((teamThreadsSync) => {
const earliest = tab === 'all' ? teamThreadsSync?.[0]?.earliest : 0;
return queryThreadsInTeam(database, teamId, getOnlyUnreads, false, true, true, earliest).observe();
return queryThreadsInTeam(database, teamId, getOnlyUnreads, true, true, true, earliest).observe();
}),
),
};
Expand Down
2 changes: 1 addition & 1 deletion app/screens/global_threads/threads_list/threads_list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ const ThreadsList = ({
const handleRefresh = useCallback(() => {
setRefreshing(true);

syncTeamThreads(serverUrl, teamId).finally(() => {
syncTeamThreads(serverUrl, teamId, {refresh: true}).finally(() => {
setRefreshing(false);
});
}, [serverUrl, teamId]);
Expand Down
4 changes: 3 additions & 1 deletion app/utils/thread/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ export function processIsCRTEnabled(preferences: PreferenceModel[]|PreferenceTyp
export const getThreadsListEdges = (threads: Thread[]) => {
// Sort a clone of 'threads' array by last_reply_at
const sortedThreads = [...threads].sort((a, b) => {
return a.last_reply_at - b.last_reply_at;
const aDate = a.last_reply_at || a.post.create_at;
const bDate = b.last_reply_at || b.post.create_at;
return aDate - bDate;
});

const earliestThread = sortedThreads[0];
Expand Down