From 9fcc6e580dd21b56d0cce6f15fb5a06d4aed9d28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Tue, 10 Jun 2025 15:30:58 +0200 Subject: [PATCH] Fix GM/DM unread threads highlighting all teams as unreads (#8905) * Fix GM/DM unread threads highlighting all teams as unreads * Use options object instead of boolean flags --------- Co-authored-by: Mattermost Build --- app/actions/local/thread.ts | 2 +- app/components/threads_button/index.ts | 4 +- app/database/subscription/unreads.ts | 10 +- app/queries/servers/team.test.ts | 142 +++++++++++++++++- app/queries/servers/team.ts | 6 +- app/queries/servers/thread.ts | 52 ++++++- app/screens/global_threads/index.tsx | 2 +- .../global_threads/threads_list/index.ts | 2 +- .../categories/unreads/index.ts | 4 +- 9 files changed, 202 insertions(+), 22 deletions(-) diff --git a/app/actions/local/thread.ts b/app/actions/local/thread.ts index 76b5344cf..f465d3f8a 100644 --- a/app/actions/local/thread.ts +++ b/app/actions/local/thread.ts @@ -249,7 +249,7 @@ export async function processReceivedThreads(serverUrl: string, threads: Thread[ export async function markTeamThreadsAsRead(serverUrl: string, teamId: string, prepareRecordsOnly = false) { try { const {database, operator} = DatabaseManager.getServerDatabaseAndOperator(serverUrl); - const threads = await queryThreadsInTeam(database, teamId, true, true, true).fetch(); + const threads = await queryThreadsInTeam(database, teamId, {onlyUnreads: true, hasReplies: true, isFollowing: true}).fetch(); const models = threads.map((thread) => thread.prepareUpdate((record) => { record.unreadMentions = 0; record.unreadReplies = 0; diff --git a/app/components/threads_button/index.ts b/app/components/threads_button/index.ts index f42ebb906..60bee78ec 100644 --- a/app/components/threads_button/index.ts +++ b/app/components/threads_button/index.ts @@ -6,7 +6,7 @@ import {switchMap} from 'rxjs/operators'; import {observeCurrentChannelId, observeCurrentTeamId} from '@queries/servers/system'; import {observeTeamLastChannelId} from '@queries/servers/team'; -import {observeUnreadsAndMentionsInTeam} from '@queries/servers/thread'; +import {observeUnreadsAndMentions} from '@queries/servers/thread'; import ThreadsButton from './threads_button'; @@ -24,7 +24,7 @@ const enhanced = withObservables([], ({database}: WithDatabaseArgs) => { ), unreadsAndMentions: currentTeamId.pipe( switchMap( - (teamId) => observeUnreadsAndMentionsInTeam(database, teamId), + (teamId) => observeUnreadsAndMentions(database, {teamId, includeDmGm: true}), ), ), }; diff --git a/app/database/subscription/unreads.ts b/app/database/subscription/unreads.ts index 469004caf..f64a40947 100644 --- a/app/database/subscription/unreads.ts +++ b/app/database/subscription/unreads.ts @@ -9,7 +9,7 @@ import {MM_TABLES} from '@constants/database'; import DatabaseManager from '@database/manager'; import {observeAllMyChannelNotifyProps} from '@queries/servers/channel'; import {queryMyTeams} from '@queries/servers/team'; -import {getIsCRTEnabled, observeThreadMentionCount, queryThreads, observeUnreadsAndMentionsInTeam} from '@queries/servers/thread'; +import {getIsCRTEnabled, observeThreadMentionCount, queryThreads, observeUnreadsAndMentions} from '@queries/servers/thread'; import type MyChannelModel from '@typings/database/models/servers/my_channel'; @@ -41,7 +41,7 @@ export const subscribeServerUnreadAndMentions = (serverUrl: string, observer: Un observeWithColumns(['is_unread', 'mentions_count']). pipe( combineLatestWith(observeAllMyChannelNotifyProps(server.database)), - combineLatestWith(observeUnreadsAndMentionsInTeam(server.database, undefined, true)), + combineLatestWith(observeUnreadsAndMentions(server.database, {includeDmGm: true})), map$(([[myChannels, settings], {unreads, mentions}]) => ({myChannels, settings, threadUnreads: unreads, threadMentionCount: mentions})), ). subscribe(observer); @@ -60,7 +60,7 @@ export const subscribeMentionsByServer = (serverUrl: string, observer: ServerUnr query(Q.on(CHANNEL, Q.where('delete_at', Q.eq(0)))). observeWithColumns(['mentions_count']). pipe( - combineLatestWith(observeThreadMentionCount(server.database, undefined, true)), + combineLatestWith(observeThreadMentionCount(server.database, {includeDmGm: true})), map$(([myChannels, threadMentionCount]) => ({myChannels, threadMentionCount})), ). subscribe(observer.bind(undefined, serverUrl)); @@ -79,7 +79,7 @@ export const subscribeUnreadAndMentionsByServer = (serverUrl: string, observer: observeWithColumns(['mentions_count', 'is_unread']). pipe( combineLatestWith(observeAllMyChannelNotifyProps(server.database)), - combineLatestWith(observeUnreadsAndMentionsInTeam(server.database, undefined, true)), + combineLatestWith(observeUnreadsAndMentions(server.database, {includeDmGm: true})), map$(([[myChannels, settings], {unreads, mentions}]) => ({myChannels, settings, threadUnreads: unreads, threadMentionCount: mentions})), ). subscribe(observer.bind(undefined, serverUrl)); @@ -108,7 +108,7 @@ export const getTotalMentionsForServer = async (serverUrl: string) => { let includeDmGm = true; const myTeamIds = await queryMyTeams(database).fetchIds(); for await (const teamId of myTeamIds) { - const threads = await queryThreads(database, teamId, false, includeDmGm).extend( + const threads = await queryThreads(database, {teamId, includeDmGm}).extend( Q.where('unread_mentions', Q.gt(0)), ).fetch(); includeDmGm = false; diff --git a/app/queries/servers/team.test.ts b/app/queries/servers/team.test.ts index 6fa9939ee..381817afb 100644 --- a/app/queries/servers/team.test.ts +++ b/app/queries/servers/team.test.ts @@ -4,7 +4,7 @@ /* eslint-disable max-lines */ import {processReceivedThreads} from '@actions/local/thread'; -import {Config, Preferences, Screens} from '@constants'; +import {ActionType, Config, Preferences, Screens} from '@constants'; import {SYSTEM_IDENTIFIERS} from '@constants/database'; import DatabaseManager from '@database/manager'; import TestHelper from '@test/test_helper'; @@ -1219,6 +1219,146 @@ describe('Team Queries', () => { await TestHelper.wait(waitTime); expect(subscriptionNext).not.toHaveBeenCalled(); }); + + it('should not consider the team unread when a gm or a dm are unread', async () => { + const subscriptionNext = jest.fn(); + const notify_props = {mark_unread: 'all' as const}; + const result = observeIsTeamUnread(database, teamId); + + result.subscribe({next: subscriptionNext}); + await TestHelper.wait(waitTime); + + // The subscription always return the first value + expect(subscriptionNext).toHaveBeenCalledWith(false); + subscriptionNext.mockClear(); + + // Setup DM channel with unreads + let channelModels = (await Promise.all((await prepareAllMyChannels( + operator, + [TestHelper.fakeChannel({id: 'dm1', team_id: '', type: 'D', total_msg_count: 30})], + [TestHelper.fakeMyChannel({ + channel_id: 'dm1', + user_id: userId, + notify_props, + msg_count: 20, + })], + false, + )))).flat(); + await operator.batchRecords([...channelModels], 'test'); + + await TestHelper.wait(waitTime); + expect(subscriptionNext).not.toHaveBeenCalled(); + + // Setup GM channel with unreads + channelModels = (await Promise.all((await prepareAllMyChannels( + operator, + [TestHelper.fakeChannel({id: 'gm1', team_id: '', type: 'G', total_msg_count: 30})], + [TestHelper.fakeMyChannel({ + channel_id: 'gm1', + user_id: userId, + notify_props, + msg_count: 20, + })], + false, + )))).flat(); + await operator.batchRecords([...channelModels], 'test'); + + await TestHelper.wait(waitTime); + expect(subscriptionNext).not.toHaveBeenCalled(); + }); + + it('should not consider the team unread if a thread in a dm or a gm has unread messages', async () => { + const subscriptionNext = jest.fn(); + const notify_props = {mark_unread: 'all' as const}; + const result = observeIsTeamUnread(database, teamId); + + result.subscribe({next: subscriptionNext}); + await TestHelper.wait(waitTime); + + // The subscription always return the first value + expect(subscriptionNext).toHaveBeenCalledWith(false); + subscriptionNext.mockClear(); + + // Setup DM channel with thread + let channelModels = (await Promise.all((await prepareAllMyChannels( + operator, + [TestHelper.fakeChannel({id: 'dm1', team_id: '', type: 'D', total_msg_count: 20})], + [TestHelper.fakeMyChannel({ + channel_id: 'dm1', + user_id: userId, + notify_props, + msg_count: 20, + })], + false, + )))).flat(); + + const dmPost = TestHelper.fakePost({id: 'dm_thread', channel_id: 'dm1'}); + + const dmThreads = [{ + ...TestHelper.fakeThread({ + id: 'dm_thread', + participants: undefined, + reply_count: 2, + last_reply_at: 123, + is_following: true, + unread_replies: 2, + unread_mentions: 1, + }), + lastFetchedAt: 0, + }]; + + const dmModels = await operator.handleThreads({threads: dmThreads, prepareRecordsOnly: false}); + let postModels = await operator.handlePosts({ + actionType: ActionType.POSTS.RECEIVED_IN_CHANNEL, + order: [], + posts: [dmPost], + prepareRecordsOnly: true, + }); + await operator.batchRecords([...channelModels, ...dmModels, ...postModels], 'test'); + + await TestHelper.wait(waitTime); + expect(subscriptionNext).not.toHaveBeenCalled(); + + // Setup GM channel with thread + channelModels = (await Promise.all((await prepareAllMyChannels( + operator, + [TestHelper.fakeChannel({id: 'gm1', team_id: '', type: 'G', total_msg_count: 20})], + [TestHelper.fakeMyChannel({ + channel_id: 'gm1', + user_id: userId, + notify_props, + msg_count: 20, + })], + false, + )))).flat(); + + const gmPost = TestHelper.fakePost({id: 'gm_thread', channel_id: 'gm1'}); + + const gmThreads = [{ + ...TestHelper.fakeThread({ + id: 'gm_thread', + participants: undefined, + reply_count: 3, + last_reply_at: 123, + is_following: true, + unread_replies: 3, + unread_mentions: 2, + }), + lastFetchedAt: 123, + }]; + + const gmModels = await operator.handleThreads({threads: gmThreads, prepareRecordsOnly: false}); + postModels = await operator.handlePosts({ + actionType: ActionType.POSTS.RECEIVED_IN_CHANNEL, + order: [], + posts: [gmPost], + prepareRecordsOnly: true, + }); + await operator.batchRecords([...channelModels, ...gmModels, ...postModels], 'test'); + + await TestHelper.wait(waitTime); + expect(subscriptionNext).not.toHaveBeenCalled(); + }); }); describe('observeSortedJoinedTeams', () => { diff --git a/app/queries/servers/team.ts b/app/queries/servers/team.ts index 218573b20..b90055d1d 100644 --- a/app/queries/servers/team.ts +++ b/app/queries/servers/team.ts @@ -14,7 +14,7 @@ import {prepareDeleteCategory} from './categories'; import {prepareDeleteChannel, getDefaultChannelForTeam, observeMyChannelMentionCount, observeMyChannelUnreads} from './channel'; import {queryPreferencesByCategoryAndName} from './preference'; import {patchTeamHistory, getConfig, getTeamHistory, observeCurrentTeamId, getCurrentTeamId} from './system'; -import {observeThreadMentionCount, observeUnreadsAndMentionsInTeam} from './thread'; +import {observeThreadMentionCount, observeUnreadsAndMentions} from './thread'; import {getCurrentUser} from './user'; import type {MyChannelModel} from '@database/models/server'; @@ -416,7 +416,7 @@ export const observeCurrentTeam = (database: Database) => { export function observeMentionCount(database: Database, teamId?: string, includeDmGm?: boolean): Observable { const channelMentionCountObservable = observeMyChannelMentionCount(database, teamId); - const threadMentionCountObservable = observeThreadMentionCount(database, teamId, includeDmGm); + const threadMentionCountObservable = observeThreadMentionCount(database, {teamId, includeDmGm}); return channelMentionCountObservable.pipe( combineLatestWith(threadMentionCountObservable), @@ -427,7 +427,7 @@ export function observeMentionCount(database: Database, teamId?: string, include export function observeIsTeamUnread(database: Database, teamId: string): Observable { const channelUnreads = observeMyChannelUnreads(database, teamId); - const threadsUnreadsAndMentions = observeUnreadsAndMentionsInTeam(database, teamId); + const threadsUnreadsAndMentions = observeUnreadsAndMentions(database, {teamId}); return channelUnreads.pipe( combineLatestWith(threadsUnreadsAndMentions), diff --git a/app/queries/servers/thread.ts b/app/queries/servers/thread.ts index d011171af..748732007 100644 --- a/app/queries/servers/thread.ts +++ b/app/queries/servers/thread.ts @@ -87,8 +87,16 @@ export const observeTeamIdByThread = (database: Database, thread: ThreadModel) = return observeTeamIdByThreadId(database, thread.id); }; -export const observeUnreadsAndMentionsInTeam = (database: Database, teamId?: string, includeDmGm?: boolean): Observable<{unreads: boolean; mentions: number}> => { - const observeThreads = () => queryThreads(database, teamId, true, includeDmGm). +type ObserveUnreadsAndMentionsOptions = { + teamId?: string; + includeDmGm?: boolean; +}; + +export const observeUnreadsAndMentions = (database: Database, { + teamId, + includeDmGm, +}: ObserveUnreadsAndMentionsOptions): Observable<{unreads: boolean; mentions: number}> => { + const observeThreads = () => queryThreads(database, {teamId, onlyUnreads: true, includeDmGm}). observeWithColumns(['unread_replies', 'unread_mentions']). pipe( switchMap((threads) => { @@ -154,7 +162,21 @@ export const prepareThreadsFromReceivedPosts = async (operator: ServerDataOperat return models; }; -export const queryThreadsInTeam = (database: Database, teamId: string, onlyUnreads?: boolean, hasReplies?: boolean, isFollowing?: boolean, sort?: boolean, earliest?: number): Query => { +type QueryThreadsInTeamOptions = { + onlyUnreads?: boolean; + hasReplies?: boolean; + isFollowing?: boolean; + sort?: boolean; + earliest?: number; +}; + +export const queryThreadsInTeam = (database: Database, teamId: string, { + onlyUnreads, + hasReplies, + isFollowing, + sort, + earliest, +}: QueryThreadsInTeamOptions): Query => { const query: Q.Clause[] = [ Q.experimentalNestedJoin(POST, CHANNEL), Q.on(POST, Q.on(CHANNEL, Q.where('delete_at', 0))), @@ -193,14 +215,32 @@ export const queryTeamThreadsSync = (database: Database, teamId: string) => { ); }; -export function observeThreadMentionCount(database: Database, teamId?: string, includeDmGm?: boolean): Observable { - return observeUnreadsAndMentionsInTeam(database, teamId, includeDmGm).pipe( +type ObserveThreadMentionCountOptions = { + teamId?: string; + includeDmGm?: boolean; +}; + +export function observeThreadMentionCount(database: Database, { + teamId, + includeDmGm, +}: ObserveThreadMentionCountOptions): Observable { + return observeUnreadsAndMentions(database, {teamId, includeDmGm}).pipe( switchMap(({mentions}) => of$(mentions)), distinctUntilChanged(), ); } -export const queryThreads = (database: Database, teamId?: string, onlyUnreads = false, includeDmGm = true): Query => { +type QueryThreadsOptions = { + teamId?: string; + onlyUnreads?: boolean; + includeDmGm?: boolean; +}; + +export const queryThreads = (database: Database, { + teamId, + onlyUnreads, + includeDmGm, +}: QueryThreadsOptions): Query => { const query: Q.Clause[] = [ Q.where('is_following', true), Q.where('reply_count', Q.gt(0)), diff --git a/app/screens/global_threads/index.tsx b/app/screens/global_threads/index.tsx index 4ba95d76e..53d37b411 100644 --- a/app/screens/global_threads/index.tsx +++ b/app/screens/global_threads/index.tsx @@ -14,7 +14,7 @@ import type {WithDatabaseArgs} from '@typings/database/database'; const enhanced = withObservables([], ({database}: WithDatabaseArgs) => { const teamId = observeCurrentTeamId(database); - const unreadsCount = teamId.pipe(switchMap((id) => queryThreadsInTeam(database, id, true, true, true).observeCount(false))); + const unreadsCount = teamId.pipe(switchMap((id) => queryThreadsInTeam(database, id, {onlyUnreads: true, hasReplies: true, isFollowing: true}).observeCount(false))); const hasUnreads = unreadsCount.pipe( switchMap((count) => of$(count > 0)), distinctUntilChanged(), diff --git a/app/screens/global_threads/threads_list/index.ts b/app/screens/global_threads/threads_list/index.ts index 19ee67c06..050e933f3 100644 --- a/app/screens/global_threads/threads_list/index.ts +++ b/app/screens/global_threads/threads_list/index.ts @@ -31,7 +31,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, true, true, true, earliest).observe(); + return queryThreadsInTeam(database, teamId, {onlyUnreads: getOnlyUnreads, hasReplies: true, isFollowing: true, sort: true, earliest}).observe(); }), ), }; diff --git a/app/screens/home/channel_list/categories_list/categories/unreads/index.ts b/app/screens/home/channel_list/categories_list/categories/unreads/index.ts index fd77a4a88..52221246e 100644 --- a/app/screens/home/channel_list/categories_list/categories/unreads/index.ts +++ b/app/screens/home/channel_list/categories_list/categories/unreads/index.ts @@ -11,7 +11,7 @@ import {filterAndSortMyChannels, makeChannelsMap} from '@helpers/database'; import {getChannelById, observeChannelsByLastPostAt, observeNotifyPropsByChannels, queryMyChannelUnreads} from '@queries/servers/channel'; import {querySidebarPreferences} from '@queries/servers/preference'; import {observeLastUnreadChannelId} from '@queries/servers/system'; -import {observeUnreadsAndMentionsInTeam} from '@queries/servers/thread'; +import {observeUnreadsAndMentions} from '@queries/servers/thread'; import UnreadCategories from './unreads'; @@ -62,7 +62,7 @@ const enhanced = withObservables(['currentTeamId', 'isTablet', 'onlyUnreads'], ( } return of$([]); })); - const unreadThreads = observeUnreadsAndMentionsInTeam(database, currentTeamId, true); + const unreadThreads = observeUnreadsAndMentions(database, {teamId: currentTeamId, includeDmGm: true}); return { unreadChannels,