diff --git a/app/actions/remote/post.test.ts b/app/actions/remote/post.test.ts index adc5bc045..9b148ddab 100644 --- a/app/actions/remote/post.test.ts +++ b/app/actions/remote/post.test.ts @@ -132,6 +132,16 @@ jest.mock('@actions/local/reactions', () => { }; }); +let mockFetchChannelStats: jest.Mock; +jest.mock('@actions/remote/channel', () => { + const original = jest.requireActual('@actions/remote/channel'); + mockFetchChannelStats = jest.fn(() => Promise.resolve({})); + return { + ...original, + fetchChannelStats: mockFetchChannelStats, + }; +}); + beforeAll(() => { // @ts-ignore NetworkManager.getClient = () => mockClient; @@ -140,6 +150,7 @@ beforeAll(() => { beforeEach(async () => { await DatabaseManager.init([serverUrl]); operator = DatabaseManager.serverDatabases[serverUrl]!.operator; + mockFetchChannelStats.mockClear(); }); afterEach(async () => { @@ -502,6 +513,119 @@ describe('create, update & delete posts', () => { expect(file2After).toBeUndefined(); }); + it('editPost - should call fetchChannelStats when files are removed', async () => { + // Setup post with files + await operator.handlePosts({ + actionType: ActionType.POSTS.RECEIVED_IN_CHANNEL, + order: [post1.id], + posts: [post1], + prepareRecordsOnly: false, + }); + + const testFiles: FileInfo[] = [ + TestHelper.fakeFileInfo({ + id: 'file-1', + post_id: post1.id, + name: 'test-file-1.jpg', + extension: 'jpg', + size: 1024, + mime_type: 'image/jpeg', + user_id: user1.id, + }), + ]; + + await operator.handleFiles({ + files: testFiles, + prepareRecordsOnly: false, + }); + + mockFetchChannelStats.mockClear(); + + // Edit post to remove files + const result = await editPost(serverUrl, post1.id, 'new message', [], ['file-1']); + + expect(result).toBeDefined(); + expect(result.error).toBeUndefined(); + expect(mockFetchChannelStats).toHaveBeenCalledWith(serverUrl, post1.channel_id); + expect(mockFetchChannelStats).toHaveBeenCalledTimes(1); + }); + + it('editPost - should call fetchChannelStats when files are added', async () => { + // Setup post without files + await operator.handlePosts({ + actionType: ActionType.POSTS.RECEIVED_IN_CHANNEL, + order: [post1.id], + posts: [post1], + prepareRecordsOnly: false, + }); + + mockFetchChannelStats.mockClear(); + + // Edit post to add files + const result = await editPost(serverUrl, post1.id, 'new message', ['new-file-1'], []); + + expect(result).toBeDefined(); + expect(result.error).toBeUndefined(); + expect(mockFetchChannelStats).toHaveBeenCalledWith(serverUrl, post1.channel_id); + expect(mockFetchChannelStats).toHaveBeenCalledTimes(1); + }); + + it('editPost - should NOT call fetchChannelStats when no files change', async () => { + // Setup post without files + await operator.handlePosts({ + actionType: ActionType.POSTS.RECEIVED_IN_CHANNEL, + order: [post1.id], + posts: [post1], + prepareRecordsOnly: false, + }); + + mockFetchChannelStats.mockClear(); + + // Edit post without changing files + const result = await editPost(serverUrl, post1.id, 'new message', [], []); + + expect(result).toBeDefined(); + expect(result.error).toBeUndefined(); + expect(mockFetchChannelStats).not.toHaveBeenCalled(); + }); + + it('editPost - should call fetchChannelStats when file IDs change', async () => { + // Setup post with files + await operator.handlePosts({ + actionType: ActionType.POSTS.RECEIVED_IN_CHANNEL, + order: [post1.id], + posts: [post1], + prepareRecordsOnly: false, + }); + + const testFiles: FileInfo[] = [ + TestHelper.fakeFileInfo({ + id: 'file-1', + post_id: post1.id, + name: 'test-file-1.jpg', + extension: 'jpg', + size: 1024, + mime_type: 'image/jpeg', + user_id: user1.id, + }), + ]; + + await operator.handleFiles({ + files: testFiles, + prepareRecordsOnly: false, + }); + + mockFetchChannelStats.mockClear(); + + // Edit post to replace files (remove one, add another) + const result = await editPost(serverUrl, post1.id, 'new message', ['new-file-2'], ['file-1']); + + expect(result).toBeDefined(); + expect(result.error).toBeUndefined(); + expect(mockFetchChannelStats).toHaveBeenCalledWith(serverUrl, post1.channel_id); + expect(mockFetchChannelStats).toHaveBeenCalledTimes(1); + }); + it('acknowledgePost - handle error', async () => { mockClient.deletePost.mockImplementationOnce(jest.fn(throwFunc)); const result = await acknowledgePost('foo', ''); diff --git a/app/actions/remote/post.ts b/app/actions/remote/post.ts index bdb2b812f..f0b0bca83 100644 --- a/app/actions/remote/post.ts +++ b/app/actions/remote/post.ts @@ -8,6 +8,7 @@ import {markChannelAsUnread, updateLastPostAt} from '@actions/local/channel'; import {addPostAcknowledgement, removePost, removePostAcknowledgement, storePostsForChannel} from '@actions/local/post'; import {addRecentReaction} from '@actions/local/reactions'; import {createThreadFromNewPost} from '@actions/local/thread'; +import {fetchChannelStats} from '@actions/remote/channel'; import {ActionType, General, Post, ServerErrors} from '@constants'; import DatabaseManager from '@database/manager'; import {filterPostsInOrderedArray} from '@helpers/api/post'; @@ -15,7 +16,7 @@ import {getNeededAtMentionedUsernames} from '@helpers/api/user'; import NetworkManager from '@managers/network_manager'; import {getMyChannel, prepareMissingChannelsForAllTeams, queryAllMyChannel} from '@queries/servers/channel'; import {queryAllCustomEmojis} from '@queries/servers/custom_emoji'; -import {getFilesByIds} from '@queries/servers/file'; +import {getFilesByIds, queryFilesForPost} from '@queries/servers/file'; import {getPostById, getRecentPostsInChannel} from '@queries/servers/post'; import {getCurrentUserId} from '@queries/servers/system'; import {getIsCRTEnabled, prepareThreadsFromReceivedPosts} from '@queries/servers/thread'; @@ -24,6 +25,7 @@ import EphemeralStore from '@store/ephemeral_store'; import {setFetchingThreadState} from '@store/fetching_thread_store'; import {getValidEmojis, matchEmoticons} from '@utils/emoji/helpers'; import {getFullErrorMessage, isServerError} from '@utils/errors'; +import {hasArrayChanged} from '@utils/helpers'; import {logDebug, logError} from '@utils/log'; import {processPostsFetched} from '@utils/post'; import {getPostIdsForCombinedUserActivityPost} from '@utils/post_list'; @@ -915,6 +917,9 @@ export const editPost = async (serverUrl: string, postId: string, postMessage: s const post = await getPostById(database, postId); if (post) { + const originalFiles = await queryFilesForPost(database, postId).fetch(); + const originalFileIds = originalFiles.map((f) => f.id); + const {update_at, edit_at, message: updatedMessage, message_source} = await client.patchPost({message: postMessage, id: postId, file_ids}); await database.write(async () => { await post.update((p) => { @@ -935,6 +940,13 @@ export const editPost = async (serverUrl: string, postId: string, postMessage: s await operator.batchRecords(fileDeleteModels, 'delete files'); } } + + const filesChanged = hasArrayChanged(originalFileIds, file_ids); + + if (filesChanged && post.channelId) { + const channelId = post.channelId; + fetchChannelStats(serverUrl, channelId); + } } return {post}; } catch (error) { diff --git a/app/actions/websocket/posts.ts b/app/actions/websocket/posts.ts index a0459404d..b3df039c5 100644 --- a/app/actions/websocket/posts.ts +++ b/app/actions/websocket/posts.ts @@ -20,7 +20,7 @@ import {getCurrentChannelId, getCurrentTeamId, getCurrentUserId} from '@queries/ import {getIsCRTEnabled} from '@queries/servers/thread'; import EphemeralStore from '@store/ephemeral_store'; import NavigationStore from '@store/navigation_store'; -import {isTablet} from '@utils/helpers'; +import {hasArrayChanged, isTablet} from '@utils/helpers'; import {isFromWebhook, isSystemMessage, shouldIgnorePost} from '@utils/post'; import type {Model} from '@nozbe/watermelondb'; @@ -223,7 +223,11 @@ export async function handlePostEdited(serverUrl: string, msg: WebSocketMessage) post.create_at = oldPost.createAt; } - if (oldPost.isPinned !== post.is_pinned) { + const oldFileIds = oldPost.metadata?.files?.map((f) => f.id).filter((id): id is string => Boolean(id)) || []; + const newFileIds = post.file_ids || []; + const filesChanged = hasArrayChanged(oldFileIds, newFileIds); + + if (oldPost.isPinned !== post.is_pinned || filesChanged) { fetchChannelStats(serverUrl, post.channel_id); } @@ -269,6 +273,12 @@ export async function handlePostDeleted(serverUrl: string, msg: WebSocketMessage models.push(deleteModel); } + // Check if the deleted post had files to refresh channel stats + const hadFiles = (oldPost.metadata?.files?.length || 0) > 0; + if (hadFiles) { + fetchChannelStats(serverUrl, post.channel_id); + } + // update thread when a reply is deleted and CRT is enabled if (post.root_id) { const isCRTEnabled = await getIsCRTEnabled(database); diff --git a/app/utils/helpers.test.ts b/app/utils/helpers.test.ts index c9cda5e36..060a03c6c 100644 --- a/app/utils/helpers.test.ts +++ b/app/utils/helpers.test.ts @@ -22,6 +22,7 @@ import { hasTrailingSpaces, isMainActivity, areBothStringArraysEqual, + hasArrayChanged, } from './helpers'; jest.mock('@mattermost/rnshare', () => ({ @@ -310,4 +311,39 @@ describe('Helpers', () => { expect(result).toBe(false); }); }); + + describe('hasArrayChanged', () => { + test('should return true for arrays with different lengths', () => { + expect(hasArrayChanged(['a', 'b'], ['a', 'b', 'c'])).toBe(true); + expect(hasArrayChanged(['a', 'b', 'c'], ['a', 'b'])).toBe(true); + }); + + test('should return true for arrays with same length but different elements', () => { + expect(hasArrayChanged(['a', 'b'], ['a', 'c'])).toBe(true); + expect(hasArrayChanged(['x', 'y'], ['a', 'b'])).toBe(true); + }); + + test('should return false for arrays with same elements (same order)', () => { + expect(hasArrayChanged(['a', 'b', 'c'], ['a', 'b', 'c'])).toBe(false); + }); + + test('should return false for arrays with same elements (different order)', () => { + expect(hasArrayChanged(['a', 'b', 'c'], ['c', 'b', 'a'])).toBe(false); + expect(hasArrayChanged(['x', 'y', 'z'], ['z', 'x', 'y'])).toBe(false); + }); + + test('should return false for empty arrays', () => { + expect(hasArrayChanged([], [])).toBe(false); + }); + + test('should return true when one array is empty', () => { + expect(hasArrayChanged([], ['a'])).toBe(true); + expect(hasArrayChanged(['a'], [])).toBe(true); + }); + + test('should handle duplicate elements correctly', () => { + expect(hasArrayChanged(['a', 'a', 'b'], ['a', 'b', 'a'])).toBe(false); + expect(hasArrayChanged(['a', 'a'], ['a', 'b'])).toBe(true); + }); + }); }); diff --git a/app/utils/helpers.ts b/app/utils/helpers.ts index 1f5acc7ad..683c04a17 100644 --- a/app/utils/helpers.ts +++ b/app/utils/helpers.ts @@ -206,3 +206,40 @@ export function areBothStringArraysEqual(a: string[], b: string[]) { return areBothEqual; } + +/** + * Efficiently compares two arrays to check if they have different elements. + * Uses O(n) complexity by comparing sets directly. + * @param oldArray - The original array + * @param newArray - The new array to compare against + * @returns true if arrays have different elements, false if they're the same + */ +export function hasArrayChanged(oldArray: string[], newArray: string[]): boolean { + if (oldArray.length !== newArray.length) { + return true; + } + + const oldSet = new Set(oldArray); + const newSet = new Set(newArray); + + // If sets have different sizes, arrays have different unique elements + if (oldSet.size !== newSet.size) { + return true; + } + + // Check both directions: all elements in oldSet exist in newSet + // AND all elements in newSet exist in oldSet + for (const item of oldSet) { + if (!newSet.has(item)) { + return true; + } + } + + for (const item of newSet) { + if (!oldSet.has(item)) { + return true; + } + } + + return false; +}