diff --git a/CHANGELOG.md b/CHANGELOG.md index 9af95885c..111cbfd65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to - 🐛(backend) close thread DB connections to fix test teardown OperationalError #2385 +- 🐛(frontend) fix crash when orphaned threads #2395 - 🐛(backend) order trashbin response by most recently deleted ## [v5.2.0] - 2026-06-03 diff --git a/src/frontend/apps/impress/src/features/docs/doc-comments/api/DocsThreadStore.tsx b/src/frontend/apps/impress/src/features/docs/doc-comments/api/DocsThreadStore.tsx index 0a47d30ac..ea108564a 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-comments/api/DocsThreadStore.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-comments/api/DocsThreadStore.tsx @@ -329,6 +329,12 @@ export class DocsThreadStore extends ThreadStore { const serverThread = (await response.json()) as ServerThread; + // Orphan threads without comments - we delete them to avoid side effects + if (!serverThread.comments || serverThread.comments.length === 0) { + void this.deleteThread({ threadId: serverThread.id }); + return; + } + const clientThread = serverThreadToClientThread(serverThread); this.upsertClientThreadData(clientThread); this.notifySubscribers(); @@ -359,6 +365,12 @@ export class DocsThreadStore extends ThreadStore { const threads = (await response.json()) as ServerThreadListResponse; const next = new Map(); threads.forEach((thread) => { + // Orphan threads without comments - we delete them to avoid side effects + if (!thread.comments || thread.comments.length === 0) { + void this.deleteThread({ threadId: thread.id }); + return; + } + const threadData: ClientThreadData = serverThreadToClientThread(thread); next.set(thread.id, threadData); }); diff --git a/src/frontend/apps/impress/src/features/docs/doc-comments/api/__tests__/DocsThreadStore.test.ts b/src/frontend/apps/impress/src/features/docs/doc-comments/api/__tests__/DocsThreadStore.test.ts new file mode 100644 index 000000000..c3e06b9fe --- /dev/null +++ b/src/frontend/apps/impress/src/features/docs/doc-comments/api/__tests__/DocsThreadStore.test.ts @@ -0,0 +1,215 @@ +import fetchMock from 'fetch-mock'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { DocsThreadStore } from '../DocsThreadStore'; +import { DocsThreadStoreAuth } from '../DocsThreadStoreAuth'; + +const BASE_URL = 'http://test.jest/api/v1.0'; +const docId = 'test-doc-id'; +const THREADS_URL = `${BASE_URL}/documents/${docId}/threads/`; +const threadUrl = (id: string) => + `${BASE_URL}/documents/${docId}/threads/${id}/`; + +const makeThread = (id: string, comments: unknown[] | null = []) => ({ + id, + created_at: '2025-12-04T08:10:47.976364Z', + updated_at: '2025-12-04T08:10:47.976377Z', + comments, + resolved: false, + metadata: {}, + abilities: {}, +}); + +const makeComment = (id: string) => ({ + id, + user: { full_name: 'Test User' }, + body: { type: 'doc', content: [] }, + created_at: '2025-12-04T08:10:47.976364Z', + updated_at: '2025-12-04T08:10:47.976364Z', + reactions: [], + metadata: {}, + abilities: {}, +}); + +describe('DocsThreadStore - Orphan Thread Handling', () => { + let threadStore: DocsThreadStore; + let mockAuth: DocsThreadStoreAuth; + let dispatchEventSpy: ReturnType; + + beforeEach(() => { + fetchMock.reset(); + dispatchEventSpy = vi + .spyOn(document, 'dispatchEvent') + .mockReturnValue(true); + + // Default: empty thread list + fetchMock.get(THREADS_URL, []); + + mockAuth = { + canSee: true, + canComment: true, + } as unknown as DocsThreadStoreAuth; + + threadStore = new DocsThreadStore(docId, undefined, mockAuth); + }); + + afterEach(() => { + vi.restoreAllMocks(); + fetchMock.reset(); + }); + + describe('initThreads()', () => { + it('should store valid threads and ignore orphan threads', async () => { + const validThread = makeThread('valid-1', [makeComment('c1')]); + const orphanEmpty = makeThread('orphan-1', []); + const orphanNull = makeThread('orphan-2', null); + + fetchMock.get(THREADS_URL, [validThread, orphanEmpty, orphanNull], { + overwriteRoutes: true, + }); + // initThreads calls deleteThread for orphans + fetchMock.delete(threadUrl('orphan-1'), 204); + fetchMock.delete(threadUrl('orphan-2'), 204); + + await threadStore.initThreads(); + + const stored = threadStore.getThreads(); + expect(stored.size).toBe(1); + expect(stored.has('valid-1')).toBe(true); + expect(stored.has('orphan-1')).toBe(false); + expect(stored.has('orphan-2')).toBe(false); + }); + + it('should delete orphan threads via the API', async () => { + fetchMock.get(THREADS_URL, [makeThread('orphan-1', [])], { + overwriteRoutes: true, + }); + fetchMock.delete(threadUrl('orphan-1'), 204); + + await threadStore.initThreads(); + + expect( + fetchMock.called(threadUrl('orphan-1'), { method: 'DELETE' }), + ).toBe(true); + }); + + it('should handle an empty thread list', async () => { + fetchMock.get(THREADS_URL, [], { overwriteRoutes: true }); + + await threadStore.initThreads(); + + expect(threadStore.getThreads().size).toBe(0); + }); + + it('should throw when the API returns an error', async () => { + fetchMock.get( + THREADS_URL, + { status: 500, body: {} }, + { overwriteRoutes: true }, + ); + + await expect(threadStore.initThreads()).rejects.toThrow( + 'Failed to get threads in document', + ); + }); + }); + + describe('refreshThread()', () => { + it('should delete an orphan thread (no comments) via the API', async () => { + const threadId = 'orphan-thread'; + fetchMock.get(threadUrl(threadId), makeThread(threadId, [])); + fetchMock.delete(threadUrl(threadId), 204); + + await threadStore.refreshThread(threadId); + + expect(fetchMock.called(threadUrl(threadId), { method: 'DELETE' })).toBe( + true, + ); + }); + + it('should store a valid thread (with comments) without deleting it', async () => { + const threadId = 'valid-thread'; + const serverThread = makeThread(threadId, [makeComment('c1')]); + fetchMock.get(threadUrl(threadId), serverThread); + + await threadStore.refreshThread(threadId); + + expect(fetchMock.called(threadUrl(threadId), { method: 'DELETE' })).toBe( + false, + ); + expect(threadStore.getThreads().has(threadId)).toBe(true); + }); + + it('should dispatch Escape and call refreshThreads on 404', async () => { + const threadId = 'deleted-thread'; + fetchMock.get(threadUrl(threadId), 404); + fetchMock.get(THREADS_URL, [], { overwriteRoutes: true }); + + await threadStore.refreshThread(threadId); + + expect(dispatchEventSpy).toHaveBeenCalledWith( + expect.objectContaining({ key: 'Escape' }), + ); + }); + }); + + describe('deleteComment()', () => { + it('should delete the thread when the last comment is removed', async () => { + const threadId = 'thread-1'; + const commentId = 'comment-1'; + + // Seed the store with a thread that has one comment + fetchMock.get( + THREADS_URL, + [makeThread(threadId, [makeComment(commentId)])], + { + overwriteRoutes: true, + }, + ); + await threadStore.initThreads(); + + fetchMock.delete( + `${BASE_URL}/documents/${docId}/threads/${threadId}/comments/${commentId}/`, + 204, + ); + fetchMock.delete(threadUrl(threadId), 204); + + await threadStore.deleteComment({ threadId, commentId }); + + expect(fetchMock.called(threadUrl(threadId), { method: 'DELETE' })).toBe( + true, + ); + expect(threadStore.getThreads().has(threadId)).toBe(false); + }); + + it('should keep the thread when other comments remain after deletion', async () => { + const threadId = 'thread-1'; + const commentToDelete = 'comment-1'; + + fetchMock.get( + THREADS_URL, + [ + makeThread(threadId, [ + makeComment(commentToDelete), + makeComment('comment-2'), + ]), + ], + { overwriteRoutes: true }, + ); + await threadStore.initThreads(); + + fetchMock.delete( + `${BASE_URL}/documents/${docId}/threads/${threadId}/comments/${commentToDelete}/`, + 204, + ); + + await threadStore.deleteComment({ threadId, commentId: commentToDelete }); + + expect(fetchMock.called(threadUrl(threadId), { method: 'DELETE' })).toBe( + false, + ); + expect(threadStore.getThreads().has(threadId)).toBe(true); + expect(threadStore.getThreads().get(threadId)?.comments).toHaveLength(1); + }); + }); +});