toki/mattermost-mobile
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Fix the count issue in channel header when removing or adding attachment in the post (#8986) (#8991)

Browse source 
* Fix the count issue in channel header when removing or adding attachment in the post

* Optimisation

* Added utils function

(cherry picked from commit 0d3369d062)

Co-authored-by: Rajat Dabade <rajatdabade1997@gmail.com>
This commit is contained in:
Mattermost Build 2025-07-14 16:05:49 +03:00 committed by GitHub
parent abbd5f1852
commit 74c5a84c0c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 222 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

124
app/actions/remote/post.test.ts
View file

@ -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', '');

14
app/actions/remote/post.ts
View file

@ -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) {

14
app/actions/websocket/posts.ts
View file

@ -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);

36
app/utils/helpers.test.ts
View file

@ -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);
});
});
});

37
app/utils/helpers.ts
View file

@ -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;
}