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 6 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
Original file line number Diff line number Diff line change
Expand Up @@ -55,26 +55,33 @@ class PushNotificationDataRunnable {
val receivingThreads = isCRTEnabled && !rootId.isNullOrEmpty()
val notificationData = Arguments.createMap()

var channel: ReadableMap? = null
var myTeam: ReadableMap? = null

if (!teamId.isNullOrEmpty()) {
val res = fetchTeamIfNeeded(db, serverUrl, teamId)
res.first?.let { notificationData.putMap("team", it) }
res.second?.let { notificationData.putMap("myTeam", it) }

myTeam = res.second
myTeam?.let { notificationData.putMap("myTeam", it) }
}

if (channelId != null && postId != null) {
val channelRes = fetchMyChannel(db, serverUrl, channelId, isCRTEnabled)
channelRes.first?.let { notificationData.putMap("channel", it) }

channel = channelRes.first
channel?.let { notificationData.putMap("channel", it) }
channelRes.second?.let { notificationData.putMap("myChannel", it) }
val loadedProfiles = channelRes.third

// Fetch categories if needed
if (!teamId.isNullOrEmpty() && notificationData.getMap("myTeam") != null) {
if (!teamId.isNullOrEmpty() && myTeam != null) {
// should load all categories
val res = fetchMyTeamCategories(db, serverUrl, teamId)
res?.let { notificationData.putMap("categories", it) }
} else if (notificationData.getMap("channel") != null) {
} else if (channel != null) {
// check if the channel is in the category for the team
val res = addToDefaultCategoryIfNeeded(db, notificationData.getMap("channel")!!)
val res = addToDefaultCategoryIfNeeded(db, channel)
res?.let { notificationData.putArray("categoryChannels", it) }
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.mattermost.helpers.database_extension

import android.util.Log
import com.facebook.react.bridge.Arguments
import com.facebook.react.bridge.NoSuchKeyException
import com.facebook.react.bridge.ReadableArray
Expand All @@ -17,7 +18,16 @@ internal fun insertThread(db: WMDatabase, thread: ReadableMap) {
val lastViewedAt = try { thread.getDouble("last_viewed_at") } catch (e: NoSuchKeyException) { 0 }
val unreadReplies = try { thread.getInt("unread_replies") } catch (e: NoSuchKeyException) { 0 }
val unreadMentions = try { thread.getInt("unread_mentions") } catch (e: NoSuchKeyException) { 0 }
val lastReplyAt = try { thread.getDouble("last_reply_at") } catch (e: NoSuchKeyException) { 0 }
val lastReplyAt = try {
var v = thread.getDouble("last_reply_at")
if (v == 0.0) {
val post = thread.getMap("post")
if (post != null) {
v = post.getDouble("create_at")
}
}
v
} catch (e: NoSuchKeyException) { 0 }
val replyCount = try { thread.getInt("reply_count") } catch (e: NoSuchKeyException) { 0 }

db.execute(
Expand All @@ -44,7 +54,16 @@ internal fun updateThread(db: WMDatabase, thread: ReadableMap, existingRecord: R
val lastViewedAt = try { thread.getDouble("last_viewed_at") } catch (e: NoSuchKeyException) { existingRecord.getDouble("last_viewed_at") }
val unreadReplies = try { thread.getInt("unread_replies") } catch (e: NoSuchKeyException) { existingRecord.getInt("unread_replies") }
val unreadMentions = try { thread.getInt("unread_mentions") } catch (e: NoSuchKeyException) { existingRecord.getInt("unread_mentions") }
val lastReplyAt = try { thread.getDouble("last_reply_at") } catch (e: NoSuchKeyException) { 0 }
val lastReplyAt = try {
var v = thread.getDouble("last_reply_at")
if (v == 0.0) {
val post = thread.getMap("post")
if (post != null) {
v = post.getDouble("create_at")
}
}
v
} catch (e: NoSuchKeyException) { 0 }
val replyCount = try { thread.getInt("reply_count") } catch (e: NoSuchKeyException) { 0 }

db.execute(
Expand Down Expand Up @@ -231,8 +250,31 @@ fun handleThreadInTeam(db: WMDatabase, thread: ReadableMap, teamId: String) {

fun handleTeamThreadsSync(db: WMDatabase, threadList: ArrayList<ReadableMap>, teamIds: ArrayList<String>) {
val sortedList = threadList.filter{ it.getBoolean("is_following") }
.sortedBy { it.getDouble("last_reply_at") }
.map { it.getDouble("last_reply_at") }
.sortedBy {
var v = it.getDouble("last_reply_at")
if (v == 0.0) {
val post = it.getMap("post");
if (post != null) {
v = post.getDouble("create_at")
} else {
Log.d("Database", "Trying to add a thread with no replies and no post")
}
}
v
}
.map {
var v = it.getDouble("last_reply_at")
if (v == 0.0) {
val post = it.getMap("post")
if (post != null) {
v = post.getDouble("create_at")
}
}
v
}
if (sortedList.isEmpty()) {
return;
}
val earliest = sortedList.first()
val latest = sortedList.last()

Expand Down
2 changes: 1 addition & 1 deletion app/actions/remote/entry/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ async function restDeferredAppEntryActions(
setTimeout(async () => {
if (chData?.channels?.length && chData.memberships?.length && initialTeamId) {
if (isCRTEnabled && initialTeamId) {
await syncTeamThreads(serverUrl, initialTeamId, false, false, groupLabel);
await syncTeamThreads(serverUrl, initialTeamId, {groupLabel});
}
fetchPostsForUnreadChannels(serverUrl, chData.channels, chData.memberships, initialChannelId, false, groupLabel);
}
Expand Down
62 changes: 46 additions & 16 deletions app/actions/remote/thread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ export const syncThreadsIfNeeded = async (

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

Expand All @@ -325,9 +325,22 @@ export const syncThreadsIfNeeded = async (
}
};

type SyncThreadOptions = {
excludeDirect?: boolean;
fetchOnly?: boolean;
refresh?: boolean;
groupLabel?: string;
}

export const syncTeamThreads = async (
serverUrl: string, teamId: string,
excludeDirect = false, fetchOnly = false, groupLabel?: string,
serverUrl: string,
teamId: string,
{
excludeDirect = false,
fetchOnly = false,
refresh = false,
groupLabel,
}: SyncThreadOptions = {},
) => {
try {
const {database, operator} = DatabaseManager.getServerDatabaseAndOperator(serverUrl);
Expand Down Expand Up @@ -368,39 +381,56 @@ export const syncTeamThreads = async (
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},
undefined,
undefined,
groupLabel,
);
const [allUnreadThreads, allNewThreads] = await Promise.all([
fetchThreads(
serverUrl,
teamId,
{unread: true, excludeDirect},
Direction.Down,
undefined,
groupLabel,
),
fetchThreads(
serverUrl,
teamId,
{deleted: true, since: refresh ? undefined : syncData.latest + 1, excludeDirect},
undefined,
1,
groupLabel,
),
]);

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
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,11 @@ extension PushNotification {
var copy = threads[index]
copy.unreadMentions = thread.unreadMentions
copy.unreadReplies = thread.unreadReplies
copy.lastReplyAt = thread.lastReplyAt
if (thread.lastReplyAt == 0) {
copy.lastReplyAt = thread.post?.createAt ?? 0
} else {
copy.lastReplyAt = thread.lastReplyAt
}
copy.lastViewedAt = thread.lastViewedAt
notificationData.threads?[index] = copy
}
Expand Down
Loading