diff --git a/.changeset/green-dragons-boil.md b/.changeset/green-dragons-boil.md new file mode 100644 index 0000000000000..47e9914c8dc19 --- /dev/null +++ b/.changeset/green-dragons-boil.md @@ -0,0 +1,6 @@ +--- +'@rocket.chat/rest-typings': patch +'@rocket.chat/meteor': patch +--- + +Fixes an issue where web clients could remain with a stale slashcommand list during a rolling workspace update diff --git a/apps/meteor/app/api/server/v1/commands.ts b/apps/meteor/app/api/server/v1/commands.ts index fda27be1d0dd9..59b90baafa8a1 100644 --- a/apps/meteor/app/api/server/v1/commands.ts +++ b/apps/meteor/app/api/server/v1/commands.ts @@ -1,3 +1,4 @@ +import { Apps } from '@rocket.chat/apps'; import { Messages } from '@rocket.chat/models'; import { Random } from '@rocket.chat/random'; import objectPath from 'object-path'; @@ -143,6 +144,19 @@ API.v1.addRoute( { authRequired: true }, { async get() { + if (!Apps.self?.isLoaded()) { + return { + statusCode: 202, // Accepted - apps are not ready, so the list is incomplete. Retry later + body: { + commands: [], + appsLoaded: false, + offset: 0, + count: 0, + total: 0, + }, + }; + } + const params = this.queryParams as Record; const { offset, count } = await getPaginationItems(params); const { sort, query } = await this.parseJsonQuery(); @@ -161,6 +175,7 @@ API.v1.addRoute( skip: offset, limit: count, }), + appsLoaded: true, offset, count: commands.length, total: totalCount, diff --git a/apps/meteor/client/hooks/useAppSlashCommands.spec.tsx b/apps/meteor/client/hooks/useAppSlashCommands.spec.tsx index 058e2c8dc4d3f..0da074aa1de3d 100644 --- a/apps/meteor/client/hooks/useAppSlashCommands.spec.tsx +++ b/apps/meteor/client/hooks/useAppSlashCommands.spec.tsx @@ -5,6 +5,7 @@ import { renderHook, waitFor } from '@testing-library/react'; import { useAppSlashCommands } from './useAppSlashCommands'; import { slashCommands } from '../../app/utils/client/slashCommand'; +import { appsQueryKeys } from '../lib/queryKeys'; const mockSlashCommands: SlashCommand[] = [ { @@ -30,6 +31,7 @@ const mockSlashCommands: SlashCommand[] = [ const mockApiResponse = { commands: mockSlashCommands, total: mockSlashCommands.length, + appsLoaded: true, }; describe('useAppSlashCommands', () => { @@ -96,7 +98,7 @@ describe('useAppSlashCommands', () => { expect(slashCommands.commands['/test']).toBeUndefined(); await waitFor(() => { - expect(queryClient.invalidateQueries).toHaveBeenCalledWith(expect.objectContaining({ queryKey: ['apps', 'slashCommands'] })); + expect(queryClient.invalidateQueries).toHaveBeenCalledWith(expect.objectContaining({ queryKey: appsQueryKeys.slashCommands() })); }); }); @@ -123,7 +125,7 @@ describe('useAppSlashCommands', () => { expect(slashCommands.commands['/test']).toBeUndefined(); await waitFor(() => { - expect(queryClient.invalidateQueries).toHaveBeenCalledWith(expect.objectContaining({ queryKey: ['apps', 'slashCommands'] })); + expect(queryClient.invalidateQueries).toHaveBeenCalledWith(expect.objectContaining({ queryKey: appsQueryKeys.slashCommands() })); }); }); @@ -155,6 +157,7 @@ describe('useAppSlashCommands', () => { }, ], total: mockSlashCommands.length + 1, + appsLoaded: true, }); streamRef.controller?.emit('apps', [['command/added', ['/newcommand']]]); @@ -188,13 +191,39 @@ describe('useAppSlashCommands', () => { streamRef.controller?.emit('apps', [['command/updated', ['/test']]]); await waitFor(() => { - expect(queryClient.invalidateQueries).toHaveBeenCalledWith(expect.objectContaining({ queryKey: ['apps', 'slashCommands'] })); + expect(queryClient.invalidateQueries).toHaveBeenCalledWith(expect.objectContaining({ queryKey: appsQueryKeys.slashCommands() })); }); expect(slashCommands.commands['/test']).toBeDefined(); expect(slashCommands.commands['/weather']).toBeDefined(); }); + it('should ignore events that do not start with command/', async () => { + const streamRef: StreamControllerRef<'apps'> = {}; + + renderHook(() => useAppSlashCommands(), { + wrapper: mockAppRoot() + .withJohnDoe() + .withQueryClient(queryClient) + .withStream('apps', streamRef) + .withEndpoint('GET', '/v1/commands.list', mockGetSlashCommands) + .build(), + }); + + expect(streamRef.controller).toBeDefined(); + + await waitFor(() => { + expect(Object.keys(slashCommands.commands)).toHaveLength(mockSlashCommands.length); + }); + + // @ts-expect-error - testing invalid event + streamRef.controller?.emit('apps', [['some/random/event', ['/test']]]); + + await new Promise((resolve) => setTimeout(resolve, 200)); + + expect(queryClient.invalidateQueries).not.toHaveBeenCalled(); + }); + it('should not set up stream listener when user ID is not available', () => { const streamRef: StreamControllerRef<'apps'> = {}; @@ -221,6 +250,7 @@ describe('useAppSlashCommands', () => { return Promise.resolve({ commands: largeMockCommands.slice(offset, offset + count), total: largeMockCommands.length, + appsLoaded: true, }); }); @@ -232,4 +262,24 @@ describe('useAppSlashCommands', () => { expect(Object.keys(slashCommands.commands)).toHaveLength(largeMockCommands.length); }); }); + + it('should not load commands when apps are not loaded', async () => { + mockGetSlashCommands.mockResolvedValue({ + commands: [], + total: 0, + appsLoaded: false, + }); + + expect(Object.keys(slashCommands.commands)).toHaveLength(0); + + renderHook(() => useAppSlashCommands(), { + wrapper: mockAppRoot().withJohnDoe().withEndpoint('GET', '/v1/commands.list', mockGetSlashCommands).build(), + }); + + await waitFor(() => { + expect(mockGetSlashCommands).toHaveBeenCalled(); + }); + + expect(Object.keys(slashCommands.commands)).toHaveLength(0); + }); }); diff --git a/apps/meteor/client/hooks/useAppSlashCommands.ts b/apps/meteor/client/hooks/useAppSlashCommands.ts index 0f2a1123c0b93..c1a6d5e33b7f8 100644 --- a/apps/meteor/client/hooks/useAppSlashCommands.ts +++ b/apps/meteor/client/hooks/useAppSlashCommands.ts @@ -48,10 +48,17 @@ export const useAppSlashCommands = () => { queryKey: appsQueryKeys.slashCommands(), enabled: !!uid, structuralSharing: false, + retry: true, + // Add a bit of randomness to avoid thundering herd problem + retryDelay: (attemptIndex) => Math.min(500 * Math.random() * 10 * 2 ** attemptIndex, 30000), queryFn: async () => { const fetchBatch = async (currentOffset: number, accumulator: SlashCommandBasicInfo[] = []): Promise => { const count = 50; - const { commands, total } = await getSlashCommands({ offset: currentOffset, count }); + const { commands, appsLoaded, total } = await getSlashCommands({ offset: currentOffset, count }); + + if (!appsLoaded) { + throw new Error('Apps not loaded, retry later'); + } const newAccumulator = [...accumulator, ...commands]; diff --git a/packages/rest-typings/src/v1/commands.ts b/packages/rest-typings/src/v1/commands.ts index 41a08435bade5..bbf3788b2b475 100644 --- a/packages/rest-typings/src/v1/commands.ts +++ b/packages/rest-typings/src/v1/commands.ts @@ -15,6 +15,7 @@ export type CommandsEndpoints = { fields?: string; }>, ) => PaginatedResult<{ + appsLoaded: boolean; commands: Pick[]; }>; };