Fix the count issue in channel header when removing or adding attachment in the post (#8986)
* Fix the count issue in channel header when removing or adding attachment in the post * Optimisation * Added utils function
This commit is contained in:
parent
b5969fdfb4
commit
0d3369d062
5 changed files with 222 additions and 3 deletions
|
|
@ -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', '');
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue