From 66275df29259ebe69d8806aebc8f41b731e85c58 Mon Sep 17 00:00:00 2001 From: jackiejou <21050234+jackiejou@users.noreply.github.com> Date: Fri, 29 May 2026 12:06:22 -0700 Subject: [PATCH 1/4] feat(popup-v2): wire reply edit and delete to threaded comments API Threads in PopupV2 can now edit and delete replies. Reply edits dispatch to PUT /undoc/comments/{id} and reply deletes to DELETE /undoc/comments/{id} via the box-ui-elements ThreadedComments client. - Adds updateReplyAction and deleteReplyAction thunks. Both reject upfront when the reply is not present in state so callers receive a typed error instead of an opaque server response. - Adds reducer cases that replace or remove the targeted reply on fulfilled. - Threads the canEdit permission through replyToTextMessage so the edit affordance reflects backend permissions. - Widens the Token type with TokenMap so consumers can pass a per-typed file id resolver for avatar fetching. --- src/@types/api.ts | 5 +- .../threadedAnnotationsAdapters-test.ts | 17 +- src/adapters/threadedAnnotationsAdapters.ts | 2 +- src/api/APIFactory.ts | 7 +- src/api/__mocks__/APIFactory.ts | 5 + src/api/types.ts | 24 ++- src/common/BaseAnnotator.ts | 4 +- src/components/Popups/PopupV2.tsx | 56 +++++-- .../Popups/__tests__/PopupV2-test.tsx | 94 ++++++++++- .../annotations/__tests__/actions-test.ts | 121 +++++++++++++- .../annotations/__tests__/reducer-test.ts | 154 ++++++++++++++++++ src/store/annotations/actions.ts | 72 ++++++++ src/store/annotations/reducer.ts | 16 ++ src/store/options/__tests__/selectors-test.ts | 11 ++ src/store/options/selectors.ts | 4 +- src/store/options/types.ts | 4 +- 16 files changed, 567 insertions(+), 29 deletions(-) diff --git a/src/@types/api.ts b/src/@types/api.ts index a357574b3..8e52d0f38 100644 --- a/src/@types/api.ts +++ b/src/@types/api.ts @@ -10,5 +10,6 @@ export interface Permissions { } export type TokenLiteral = null | undefined | string | { read: string; write?: string }; -export type TokenResolver = () => TokenLiteral | Promise; -export type Token = TokenLiteral | TokenResolver; +export type TokenMap = { [typedFileId: string]: TokenLiteral }; +export type TokenResolver = (typedFileId?: string) => TokenLiteral | TokenMap | Promise; +export type Token = TokenLiteral | TokenMap | TokenResolver; diff --git a/src/adapters/__tests__/threadedAnnotationsAdapters-test.ts b/src/adapters/__tests__/threadedAnnotationsAdapters-test.ts index 44607d2c4..999715b29 100644 --- a/src/adapters/__tests__/threadedAnnotationsAdapters-test.ts +++ b/src/adapters/__tests__/threadedAnnotationsAdapters-test.ts @@ -144,13 +144,28 @@ describe('threadedAnnotationsAdapters', () => { }); }); - test('should map reply permissions from backend payload, forcing canEdit false', () => { + test('should map reply permissions from backend payload', () => { const reply: Reply = { ...mockReply, permissions: { can_delete: true, can_edit: true, can_reply: true, can_resolve: true }, }; const result = replyToTextMessage(reply); + expect(result.permissions).toEqual({ + canDelete: true, + canEdit: true, + canReply: true, + canResolve: true, + }); + }); + + test('should default canEdit to false when backend payload omits it', () => { + const reply: Reply = { + ...mockReply, + permissions: { can_delete: true, can_reply: true, can_resolve: true }, + }; + const result = replyToTextMessage(reply); + expect(result.permissions).toEqual({ canDelete: true, canEdit: false, diff --git a/src/adapters/threadedAnnotationsAdapters.ts b/src/adapters/threadedAnnotationsAdapters.ts index 0029056ab..d1080b6bf 100644 --- a/src/adapters/threadedAnnotationsAdapters.ts +++ b/src/adapters/threadedAnnotationsAdapters.ts @@ -83,7 +83,7 @@ export const replyToTextMessage = (reply: Reply): TextMessageTypeV2 => ({ message: deserializeMentionMarkup(reply.message), permissions: { canDelete: reply.permissions?.can_delete ?? false, - canEdit: false, + canEdit: reply.permissions?.can_edit ?? false, canReply: reply.permissions?.can_reply ?? false, canResolve: reply.permissions?.can_resolve ?? false, }, diff --git a/src/api/APIFactory.ts b/src/api/APIFactory.ts index 7050f9410..ea820afef 100644 --- a/src/api/APIFactory.ts +++ b/src/api/APIFactory.ts @@ -1,7 +1,8 @@ import Annotations from 'box-ui-elements/es/api/Annotations'; import FileCollaborators from 'box-ui-elements/es/api/FileCollaborators'; +import ThreadedComments from 'box-ui-elements/es/api/ThreadedComments'; import { DEFAULT_HOSTNAME_API } from 'box-ui-elements/es/constants'; -import { AnnotationsAPI, CollaboratorsAPI, APIOptions } from './types'; +import { AnnotationsAPI, CollaboratorsAPI, APIOptions, ThreadedCommentsAPI } from './types'; export default class APIFactory { options: APIOptions; @@ -21,4 +22,8 @@ export default class APIFactory { getCollaboratorsAPI(): CollaboratorsAPI { return new FileCollaborators(this.options); } + + getThreadedCommentsAPI(): ThreadedCommentsAPI { + return new ThreadedComments(this.options); + } } diff --git a/src/api/__mocks__/APIFactory.ts b/src/api/__mocks__/APIFactory.ts index 10b3e7037..2c2116320 100644 --- a/src/api/__mocks__/APIFactory.ts +++ b/src/api/__mocks__/APIFactory.ts @@ -12,4 +12,9 @@ export default jest.fn(() => ({ ), destroy: jest.fn(), })), + getThreadedCommentsAPI: jest.fn(() => ({ + deleteComment: jest.fn(({ successCallback }) => successCallback()), + updateComment: jest.fn(({ successCallback }) => successCallback({ id: 'reply_1', message: 'updated' })), + destroy: jest.fn(), + })), })); diff --git a/src/api/types.ts b/src/api/types.ts index 8b4796701..9b4baf733 100644 --- a/src/api/types.ts +++ b/src/api/types.ts @@ -1,4 +1,4 @@ -import { Annotation, Collaborator, NewAnnotation, Permissions, Reply, Token } from '../@types'; +import { Annotation, Collaborator, NewAnnotation, Permissions, Reply, ReplyPermissions, Token } from '../@types'; export type APICollection = { entries: R[]; @@ -73,6 +73,28 @@ export interface AnnotationsAPI { destroy(): void; } +export interface ThreadedCommentsAPI { + deleteComment(args: { + commentId: string; + errorCallback: (error: APIError) => void; + fileId: string | null; + permissions: ReplyPermissions; + successCallback: () => void; + }): void; + + updateComment(args: { + commentId: string; + errorCallback: (error: APIError) => void; + fileId: string | null; + message?: string; + permissions: ReplyPermissions; + status?: string; + successCallback: (reply: Reply) => void; + }): void; + + destroy(): void; +} + export interface CollaboratorsAPI { getFileCollaborators( fileId: string | null, diff --git a/src/common/BaseAnnotator.ts b/src/common/BaseAnnotator.ts index f5e74fb1f..8e980ca96 100644 --- a/src/common/BaseAnnotator.ts +++ b/src/common/BaseAnnotator.ts @@ -6,7 +6,7 @@ import DeselectManager from './DeselectManager'; import EventEmitter from './EventEmitter'; import i18n from '../utils/i18n'; import messages from '../messages'; -import { Event, IntlOptions, LegacyEvent, Permissions } from '../@types'; +import { Event, IntlOptions, LegacyEvent, Permissions, Token } from '../@types'; import { BoundingBox, getBoundingBoxHighlights } from '../store/boundingBoxHighlights'; import { ViewMode } from '../store/options/types'; import { Features } from '../BoxAnnotations'; @@ -45,7 +45,7 @@ export type Options = { intl: IntlOptions; locale?: string; onCopyLink?: (params: { annotationId: string; fileVersionId: string }) => void; - token: string; + token: Token; }; export const CSS_CONTAINER_CLASS = 'ba'; diff --git a/src/components/Popups/PopupV2.tsx b/src/components/Popups/PopupV2.tsx index 81b3a3d31..6df30a9eb 100644 --- a/src/components/Popups/PopupV2.tsx +++ b/src/components/Popups/PopupV2.tsx @@ -21,13 +21,16 @@ import AnnotationCallbacksContext from '../../common/AnnotationCallbacksContext' import { createReplyAction, deleteAnnotationAction, + deleteReplyAction, setActiveAnnotationIdAction, updateAnnotationAction, + updateReplyAction, } from '../../store/annotations/actions'; import { getAnnotation } from '../../store/annotations/selectors'; -import { getApiHost, getFileVersionId, getToken } from '../../store/options'; +import { getApiHost, getFileId, getFileVersionId, getToken } from '../../store/options'; import { fetchCollaboratorsAction } from '../../store/users/actions'; +import type { Token, TokenLiteral, TokenMap } from '../../@types'; import type { AppState, AppThunkDispatch } from '../../store/types'; import createPopper, { PopupReference } from './Popper'; @@ -63,12 +66,32 @@ const createDocumentNode = (content: JSONContent | null): DocumentNodeV2 => { return { type: 'doc', content: [content] } as DocumentNodeV2; }; -// Callers render initials as a fallback on null. -// A persistent null across all users usually indicates a stale token. -const fetchAvatarBlob = async (apiHost: string, token: string, userId: string): Promise => { +const literalToString = (literal: TokenLiteral): string | null => { + if (!literal) return null; + if (typeof literal === 'string') return literal; + return literal.read ?? literal.write ?? null; +}; + +const resolveStringToken = async (token: Token, typedFileId: string): Promise => { + const resolved = typeof token === 'function' ? await token(typedFileId) : token; + if (!resolved) return null; + if (typeof resolved === 'string') return resolved; + if ('read' in resolved) return literalToString(resolved as TokenLiteral); + return literalToString((resolved as TokenMap)[typedFileId]); +}; + +const fetchAvatarBlob = async ( + apiHost: string, + token: Token, + fileId: string | null, + userId: string, +): Promise => { try { + if (!fileId) return null; + const stringToken = await resolveStringToken(token, `file_${fileId}`); + if (!stringToken) return null; const response = await fetch(`${apiHost}/2.0/users/${userId}/avatar?pic_type=large`, { - headers: { Authorization: `Bearer ${token}` }, + headers: { Authorization: `Bearer ${stringToken}` }, }); if (!response.ok) return null; const blob = await response.blob(); @@ -87,6 +110,7 @@ const PopupV2 = ({ annotationId, onSubmit, popupPortalEl, reference }: Props): J const optionsRef = React.useRef>(getPopupOptions()); const apiHost = useSelector(getApiHost); + const fileId = useSelector(getFileId); const fileVersionId = useSelector(getFileVersionId); const token = useSelector(getToken); const onCopyLink = React.useMemo( @@ -111,7 +135,7 @@ const PopupV2 = ({ annotationId, onSubmit, popupPortalEl, reference }: Props): J if (cached) return cached; const capturedApiHost = apiHost; const capturedToken = token; - const url = await fetchAvatarBlob(capturedApiHost, capturedToken, userId); + const url = await fetchAvatarBlob(capturedApiHost, capturedToken, fileId, userId); if (!url) return null; if ( credentialsRef.current.apiHost !== capturedApiHost || @@ -128,7 +152,7 @@ const PopupV2 = ({ annotationId, onSubmit, popupPortalEl, reference }: Props): J avatarCacheRef.current.set(userId, url); return url; }, - [apiHost, token], + [apiHost, fileId, token], ); React.useEffect(() => { @@ -247,10 +271,14 @@ const PopupV2 = ({ annotationId, onSubmit, popupPortalEl, reference }: Props): J const handleEdit = React.useCallback( async (id: string, content: JSONContent | null): Promise => { - if (!annotationId || id !== annotationId) return; + if (!annotationId) return; const doc = createDocumentNode(content); const { text } = serializeMentionMarkup(doc); - await dispatch(updateAnnotationAction({ annotationId, payload: { message: text } })); + if (id === annotationId) { + await dispatch(updateAnnotationAction({ annotationId, payload: { message: text } })); + return; + } + await dispatch(updateReplyAction({ annotationId, replyId: id, payload: { message: text } })); }, [annotationId, dispatch], ); @@ -264,6 +292,14 @@ const PopupV2 = ({ annotationId, onSubmit, popupPortalEl, reference }: Props): J [annotationId, dispatch], ); + const handleDelete = React.useCallback( + async (id: string): Promise => { + if (!annotationId || id === annotationId) return; + await dispatch(deleteReplyAction({ annotationId, replyId: id })); + }, + [annotationId, dispatch], + ); + const handleResolve = React.useCallback( async (): Promise => { if (!annotationId) return; @@ -314,7 +350,7 @@ const PopupV2 = ({ annotationId, onSubmit, popupPortalEl, reference }: Props): J messages={threadMessages} onAvatarClick={noop} onCopyLink={onCopyLink} - onDelete={noop} + onDelete={handleDelete} onEdit={handleEdit} onPost={handlePost} onResolve={handleResolve} diff --git a/src/components/Popups/__tests__/PopupV2-test.tsx b/src/components/Popups/__tests__/PopupV2-test.tsx index 57e19eab2..81c922e4d 100644 --- a/src/components/Popups/__tests__/PopupV2-test.tsx +++ b/src/components/Popups/__tests__/PopupV2-test.tsx @@ -4,8 +4,12 @@ import { useDispatch, useSelector } from 'react-redux'; import type { ThreadedAnnotationsPropsV2 } from '@box/threaded-annotations'; import AnnotationCallbacksContext from '../../../common/AnnotationCallbacksContext'; import PopupV2, { Props } from '../PopupV2'; -import { updateAnnotationAction } from '../../../store/annotations/actions'; -import { getApiHost, getFileVersionId, getToken } from '../../../store/options'; +import { + deleteReplyAction, + updateAnnotationAction, + updateReplyAction, +} from '../../../store/annotations/actions'; +import { getApiHost, getFileId, getFileVersionId, getToken } from '../../../store/options'; jest.mock('react-redux', () => ({ useDispatch: jest.fn(), @@ -62,10 +66,12 @@ jest.mock('@box/threaded-annotations', () => { jest.mock('../../../store/annotations/actions', () => ({ createReplyAction: jest.fn(), deleteAnnotationAction: jest.fn(), + deleteReplyAction: jest.fn(), setActiveAnnotationIdAction: jest.fn(), updateAnnotationAction: Object.assign(jest.fn(), { fulfilled: { match: jest.fn().mockReturnValue(true) }, }), + updateReplyAction: jest.fn(), })); jest.mock('../../../store/users/actions', () => ({ @@ -80,6 +86,7 @@ const mockUseSelector = useSelector as jest.MockedFunction; const mockSelectorValues = (annotation?: unknown): void => { mockUseSelector.mockImplementation(selector => { if (selector === getApiHost) return 'https://api.box.com'; + if (selector === getFileId) return '12345'; if (selector === getFileVersionId) return 'fv-1'; if (selector === getToken) return 'test-token'; return annotation; @@ -243,13 +250,39 @@ describe('PopupV2', () => { }); }); - test('should not dispatch updateAnnotationAction when editing a reply', async () => { + test('should dispatch updateReplyAction (not updateAnnotationAction) when editing a reply', async () => { render(); await flushPromises(); await lastThreadedAnnotationsProps.onEdit?.('reply-1', { type: 'doc', content: [] }); expect(updateAnnotationAction).not.toHaveBeenCalled(); + expect(updateReplyAction).toHaveBeenCalledWith({ + annotationId: 'annotation-1', + replyId: 'reply-1', + payload: { message: 'serialized text' }, + }); + }); + + test('should dispatch deleteReplyAction when a reply is deleted', async () => { + render(); + await flushPromises(); + + await lastThreadedAnnotationsProps.onDelete?.('reply-1'); + + expect(deleteReplyAction).toHaveBeenCalledWith({ + annotationId: 'annotation-1', + replyId: 'reply-1', + }); + }); + + test('should not dispatch deleteReplyAction when deleting the root annotation id', async () => { + render(); + await flushPromises(); + + await lastThreadedAnnotationsProps.onDelete?.('annotation-1'); + + expect(deleteReplyAction).not.toHaveBeenCalled(); }); test('should invoke context onCopyLink with the root annotationId and fileVersionId regardless of clicked message id', async () => { @@ -270,6 +303,7 @@ describe('PopupV2', () => { const onCopyLink = jest.fn(); mockUseSelector.mockImplementation(selector => { if (selector === getApiHost) return 'https://api.box.com'; + if (selector === getFileId) return '12345'; if (selector === getFileVersionId) return null; if (selector === getToken) return 'test-token'; return mockAnnotation; @@ -310,6 +344,60 @@ describe('PopupV2', () => { const [calledUrl] = mockFetch.mock.calls[0]; expect(calledUrl).not.toContain('access_token'); }); + + test('should resolve a function token by typed file id before building Authorization header', async () => { + const tokenResolver = jest.fn().mockResolvedValue('resolved-token'); + mockUseSelector.mockImplementation(selector => { + if (selector === getApiHost) return 'https://api.box.com'; + if (selector === getFileId) return '12345'; + if (selector === getFileVersionId) return 'fv-1'; + if (selector === getToken) return tokenResolver; + return mockAnnotation; + }); + + render(); + await flushPromises(); + + expect(tokenResolver).toHaveBeenCalledWith('file_12345'); + expect(mockFetch).toHaveBeenCalledWith( + 'https://api.box.com/2.0/users/100/avatar?pic_type=large', + { headers: { Authorization: 'Bearer resolved-token' } }, + ); + }); + + test('should resolve a per-file map token by extracting the read string', async () => { + const tokenMap = { file_12345: { read: 'read-token', write: 'write-token' } }; + mockUseSelector.mockImplementation(selector => { + if (selector === getApiHost) return 'https://api.box.com'; + if (selector === getFileId) return '12345'; + if (selector === getFileVersionId) return 'fv-1'; + if (selector === getToken) return tokenMap; + return mockAnnotation; + }); + + render(); + await flushPromises(); + + expect(mockFetch).toHaveBeenCalledWith( + 'https://api.box.com/2.0/users/100/avatar?pic_type=large', + { headers: { Authorization: 'Bearer read-token' } }, + ); + }); + + test('should not call fetch when fileId is missing', async () => { + mockUseSelector.mockImplementation(selector => { + if (selector === getApiHost) return 'https://api.box.com'; + if (selector === getFileId) return null; + if (selector === getFileVersionId) return 'fv-1'; + if (selector === getToken) return 'test-token'; + return mockAnnotation; + }); + + render(); + await flushPromises(); + + expect(mockFetch).not.toHaveBeenCalled(); + }); }); test('should set aria-label on popup container', () => { diff --git a/src/store/annotations/__tests__/actions-test.ts b/src/store/annotations/__tests__/actions-test.ts index 7b78d9494..3bc534c13 100644 --- a/src/store/annotations/__tests__/actions-test.ts +++ b/src/store/annotations/__tests__/actions-test.ts @@ -1,13 +1,21 @@ import API from '../../../api'; -import { createAnnotationAction, fetchAnnotationsAction } from '../actions'; -import { NewAnnotation } from '../../../@types'; +import { + createAnnotationAction, + deleteReplyAction, + fetchAnnotationsAction, + updateReplyAction, +} from '../actions'; +import { Annotation, NewAnnotation, Reply } from '../../../@types'; jest.mock('../../../api/APIFactory'); describe('store/annotations/actions', () => { const api = new API({ token: 'token_1234' }); const dispatch = jest.fn(); - const getState = jest.fn().mockReturnValue({ + const baseState = { + annotations: { + byId: {} as Record, + }, options: { fileId: '12345', fileVersionId: '67890', @@ -16,7 +24,8 @@ describe('store/annotations/actions', () => { can_view_annotations: true, }, }, - }); + }; + const getState = jest.fn().mockReturnValue(baseState); describe('createAnnotationAction', () => { const arg = { target: { shape: { x: 10, y: 10 } } } as NewAnnotation; @@ -64,4 +73,108 @@ describe('store/annotations/actions', () => { expect(result.payload).toBe(undefined); }); }); + + describe('updateReplyAction', () => { + const annotationId = 'anno_1'; + const replyId = 'reply_1'; + const arg = { annotationId, replyId, payload: { message: 'updated' } }; + const reply = { id: replyId, message: 'old', permissions: { can_edit: true } } as unknown as Reply; + const annotation = { id: annotationId, replies: [reply] } as unknown as Annotation; + + beforeEach(() => { + getState.mockReturnValue({ + ...baseState, + annotations: { ...baseState.annotations, byId: { [annotationId]: annotation } }, + }); + }); + + afterEach(() => { + getState.mockReturnValue(baseState); + }); + + test('should resolve with annotationId and updated reply from threaded comments API', async () => { + const result = await updateReplyAction(arg)(dispatch, getState, { api }); + + expect(result.payload).toEqual({ annotationId, reply: { id: 'reply_1', message: 'updated' } }); + }); + + test('should reject with a clear error when the reply is not in state', async () => { + getState.mockReturnValue(baseState); + + const result = await updateReplyAction(arg)(dispatch, getState, { api }); + + expect(result.type).toBe('UPDATE_REPLY/rejected'); + expect(result.payload).toBeUndefined(); + const {error} = (result as { error: { message: string } }); + expect(error.message).toContain('reply reply_1 not found'); + + getState.mockReturnValue({ + ...baseState, + annotations: { ...baseState.annotations, byId: { [annotationId]: annotation } }, + }); + }); + + test('should abort the request if the action abort method is called', async () => { + const action = updateReplyAction(arg)(dispatch, getState, { api }); + + action.abort(); + + const result = await action; + + expect(result.meta).toMatchObject({ aborted: true }); + expect(result.payload).toBe(undefined); + }); + }); + + describe('deleteReplyAction', () => { + const annotationId = 'anno_1'; + const replyId = 'reply_1'; + const arg = { annotationId, replyId }; + const reply = { id: replyId, permissions: { can_delete: true } } as unknown as Reply; + const annotation = { id: annotationId, replies: [reply] } as unknown as Annotation; + + beforeEach(() => { + getState.mockReturnValue({ + ...baseState, + annotations: { ...baseState.annotations, byId: { [annotationId]: annotation } }, + }); + }); + + afterEach(() => { + getState.mockReturnValue(baseState); + }); + + test('should resolve with the annotationId and replyId via threaded comments API', async () => { + const result = await deleteReplyAction(arg)(dispatch, getState, { api }); + + expect(result.payload).toEqual({ annotationId, replyId }); + }); + + test('should reject with a clear error when the reply is not in state', async () => { + getState.mockReturnValue(baseState); + + const result = await deleteReplyAction(arg)(dispatch, getState, { api }); + + expect(result.type).toBe('DELETE_REPLY/rejected'); + expect(result.payload).toBeUndefined(); + const {error} = (result as { error: { message: string } }); + expect(error.message).toContain('reply reply_1 not found'); + + getState.mockReturnValue({ + ...baseState, + annotations: { ...baseState.annotations, byId: { [annotationId]: annotation } }, + }); + }); + + test('should abort the request if the action abort method is called', async () => { + const action = deleteReplyAction(arg)(dispatch, getState, { api }); + + action.abort(); + + const result = await action; + + expect(result.meta).toMatchObject({ aborted: true }); + expect(result.payload).toBe(undefined); + }); + }); }); diff --git a/src/store/annotations/__tests__/reducer-test.ts b/src/store/annotations/__tests__/reducer-test.ts index 973160e8d..035d6274e 100644 --- a/src/store/annotations/__tests__/reducer-test.ts +++ b/src/store/annotations/__tests__/reducer-test.ts @@ -6,11 +6,13 @@ import { createAnnotationAction, createReplyAction, deleteAnnotationAction, + deleteReplyAction, fetchAnnotationsAction, removeAnnotationAction, setActiveAnnotationIdAction, setIsInitialized, updateAnnotationAction, + updateReplyAction, } from '../actions'; import { setViewModeAction } from '../../options/actions'; @@ -219,6 +221,158 @@ describe('store/annotations/reducer', () => { }); }); + describe('updateReplyAction', () => { + const existingReply = { + created_at: '2026-01-01T00:00:00Z', + created_by: { id: '1', login: 'user@box.com', name: 'User', type: 'user' }, + id: 'reply-1', + message: 'old message', + parent: { id: 'test1', type: 'annotation' }, + type: 'reply', + } as Reply; + + test('should replace the matching reply in the annotation replies list', () => { + const updatedReply = { ...existingReply, message: 'new message' } as Reply; + const stateWithReply = { + ...state, + byId: { + ...state.byId, + test1: { ...state.byId.test1, replies: [existingReply] } as unknown as Annotation, + }, + }; + + const newState = reducer( + stateWithReply, + updateReplyAction.fulfilled( + { annotationId: 'test1', reply: updatedReply }, + 'test', + { annotationId: 'test1', replyId: 'reply-1', payload: { message: 'new message' } }, + ), + ); + + expect(newState.byId.test1.replies).toHaveLength(1); + expect(newState.byId.test1.replies![0]).toEqual(updatedReply); + }); + + test('should leave other replies untouched when one reply is updated', () => { + const otherReply = { ...existingReply, id: 'reply-2', message: 'other' } as Reply; + const updatedReply = { ...existingReply, message: 'new message' } as Reply; + const stateWithReplies = { + ...state, + byId: { + ...state.byId, + test1: { ...state.byId.test1, replies: [existingReply, otherReply] } as unknown as Annotation, + }, + }; + + const newState = reducer( + stateWithReplies, + updateReplyAction.fulfilled( + { annotationId: 'test1', reply: updatedReply }, + 'test', + { annotationId: 'test1', replyId: 'reply-1', payload: { message: 'new message' } }, + ), + ); + + expect(newState.byId.test1.replies).toEqual([updatedReply, otherReply]); + }); + + test('should not modify state if annotation does not exist', () => { + const updatedReply = { ...existingReply, message: 'new message' } as Reply; + const newState = reducer( + state, + updateReplyAction.fulfilled( + { annotationId: 'nonexistent', reply: updatedReply }, + 'test', + { annotationId: 'nonexistent', replyId: 'reply-1', payload: { message: 'new message' } }, + ), + ); + + expect(newState.byId).toEqual(state.byId); + }); + + test('should not modify state if annotation has no replies', () => { + const updatedReply = { ...existingReply, message: 'new message' } as Reply; + const newState = reducer( + state, + updateReplyAction.fulfilled( + { annotationId: 'test1', reply: updatedReply }, + 'test', + { annotationId: 'test1', replyId: 'reply-1', payload: { message: 'new message' } }, + ), + ); + + expect(newState.byId.test1).toEqual(state.byId.test1); + }); + }); + + describe('deleteReplyAction', () => { + const replyA = { + created_at: '2026-01-01T00:00:00Z', + created_by: { id: '1', login: 'user@box.com', name: 'User', type: 'user' }, + id: 'reply-1', + message: 'first', + parent: { id: 'test1', type: 'annotation' }, + type: 'reply', + } as Reply; + const replyB = { ...replyA, id: 'reply-2', message: 'second' } as Reply; + + test('should remove the targeted reply from the replies list', () => { + const stateWithReplies = { + ...state, + byId: { + ...state.byId, + test1: { ...state.byId.test1, replies: [replyA, replyB] } as unknown as Annotation, + }, + }; + + const newState = reducer( + stateWithReplies, + deleteReplyAction.fulfilled( + { annotationId: 'test1', replyId: 'reply-1' }, + 'test', + { annotationId: 'test1', replyId: 'reply-1' }, + ), + ); + + expect(newState.byId.test1.replies).toEqual([replyB]); + }); + + test('should leave replies unchanged when replyId is not found', () => { + const stateWithReplies = { + ...state, + byId: { + ...state.byId, + test1: { ...state.byId.test1, replies: [replyA] } as unknown as Annotation, + }, + }; + + const newState = reducer( + stateWithReplies, + deleteReplyAction.fulfilled( + { annotationId: 'test1', replyId: 'reply-other' }, + 'test', + { annotationId: 'test1', replyId: 'reply-other' }, + ), + ); + + expect(newState.byId.test1.replies).toEqual([replyA]); + }); + + test('should not modify state if annotation does not exist', () => { + const newState = reducer( + state, + deleteReplyAction.fulfilled( + { annotationId: 'nonexistent', replyId: 'reply-1' }, + 'test', + { annotationId: 'nonexistent', replyId: 'reply-1' }, + ), + ); + + expect(newState.byId).toEqual(state.byId); + }); + }); + describe('setViewModeAction', () => { test('should clear activeId when switching to boundingBoxes mode', () => { const stateWithActiveAnnotation = { diff --git a/src/store/annotations/actions.ts b/src/store/annotations/actions.ts index 408d7e836..a87e08aa2 100644 --- a/src/store/annotations/actions.ts +++ b/src/store/annotations/actions.ts @@ -121,6 +121,78 @@ export const updateAnnotationAction = createAsyncThunk< }, ); +export const updateReplyAction = createAsyncThunk< + { annotationId: string; reply: Reply }, + { annotationId: string; replyId: string; payload: { message?: string; status?: string } }, + AppThunkAPI +>( + 'UPDATE_REPLY', + async ({ annotationId, replyId, payload }, { extra, getState, signal }) => { + const client = extra.api.getThreadedCommentsAPI(); + const state = getState(); + const fileId = getFileId(state); + const annotation = getAnnotation(state, annotationId); + const reply = annotation?.replies?.find(r => r.id === replyId); + + if (!reply) { + throw new Error(`updateReplyAction: reply ${replyId} not found on annotation ${annotationId}`); + } + + signal.addEventListener('abort', () => { + client.destroy(); + }); + + const updated = await new Promise((resolve, reject) => { + client.updateComment({ + commentId: replyId, + errorCallback: reject, + fileId, + message: payload.message, + permissions: reply.permissions ?? {}, + status: payload.status, + successCallback: resolve, + }); + }); + + return { annotationId, reply: updated }; + }, +); + +export const deleteReplyAction = createAsyncThunk< + { annotationId: string; replyId: string }, + { annotationId: string; replyId: string }, + AppThunkAPI +>( + 'DELETE_REPLY', + async ({ annotationId, replyId }, { extra, getState, signal }) => { + const client = extra.api.getThreadedCommentsAPI(); + const state = getState(); + const fileId = getFileId(state); + const annotation = getAnnotation(state, annotationId); + const reply = annotation?.replies?.find(r => r.id === replyId); + + if (!reply) { + throw new Error(`deleteReplyAction: reply ${replyId} not found on annotation ${annotationId}`); + } + + signal.addEventListener('abort', () => { + client.destroy(); + }); + + await new Promise((resolve, reject) => { + client.deleteComment({ + commentId: replyId, + errorCallback: reject, + fileId, + permissions: reply.permissions ?? {}, + successCallback: resolve, + }); + }); + + return { annotationId, replyId }; + }, +); + export const removeAnnotationAction = createAction('REMOVE_ANNOTATION'); export const setActiveAnnotationIdAction = createAction('SET_ACTIVE_ANNOTATION_ID'); export const setIsInitialized = createAction('SET_IS_INITIALIZED'); diff --git a/src/store/annotations/reducer.ts b/src/store/annotations/reducer.ts index 5485820fc..5a868f6d8 100644 --- a/src/store/annotations/reducer.ts +++ b/src/store/annotations/reducer.ts @@ -5,11 +5,13 @@ import { createAnnotationAction, createReplyAction, deleteAnnotationAction, + deleteReplyAction, fetchAnnotationsAction, removeAnnotationAction, setActiveAnnotationIdAction, setIsInitialized, updateAnnotationAction, + updateReplyAction, } from './actions'; import { setViewModeAction } from '../options/actions'; @@ -58,6 +60,20 @@ const annotationsById = createReducer({}, builder => .addCase(updateAnnotationAction.fulfilled, (state, { payload }) => { state[payload.id] = isDrawing(payload) ? formatDrawing(payload) : payload; }) + .addCase(updateReplyAction.fulfilled, (state, { payload: { annotationId, reply } }) => { + const annotation = state[annotationId]; + if (annotation && annotation.replies) { + annotation.replies = annotation.replies.map(existing => + existing.id === reply.id ? reply : existing, + ); + } + }) + .addCase(deleteReplyAction.fulfilled, (state, { payload: { annotationId, replyId } }) => { + const annotation = state[annotationId]; + if (annotation && annotation.replies) { + annotation.replies = annotation.replies.filter(existing => existing.id !== replyId); + } + }) .addCase(fetchAnnotationsAction.fulfilled, (state, { payload }) => { payload.entries.forEach(annotation => { state[annotation.id] = isDrawing(annotation) ? formatDrawing(annotation) : annotation; diff --git a/src/store/options/__tests__/selectors-test.ts b/src/store/options/__tests__/selectors-test.ts index 3595e968d..f6501925e 100644 --- a/src/store/options/__tests__/selectors-test.ts +++ b/src/store/options/__tests__/selectors-test.ts @@ -5,6 +5,7 @@ import { getPermissions, getRotation, getScale, + getToken, getViewMode, isFeatureEnabled, } from '../selectors'; @@ -65,6 +66,16 @@ describe('store/options/selectors', () => { }); }); + describe('getToken', () => { + test.each([ + ['string', 'test-token'], + ['function resolver', () => 'resolved'], + ])('should return a %s token unchanged so callers can resolve it', (_label, token) => { + const stateWithToken = { options: { ...optionsState, token } }; + expect(getToken(stateWithToken)).toBe(token); + }); + }); + describe('getViewMode', () => { test('should return the current view mode', () => { expect(getViewMode({ options: optionsState })).toBe('annotations'); diff --git a/src/store/options/selectors.ts b/src/store/options/selectors.ts index 0f7031d87..af568ba7d 100644 --- a/src/store/options/selectors.ts +++ b/src/store/options/selectors.ts @@ -1,6 +1,6 @@ import getProp from 'lodash/get'; import { AppState } from '../types'; -import { Permissions } from '../../@types'; +import { Permissions, Token } from '../../@types'; import { Features } from '../../BoxAnnotations'; import { ViewMode } from './types'; @@ -15,6 +15,6 @@ export const getIsCurrentFileVersion = (state: State): boolean => state.options. export const getPermissions = (state: State): Permissions => state.options.permissions; export const getRotation = (state: State): number => state.options.rotation; export const getScale = (state: State): number => state.options.scale; -export const getToken = (state: State): string => state.options.token; +export const getToken = (state: State): Token => state.options.token; export const isFeatureEnabled = (state: State, featurename: string): boolean => getProp(getFeatures(state), featurename, false); diff --git a/src/store/options/types.ts b/src/store/options/types.ts index 9abfd9ab0..65bf15f51 100644 --- a/src/store/options/types.ts +++ b/src/store/options/types.ts @@ -1,5 +1,5 @@ import { Features } from '../../BoxAnnotations'; -import { Permissions } from '../../@types'; +import { Permissions, Token } from '../../@types'; /** View mode: annotations and bounding boxes are mutually exclusive. */ export type ViewMode = 'annotations' | 'boundingBoxes'; @@ -13,6 +13,6 @@ export type OptionsState = { permissions: Permissions; rotation: number; scale: number; - token: string; + token: Token; viewMode: ViewMode; }; From bf8809a02365e907eee9f6945d6126b2ef3c3fc7 Mon Sep 17 00:00:00 2001 From: jackiejou <21050234+jackiejou@users.noreply.github.com> Date: Fri, 29 May 2026 14:17:49 -0700 Subject: [PATCH 2/4] fix(adapters): map reply modified_at to updatedAt for edit badge replyToTextMessage was not setting updatedAt on the message, so the edit indicator never appeared on edited replies even though the backend returns modified_at on the comment object. The Reply type also did not declare the field. --- src/@types/model.ts | 1 + .../threadedAnnotationsAdapters-test.ts | 20 +++++++++++++ src/adapters/threadedAnnotationsAdapters.ts | 28 ++++++++++--------- 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/@types/model.ts b/src/@types/model.ts index 1bfaece72..1810ca415 100644 --- a/src/@types/model.ts +++ b/src/@types/model.ts @@ -98,6 +98,7 @@ export interface Reply { created_by: User; id: string; message: string; + modified_at?: string; parent: { id: string; type: string; diff --git a/src/adapters/__tests__/threadedAnnotationsAdapters-test.ts b/src/adapters/__tests__/threadedAnnotationsAdapters-test.ts index 999715b29..7811d7602 100644 --- a/src/adapters/__tests__/threadedAnnotationsAdapters-test.ts +++ b/src/adapters/__tests__/threadedAnnotationsAdapters-test.ts @@ -173,6 +173,26 @@ describe('threadedAnnotationsAdapters', () => { canResolve: true, }); }); + + test('should leave updatedAt undefined when reply has no modified_at', () => { + const result = replyToTextMessage(mockReply); + + expect(result.updatedAt).toBeUndefined(); + }); + + test('should leave updatedAt undefined when modified_at equals created_at', () => { + const reply: Reply = { ...mockReply, modified_at: mockReply.created_at }; + const result = replyToTextMessage(reply); + + expect(result.updatedAt).toBeUndefined(); + }); + + test('should set updatedAt to modified_at instant when reply was edited', () => { + const reply: Reply = { ...mockReply, modified_at: '2026-03-15T11:00:00Z' }; + const result = replyToTextMessage(reply); + + expect(result.updatedAt).toBe(new Date('2026-03-15T11:00:00Z').getTime()); + }); }); describe('annotationToMessages', () => { diff --git a/src/adapters/threadedAnnotationsAdapters.ts b/src/adapters/threadedAnnotationsAdapters.ts index d1080b6bf..7157fcb08 100644 --- a/src/adapters/threadedAnnotationsAdapters.ts +++ b/src/adapters/threadedAnnotationsAdapters.ts @@ -69,6 +69,20 @@ export const deserializeMentionMarkup = (text: string): DocumentNodeV2 => { return { type: 'doc', content }; }; +/** + * Returns the edit timestamp consumers use to render an edited indicator. + * Compares parsed instants, not raw strings, so equivalent ISO formats + * (Z vs +00:00, fractional precision) are treated as unedited. + */ +const toUpdatedAt = (createdAt: string, modifiedAt: string | undefined): number | undefined => { + if (!modifiedAt) return undefined; + const modifiedMs = new Date(modifiedAt).getTime(); + if (Number.isNaN(modifiedMs)) return undefined; + const createdMs = new Date(createdAt).getTime(); + if (modifiedMs === createdMs) return undefined; + return modifiedMs; +}; + /** * Converts a box-annotations Reply to a threaded-annotations TextMessageType. */ @@ -87,21 +101,9 @@ export const replyToTextMessage = (reply: Reply): TextMessageTypeV2 => ({ canReply: reply.permissions?.can_reply ?? false, canResolve: reply.permissions?.can_resolve ?? false, }, + updatedAt: toUpdatedAt(reply.created_at, reply.modified_at), }); -/** - * Returns the edit timestamp consumers use to render an edited indicator. - * Compares parsed instants, not raw strings, so equivalent ISO formats - * (Z vs +00:00, fractional precision) are treated as unedited. - */ -const toUpdatedAt = (createdAt: string, modifiedAt: string): number | undefined => { - const modifiedMs = new Date(modifiedAt).getTime(); - if (Number.isNaN(modifiedMs)) return undefined; - const createdMs = new Date(createdAt).getTime(); - if (modifiedMs === createdMs) return undefined; - return modifiedMs; -}; - // The root message shares the annotation's author and permissions; description // comes back sparse ({ message } only) from the list endpoint. const descriptionToTextMessage = (annotation: Annotation): TextMessageTypeV2 => ({ From ed325993cd5c0f00de2017724307e68ec4abcb2f Mon Sep 17 00:00:00 2001 From: jackiejou <21050234+jackiejou@users.noreply.github.com> Date: Fri, 29 May 2026 14:06:17 -0700 Subject: [PATCH 3/4] feat(eventing): emit lifecycle events for update, delete, reply CRUD box-annotations only emitted lifecycle events for the create flow. Other thunks ran their API calls and updated internal state, but withAnnotations listeners on the box-ui-elements side never fired, so the activity feed went stale until reload. Adds outbound eventing for five thunks following the existing create.ts pattern: - updateAnnotationAction emits annotations_update (covers resolve and unresolve via annotation.status) - deleteAnnotationAction emits annotations_delete - createReplyAction emits annotations_reply_create - updateReplyAction emits annotations_reply_update - deleteReplyAction emits annotations_reply_delete Each handler emits via the singleton eventManager with the { annotation, error, meta: { requestId, status } } envelope. Reply handlers add an annotationReply field carrying the reply object. Generalizes AsyncAction over arg/payload so each handler can type its action precisely. Each success-phase handler short-circuits when the necessary payload is missing instead of emitting a contradictory success event. --- src/@types/events.ts | 5 ++ src/store/eventing/__tests__/delete-test.ts | 61 ++++++++++++++ .../eventing/__tests__/middleware-test.ts | 25 +++++- .../eventing/__tests__/replyCreate-test.ts | 67 +++++++++++++++ .../eventing/__tests__/replyDelete-test.ts | 68 +++++++++++++++ .../eventing/__tests__/replyUpdate-test.ts | 69 ++++++++++++++++ src/store/eventing/__tests__/update-test.ts | 82 +++++++++++++++++++ src/store/eventing/create.ts | 6 +- src/store/eventing/delete.ts | 27 ++++++ src/store/eventing/middleware.ts | 44 +++++++++- src/store/eventing/replyCreate.ts | 34 ++++++++ src/store/eventing/replyDelete.ts | 35 ++++++++ src/store/eventing/replyUpdate.ts | 39 +++++++++ src/store/eventing/types.ts | 8 +- src/store/eventing/update.ts | 29 +++++++ 15 files changed, 587 insertions(+), 12 deletions(-) create mode 100644 src/store/eventing/__tests__/delete-test.ts create mode 100644 src/store/eventing/__tests__/replyCreate-test.ts create mode 100644 src/store/eventing/__tests__/replyDelete-test.ts create mode 100644 src/store/eventing/__tests__/replyUpdate-test.ts create mode 100644 src/store/eventing/__tests__/update-test.ts create mode 100644 src/store/eventing/delete.ts create mode 100644 src/store/eventing/replyCreate.ts create mode 100644 src/store/eventing/replyDelete.ts create mode 100644 src/store/eventing/replyUpdate.ts create mode 100644 src/store/eventing/update.ts diff --git a/src/@types/events.ts b/src/@types/events.ts index a74ef9ad6..2ecb29287 100644 --- a/src/@types/events.ts +++ b/src/@types/events.ts @@ -4,8 +4,13 @@ enum Event { CREATOR_STAGED_CHANGE = 'creator_staged_change', CREATOR_STATUS_CHANGE = 'creator_status_change', ANNOTATION_CREATE = 'annotations_create', + ANNOTATION_DELETE = 'annotations_delete', ANNOTATION_FETCH_ERROR = 'annotations_fetch_error', ANNOTATION_REMOVE = 'annotations_remove', + ANNOTATION_REPLY_CREATE = 'annotations_reply_create', + ANNOTATION_REPLY_DELETE = 'annotations_reply_delete', + ANNOTATION_REPLY_UPDATE = 'annotations_reply_update', + ANNOTATION_UPDATE = 'annotations_update', ANNOTATIONS_INITIALIZED = 'annotations_initialized', ANNOTATIONS_MODE_CHANGE = 'annotations_mode_change', COLOR_SET = 'annotations_color_set', diff --git a/src/store/eventing/__tests__/delete-test.ts b/src/store/eventing/__tests__/delete-test.ts new file mode 100644 index 000000000..acf43c3d6 --- /dev/null +++ b/src/store/eventing/__tests__/delete-test.ts @@ -0,0 +1,61 @@ +import { Action } from '@reduxjs/toolkit'; +import { handleDeleteErrorEvents, handleDeletePendingEvents, handleDeleteSuccessEvents } from '../delete'; +import eventManager from '../../../common/EventManager'; +import { AppState } from '../../types'; + +jest.mock('../../../common/EventManager'); + +describe('store/eventing/delete', () => { + const prevState = {} as AppState; + const nextState = {} as AppState; + + const arg = 'anno_1'; + const action = { + type: 'action', + meta: { arg, requestId: '123' }, + } as Action; + + describe('handleDeleteErrorEvents()', () => { + const error = new Error('foo'); + const actionWithError = { ...action, error } as Action; + + test('should emit delete event with error status and arg id', () => { + handleDeleteErrorEvents(prevState, nextState, actionWithError); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_delete', { + annotation: { id: 'anno_1' }, + error, + meta: { requestId: '123', status: 'error' }, + }); + }); + }); + + describe('handleDeletePendingEvents()', () => { + test('should emit delete event with pending status and arg id', () => { + handleDeletePendingEvents(prevState, nextState, action); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_delete', { + annotation: { id: 'anno_1' }, + meta: { requestId: '123', status: 'pending' }, + }); + }); + }); + + describe('handleDeleteSuccessEvents()', () => { + test('should emit delete event with success status and payload id', () => { + const actionWithPayload = { ...action, payload: 'anno_1' }; + handleDeleteSuccessEvents(prevState, nextState, actionWithPayload); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_delete', { + annotation: { id: 'anno_1' }, + meta: { requestId: '123', status: 'success' }, + }); + }); + + test('should not emit when success arrives with no payload', () => { + handleDeleteSuccessEvents(prevState, nextState, action); + + expect(eventManager.emit).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/src/store/eventing/__tests__/middleware-test.ts b/src/store/eventing/__tests__/middleware-test.ts index 796017dba..d858a4077 100644 --- a/src/store/eventing/__tests__/middleware-test.ts +++ b/src/store/eventing/__tests__/middleware-test.ts @@ -1,4 +1,12 @@ -import getEventingMiddleware from '../middleware'; +import getEventingMiddleware, { eventHandlers } from '../middleware'; +import { + createAnnotationAction, + createReplyAction, + deleteAnnotationAction, + deleteReplyAction, + updateAnnotationAction, + updateReplyAction, +} from '../../annotations/actions'; describe('store/eventing/middleware', () => { describe('getEventingMiddleware()', () => { @@ -29,4 +37,19 @@ describe('store/eventing/middleware', () => { expect(mockHandler).not.toHaveBeenCalled(); }); }); + + describe('eventHandlers registration', () => { + test.each([ + createAnnotationAction, + createReplyAction, + deleteAnnotationAction, + deleteReplyAction, + updateAnnotationAction, + updateReplyAction, + ])('should register fulfilled, pending, and rejected handlers for $typePrefix', thunk => { + expect(eventHandlers).toHaveProperty(thunk.fulfilled.toString()); + expect(eventHandlers).toHaveProperty(thunk.pending.toString()); + expect(eventHandlers).toHaveProperty(thunk.rejected.toString()); + }); + }); }); diff --git a/src/store/eventing/__tests__/replyCreate-test.ts b/src/store/eventing/__tests__/replyCreate-test.ts new file mode 100644 index 000000000..d86c02cfe --- /dev/null +++ b/src/store/eventing/__tests__/replyCreate-test.ts @@ -0,0 +1,67 @@ +import { Action } from '@reduxjs/toolkit'; +import { + handleReplyCreateErrorEvents, + handleReplyCreatePendingEvents, + handleReplyCreateSuccessEvents, +} from '../replyCreate'; +import eventManager from '../../../common/EventManager'; +import { AppState } from '../../types'; + +jest.mock('../../../common/EventManager'); + +describe('store/eventing/replyCreate', () => { + const prevState = {} as AppState; + const nextState = {} as AppState; + + const arg = { annotationId: 'anno_1', message: 'hello' }; + const reply = { id: 'reply_1', message: 'hello' }; + const action = { + type: 'action', + meta: { arg, requestId: '123' }, + } as Action; + + describe('handleReplyCreateErrorEvents()', () => { + const error = new Error('foo'); + const actionWithError = { ...action, error } as Action; + + test('should emit reply create event with error status and arg-derived annotation id', () => { + handleReplyCreateErrorEvents(prevState, nextState, actionWithError); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_reply_create', { + annotation: { id: 'anno_1' }, + error, + meta: { requestId: '123', status: 'error' }, + }); + }); + }); + + describe('handleReplyCreatePendingEvents()', () => { + test('should emit reply create event with pending status and arg-derived annotation id', () => { + handleReplyCreatePendingEvents(prevState, nextState, action); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_reply_create', { + annotation: { id: 'anno_1' }, + meta: { requestId: '123', status: 'pending' }, + }); + }); + }); + + describe('handleReplyCreateSuccessEvents()', () => { + test('should emit reply create event with success status and reply mapped to annotationReply', () => { + const actionWithPayload = { ...action, payload: { annotationId: 'anno_1', reply } }; + handleReplyCreateSuccessEvents(prevState, nextState, actionWithPayload); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_reply_create', { + annotation: { id: 'anno_1' }, + annotationReply: reply, + meta: { requestId: '123', status: 'success' }, + }); + }); + + test('should not emit when success arrives with no reply payload', () => { + handleReplyCreateSuccessEvents(prevState, nextState, action); + + expect(eventManager.emit).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/src/store/eventing/__tests__/replyDelete-test.ts b/src/store/eventing/__tests__/replyDelete-test.ts new file mode 100644 index 000000000..64dcf9e49 --- /dev/null +++ b/src/store/eventing/__tests__/replyDelete-test.ts @@ -0,0 +1,68 @@ +import { Action } from '@reduxjs/toolkit'; +import { + handleReplyDeleteErrorEvents, + handleReplyDeletePendingEvents, + handleReplyDeleteSuccessEvents, +} from '../replyDelete'; +import eventManager from '../../../common/EventManager'; +import { AppState } from '../../types'; + +jest.mock('../../../common/EventManager'); + +describe('store/eventing/replyDelete', () => { + const prevState = {} as AppState; + const nextState = {} as AppState; + + const arg = { annotationId: 'anno_1', replyId: 'reply_1' }; + const action = { + type: 'action', + meta: { arg, requestId: '123' }, + } as Action; + + describe('handleReplyDeleteErrorEvents()', () => { + const error = new Error('foo'); + const actionWithError = { ...action, error } as Action; + + test('should emit reply delete event with error status and arg-derived ids', () => { + handleReplyDeleteErrorEvents(prevState, nextState, actionWithError); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_reply_delete', { + annotation: { id: 'anno_1' }, + annotationReply: { id: 'reply_1' }, + error, + meta: { requestId: '123', status: 'error' }, + }); + }); + }); + + describe('handleReplyDeletePendingEvents()', () => { + test('should emit reply delete event with pending status and arg-derived ids', () => { + handleReplyDeletePendingEvents(prevState, nextState, action); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_reply_delete', { + annotation: { id: 'anno_1' }, + annotationReply: { id: 'reply_1' }, + meta: { requestId: '123', status: 'pending' }, + }); + }); + }); + + describe('handleReplyDeleteSuccessEvents()', () => { + test('should emit reply delete event with success status and payload-derived ids', () => { + const actionWithPayload = { ...action, payload: { annotationId: 'anno_1', replyId: 'reply_1' } }; + handleReplyDeleteSuccessEvents(prevState, nextState, actionWithPayload); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_reply_delete', { + annotation: { id: 'anno_1' }, + annotationReply: { id: 'reply_1' }, + meta: { requestId: '123', status: 'success' }, + }); + }); + + test('should not emit when success arrives with no payload', () => { + handleReplyDeleteSuccessEvents(prevState, nextState, action); + + expect(eventManager.emit).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/src/store/eventing/__tests__/replyUpdate-test.ts b/src/store/eventing/__tests__/replyUpdate-test.ts new file mode 100644 index 000000000..2a99be0a8 --- /dev/null +++ b/src/store/eventing/__tests__/replyUpdate-test.ts @@ -0,0 +1,69 @@ +import { Action } from '@reduxjs/toolkit'; +import { + handleReplyUpdateErrorEvents, + handleReplyUpdatePendingEvents, + handleReplyUpdateSuccessEvents, +} from '../replyUpdate'; +import eventManager from '../../../common/EventManager'; +import { AppState } from '../../types'; + +jest.mock('../../../common/EventManager'); + +describe('store/eventing/replyUpdate', () => { + const prevState = {} as AppState; + const nextState = {} as AppState; + + const arg = { annotationId: 'anno_1', replyId: 'reply_1', payload: { message: 'updated' } }; + const reply = { id: 'reply_1', message: 'updated' }; + const action = { + type: 'action', + meta: { arg, requestId: '123' }, + } as Action; + + describe('handleReplyUpdateErrorEvents()', () => { + const error = new Error('foo'); + const actionWithError = { ...action, error } as Action; + + test('should emit reply update event with error status and arg-derived ids', () => { + handleReplyUpdateErrorEvents(prevState, nextState, actionWithError); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_reply_update', { + annotation: { id: 'anno_1' }, + annotationReply: { id: 'reply_1' }, + error, + meta: { requestId: '123', status: 'error' }, + }); + }); + }); + + describe('handleReplyUpdatePendingEvents()', () => { + test('should emit reply update event with pending status and arg-derived ids', () => { + handleReplyUpdatePendingEvents(prevState, nextState, action); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_reply_update', { + annotation: { id: 'anno_1' }, + annotationReply: { id: 'reply_1' }, + meta: { requestId: '123', status: 'pending' }, + }); + }); + }); + + describe('handleReplyUpdateSuccessEvents()', () => { + test('should emit reply update event with success status and reply mapped to annotationReply', () => { + const actionWithPayload = { ...action, payload: { annotationId: 'anno_1', reply } }; + handleReplyUpdateSuccessEvents(prevState, nextState, actionWithPayload); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_reply_update', { + annotation: { id: 'anno_1' }, + annotationReply: reply, + meta: { requestId: '123', status: 'success' }, + }); + }); + + test('should not emit when success arrives with no reply payload', () => { + handleReplyUpdateSuccessEvents(prevState, nextState, action); + + expect(eventManager.emit).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/src/store/eventing/__tests__/update-test.ts b/src/store/eventing/__tests__/update-test.ts new file mode 100644 index 000000000..d2a4ebc4b --- /dev/null +++ b/src/store/eventing/__tests__/update-test.ts @@ -0,0 +1,82 @@ +import { Action } from '@reduxjs/toolkit'; +import { handleUpdateErrorEvents, handleUpdatePendingEvents, handleUpdateSuccessEvents } from '../update'; +import eventManager from '../../../common/EventManager'; +import { AppState } from '../../types'; +import { annotation as payload } from '../../../region/__mocks__/data'; + +jest.mock('../../../common/EventManager'); + +describe('store/eventing/update', () => { + const prevState = {} as AppState; + const nextState = {} as AppState; + + const arg = { annotationId: 'anno_1', payload: { message: 'updated' } }; + const action = { + type: 'action', + meta: { arg, requestId: '123' }, + } as Action; + + describe('handleUpdateErrorEvents()', () => { + const error = new Error('foo'); + const actionWithError = { ...action, error } as Action; + + test('should emit update event with error status and arg-derived id', () => { + handleUpdateErrorEvents(prevState, nextState, actionWithError); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_update', { + annotation: { id: 'anno_1' }, + error, + meta: { requestId: '123', status: 'error' }, + }); + }); + }); + + describe('handleUpdatePendingEvents()', () => { + test('should emit update event with pending status and arg-derived id', () => { + handleUpdatePendingEvents(prevState, nextState, action); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_update', { + annotation: { id: 'anno_1' }, + meta: { requestId: '123', status: 'pending' }, + }); + }); + }); + + describe('handleUpdateSuccessEvents()', () => { + test('should emit update event with success status and full annotation payload', () => { + const actionWithPayload = { ...action, payload }; + handleUpdateSuccessEvents(prevState, nextState, actionWithPayload); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_update', { + annotation: payload, + meta: { requestId: '123', status: 'success' }, + }); + }); + + test('should not emit when success arrives with no payload', () => { + handleUpdateSuccessEvents(prevState, nextState, action); + + expect(eventManager.emit).not.toHaveBeenCalled(); + }); + + test.each([ + ['resolved', 'resolved'], + ['open', 'unresolved'], + ])('should emit update event with annotation.status=%s for %s flow', annotationStatus => { + const resolvePayload = { ...payload, status: annotationStatus }; + const resolveArg = { annotationId: 'anno_1', payload: { status: annotationStatus } }; + const resolveAction = { + type: 'action', + meta: { arg: resolveArg, requestId: '123' }, + payload: resolvePayload, + } as Action; + + handleUpdateSuccessEvents(prevState, nextState, resolveAction); + + expect(eventManager.emit).toHaveBeenLastCalledWith('annotations_update', { + annotation: expect.objectContaining({ status: annotationStatus }), + meta: { requestId: '123', status: 'success' }, + }); + }); + }); +}); diff --git a/src/store/eventing/create.ts b/src/store/eventing/create.ts index d083a24f1..5096b18f4 100644 --- a/src/store/eventing/create.ts +++ b/src/store/eventing/create.ts @@ -1,9 +1,9 @@ import eventManager from '../../common/EventManager'; import { AsyncAction, Status } from './types'; import { AppState } from '../types'; -import { Event } from '../../@types'; +import { Annotation, Event, NewAnnotation } from '../../@types'; -const emitCreateEvent = (action: AsyncAction, status: Status): void => { +const emitCreateEvent = (action: AsyncAction, status: Status): void => { const { error, meta: { arg, requestId } = {}, payload } = action; eventManager.emit(Event.ANNOTATION_CREATE, { annotation: payload || arg, @@ -16,7 +16,7 @@ const emitCreateEvent = (action: AsyncAction, status: Status): void => { }; const createHandler = (status: Status) => (prevState: AppState, nextState: AppState, action: AsyncAction): void => - emitCreateEvent(action, status); + emitCreateEvent(action as AsyncAction, status); const handleCreateErrorEvents = createHandler(Status.ERROR); const handleCreatePendingEvents = createHandler(Status.PENDING); diff --git a/src/store/eventing/delete.ts b/src/store/eventing/delete.ts new file mode 100644 index 000000000..77500b938 --- /dev/null +++ b/src/store/eventing/delete.ts @@ -0,0 +1,27 @@ +import eventManager from '../../common/EventManager'; +import { AppState } from '../types'; +import { AsyncAction, Status } from './types'; +import { Event } from '../../@types'; + +const emitDeleteEvent = (action: AsyncAction, status: Status): void => { + const { error, meta: { arg, requestId } = {}, payload } = action; + if (status === Status.SUCCESS && !payload) return; + const id = payload ?? arg; + eventManager.emit(Event.ANNOTATION_DELETE, { + annotation: id ? { id } : undefined, + error, + meta: { requestId, status }, + }); +}; + +const deleteHandler = (status: Status) => ( + _prev: AppState, + _next: AppState, + action: AsyncAction, +): void => emitDeleteEvent(action as AsyncAction, status); + +const handleDeleteErrorEvents = deleteHandler(Status.ERROR); +const handleDeletePendingEvents = deleteHandler(Status.PENDING); +const handleDeleteSuccessEvents = deleteHandler(Status.SUCCESS); + +export { handleDeleteErrorEvents, handleDeletePendingEvents, handleDeleteSuccessEvents }; diff --git a/src/store/eventing/middleware.ts b/src/store/eventing/middleware.ts index c216b2733..5d2205984 100644 --- a/src/store/eventing/middleware.ts +++ b/src/store/eventing/middleware.ts @@ -2,28 +2,58 @@ import noop from 'lodash/noop'; import { Action, Dispatch, Middleware, MiddlewareAPI } from '@reduxjs/toolkit'; import { createAnnotationAction, + createReplyAction, + deleteAnnotationAction, + deleteReplyAction, fetchAnnotationsAction, setActiveAnnotationIdAction, setIsInitialized, + updateAnnotationAction, + updateReplyAction, } from '../annotations/actions'; import { navigateBoundingBoxHighlightAction } from '../boundingBoxHighlights/actions'; import { EventHandlerMap } from './types'; import { handleActiveAnnotationEvents } from './active'; import { handleAnnotationsInitialized } from './init'; +import { handleNavigateBoundingBoxHighlight } from './boundingBoxHighlightNav'; import { handleCreateErrorEvents, handleCreatePendingEvents, handleCreateSuccessEvents } from './create'; +import { handleDeleteErrorEvents, handleDeletePendingEvents, handleDeleteSuccessEvents } from './delete'; import { handleFetchErrorEvents } from './fetch'; -import { handleNavigateBoundingBoxHighlight } from './boundingBoxHighlightNav'; +import { handleToggleAnnotationModeAction } from './mode'; +import { + handleReplyCreateErrorEvents, + handleReplyCreatePendingEvents, + handleReplyCreateSuccessEvents, +} from './replyCreate'; +import { + handleReplyDeleteErrorEvents, + handleReplyDeletePendingEvents, + handleReplyDeleteSuccessEvents, +} from './replyDelete'; +import { + handleReplyUpdateErrorEvents, + handleReplyUpdatePendingEvents, + handleReplyUpdateSuccessEvents, +} from './replyUpdate'; import { handleResetCreatorAction, handleSetStagedAction } from './staged'; import { handleSetStatusAction } from './status'; -import { handleToggleAnnotationModeAction } from './mode'; +import { handleUpdateErrorEvents, handleUpdatePendingEvents, handleUpdateSuccessEvents } from './update'; import { resetCreatorAction, setStagedAction, setStatusAction } from '../creator'; import { toggleAnnotationModeAction } from '../common/actions'; -// Array of event handlers based on redux action. To add handling for new events add an entry keyed by action -const eventHandlers: EventHandlerMap = { +export const eventHandlers: EventHandlerMap = { [createAnnotationAction.fulfilled.toString()]: handleCreateSuccessEvents, [createAnnotationAction.pending.toString()]: handleCreatePendingEvents, [createAnnotationAction.rejected.toString()]: handleCreateErrorEvents, + [createReplyAction.fulfilled.toString()]: handleReplyCreateSuccessEvents, + [createReplyAction.pending.toString()]: handleReplyCreatePendingEvents, + [createReplyAction.rejected.toString()]: handleReplyCreateErrorEvents, + [deleteAnnotationAction.fulfilled.toString()]: handleDeleteSuccessEvents, + [deleteAnnotationAction.pending.toString()]: handleDeletePendingEvents, + [deleteAnnotationAction.rejected.toString()]: handleDeleteErrorEvents, + [deleteReplyAction.fulfilled.toString()]: handleReplyDeleteSuccessEvents, + [deleteReplyAction.pending.toString()]: handleReplyDeletePendingEvents, + [deleteReplyAction.rejected.toString()]: handleReplyDeleteErrorEvents, [fetchAnnotationsAction.rejected.toString()]: handleFetchErrorEvents, [navigateBoundingBoxHighlightAction.toString()]: handleNavigateBoundingBoxHighlight, [resetCreatorAction.toString()]: handleResetCreatorAction, @@ -32,6 +62,12 @@ const eventHandlers: EventHandlerMap = { [setStagedAction.toString()]: handleSetStagedAction, [setStatusAction.toString()]: handleSetStatusAction, [toggleAnnotationModeAction.toString()]: handleToggleAnnotationModeAction, + [updateAnnotationAction.fulfilled.toString()]: handleUpdateSuccessEvents, + [updateAnnotationAction.pending.toString()]: handleUpdatePendingEvents, + [updateAnnotationAction.rejected.toString()]: handleUpdateErrorEvents, + [updateReplyAction.fulfilled.toString()]: handleReplyUpdateSuccessEvents, + [updateReplyAction.pending.toString()]: handleReplyUpdatePendingEvents, + [updateReplyAction.rejected.toString()]: handleReplyUpdateErrorEvents, }; function getEventingMiddleware(handlers: EventHandlerMap = eventHandlers): Middleware { diff --git a/src/store/eventing/replyCreate.ts b/src/store/eventing/replyCreate.ts new file mode 100644 index 000000000..29ce74d14 --- /dev/null +++ b/src/store/eventing/replyCreate.ts @@ -0,0 +1,34 @@ +import eventManager from '../../common/EventManager'; +import { Reply , Event } from '../../@types'; +import { AppState } from '../types'; +import { AsyncAction, Status } from './types'; + +type ReplyCreateArg = { annotationId: string; message: string }; +type ReplyCreatePayload = { annotationId: string; reply: Reply }; + +const emitReplyCreateEvent = ( + action: AsyncAction, + status: Status, +): void => { + const { error, meta: { arg, requestId } = {}, payload } = action; + if (status === Status.SUCCESS && !payload?.reply) return; + const annotationId = payload?.annotationId ?? arg?.annotationId; + eventManager.emit(Event.ANNOTATION_REPLY_CREATE, { + annotation: annotationId ? { id: annotationId } : undefined, + annotationReply: payload?.reply, + error, + meta: { requestId, status }, + }); +}; + +const replyCreateHandler = (status: Status) => ( + _prev: AppState, + _next: AppState, + action: AsyncAction, +): void => emitReplyCreateEvent(action as AsyncAction, status); + +const handleReplyCreateErrorEvents = replyCreateHandler(Status.ERROR); +const handleReplyCreatePendingEvents = replyCreateHandler(Status.PENDING); +const handleReplyCreateSuccessEvents = replyCreateHandler(Status.SUCCESS); + +export { handleReplyCreateErrorEvents, handleReplyCreatePendingEvents, handleReplyCreateSuccessEvents }; diff --git a/src/store/eventing/replyDelete.ts b/src/store/eventing/replyDelete.ts new file mode 100644 index 000000000..4cd2d9b5a --- /dev/null +++ b/src/store/eventing/replyDelete.ts @@ -0,0 +1,35 @@ +import eventManager from '../../common/EventManager'; +import { AppState } from '../types'; +import { AsyncAction, Status } from './types'; +import { Event } from '../../@types'; + +type ReplyDeleteArg = { annotationId: string; replyId: string }; +type ReplyDeletePayload = { annotationId: string; replyId: string }; + +const emitReplyDeleteEvent = ( + action: AsyncAction, + status: Status, +): void => { + const { error, meta: { arg, requestId } = {}, payload } = action; + if (status === Status.SUCCESS && !payload) return; + const annotationId = payload?.annotationId ?? arg?.annotationId; + const replyId = payload?.replyId ?? arg?.replyId; + eventManager.emit(Event.ANNOTATION_REPLY_DELETE, { + annotation: annotationId ? { id: annotationId } : undefined, + annotationReply: replyId ? { id: replyId } : undefined, + error, + meta: { requestId, status }, + }); +}; + +const replyDeleteHandler = (status: Status) => ( + _prev: AppState, + _next: AppState, + action: AsyncAction, +): void => emitReplyDeleteEvent(action as AsyncAction, status); + +const handleReplyDeleteErrorEvents = replyDeleteHandler(Status.ERROR); +const handleReplyDeletePendingEvents = replyDeleteHandler(Status.PENDING); +const handleReplyDeleteSuccessEvents = replyDeleteHandler(Status.SUCCESS); + +export { handleReplyDeleteErrorEvents, handleReplyDeletePendingEvents, handleReplyDeleteSuccessEvents }; diff --git a/src/store/eventing/replyUpdate.ts b/src/store/eventing/replyUpdate.ts new file mode 100644 index 000000000..ce345e83a --- /dev/null +++ b/src/store/eventing/replyUpdate.ts @@ -0,0 +1,39 @@ +import eventManager from '../../common/EventManager'; +import { Reply , Event } from '../../@types'; +import { AppState } from '../types'; +import { AsyncAction, Status } from './types'; + +type ReplyUpdateArg = { + annotationId: string; + replyId: string; + payload: { message?: string; status?: string }; +}; +type ReplyUpdatePayload = { annotationId: string; reply: Reply }; + +const emitReplyUpdateEvent = ( + action: AsyncAction, + status: Status, +): void => { + const { error, meta: { arg, requestId } = {}, payload } = action; + if (status === Status.SUCCESS && !payload?.reply) return; + const annotationId = payload?.annotationId ?? arg?.annotationId; + const annotationReply = payload?.reply ?? (arg ? { id: arg.replyId } : undefined); + eventManager.emit(Event.ANNOTATION_REPLY_UPDATE, { + annotation: annotationId ? { id: annotationId } : undefined, + annotationReply, + error, + meta: { requestId, status }, + }); +}; + +const replyUpdateHandler = (status: Status) => ( + _prev: AppState, + _next: AppState, + action: AsyncAction, +): void => emitReplyUpdateEvent(action as AsyncAction, status); + +const handleReplyUpdateErrorEvents = replyUpdateHandler(Status.ERROR); +const handleReplyUpdatePendingEvents = replyUpdateHandler(Status.PENDING); +const handleReplyUpdateSuccessEvents = replyUpdateHandler(Status.SUCCESS); + +export { handleReplyUpdateErrorEvents, handleReplyUpdatePendingEvents, handleReplyUpdateSuccessEvents }; diff --git a/src/store/eventing/types.ts b/src/store/eventing/types.ts index c4dac8ce7..0dea87648 100644 --- a/src/store/eventing/types.ts +++ b/src/store/eventing/types.ts @@ -1,6 +1,6 @@ import { Action, SerializedError } from '@reduxjs/toolkit'; import { AppState } from '../types'; -import { Annotation, NewAnnotation } from '../../@types'; +import { Annotation } from '../../@types'; export enum Status { ERROR = 'error', @@ -22,10 +22,10 @@ export interface ThunkMeta { requestId: string; } -export interface AsyncAction extends Action { +export interface AsyncAction extends Action { error?: SerializedError; - meta?: ThunkMeta; - payload?: Annotation; + meta?: ThunkMeta; + payload?: TPayload; } export type EventHandler = (prevState: AppState, nextState: AppState, action: Action | AsyncAction) => void; diff --git a/src/store/eventing/update.ts b/src/store/eventing/update.ts new file mode 100644 index 000000000..5a3fb97e5 --- /dev/null +++ b/src/store/eventing/update.ts @@ -0,0 +1,29 @@ +import eventManager from '../../common/EventManager'; +import { Annotation , Event } from '../../@types'; +import { AppState } from '../types'; +import { AsyncAction, Status } from './types'; + +type UpdateArg = { annotationId: string; payload: { message?: string; status?: string } }; + +const emitUpdateEvent = (action: AsyncAction, status: Status): void => { + const { error, meta: { arg, requestId } = {}, payload } = action; + if (status === Status.SUCCESS && !payload) return; + const annotation = payload ?? (arg ? { id: arg.annotationId } : undefined); + eventManager.emit(Event.ANNOTATION_UPDATE, { + annotation, + error, + meta: { requestId, status }, + }); +}; + +const updateHandler = (status: Status) => ( + _prev: AppState, + _next: AppState, + action: AsyncAction, +): void => emitUpdateEvent(action as AsyncAction, status); + +const handleUpdateErrorEvents = updateHandler(Status.ERROR); +const handleUpdatePendingEvents = updateHandler(Status.PENDING); +const handleUpdateSuccessEvents = updateHandler(Status.SUCCESS); + +export { handleUpdateErrorEvents, handleUpdatePendingEvents, handleUpdateSuccessEvents }; From f99fb8b267b380519652093be8bf3b873e73051f Mon Sep 17 00:00:00 2001 From: jackiejou <21050234+jackiejou@users.noreply.github.com> Date: Fri, 29 May 2026 15:29:13 -0700 Subject: [PATCH 4/4] feat(annotations): apply sidebar lifecycle events to local store Listens to inbound sidebar.* events emitted by the activity feed and applies them to the document-side redux store, keeping the in-document popup in sync with sidebar-initiated edits, replies, and resolve flows. - New SidebarEvent enum with four entries: SIDEBAR_ANNOTATION_UPDATE, SIDEBAR_REPLY_CREATE, SIDEBAR_REPLY_UPDATE, SIDEBAR_REPLY_DELETE. - Four plain createAction actions (apply*) dispatched on each event. - Four idempotent reducer cases on annotations.byId: merge for annotation update, dedupe-by-id for reply create, in-place merge for reply update, filter-by-id for reply delete. All bail when the parent annotation is missing from local state. - The annotation update reducer skips explicit-undefined payload keys so a sparse update cannot erase fields like permissions or status. - BaseAnnotator subscribes only to the success-phase events and unsubscribes in destroy(). - Annotation delete is unchanged; it continues to flow through the existing annotations_remove listener. - Middleware regression test asserts the new actions are absent from the eventing handler map. --- src/@types/events.ts | 9 +- src/common/BaseAnnotator.ts | 27 ++- src/common/__tests__/BaseAnnotator-test.ts | 38 +++- .../annotations/__tests__/reducer-test.ts | 212 ++++++++++++++++++ src/store/annotations/actions.ts | 17 ++ src/store/annotations/reducer.ts | 36 +++ .../eventing/__tests__/middleware-test.ts | 13 ++ 7 files changed, 349 insertions(+), 3 deletions(-) diff --git a/src/@types/events.ts b/src/@types/events.ts index 2ecb29287..b0857584d 100644 --- a/src/@types/events.ts +++ b/src/@types/events.ts @@ -21,6 +21,13 @@ enum Event { VIEW_MODE_SET = 'view_mode_set', } +enum SidebarEvent { + SIDEBAR_ANNOTATION_UPDATE = 'sidebar.annotations_update', + SIDEBAR_REPLY_CREATE = 'sidebar.annotations_reply_create', + SIDEBAR_REPLY_DELETE = 'sidebar.annotations_reply_delete', + SIDEBAR_REPLY_UPDATE = 'sidebar.annotations_reply_update', +} + // Existing legacy events, don't rename enum LegacyEvent { ANNOTATOR = 'annotatorevent', @@ -28,4 +35,4 @@ enum LegacyEvent { SCALE = 'scaleannotations', } -export { Event, LegacyEvent }; +export { Event, LegacyEvent, SidebarEvent }; diff --git a/src/common/BaseAnnotator.ts b/src/common/BaseAnnotator.ts index 8e980ca96..3459e65a3 100644 --- a/src/common/BaseAnnotator.ts +++ b/src/common/BaseAnnotator.ts @@ -6,7 +6,7 @@ import DeselectManager from './DeselectManager'; import EventEmitter from './EventEmitter'; import i18n from '../utils/i18n'; import messages from '../messages'; -import { Event, IntlOptions, LegacyEvent, Permissions, Token } from '../@types'; +import { Event, IntlOptions, LegacyEvent, Permissions, SidebarEvent, Token } from '../@types'; import { BoundingBox, getBoundingBoxHighlights } from '../store/boundingBoxHighlights'; import { ViewMode } from '../store/options/types'; import { Features } from '../BoxAnnotations'; @@ -131,6 +131,11 @@ export default class BaseAnnotator extends EventEmitter { this.addListener(Event.BOUNDING_BOX_HIGHLIGHT_SELECT, this.handleSelectBoundingBoxHighlight); this.addListener(Event.VIEW_MODE_SET, this.handleSetViewMode); + this.addListener(SidebarEvent.SIDEBAR_ANNOTATION_UPDATE, this.handleSidebarAnnotationUpdate); + this.addListener(SidebarEvent.SIDEBAR_REPLY_CREATE, this.handleSidebarReplyCreate); + this.addListener(SidebarEvent.SIDEBAR_REPLY_UPDATE, this.handleSidebarReplyUpdate); + this.addListener(SidebarEvent.SIDEBAR_REPLY_DELETE, this.handleSidebarReplyDelete); + // Load any required data at startup this.hydrate(); } @@ -164,6 +169,10 @@ export default class BaseAnnotator extends EventEmitter { this.removeListener(Event.BOUNDING_BOX_HIGHLIGHT_NAVIGATE, this.handleNavigateBoundingBoxHighlight); this.removeListener(Event.BOUNDING_BOX_HIGHLIGHT_SELECT, this.handleSelectBoundingBoxHighlight); this.removeListener(Event.VIEW_MODE_SET, this.handleSetViewMode); + this.removeListener(SidebarEvent.SIDEBAR_ANNOTATION_UPDATE, this.handleSidebarAnnotationUpdate); + this.removeListener(SidebarEvent.SIDEBAR_REPLY_CREATE, this.handleSidebarReplyCreate); + this.removeListener(SidebarEvent.SIDEBAR_REPLY_UPDATE, this.handleSidebarReplyUpdate); + this.removeListener(SidebarEvent.SIDEBAR_REPLY_DELETE, this.handleSidebarReplyDelete); } public init(scale = 1, rotation = 0): void { @@ -357,6 +366,22 @@ export default class BaseAnnotator extends EventEmitter { this.setViewMode(viewMode); }; + protected handleSidebarAnnotationUpdate = (annotation: store.SidebarAnnotationUpdatePayload): void => { + this.store.dispatch(store.applySidebarAnnotationUpdateAction(annotation)); + }; + + protected handleSidebarReplyCreate = (payload: store.SidebarReplyMutationPayload): void => { + this.store.dispatch(store.applySidebarReplyCreateAction(payload)); + }; + + protected handleSidebarReplyUpdate = (payload: store.SidebarReplyMutationPayload): void => { + this.store.dispatch(store.applySidebarReplyUpdateAction(payload)); + }; + + protected handleSidebarReplyDelete = ({ annotationId, id }: { annotationId: string; id: string }): void => { + this.store.dispatch(store.applySidebarReplyDeleteAction({ annotationId, replyId: id })); + }; + protected hydrate(): void { // Redux dispatch method signature doesn't seem to like async actions this.store.dispatch(store.fetchAnnotationsAction()); // eslint-disable-line @typescript-eslint/no-explicit-any diff --git a/src/common/__tests__/BaseAnnotator-test.ts b/src/common/__tests__/BaseAnnotator-test.ts index 408bedd4a..b3200cf95 100644 --- a/src/common/__tests__/BaseAnnotator-test.ts +++ b/src/common/__tests__/BaseAnnotator-test.ts @@ -3,7 +3,7 @@ import APIFactory from '../../api'; import BaseAnnotator, { ANNOTATION_CLASSES, CSS_CONTAINER_CLASS, CSS_LOADED_CLASS } from '../BaseAnnotator'; import DeselectManager from '../DeselectManager'; import { ANNOTATOR_EVENT } from '../../constants'; -import { Event, LegacyEvent } from '../../@types'; +import { Event, LegacyEvent, SidebarEvent } from '../../@types'; import { Mode } from '../../store/common'; import { setIsInitialized } from '../../store'; @@ -220,6 +220,10 @@ describe('BaseAnnotator', () => { expect(annotator.removeListener).toBeCalledWith(Event.COLOR_SET, expect.any(Function)); expect(annotator.removeListener).toBeCalledWith(Event.VISIBLE_SET, expect.any(Function)); expect(annotator.removeListener).toBeCalledWith(LegacyEvent.SCALE, expect.any(Function)); + expect(annotator.removeListener).toBeCalledWith(SidebarEvent.SIDEBAR_ANNOTATION_UPDATE, expect.any(Function)); + expect(annotator.removeListener).toBeCalledWith(SidebarEvent.SIDEBAR_REPLY_CREATE, expect.any(Function)); + expect(annotator.removeListener).toBeCalledWith(SidebarEvent.SIDEBAR_REPLY_UPDATE, expect.any(Function)); + expect(annotator.removeListener).toBeCalledWith(SidebarEvent.SIDEBAR_REPLY_DELETE, expect.any(Function)); }); test('should destroy DeselectManager', () => { @@ -284,6 +288,38 @@ describe('BaseAnnotator', () => { annotator.emit(Event.COLOR_SET, '#000'); expect(annotator.setColor).toHaveBeenCalledWith('#000'); }); + + test('should dispatch applySidebarAnnotationUpdate when sidebar emits annotation update', () => { + const partial = { id: 'anno_1', status: 'resolved' as const }; + + annotator.emit(SidebarEvent.SIDEBAR_ANNOTATION_UPDATE, partial); + + expect(annotator.store.dispatch).toHaveBeenCalledWith(store.applySidebarAnnotationUpdateAction(partial)); + }); + + test('should dispatch applySidebarReplyCreate when sidebar emits reply create', () => { + const payload = { annotationId: 'anno_1', reply: { id: 'r1' } as never }; + + annotator.emit(SidebarEvent.SIDEBAR_REPLY_CREATE, payload); + + expect(annotator.store.dispatch).toHaveBeenCalledWith(store.applySidebarReplyCreateAction(payload)); + }); + + test('should dispatch applySidebarReplyUpdate when sidebar emits reply update', () => { + const payload = { annotationId: 'anno_1', reply: { id: 'r1', message: 'updated' } as never }; + + annotator.emit(SidebarEvent.SIDEBAR_REPLY_UPDATE, payload); + + expect(annotator.store.dispatch).toHaveBeenCalledWith(store.applySidebarReplyUpdateAction(payload)); + }); + + test('should translate sidebar emit `id` to action payload `replyId` and dispatch applySidebarReplyDelete', () => { + annotator.emit(SidebarEvent.SIDEBAR_REPLY_DELETE, { annotationId: 'anno_1', id: 'r1' }); + + expect(annotator.store.dispatch).toHaveBeenCalledWith( + store.applySidebarReplyDeleteAction({ annotationId: 'anno_1', replyId: 'r1' }), + ); + }); }); describe('scrollToAnnotation()', () => { diff --git a/src/store/annotations/__tests__/reducer-test.ts b/src/store/annotations/__tests__/reducer-test.ts index 035d6274e..ac6f3f4ea 100644 --- a/src/store/annotations/__tests__/reducer-test.ts +++ b/src/store/annotations/__tests__/reducer-test.ts @@ -3,6 +3,10 @@ import {annotationState as state} from '../__mocks__/annotationsState'; import { Annotation, AnnotationDrawing, NewAnnotation, PathGroup, Reply } from '../../../@types'; import { APICollection } from '../../../api'; import { + applySidebarAnnotationUpdateAction, + applySidebarReplyCreateAction, + applySidebarReplyDeleteAction, + applySidebarReplyUpdateAction, createAnnotationAction, createReplyAction, deleteAnnotationAction, @@ -373,6 +377,214 @@ describe('store/annotations/reducer', () => { }); }); + describe('applySidebarAnnotationUpdateAction', () => { + test('should merge partial annotation into existing state without erasing other fields', () => { + const description = { message: 'original' } as unknown as Reply; + const stateWithDescription = { + ...state, + byId: { + ...state.byId, + test1: { ...state.byId.test1, description, status: 'open' } as unknown as Annotation, + }, + }; + + const newState = reducer( + stateWithDescription, + applySidebarAnnotationUpdateAction({ id: 'test1', status: 'resolved' }), + ); + + expect(newState.byId.test1).toMatchObject({ id: 'test1', description, status: 'resolved' }); + }); + + test('should ignore updates for annotations not in state', () => { + const newState = reducer( + state, + applySidebarAnnotationUpdateAction({ id: 'unknown' }), + ); + + expect(newState.byId).toEqual(state.byId); + }); + + test.each([['resolved' as const], ['open' as const]])( + 'should apply status=%s for resolve/unresolve flow', + annotationStatus => { + const newState = reducer(state, applySidebarAnnotationUpdateAction({ id: 'test1', status: annotationStatus })); + + expect(newState.byId.test1).toMatchObject({ status: annotationStatus }); + }, + ); + + test('should not erase existing fields when payload key value is undefined', () => { + const permissions = { can_delete: true, can_edit: true } as const; + const stateWithPermissions = { + ...state, + byId: { + ...state.byId, + test1: { ...state.byId.test1, permissions, status: 'open' } as unknown as Annotation, + }, + }; + + const newState = reducer( + stateWithPermissions, + applySidebarAnnotationUpdateAction({ + id: 'test1', + status: 'resolved', + permissions: undefined, + } as unknown as ReturnType['payload']), + ); + + expect(newState.byId.test1).toMatchObject({ permissions, status: 'resolved' }); + }); + }); + + describe('applySidebarReplyCreateAction', () => { + const reply = { + created_at: '2026-01-01T00:00:00Z', + created_by: { id: '1', login: 'user@box.com', name: 'User', type: 'user' }, + id: 'reply-1', + message: 'A reply', + parent: { id: 'test1', type: 'annotation' }, + type: 'reply', + } as Reply; + + test('should append the reply when annotation has no replies yet', () => { + const newState = reducer(state, applySidebarReplyCreateAction({ annotationId: 'test1', reply })); + + expect(newState.byId.test1.replies).toHaveLength(1); + expect(newState.byId.test1.replies![0]).toEqual(reply); + }); + + test('should dedupe by reply.id when the same reply is applied twice', () => { + const stateWithReply = { + ...state, + byId: { + ...state.byId, + test1: { ...state.byId.test1, replies: [reply] } as unknown as Annotation, + }, + }; + + const newState = reducer( + stateWithReply, + applySidebarReplyCreateAction({ annotationId: 'test1', reply }), + ); + + expect(newState.byId.test1.replies).toHaveLength(1); + }); + + test('should ignore create for annotations not in state', () => { + const newState = reducer( + state, + applySidebarReplyCreateAction({ annotationId: 'unknown', reply }), + ); + + expect(newState.byId).toEqual(state.byId); + }); + }); + + describe('applySidebarReplyUpdateAction', () => { + const existingReply = { + created_at: '2026-01-01T00:00:00Z', + created_by: { id: '1', login: 'user@box.com', name: 'User', type: 'user' }, + id: 'reply-1', + message: 'old', + parent: { id: 'test1', type: 'annotation' }, + type: 'reply', + } as Reply; + + test('should merge updated fields into the matching reply', () => { + const stateWithReply = { + ...state, + byId: { + ...state.byId, + test1: { ...state.byId.test1, replies: [existingReply] } as unknown as Annotation, + }, + }; + + const updatedReply = { ...existingReply, message: 'new' } as Reply; + const newState = reducer( + stateWithReply, + applySidebarReplyUpdateAction({ annotationId: 'test1', reply: updatedReply }), + ); + + expect(newState.byId.test1.replies![0]).toMatchObject({ id: 'reply-1', message: 'new' }); + }); + + test('should leave other replies untouched', () => { + const otherReply = { ...existingReply, id: 'reply-2', message: 'other' } as Reply; + const stateWithReplies = { + ...state, + byId: { + ...state.byId, + test1: { ...state.byId.test1, replies: [existingReply, otherReply] } as unknown as Annotation, + }, + }; + + const updatedReply = { ...existingReply, message: 'new' } as Reply; + const newState = reducer( + stateWithReplies, + applySidebarReplyUpdateAction({ annotationId: 'test1', reply: updatedReply }), + ); + + expect(newState.byId.test1.replies![1]).toEqual(otherReply); + }); + + test('should ignore update when annotation does not exist', () => { + const updatedReply = { ...existingReply, message: 'new' } as Reply; + const newState = reducer( + state, + applySidebarReplyUpdateAction({ annotationId: 'unknown', reply: updatedReply }), + ); + + expect(newState.byId).toEqual(state.byId); + }); + }); + + describe('applySidebarReplyDeleteAction', () => { + const replyA = { + created_at: '2026-01-01T00:00:00Z', + created_by: { id: '1', login: 'user@box.com', name: 'User', type: 'user' }, + id: 'reply-1', + message: 'first', + parent: { id: 'test1', type: 'annotation' }, + type: 'reply', + } as Reply; + const replyB = { ...replyA, id: 'reply-2', message: 'second' } as Reply; + + test('should remove the targeted reply by id', () => { + const stateWithReplies = { + ...state, + byId: { + ...state.byId, + test1: { ...state.byId.test1, replies: [replyA, replyB] } as unknown as Annotation, + }, + }; + + const newState = reducer( + stateWithReplies, + applySidebarReplyDeleteAction({ annotationId: 'test1', replyId: 'reply-1' }), + ); + + expect(newState.byId.test1.replies).toEqual([replyB]); + }); + + test('should leave replies unchanged when id does not match', () => { + const stateWithReplies = { + ...state, + byId: { + ...state.byId, + test1: { ...state.byId.test1, replies: [replyA] } as unknown as Annotation, + }, + }; + + const newState = reducer( + stateWithReplies, + applySidebarReplyDeleteAction({ annotationId: 'test1', replyId: 'reply-other' }), + ); + + expect(newState.byId.test1.replies).toEqual([replyA]); + }); + }); + describe('setViewModeAction', () => { test('should clear activeId when switching to boundingBoxes mode', () => { const stateWithActiveAnnotation = { diff --git a/src/store/annotations/actions.ts b/src/store/annotations/actions.ts index a87e08aa2..7af817ea2 100644 --- a/src/store/annotations/actions.ts +++ b/src/store/annotations/actions.ts @@ -193,6 +193,23 @@ export const deleteReplyAction = createAsyncThunk< }, ); +export type SidebarAnnotationUpdatePayload = Partial & { id: string }; +export type SidebarReplyMutationPayload = { annotationId: string; reply: Reply }; +export type SidebarReplyDeletePayload = { annotationId: string; replyId: string }; + +export const applySidebarAnnotationUpdateAction = createAction( + 'APPLY_SIDEBAR_ANNOTATION_UPDATE', +); +export const applySidebarReplyCreateAction = createAction( + 'APPLY_SIDEBAR_REPLY_CREATE', +); +export const applySidebarReplyDeleteAction = createAction( + 'APPLY_SIDEBAR_REPLY_DELETE', +); +export const applySidebarReplyUpdateAction = createAction( + 'APPLY_SIDEBAR_REPLY_UPDATE', +); + export const removeAnnotationAction = createAction('REMOVE_ANNOTATION'); export const setActiveAnnotationIdAction = createAction('SET_ACTIVE_ANNOTATION_ID'); export const setIsInitialized = createAction('SET_IS_INITIALIZED'); diff --git a/src/store/annotations/reducer.ts b/src/store/annotations/reducer.ts index 5a868f6d8..98cd8e699 100644 --- a/src/store/annotations/reducer.ts +++ b/src/store/annotations/reducer.ts @@ -2,6 +2,10 @@ import { createReducer, combineReducers } from '@reduxjs/toolkit'; import { formatDrawing, isDrawing } from '../../drawing/drawingUtil'; import { AnnotationsState } from './types'; import { + applySidebarAnnotationUpdateAction, + applySidebarReplyCreateAction, + applySidebarReplyDeleteAction, + applySidebarReplyUpdateAction, createAnnotationAction, createReplyAction, deleteAnnotationAction, @@ -74,6 +78,38 @@ const annotationsById = createReducer({}, builder => annotation.replies = annotation.replies.filter(existing => existing.id !== replyId); } }) + .addCase(applySidebarAnnotationUpdateAction, (state, { payload }) => { + const existing = state[payload.id]; + if (!existing) return; + // Skip explicit-undefined keys so a sparse payload does not erase fields like permissions. + const merged = { ...existing }; + (Object.keys(payload) as Array).forEach(key => { + const value = payload[key]; + if (value !== undefined) { + (merged as Record)[key as string] = value; + } + }); + state[payload.id] = merged; + }) + .addCase(applySidebarReplyCreateAction, (state, { payload: { annotationId, reply } }) => { + const annotation = state[annotationId]; + if (!annotation) return; + const replies = annotation.replies ?? []; + if (replies.some(existing => existing.id === reply.id)) return; + annotation.replies = [...replies, reply]; + }) + .addCase(applySidebarReplyUpdateAction, (state, { payload: { annotationId, reply } }) => { + const annotation = state[annotationId]; + if (!annotation || !annotation.replies) return; + annotation.replies = annotation.replies.map(existing => + existing.id === reply.id ? { ...existing, ...reply } : existing, + ); + }) + .addCase(applySidebarReplyDeleteAction, (state, { payload: { annotationId, replyId } }) => { + const annotation = state[annotationId]; + if (!annotation || !annotation.replies) return; + annotation.replies = annotation.replies.filter(existing => existing.id !== replyId); + }) .addCase(fetchAnnotationsAction.fulfilled, (state, { payload }) => { payload.entries.forEach(annotation => { state[annotation.id] = isDrawing(annotation) ? formatDrawing(annotation) : annotation; diff --git a/src/store/eventing/__tests__/middleware-test.ts b/src/store/eventing/__tests__/middleware-test.ts index d858a4077..8f28ec15e 100644 --- a/src/store/eventing/__tests__/middleware-test.ts +++ b/src/store/eventing/__tests__/middleware-test.ts @@ -1,5 +1,9 @@ import getEventingMiddleware, { eventHandlers } from '../middleware'; import { + applySidebarAnnotationUpdateAction, + applySidebarReplyCreateAction, + applySidebarReplyDeleteAction, + applySidebarReplyUpdateAction, createAnnotationAction, createReplyAction, deleteAnnotationAction, @@ -51,5 +55,14 @@ describe('store/eventing/middleware', () => { expect(eventHandlers).toHaveProperty(thunk.pending.toString()); expect(eventHandlers).toHaveProperty(thunk.rejected.toString()); }); + + test.each([ + applySidebarAnnotationUpdateAction, + applySidebarReplyCreateAction, + applySidebarReplyDeleteAction, + applySidebarReplyUpdateAction, + ])('should NOT register sidebar inbound action $type in the eventing middleware', action => { + expect(eventHandlers).not.toHaveProperty(action.toString()); + }); }); });