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/@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 44607d2c4..7811d7602 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, @@ -158,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 0029056ab..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. */ @@ -83,25 +97,13 @@ 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, }, + 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 => ({ 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..173d391b8 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 (typedFileId in resolved) return literalToString((resolved as TokenMap)[typedFileId]); + return literalToString(resolved as TokenLiteral); +}; + +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..40501d1da 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', () => ({ @@ -77,11 +83,26 @@ jest.mock('../../../store/users/actions', () => ({ const mockUseDispatch = useDispatch as jest.MockedFunction; const mockUseSelector = useSelector as jest.MockedFunction; -const mockSelectorValues = (annotation?: unknown): void => { +type SelectorOverrides = { + annotation?: unknown; + apiHost?: string; + fileId?: string | null; + fileVersionId?: string | null; + token?: unknown; +}; + +const mockSelectorValues = ({ + annotation, + apiHost = 'https://api.box.com', + fileId = '12345', + fileVersionId = 'fv-1', + token = 'test-token', +}: SelectorOverrides = {}): void => { mockUseSelector.mockImplementation(selector => { - if (selector === getApiHost) return 'https://api.box.com'; - if (selector === getFileVersionId) return 'fv-1'; - if (selector === getToken) return 'test-token'; + if (selector === getApiHost) return apiHost; + if (selector === getFileId) return fileId; + if (selector === getFileVersionId) return fileVersionId; + if (selector === getToken) return token; return annotation; }); }; @@ -154,7 +175,7 @@ describe('PopupV2', () => { }; beforeEach(() => { - mockSelectorValues(undefined); + mockSelectorValues(); }); test('should render MessageEditorV2 with FocusTrap and MentionContextProvider', () => { @@ -189,7 +210,7 @@ describe('PopupV2', () => { }; beforeEach(() => { - mockSelectorValues(mockAnnotation); + mockSelectorValues({ annotation: mockAnnotation }); }); test('should render ThreadedAnnotationsV2 with FocusTrap and MentionContextProvider', async () => { @@ -212,7 +233,7 @@ describe('PopupV2', () => { }); test('should render empty messages when annotation is not found', async () => { - mockSelectorValues(undefined); + mockSelectorValues(); render(); await flushPromises(); @@ -243,13 +264,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 () => { @@ -268,12 +315,7 @@ describe('PopupV2', () => { test('should leave onCopyLink undefined when fileVersionId is missing from the store', async () => { const onCopyLink = jest.fn(); - mockUseSelector.mockImplementation(selector => { - if (selector === getApiHost) return 'https://api.box.com'; - if (selector === getFileVersionId) return null; - if (selector === getToken) return 'test-token'; - return mockAnnotation; - }); + mockSelectorValues({ annotation: mockAnnotation, fileVersionId: null }); render( @@ -310,17 +352,53 @@ 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'); + mockSelectorValues({ annotation: mockAnnotation, token: tokenResolver }); + + 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' } }; + mockSelectorValues({ annotation: mockAnnotation, token: tokenMap }); + + 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 () => { + mockSelectorValues({ annotation: mockAnnotation, fileId: null }); + + render(); + await flushPromises(); + + expect(mockFetch).not.toHaveBeenCalled(); + }); }); test('should set aria-label on popup container', () => { - mockSelectorValues(undefined); + mockSelectorValues(); render(); expect(screen.getByRole('presentation')).toHaveAttribute('aria-label', 'Comment'); }); test('should render portal container for threaded-annotations popovers', () => { - mockSelectorValues(undefined); + mockSelectorValues(); render(); const portal = screen.getByRole('presentation').querySelector('[data-threaded-annotations-portal]'); @@ -328,7 +406,7 @@ describe('PopupV2', () => { }); test('should render popup into popupPortalEl, not the render container', () => { - mockSelectorValues(undefined); + mockSelectorValues(); const portalEl = makePortalEl(); const { container } = render( , @@ -339,7 +417,7 @@ describe('PopupV2', () => { }); test('should render nothing when popupPortalEl is missing', () => { - mockSelectorValues(undefined); + mockSelectorValues(); const { container } = render( , ); diff --git a/src/store/annotations/__tests__/actions-test.ts b/src/store/annotations/__tests__/actions-test.ts index 7b78d9494..458c74ae4 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,98 @@ 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'); + }); + + 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'); + }); + + 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; };