From de3d60f894a64bf76dc169392dc000357adc3dad Mon Sep 17 00:00:00 2001 From: Jackie Jou Date: Thu, 14 May 2026 13:15:27 -0700 Subject: [PATCH 1/7] feat(activity-feed-v2): Wire edit, copy link, scroll to end - Bump @box/activity-feed to 1.19.4 and @box/threaded-annotations to 1.89.1 with matching blueprint, collaboration-popover, readable-time, and user-selector peer versions - Route the new ThreadedAnnotation onEdit to onCommentUpdate, onAnnotationEdit, or onReplyUpdate based on message id, and pass isEditDisabled when the thread is disabled or resolved - Add onCommentCopyLink and onAnnotationCopyLink pass-throughs with { id, rootId } so hosts can build permalinks - Pass page number alongside highlightedText for highlight annotation badges so they can fall back to the page - Scroll the activity tab to the deep-linked entry on reload, or to the last visible row when no deep link is active; both retry until the target row is mounted and run once thereafter - Log via console.error and onEditError for malformed editor content and missing reply permissions, matching the repo errorCallback pattern --- package.json | 16 +- .../content-sidebar/ActivitySidebar.js | 8 + .../activity-feed-v2/ActivityFeedV2.tsx | 38 ++- .../activity-feed-v2/FeedItemRow.tsx | 110 ++++++++- .../__tests__/ActivityFeedV2.test.tsx | 92 ++++++- .../__tests__/FeedItemRow.test.tsx | 232 +++++++++++++++++- .../content-sidebar/activity-feed-v2/types.ts | 11 + yarn.lock | 32 +-- 8 files changed, 494 insertions(+), 45 deletions(-) diff --git a/package.json b/package.json index dacd33945d..53cf776312 100644 --- a/package.json +++ b/package.json @@ -124,14 +124,14 @@ "@babel/preset-typescript": "^7.24.7", "@babel/template": "^7.24.7", "@babel/types": "^7.24.7", - "@box/activity-feed": "^1.18.3", + "@box/activity-feed": "^1.19.4", "@box/blueprint-web": "^14.30.1", "@box/blueprint-web-assets": "^4.117.3", "@box/box-ai-agent-selector": "^1.39.21", "@box/box-ai-content-answers": "^1.43.22", "@box/box-item-type-selector": "^1.39.21", "@box/cldr-data": "^34.2.0", - "@box/collaboration-popover": "^1.61.30", + "@box/collaboration-popover": "^1.62.3", "@box/combobox-with-api": "^1.42.22", "@box/copy-input": "^1.42.5", "@box/content-field": "^1.40.23", @@ -142,8 +142,8 @@ "@box/metadata-filter": "^1.80.23", "@box/metadata-view": "^1.53.26", "@box/react-virtualized": "^9.22.3-rc-box.10", - "@box/readable-time": "^1.40.30", - "@box/threaded-annotations": "^1.86.0", + "@box/readable-time": "^1.41.3", + "@box/threaded-annotations": "^1.89.1", "@box/types": "^2.1.8", "@box/unified-share-modal": "^2.15.5", "@box/user-selector": "^1.76.5", @@ -292,14 +292,14 @@ "webpack-dev-server": "^5.2.3" }, "peerDependencies": { - "@box/activity-feed": "^1.18.3", + "@box/activity-feed": "^1.19.4", "@box/blueprint-web": "^14.30.1", "@box/blueprint-web-assets": "^4.117.3", "@box/box-ai-agent-selector": "^1.39.21", "@box/box-ai-content-answers": "^1.43.22", "@box/box-item-type-selector": "^1.39.21", "@box/cldr-data": ">=34.2.0", - "@box/collaboration-popover": "^1.61.30", + "@box/collaboration-popover": "^1.62.3", "@box/combobox-with-api": "^1.42.22", "@box/copy-input": "^1.42.5", "@box/content-field": "^1.40.23", @@ -308,8 +308,8 @@ "@box/metadata-filter": "^1.80.23", "@box/metadata-view": "^1.53.26", "@box/react-virtualized": "^9.22.3-rc-box.10", - "@box/readable-time": "^1.40.30", - "@box/threaded-annotations": "^1.86.0", + "@box/readable-time": "^1.41.3", + "@box/threaded-annotations": "^1.89.1", "@box/types": "^2.1.8", "@box/unified-share-modal": "^2.15.5", "@box/user-selector": "^1.76.5", diff --git a/src/elements/content-sidebar/ActivitySidebar.js b/src/elements/content-sidebar/ActivitySidebar.js index 7fe66cb29e..650c72939f 100644 --- a/src/elements/content-sidebar/ActivitySidebar.js +++ b/src/elements/content-sidebar/ActivitySidebar.js @@ -92,6 +92,8 @@ type ExternalProps = { hasVersions?: boolean, internalSidebarNavigation?: InternalSidebarNavigation, internalSidebarNavigationHandler?: InternalSidebarNavigationHandler, + onAnnotationCopyLink?: (params: { id: string, rootId: string }) => void, + onCommentCopyLink?: (params: { id: string, rootId: string }) => void, onCommentCreate: Function, onCommentDelete: (comment: Comment) => any, onCommentUpdate: () => any, @@ -1371,6 +1373,8 @@ class ActivitySidebar extends React.PureComponent { hasReplies, hasVersions, isDisabled = false, + onAnnotationCopyLink, + onCommentCopyLink, onVersionHistoryClick, getUserProfileUrl, onTaskView, @@ -1401,13 +1405,17 @@ class ActivitySidebar extends React.PureComponent { getMentionAsync={this.getMentionAsync} hasTasks={this.props.hasTasks} isDisabled={isDisabled} + onAnnotationCopyLink={onAnnotationCopyLink} onAnnotationDelete={this.handleAnnotationDelete} + onAnnotationEdit={this.handleAnnotationEdit} onAnnotationSelect={this.handleAnnotationSelect} onAnnotationStatusChange={this.handleAnnotationStatusChange} + onCommentCopyLink={onCommentCopyLink} onCommentCreate={this.createComment} onCommentDelete={this.deleteComment} onCommentUpdate={this.updateComment} onReplyCreate={this.createReply} + onReplyUpdate={this.updateReply} onTaskDelete={this.deleteTask} onTaskView={onTaskView} onVersionHistoryClick={onVersionHistoryClick} diff --git a/src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx b/src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx index ae756d0060..38435f43ba 100644 --- a/src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx +++ b/src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx @@ -38,13 +38,17 @@ const ActivityFeedV2 = ({ getMentionAsync, hasTasks = true, isDisabled = false, + onAnnotationCopyLink, onAnnotationDelete, + onAnnotationEdit, onAnnotationSelect, onAnnotationStatusChange, + onCommentCopyLink, onCommentCreate, onCommentDelete, onCommentUpdate, onReplyCreate, + onReplyUpdate, onTaskDelete, onTaskView, onVersionHistoryClick, @@ -54,11 +58,8 @@ const ActivityFeedV2 = ({ const currentUserId = currentUser?.id; const headerTitle = intl.formatMessage(commonMessages.sidebarActivityTitle); - React.useEffect(() => { - if (activeFeedEntryId && scrollHandle) { - scrollHandle.scrollTo(activeFeedEntryId); - } - }, [activeFeedEntryId, scrollHandle]); + const scrolledEntryIdRef = React.useRef(null); + const hasScrolledToEndRef = React.useRef(false); const fetchUsers = React.useCallback( async (inputValue: string): Promise => { @@ -180,6 +181,29 @@ const ActivityFeedV2 = ({ }); }, [currentUserId, mentionMe, showResolved, transformedItems]); + React.useEffect(() => { + const alreadyScrolledToThisEntry = scrolledEntryIdRef.current === activeFeedEntryId; + if (!activeFeedEntryId || !scrollHandle || alreadyScrolledToThisEntry) { + return; + } + const didScroll = scrollHandle.scrollTo(activeFeedEntryId); + if (didScroll) { + scrolledEntryIdRef.current = activeFeedEntryId; + } + }, [activeFeedEntryId, filteredItems, scrollHandle]); + + React.useEffect(() => { + const hasDeepLink = Boolean(activeFeedEntryId); + if (hasScrolledToEndRef.current || hasDeepLink || !scrollHandle || filteredItems.length === 0) { + return; + } + const lastItemId = filteredItems[filteredItems.length - 1].id; + const didScroll = scrollHandle.scrollTo(lastItemId); + if (didScroll) { + hasScrolledToEndRef.current = true; + } + }, [activeFeedEntryId, filteredItems, scrollHandle]); + const handleCommentPost = React.useCallback( async (content: unknown) => { if (!onCommentCreate) return; @@ -231,12 +255,16 @@ const ActivityFeedV2 = ({ currentUserId={currentUserId} isDisabled={isDisabled} item={item} + onAnnotationCopyLink={onAnnotationCopyLink} onAnnotationDelete={onAnnotationDelete} + onAnnotationEdit={onAnnotationEdit} onAnnotationSelect={onAnnotationSelect} onAnnotationStatusChange={onAnnotationStatusChange} + onCommentCopyLink={onCommentCopyLink} onCommentDelete={onCommentDelete} onCommentUpdate={onCommentUpdate} onReplyCreate={onReplyCreate} + onReplyUpdate={onReplyUpdate} onTaskDelete={onTaskDelete} onTaskView={onTaskView} onVersionHistoryClick={onVersionHistoryClick} diff --git a/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx b/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx index 6f0cf32cc3..e50c9d44a8 100644 --- a/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx +++ b/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx @@ -13,21 +13,31 @@ import type { Annotation, AnnotationPermission, Target } from '../../../common/t import type { BoxCommentPermission, CommentFeedItemType, FeedItemStatus } from '../../../common/types/feed'; import type { TaskNew } from '../../../common/types/tasks'; -import type { AnnotationBadgeTargetType, TransformedFeedItem, UserSelectorProps } from './types'; +import type { + AnnotationBadgeTargetType, + TransformedCommentItem, + TransformedFeedItem, + UserSelectorProps, +} from './types'; import { FEED_ITEM_TYPE_ANNOTATION, FEED_ITEM_TYPE_COMMENT } from '../../../constants'; +type EditorContent = Parameters[0]; + type FeedItemRowProps = { currentUserId?: string; isDisabled: boolean; item: TransformedFeedItem; + onAnnotationCopyLink?: (params: { id: string; rootId: string }) => void; onAnnotationDelete?: (params: { id: string; permissions: AnnotationPermission }) => void; + onAnnotationEdit?: (params: { id: string; permissions: AnnotationPermission; text: string }) => void; onAnnotationSelect?: (annotation: Annotation) => void; onAnnotationStatusChange?: (params: { id: string; permissions: AnnotationPermission; status: FeedItemStatus; }) => void; + onCommentCopyLink?: (params: { id: string; rootId: string }) => void; onCommentDelete?: (params: { id: string; permissions: BoxCommentPermission }) => void; onCommentUpdate?: ( id: string, @@ -39,6 +49,14 @@ type FeedItemRowProps = { onError?: (() => void) | null, ) => void; onReplyCreate?: (parentId: string, parentType: CommentFeedItemType, text: string) => void; + onReplyUpdate?: ( + id: string, + parentId: string, + text: string, + permissions: BoxCommentPermission, + onSuccess?: (() => void) | null, + onError?: (() => void) | null, + ) => void; onTaskDelete?: (task: TaskNew) => void; onTaskView?: (id: string, isCreator: boolean) => void; onVersionHistoryClick?: (version: { id: string; version_number: number }) => void; @@ -55,7 +73,7 @@ const annotationTargetToBadge = (target?: Target): AnnotationBadgeTargetType | u case 'drawing': return { page, type: AnnotationBadgeType.Drawing }; case 'highlight': - return { highlightedText: '', type: AnnotationBadgeType.Highlight }; + return { highlightedText: '', page, type: AnnotationBadgeType.Highlight }; case 'point': return { page, type: AnnotationBadgeType.Point }; case 'region': @@ -65,6 +83,16 @@ const annotationTargetToBadge = (target?: Target): AnnotationBadgeTargetType | u } }; +const serializeEditorContent = (content: unknown): { hasMention: boolean; text: string } | null => { + try { + return serializeMentionMarkup(content as EditorContent); + } catch (error) { + // eslint-disable-next-line no-console + console.error('ActivityFeedV2: failed to serialize editor content', error); + return null; + } +}; + const buildReplyPost = ( parentId: string, @@ -74,26 +102,46 @@ const buildReplyPost = ) => async (content: unknown) => { if (isDisabled || !onReplyCreate) return; - let serialized; - try { - serialized = serializeMentionMarkup(content as Parameters[0]); - } catch { - return; - } - if (!serialized.text.trim()) return; + const serialized = serializeEditorContent(content); + if (!serialized || !serialized.text.trim()) return; onReplyCreate(parentId, parentType, serialized.text); }; +const findMessagePermissions = ( + messages: TransformedCommentItem['messages'], + id: string, +): BoxCommentPermission | undefined => { + const message = messages.find(m => m.id === id); + if (!message) return undefined; + const { canDelete, canEdit, canReply, canResolve } = message.permissions; + return { + can_delete: canDelete, + can_edit: canEdit, + can_reply: canReply, + can_resolve: canResolve, + }; +}; + +const logEditError = (error: unknown): string | undefined => { + // eslint-disable-next-line no-console + console.error('ActivityFeedV2: failed to save edit', error); + return undefined; +}; + const FeedItemRow = ({ currentUserId, isDisabled, item, + onAnnotationCopyLink, onAnnotationDelete, + onAnnotationEdit, onAnnotationSelect, onAnnotationStatusChange, + onCommentCopyLink, onCommentDelete, onCommentUpdate, onReplyCreate, + onReplyUpdate, onTaskDelete, onTaskView, onVersionHistoryClick, @@ -110,14 +158,36 @@ const FeedItemRow = ({ if (isDisabled) return; onCommentUpdate?.(id, item.originalText, status, false, permissions); }; + const handleEdit = (id: string, content: unknown) => { + if (isDisabled) return; + const serialized = serializeEditorContent(content); + if (!serialized) return; + if (id === item.id) { + onCommentUpdate?.(id, serialized.text, undefined, serialized.hasMention, permissions); + return; + } + const replyPermissions = findMessagePermissions(item.messages, id); + if (!replyPermissions) { + // eslint-disable-next-line no-console + console.error(`ActivityFeedV2: no permissions found for reply "${id}" in thread "${item.id}"`); + return; + } + onReplyUpdate?.(id, item.id, serialized.text, replyPermissions); + }; return ( onCommentCopyLink({ id, rootId: item.id }) : undefined + } onDelete={handleDelete} + onEdit={handleEdit} + onEditError={logEditError} onPost={buildReplyPost(item.id, FEED_ITEM_TYPE_COMMENT, isDisabled, onReplyCreate)} onResolve={handleStatusChange('resolved')} onThreadDelete={() => handleDelete(item.id)} @@ -139,16 +209,38 @@ const FeedItemRow = ({ if (isDisabled) return; onAnnotationStatusChange?.({ id, permissions, status }); }; + const handleEdit = (id: string, content: unknown) => { + if (isDisabled) return; + const serialized = serializeEditorContent(content); + if (!serialized) return; + if (id === item.id) { + onAnnotationEdit?.({ id, permissions, text: serialized.text }); + return; + } + const replyPermissions = findMessagePermissions(item.messages, id); + if (!replyPermissions) { + // eslint-disable-next-line no-console + console.error(`ActivityFeedV2: no permissions found for reply "${id}" in thread "${item.id}"`); + return; + } + onReplyUpdate?.(id, item.id, serialized.text, replyPermissions); + }; return ( onAnnotationSelect?.(item.annotation)} onAvatarClick={noop} + onCopyLink={ + onAnnotationCopyLink ? (id: string) => onAnnotationCopyLink({ id, rootId: item.id }) : undefined + } onDelete={handleDelete} + onEdit={handleEdit} + onEditError={logEditError} onPost={buildReplyPost(item.id, FEED_ITEM_TYPE_ANNOTATION, isDisabled, onReplyCreate)} onResolve={handleStatusChange('resolved')} onThreadDelete={() => handleDelete(item.id)} diff --git a/src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx b/src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx index b93999c058..adb2059454 100644 --- a/src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx +++ b/src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx @@ -14,6 +14,8 @@ jest.mock('@box/threaded-annotations', () => ({ serializeMentionMarkup: (doc: unknown) => ({ hasMention: false, text: JSON.stringify(doc) }), })); +const mockScrollTo = jest.fn(() => true); + jest.mock('@box/activity-feed', () => { const actual = jest.requireActual('@box/activity-feed'); const ActivityFeedRoot = ({ children }: { children: React.ReactNode }) => ( @@ -41,7 +43,7 @@ jest.mock('@box/activity-feed', () => { List: ActivityFeedList, Root: ActivityFeedRoot, }, - useActivityFeedScroll: () => ({ scrollTo: jest.fn() }), + useActivityFeedScroll: () => ({ scrollTo: mockScrollTo }), }; }); @@ -129,6 +131,10 @@ const feedItems = [ ] as ActivityFeedV2Props['feedItems']; describe('elements/content-sidebar/activity-feed-v2/ActivityFeedV2', () => { + beforeEach(() => { + mockScrollTo.mockReturnValue(true); + }); + afterEach(() => { jest.clearAllMocks(); }); @@ -296,4 +302,88 @@ describe('elements/content-sidebar/activity-feed-v2/ActivityFeedV2', () => { ); expect(screen.getByTestId('activity-feed-root')).toBeVisible(); }); + + describe('scroll to end on mount', () => { + test('should scroll to the last rendered items id when the tab opens', () => { + render(); + expect(mockScrollTo).toHaveBeenCalledWith('app-activity-1'); + }); + + test('should not scroll when activeFeedEntryId is set (deep link takes precedence)', () => { + render( + , + ); + expect(mockScrollTo).toHaveBeenCalledWith('comment-1'); + expect(mockScrollTo).not.toHaveBeenCalledWith('app-activity-1'); + }); + + test('should retry scroll-to-entry on later renders when first attempt returns false', () => { + mockScrollTo.mockReturnValueOnce(false).mockReturnValue(true); + const { rerender } = render( + , + ); + expect(mockScrollTo).toHaveBeenCalledWith('annotation-1'); + expect(mockScrollTo).toHaveBeenCalledTimes(1); + + rerender( + , + ); + expect(mockScrollTo).toHaveBeenCalledTimes(2); + expect(mockScrollTo).toHaveBeenLastCalledWith('annotation-1'); + }); + + test('should not re-scroll to scroll-to-entry target after success', () => { + const { rerender } = render( + , + ); + expect(mockScrollTo).toHaveBeenCalledTimes(1); + + rerender( + , + ); + expect(mockScrollTo).toHaveBeenCalledTimes(1); + }); + + test('should not scroll when feedItems is empty', () => { + render(); + expect(mockScrollTo).not.toHaveBeenCalled(); + }); + + test('should scroll to the last visible row after filters remove the tail', () => { + const trailingAppActivity = { ...mockAppActivity, id: 'app-activity-2' }; + const resolvedCommentAfterTail = { ...mockComment, id: 'resolved-last', status: 'resolved' }; + render( + , + ); + expect(mockScrollTo).toHaveBeenLastCalledWith('app-activity-2'); + }); + + test('should retry scroll-to-end on later renders when first attempt returns false', () => { + mockScrollTo.mockReturnValueOnce(false).mockReturnValue(true); + const { rerender } = render(); + expect(mockScrollTo).toHaveBeenCalledTimes(1); + rerender(); + expect(mockScrollTo).toHaveBeenCalledTimes(2); + }); + + test('should not re-scroll to the end after a successful scroll when filters change', () => { + const { rerender } = render(); + expect(mockScrollTo).toHaveBeenCalledTimes(1); + rerender(); + expect(mockScrollTo).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx b/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx index b3118fbcc1..35b05e2ee5 100644 --- a/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx +++ b/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx @@ -45,9 +45,11 @@ jest.mock('@box/threaded-annotations', () => ({ Point: 'point', Region: 'region', }, - serializeMentionMarkup: () => ({ hasMention: false, text: 'serialized-text' }), + serializeMentionMarkup: jest.fn(() => ({ hasMention: false, text: 'serialized-text' })), })); +const mockedSerialize = jest.mocked(jest.requireMock('@box/threaded-annotations').serializeMentionMarkup); + const userSelectorProps: UserSelectorProps = { ariaRoleDescription: 'user selector', fetchAvatarUrls: () => Promise.resolve({}), @@ -57,6 +59,8 @@ const userSelectorProps: UserSelectorProps = { const commentPermissions = { can_delete: true, can_edit: true, can_reply: true, can_resolve: true }; +const replyPermissions = { canDelete: true, canEdit: true, canReply: false, canResolve: false }; + const mockComment: TransformedCommentItem = { id: 'comment-1', isResolved: false, @@ -68,6 +72,13 @@ const mockComment: TransformedCommentItem = { author: { name: 'User', id: 1, email: 'u@b.com' }, permissions: { canDelete: true, canEdit: true, canReply: true, canResolve: true }, }, + { + id: 'reply-1', + message: { type: 'doc', content: [] }, + createdAt: 0, + author: { name: 'User', id: 1, email: 'u@b.com' }, + permissions: replyPermissions, + }, ], originalText: 'Hello world', permissions: commentPermissions, @@ -99,6 +110,13 @@ const mockAnnotation: TransformedAnnotationItem = { author: { name: 'User', id: 1, email: 'u@b.com' }, permissions: { canDelete: true, canEdit: true, canReply: true, canResolve: true }, }, + { + id: 'annotation-reply-1', + message: { type: 'doc', content: [] }, + createdAt: 0, + author: { name: 'User', id: 1, email: 'u@b.com' }, + permissions: replyPermissions, + }, ], permissions: annotationPermissions, type: 'annotation', @@ -168,6 +186,7 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { lastThreadedAnnotationProps = {}; lastTaskProps = {}; lastVersionProps = {}; + mockedSerialize.mockReturnValue({ hasMention: false, text: 'serialized-text' }); }); afterEach(() => { @@ -232,6 +251,123 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { expect(onReplyCreate).toHaveBeenCalledWith('comment-1', 'comment', 'serialized-text'); }); + + test('should pass isEditDisabled=false when not disabled and not resolved', () => { + render(); + expect(lastThreadedAnnotationProps.isEditDisabled).toBe(false); + }); + + test('should pass isEditDisabled=true when resolved', () => { + render(); + expect(lastThreadedAnnotationProps.isEditDisabled).toBe(true); + }); + + test('should route onEdit of root id to onCommentUpdate with serialized text and hasMention', () => { + mockedSerialize.mockReturnValue({ hasMention: true, text: 'edited-text' }); + const onCommentUpdate = jest.fn(); + render(); + + lastThreadedAnnotationProps.onEdit?.('comment-1', { type: 'doc', content: [] }); + + expect(onCommentUpdate).toHaveBeenCalledWith( + 'comment-1', + 'edited-text', + undefined, + true, + commentPermissions, + ); + }); + + test('should route onEdit of reply id to onReplyUpdate with reply permissions', () => { + mockedSerialize.mockReturnValue({ hasMention: false, text: 'edited-reply' }); + const onReplyUpdate = jest.fn(); + const onCommentUpdate = jest.fn(); + render( + , + ); + + lastThreadedAnnotationProps.onEdit?.('reply-1', { type: 'doc', content: [] }); + + expect(onReplyUpdate).toHaveBeenCalledWith('reply-1', 'comment-1', 'edited-reply', { + can_delete: true, + can_edit: true, + can_reply: false, + can_resolve: false, + }); + expect(onCommentUpdate).not.toHaveBeenCalled(); + }); + + test('should pass onCopyLink when onCommentCopyLink is provided', () => { + const onCommentCopyLink = jest.fn(); + render(); + + lastThreadedAnnotationProps.onCopyLink?.('reply-1'); + + expect(onCommentCopyLink).toHaveBeenCalledWith({ id: 'reply-1', rootId: 'comment-1' }); + }); + + test('should omit onCopyLink when onCommentCopyLink is not provided', () => { + render(); + expect(lastThreadedAnnotationProps.onCopyLink).toBeUndefined(); + }); + + test('should log and skip onEdit when serialize fails', () => { + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined); + mockedSerialize.mockImplementation(() => { + throw new Error('bad content'); + }); + const onCommentUpdate = jest.fn(); + const onReplyUpdate = jest.fn(); + render( + , + ); + + lastThreadedAnnotationProps.onEdit?.('comment-1', { type: 'doc', content: [] }); + + expect(onCommentUpdate).not.toHaveBeenCalled(); + expect(onReplyUpdate).not.toHaveBeenCalled(); + expect(consoleError).toHaveBeenCalledWith( + 'ActivityFeedV2: failed to serialize editor content', + expect.any(Error), + ); + consoleError.mockRestore(); + }); + + test('should log and skip onEdit when a reply id has no resolvable permissions', () => { + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined); + const onReplyUpdate = jest.fn(); + const orphanReplyComment = { ...mockComment, messages: [mockComment.messages[0]] }; + render(); + + lastThreadedAnnotationProps.onEdit?.('reply-1', { type: 'doc', content: [] }); + + expect(onReplyUpdate).not.toHaveBeenCalled(); + expect(consoleError).toHaveBeenCalledWith( + 'ActivityFeedV2: no permissions found for reply "reply-1" in thread "comment-1"', + ); + consoleError.mockRestore(); + }); + + test('should log via onEditError so failed edits leave a trail', () => { + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined); + render(); + + const message = lastThreadedAnnotationProps.onEditError?.(new Error('save failed')); + + expect(message).toBeUndefined(); + expect(consoleError).toHaveBeenCalledWith('ActivityFeedV2: failed to save edit', expect.any(Error)); + consoleError.mockRestore(); + }); }); describe('annotation rendering', () => { @@ -338,16 +474,20 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { expect(lastThreadedAnnotationProps.annotationTarget).toEqual({ page: 1, type: 'drawing' }); }); - test('should pass highlight badge with empty text for highlight annotation target', () => { + test('should pass highlight badge with page number for highlight annotation target', () => { const highlightAnnotation: TransformedAnnotationItem = { ...mockAnnotation, annotation: { ...mockAnnotation.annotation, - target: { location: { type: 'page', value: 1 }, type: 'highlight' }, + target: { location: { type: 'page', value: 4 }, type: 'highlight' }, } as TransformedAnnotationItem['annotation'], }; render(); - expect(lastThreadedAnnotationProps.annotationTarget).toEqual({ highlightedText: '', type: 'highlight' }); + expect(lastThreadedAnnotationProps.annotationTarget).toEqual({ + highlightedText: '', + page: 4, + type: 'highlight', + }); }); test('should pass undefined badge for unknown annotation target type', () => { @@ -370,6 +510,63 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { expect(onReplyCreate).toHaveBeenCalledWith('annotation-1', 'annotation', 'serialized-text'); }); + + test('should pass isEditDisabled=true when resolved', () => { + render(); + expect(lastThreadedAnnotationProps.isEditDisabled).toBe(true); + }); + + test('should route onEdit of root id to onAnnotationEdit with serialized text', () => { + mockedSerialize.mockReturnValue({ hasMention: false, text: 'edited-annotation' }); + const onAnnotationEdit = jest.fn(); + render(); + + lastThreadedAnnotationProps.onEdit?.('annotation-1', { type: 'doc', content: [] }); + + expect(onAnnotationEdit).toHaveBeenCalledWith({ + id: 'annotation-1', + permissions: annotationPermissions, + text: 'edited-annotation', + }); + }); + + test('should route onEdit of reply id to onReplyUpdate with reply permissions', () => { + mockedSerialize.mockReturnValue({ hasMention: false, text: 'edited-reply' }); + const onAnnotationEdit = jest.fn(); + const onReplyUpdate = jest.fn(); + render( + , + ); + + lastThreadedAnnotationProps.onEdit?.('annotation-reply-1', { type: 'doc', content: [] }); + + expect(onReplyUpdate).toHaveBeenCalledWith('annotation-reply-1', 'annotation-1', 'edited-reply', { + can_delete: true, + can_edit: true, + can_reply: false, + can_resolve: false, + }); + expect(onAnnotationEdit).not.toHaveBeenCalled(); + }); + + test('should pass onCopyLink when onAnnotationCopyLink is provided', () => { + const onAnnotationCopyLink = jest.fn(); + render(); + + lastThreadedAnnotationProps.onCopyLink?.('annotation-1'); + + expect(onAnnotationCopyLink).toHaveBeenCalledWith({ id: 'annotation-1', rootId: 'annotation-1' }); + }); + + test('should omit onCopyLink when onAnnotationCopyLink is not provided', () => { + render(); + expect(lastThreadedAnnotationProps.onCopyLink).toBeUndefined(); + }); }); describe('task rendering', () => { @@ -433,10 +630,11 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { }); describe('isDisabled', () => { - test('should not fire comment mutation callbacks (delete, resolve, unresolve, reply) when isDisabled', async () => { + test('should not fire comment mutation callbacks (delete, resolve, unresolve, reply, edit) when isDisabled', async () => { const onCommentDelete = jest.fn(); const onCommentUpdate = jest.fn(); const onReplyCreate = jest.fn(); + const onReplyUpdate = jest.fn(); render( { onCommentDelete={onCommentDelete} onCommentUpdate={onCommentUpdate} onReplyCreate={onReplyCreate} + onReplyUpdate={onReplyUpdate} />, ); @@ -453,24 +652,32 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { lastThreadedAnnotationProps.onResolve?.('comment-1'); lastThreadedAnnotationProps.onUnresolve?.('comment-1'); await lastThreadedAnnotationProps.onPost?.({ type: 'doc', content: [] }); + lastThreadedAnnotationProps.onEdit?.('comment-1', { type: 'doc', content: [] }); + lastThreadedAnnotationProps.onEdit?.('reply-1', { type: 'doc', content: [] }); expect(onCommentDelete).not.toHaveBeenCalled(); expect(onCommentUpdate).not.toHaveBeenCalled(); expect(onReplyCreate).not.toHaveBeenCalled(); + expect(onReplyUpdate).not.toHaveBeenCalled(); + expect(lastThreadedAnnotationProps.isEditDisabled).toBe(true); }); - test('should not fire annotation mutation callbacks (delete, resolve, unresolve, reply) when isDisabled', async () => { + test('should not fire annotation mutation callbacks (delete, resolve, unresolve, reply, edit) when isDisabled', async () => { const onAnnotationDelete = jest.fn(); + const onAnnotationEdit = jest.fn(); const onAnnotationStatusChange = jest.fn(); const onReplyCreate = jest.fn(); + const onReplyUpdate = jest.fn(); render( , ); @@ -479,10 +686,15 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { lastThreadedAnnotationProps.onResolve?.('annotation-1'); lastThreadedAnnotationProps.onUnresolve?.('annotation-1'); await lastThreadedAnnotationProps.onPost?.({ type: 'doc', content: [] }); + lastThreadedAnnotationProps.onEdit?.('annotation-1', { type: 'doc', content: [] }); + lastThreadedAnnotationProps.onEdit?.('annotation-reply-1', { type: 'doc', content: [] }); expect(onAnnotationDelete).not.toHaveBeenCalled(); + expect(onAnnotationEdit).not.toHaveBeenCalled(); expect(onAnnotationStatusChange).not.toHaveBeenCalled(); expect(onReplyCreate).not.toHaveBeenCalled(); + expect(onReplyUpdate).not.toHaveBeenCalled(); + expect(lastThreadedAnnotationProps.isEditDisabled).toBe(true); }); }); @@ -494,6 +706,8 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { expect(() => lastThreadedAnnotationProps.onResolve?.('c1')).not.toThrow(); expect(() => lastThreadedAnnotationProps.onUnresolve?.('c1')).not.toThrow(); expect(() => lastThreadedAnnotationProps.onThreadDelete?.()).not.toThrow(); + expect(() => lastThreadedAnnotationProps.onEdit?.('comment-1', { type: 'doc', content: [] })).not.toThrow(); + expect(() => lastThreadedAnnotationProps.onEdit?.('reply-1', { type: 'doc', content: [] })).not.toThrow(); }); test('should not throw when annotation callbacks are not provided', () => { @@ -503,6 +717,12 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { expect(() => lastThreadedAnnotationProps.onResolve?.('a1')).not.toThrow(); expect(() => lastThreadedAnnotationProps.onAnnotationBadgeClick?.('a1')).not.toThrow(); expect(() => lastThreadedAnnotationProps.onThreadDelete?.()).not.toThrow(); + expect(() => + lastThreadedAnnotationProps.onEdit?.('annotation-1', { type: 'doc', content: [] }), + ).not.toThrow(); + expect(() => + lastThreadedAnnotationProps.onEdit?.('annotation-reply-1', { type: 'doc', content: [] }), + ).not.toThrow(); }); }); }); diff --git a/src/elements/content-sidebar/activity-feed-v2/types.ts b/src/elements/content-sidebar/activity-feed-v2/types.ts index c858c7318a..8b66548fe5 100644 --- a/src/elements/content-sidebar/activity-feed-v2/types.ts +++ b/src/elements/content-sidebar/activity-feed-v2/types.ts @@ -70,13 +70,16 @@ export type ActivityFeedV2Props = { getMentionAsync?: (searchStr: string) => Promise>>; hasTasks?: boolean; isDisabled?: boolean; + onAnnotationCopyLink?: (params: { id: string; rootId: string }) => void; onAnnotationDelete?: (params: { id: string; permissions: AnnotationPermission }) => void; + onAnnotationEdit?: (params: { id: string; permissions: AnnotationPermission; text: string }) => void; onAnnotationSelect?: (annotation: Annotation) => void; onAnnotationStatusChange?: (params: { id: string; permissions: AnnotationPermission; status: FeedItemStatus; }) => void; + onCommentCopyLink?: (params: { id: string; rootId: string }) => void; onCommentCreate?: (text: string, hasMention: boolean) => void; onCommentDelete?: (params: { id: string; permissions: BoxCommentPermission }) => void; onCommentUpdate?: ( @@ -89,6 +92,14 @@ export type ActivityFeedV2Props = { onError?: (() => void) | null, ) => void; onReplyCreate?: (parentId: string, parentType: CommentFeedItemType, text: string) => void; + onReplyUpdate?: ( + id: string, + parentId: string, + text: string, + permissions: BoxCommentPermission, + onSuccess?: (() => void) | null, + onError?: (() => void) | null, + ) => void; onTaskDelete?: (task: TaskNew) => void; onTaskView?: (id: string, isCreator: boolean) => void; onVersionHistoryClick?: (version: { id: string; version_number: number }) => void; diff --git a/yarn.lock b/yarn.lock index 5b9b5b6b95..3c34fa2942 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1067,10 +1067,10 @@ resolved "https://registry.yarnpkg.com/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz#75a2e8b51cb758a7553d6804a5932d7aace75c39" integrity sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw== -"@box/activity-feed@^1.18.3": - version "1.18.3" - resolved "https://registry.yarnpkg.com/@box/activity-feed/-/activity-feed-1.18.3.tgz#d97d13c4fcb73424bf11ee6d1a773fc3ba7dab35" - integrity sha512-zFWpCRldecWDnAvgaKfNzdA87C7UND0+xIXYCviSeOfEs9+2Zqg86WGmwjwWu6ItX2wQhxVrxZs7ffRAchanNg== +"@box/activity-feed@^1.19.4": + version "1.19.4" + resolved "https://registry.yarnpkg.com/@box/activity-feed/-/activity-feed-1.19.4.tgz#7e24f48f0f7eefdd9377b75886061d482c921a9b" + integrity sha512-O4arzJZNPdqCkULyKfA3W2BtuKFlvcBWUmjlb9eob3lADVqROuoFEMAEXqLKx3QOeYvBeqbOfiQ42mLLCWpNsA== "@box/blueprint-web-assets@^4.117.3": version "4.117.3" @@ -1136,10 +1136,10 @@ resolved "https://registry.yarnpkg.com/@box/cldr-data/-/cldr-data-34.8.0.tgz#36e6ddcea8e20653326aba2e0d13e07f34b7704f" integrity sha512-jsTnhhpFy/eMossMr3cP9+1VFqOxOzO1GX/csw0LzasPl0Dg2Jhn8ypeNTBnFwlOB2Dp6XoxdvQHkBG4eVQe/Q== -"@box/collaboration-popover@^1.61.30": - version "1.61.30" - resolved "https://registry.yarnpkg.com/@box/collaboration-popover/-/collaboration-popover-1.61.30.tgz#269a82fabd9b574e673f4cbf408b003e62e1dfa7" - integrity sha512-vdbJCDEOhy8OstSAbDG8DDxRyYDtc8cAOr0WKV74hrK0QPHIEy3nY68Gah9iyvDe3iIJQ7WTo8JkacBRoONKvw== +"@box/collaboration-popover@^1.62.3": + version "1.62.3" + resolved "https://registry.yarnpkg.com/@box/collaboration-popover/-/collaboration-popover-1.62.3.tgz#b1cea34df5df771373af57ad0a27dc85a830a3e6" + integrity sha512-eu71Hra6p4QPwaVjQbvR36oLbyukIGSt+nEVvSQzM3pnL+KwD05JJEn2OFnzrPaQUQgUV0bhYzBRFvOADzLS4w== "@box/combobox-with-api@^1.42.22": version "1.42.22" @@ -1205,15 +1205,15 @@ prop-types "^15.7.2" react-lifecycles-compat "^3.0.4" -"@box/readable-time@^1.40.30": - version "1.40.30" - resolved "https://registry.yarnpkg.com/@box/readable-time/-/readable-time-1.40.30.tgz#dc9a8a4b146532b191b519a2300b21713a6dca6d" - integrity sha512-uvMvH4Bl2e/6jNP/MadvckvWSgHuc3xnZD59fhZtdRYlYgi/e+eaGRHjOYJm0shDoMEDBnUeyHbEZpbkq0yu9g== +"@box/readable-time@^1.41.3": + version "1.41.3" + resolved "https://registry.yarnpkg.com/@box/readable-time/-/readable-time-1.41.3.tgz#423d19e3bb97b08f233a3a1543586dc619b953ff" + integrity sha512-6igs7l8gxXcG8f3fQ+uSl+KE+d3Z+rFU2oyWITw7WoswzQn0w3shezKa27vLT/7Rxa22yxzzkJXUEPZ/j7gIHw== -"@box/threaded-annotations@^1.86.0": - version "1.86.0" - resolved "https://registry.yarnpkg.com/@box/threaded-annotations/-/threaded-annotations-1.86.0.tgz#405e26d2741abfc0862444dea4d792f46cab7e5a" - integrity sha512-0b2xdKCLPfEodG5tLuTe/ErmdHSGWEUR9XDP9DDDrurolRh2eH8lmf/3GznJlFSmuUUUr5xhQTJEos6ZBfB1Nw== +"@box/threaded-annotations@^1.89.1": + version "1.89.1" + resolved "https://registry.yarnpkg.com/@box/threaded-annotations/-/threaded-annotations-1.89.1.tgz#c777560d04150fa4c4039fbeef11edcd59fb2c47" + integrity sha512-vqWhinWEriUjzw1iqVA6/PYN8FI2RBDWaOqIzS5qRgLldr/prwbyjTLGnORvYWPQ0Hsx+1dXjuR2KjCh6pFZBA== dependencies: "@tanstack/react-virtual" "^3.10.8" "@tiptap/core" "2.0.2" From 536f8a13518123069e76ea4f6a754dd0750370b3 Mon Sep 17 00:00:00 2001 From: Jackie Jou Date: Thu, 14 May 2026 13:56:19 -0700 Subject: [PATCH 2/7] refactor(activity-feed-v2): Convert onReplyUpdate to object params Align onReplyUpdate with the other new callbacks (onAnnotationCopyLink, onAnnotationEdit, onCommentCopyLink) which take a single params object. ActivitySidebar adapts the object back to its legacy positional updateReply signature at the prop-passing site. --- .../content-sidebar/ActivitySidebar.js | 4 +++- .../activity-feed-v2/FeedItemRow.tsx | 20 +++++++++---------- .../__tests__/FeedItemRow.test.tsx | 20 +++++++++---------- .../content-sidebar/activity-feed-v2/types.ts | 16 +++++++-------- 4 files changed, 31 insertions(+), 29 deletions(-) diff --git a/src/elements/content-sidebar/ActivitySidebar.js b/src/elements/content-sidebar/ActivitySidebar.js index 650c72939f..ad35db2a69 100644 --- a/src/elements/content-sidebar/ActivitySidebar.js +++ b/src/elements/content-sidebar/ActivitySidebar.js @@ -1415,7 +1415,9 @@ class ActivitySidebar extends React.PureComponent { onCommentDelete={this.deleteComment} onCommentUpdate={this.updateComment} onReplyCreate={this.createReply} - onReplyUpdate={this.updateReply} + onReplyUpdate={({ id, onError, onSuccess, parentId, permissions, text }) => + this.updateReply(id, parentId, text, permissions, onSuccess, onError) + } onTaskDelete={this.deleteTask} onTaskView={onTaskView} onVersionHistoryClick={onVersionHistoryClick} diff --git a/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx b/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx index e50c9d44a8..672f137dd7 100644 --- a/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx +++ b/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx @@ -49,14 +49,14 @@ type FeedItemRowProps = { onError?: (() => void) | null, ) => void; onReplyCreate?: (parentId: string, parentType: CommentFeedItemType, text: string) => void; - onReplyUpdate?: ( - id: string, - parentId: string, - text: string, - permissions: BoxCommentPermission, - onSuccess?: (() => void) | null, - onError?: (() => void) | null, - ) => void; + onReplyUpdate?: (params: { + id: string; + onError?: () => void; + onSuccess?: () => void; + parentId: string; + permissions: BoxCommentPermission; + text: string; + }) => void; onTaskDelete?: (task: TaskNew) => void; onTaskView?: (id: string, isCreator: boolean) => void; onVersionHistoryClick?: (version: { id: string; version_number: number }) => void; @@ -172,7 +172,7 @@ const FeedItemRow = ({ console.error(`ActivityFeedV2: no permissions found for reply "${id}" in thread "${item.id}"`); return; } - onReplyUpdate?.(id, item.id, serialized.text, replyPermissions); + onReplyUpdate?.({ id, parentId: item.id, permissions: replyPermissions, text: serialized.text }); }; return ( { lastThreadedAnnotationProps.onEdit?.('reply-1', { type: 'doc', content: [] }); - expect(onReplyUpdate).toHaveBeenCalledWith('reply-1', 'comment-1', 'edited-reply', { - can_delete: true, - can_edit: true, - can_reply: false, - can_resolve: false, + expect(onReplyUpdate).toHaveBeenCalledWith({ + id: 'reply-1', + parentId: 'comment-1', + permissions: { can_delete: true, can_edit: true, can_reply: false, can_resolve: false }, + text: 'edited-reply', }); expect(onCommentUpdate).not.toHaveBeenCalled(); }); @@ -545,11 +545,11 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { lastThreadedAnnotationProps.onEdit?.('annotation-reply-1', { type: 'doc', content: [] }); - expect(onReplyUpdate).toHaveBeenCalledWith('annotation-reply-1', 'annotation-1', 'edited-reply', { - can_delete: true, - can_edit: true, - can_reply: false, - can_resolve: false, + expect(onReplyUpdate).toHaveBeenCalledWith({ + id: 'annotation-reply-1', + parentId: 'annotation-1', + permissions: { can_delete: true, can_edit: true, can_reply: false, can_resolve: false }, + text: 'edited-reply', }); expect(onAnnotationEdit).not.toHaveBeenCalled(); }); diff --git a/src/elements/content-sidebar/activity-feed-v2/types.ts b/src/elements/content-sidebar/activity-feed-v2/types.ts index 8b66548fe5..bfd1dd68cc 100644 --- a/src/elements/content-sidebar/activity-feed-v2/types.ts +++ b/src/elements/content-sidebar/activity-feed-v2/types.ts @@ -92,14 +92,14 @@ export type ActivityFeedV2Props = { onError?: (() => void) | null, ) => void; onReplyCreate?: (parentId: string, parentType: CommentFeedItemType, text: string) => void; - onReplyUpdate?: ( - id: string, - parentId: string, - text: string, - permissions: BoxCommentPermission, - onSuccess?: (() => void) | null, - onError?: (() => void) | null, - ) => void; + onReplyUpdate?: (params: { + id: string; + onError?: () => void; + onSuccess?: () => void; + parentId: string; + permissions: BoxCommentPermission; + text: string; + }) => void; onTaskDelete?: (task: TaskNew) => void; onTaskView?: (id: string, isCreator: boolean) => void; onVersionHistoryClick?: (version: { id: string; version_number: number }) => void; From 5ae7b51cb33d371b0c95244d797b582a4a467823 Mon Sep 17 00:00:00 2001 From: Jackie Jou Date: Thu, 14 May 2026 14:18:52 -0700 Subject: [PATCH 3/7] fix(activity-feed-v2): Block whitespace-only edits before dispatch Matches the existing guard in buildReplyPost so comment, annotation, and reply edits with only whitespace (or empty content) no longer fire onCommentUpdate, onAnnotationEdit, or onReplyUpdate. --- .../activity-feed-v2/FeedItemRow.tsx | 4 +- .../__tests__/FeedItemRow.test.tsx | 40 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx b/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx index 672f137dd7..d248974073 100644 --- a/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx +++ b/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx @@ -161,7 +161,7 @@ const FeedItemRow = ({ const handleEdit = (id: string, content: unknown) => { if (isDisabled) return; const serialized = serializeEditorContent(content); - if (!serialized) return; + if (!serialized || !serialized.text.trim()) return; if (id === item.id) { onCommentUpdate?.(id, serialized.text, undefined, serialized.hasMention, permissions); return; @@ -212,7 +212,7 @@ const FeedItemRow = ({ const handleEdit = (id: string, content: unknown) => { if (isDisabled) return; const serialized = serializeEditorContent(content); - if (!serialized) return; + if (!serialized || !serialized.text.trim()) return; if (id === item.id) { onAnnotationEdit?.({ id, permissions, text: serialized.text }); return; diff --git a/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx b/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx index 441190a4c5..368789207c 100644 --- a/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx +++ b/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx @@ -343,6 +343,26 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { consoleError.mockRestore(); }); + test('should skip onEdit when serialized text is whitespace only', () => { + mockedSerialize.mockReturnValue({ hasMention: false, text: ' \n\t ' }); + const onCommentUpdate = jest.fn(); + const onReplyUpdate = jest.fn(); + render( + , + ); + + lastThreadedAnnotationProps.onEdit?.('comment-1', { type: 'doc', content: [] }); + lastThreadedAnnotationProps.onEdit?.('reply-1', { type: 'doc', content: [] }); + + expect(onCommentUpdate).not.toHaveBeenCalled(); + expect(onReplyUpdate).not.toHaveBeenCalled(); + }); + test('should log and skip onEdit when a reply id has no resolvable permissions', () => { const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined); const onReplyUpdate = jest.fn(); @@ -567,6 +587,26 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { render(); expect(lastThreadedAnnotationProps.onCopyLink).toBeUndefined(); }); + + test('should skip onEdit when serialized text is whitespace only', () => { + mockedSerialize.mockReturnValue({ hasMention: false, text: ' \n\t ' }); + const onAnnotationEdit = jest.fn(); + const onReplyUpdate = jest.fn(); + render( + , + ); + + lastThreadedAnnotationProps.onEdit?.('annotation-1', { type: 'doc', content: [] }); + lastThreadedAnnotationProps.onEdit?.('annotation-reply-1', { type: 'doc', content: [] }); + + expect(onAnnotationEdit).not.toHaveBeenCalled(); + expect(onReplyUpdate).not.toHaveBeenCalled(); + }); }); describe('task rendering', () => { From d8ac042cc4c374846cddca9ddc7bd42d84f6b1e1 Mon Sep 17 00:00:00 2001 From: Jackie Jou Date: Thu, 14 May 2026 16:18:05 -0700 Subject: [PATCH 4/7] refactor(activity-feed-v2): Rename copy-link id to annotationId Annotation deep links target the thread root regardless of which message was clicked, so the field is renamed from id to annotationId to make the root-only contract self-documenting alongside the per-message id used by onCommentCopyLink. --- .../content-sidebar/ActivitySidebar.js | 4 +-- .../activity-feed-v2/FeedItemRow.tsx | 16 +++++++----- .../__tests__/FeedItemRow.test.tsx | 26 +++++++++++++++---- .../content-sidebar/activity-feed-v2/types.ts | 4 +-- 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/elements/content-sidebar/ActivitySidebar.js b/src/elements/content-sidebar/ActivitySidebar.js index ad35db2a69..1a606a61fe 100644 --- a/src/elements/content-sidebar/ActivitySidebar.js +++ b/src/elements/content-sidebar/ActivitySidebar.js @@ -92,8 +92,8 @@ type ExternalProps = { hasVersions?: boolean, internalSidebarNavigation?: InternalSidebarNavigation, internalSidebarNavigationHandler?: InternalSidebarNavigationHandler, - onAnnotationCopyLink?: (params: { id: string, rootId: string }) => void, - onCommentCopyLink?: (params: { id: string, rootId: string }) => void, + onAnnotationCopyLink?: (params: { annotationId: string, fileVersionId: string }) => void, + onCommentCopyLink?: (params: { id: string }) => void, onCommentCreate: Function, onCommentDelete: (comment: Comment) => any, onCommentUpdate: () => any, diff --git a/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx b/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx index d248974073..f72074e505 100644 --- a/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx +++ b/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx @@ -28,7 +28,7 @@ type FeedItemRowProps = { currentUserId?: string; isDisabled: boolean; item: TransformedFeedItem; - onAnnotationCopyLink?: (params: { id: string; rootId: string }) => void; + onAnnotationCopyLink?: (params: { annotationId: string; fileVersionId: string }) => void; onAnnotationDelete?: (params: { id: string; permissions: AnnotationPermission }) => void; onAnnotationEdit?: (params: { id: string; permissions: AnnotationPermission; text: string }) => void; onAnnotationSelect?: (annotation: Annotation) => void; @@ -37,7 +37,7 @@ type FeedItemRowProps = { permissions: AnnotationPermission; status: FeedItemStatus; }) => void; - onCommentCopyLink?: (params: { id: string; rootId: string }) => void; + onCommentCopyLink?: (params: { id: string }) => void; onCommentDelete?: (params: { id: string; permissions: BoxCommentPermission }) => void; onCommentUpdate?: ( id: string, @@ -182,9 +182,7 @@ const FeedItemRow = ({ isResolved={item.isResolved} messages={item.messages} onAvatarClick={noop} - onCopyLink={ - onCommentCopyLink ? (id: string) => onCommentCopyLink({ id, rootId: item.id }) : undefined - } + onCopyLink={onCommentCopyLink ? (id: string) => onCommentCopyLink({ id }) : undefined} onDelete={handleDelete} onEdit={handleEdit} onEditError={logEditError} @@ -236,7 +234,13 @@ const FeedItemRow = ({ onAnnotationBadgeClick={() => onAnnotationSelect?.(item.annotation)} onAvatarClick={noop} onCopyLink={ - onAnnotationCopyLink ? (id: string) => onAnnotationCopyLink({ id, rootId: item.id }) : undefined + onAnnotationCopyLink && item.annotation.file_version?.id + ? () => + onAnnotationCopyLink({ + annotationId: item.id, + fileVersionId: item.annotation.file_version.id, + }) + : undefined } onDelete={handleDelete} onEdit={handleEdit} diff --git a/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx b/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx index 368789207c..02e1e610b0 100644 --- a/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx +++ b/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx @@ -302,13 +302,13 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { expect(onCommentUpdate).not.toHaveBeenCalled(); }); - test('should pass onCopyLink when onCommentCopyLink is provided', () => { + test('should pass onCopyLink with the clicked message id when onCommentCopyLink is provided', () => { const onCommentCopyLink = jest.fn(); render(); lastThreadedAnnotationProps.onCopyLink?.('reply-1'); - expect(onCommentCopyLink).toHaveBeenCalledWith({ id: 'reply-1', rootId: 'comment-1' }); + expect(onCommentCopyLink).toHaveBeenCalledWith({ id: 'reply-1' }); }); test('should omit onCopyLink when onCommentCopyLink is not provided', () => { @@ -574,13 +574,13 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { expect(onAnnotationEdit).not.toHaveBeenCalled(); }); - test('should pass onCopyLink when onAnnotationCopyLink is provided', () => { + test('should pass onCopyLink with annotationId (thread root) and fileVersionId regardless of which message was clicked', () => { const onAnnotationCopyLink = jest.fn(); render(); - lastThreadedAnnotationProps.onCopyLink?.('annotation-1'); + lastThreadedAnnotationProps.onCopyLink?.('annotation-reply-1'); - expect(onAnnotationCopyLink).toHaveBeenCalledWith({ id: 'annotation-1', rootId: 'annotation-1' }); + expect(onAnnotationCopyLink).toHaveBeenCalledWith({ annotationId: 'annotation-1', fileVersionId: 'fv1' }); }); test('should omit onCopyLink when onAnnotationCopyLink is not provided', () => { @@ -588,6 +588,22 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { expect(lastThreadedAnnotationProps.onCopyLink).toBeUndefined(); }); + test('should omit onCopyLink when annotation has no file_version', () => { + const annotationWithoutVersion = { + ...mockAnnotation, + annotation: { ...mockAnnotation.annotation, file_version: null }, + }; + const onAnnotationCopyLink = jest.fn(); + render( + , + ); + expect(lastThreadedAnnotationProps.onCopyLink).toBeUndefined(); + }); + test('should skip onEdit when serialized text is whitespace only', () => { mockedSerialize.mockReturnValue({ hasMention: false, text: ' \n\t ' }); const onAnnotationEdit = jest.fn(); diff --git a/src/elements/content-sidebar/activity-feed-v2/types.ts b/src/elements/content-sidebar/activity-feed-v2/types.ts index bfd1dd68cc..c67da576d5 100644 --- a/src/elements/content-sidebar/activity-feed-v2/types.ts +++ b/src/elements/content-sidebar/activity-feed-v2/types.ts @@ -70,7 +70,7 @@ export type ActivityFeedV2Props = { getMentionAsync?: (searchStr: string) => Promise>>; hasTasks?: boolean; isDisabled?: boolean; - onAnnotationCopyLink?: (params: { id: string; rootId: string }) => void; + onAnnotationCopyLink?: (params: { annotationId: string; fileVersionId: string }) => void; onAnnotationDelete?: (params: { id: string; permissions: AnnotationPermission }) => void; onAnnotationEdit?: (params: { id: string; permissions: AnnotationPermission; text: string }) => void; onAnnotationSelect?: (annotation: Annotation) => void; @@ -79,7 +79,7 @@ export type ActivityFeedV2Props = { permissions: AnnotationPermission; status: FeedItemStatus; }) => void; - onCommentCopyLink?: (params: { id: string; rootId: string }) => void; + onCommentCopyLink?: (params: { id: string }) => void; onCommentCreate?: (text: string, hasMention: boolean) => void; onCommentDelete?: (params: { id: string; permissions: BoxCommentPermission }) => void; onCommentUpdate?: ( From dee6bdc3a1f2866214e84ad2c75388f0a78b1499 Mon Sep 17 00:00:00 2001 From: Jackie Jou Date: Thu, 14 May 2026 21:38:53 -0700 Subject: [PATCH 5/7] refactor(activity-feed-v2): Extract row helpers into helpers.ts - Move serializeEditorContent, findMessagePermissions, logEditError, and the new dispatchReplyEdit (object-param) into helpers.ts; FeedItemRow now imports them and tests mock them via the standard jest.mocked pattern - Move annotationTargetToBadge into transformers.ts where the other shape converters live, and exhaust the Target discriminant directly instead of casting through string - Extend the Target type with TargetDrawing and TargetHighlight to match the runtime shapes that box-annotations emits, and pass through the highlight target text so the badge can display the excerpt - Promote OnReplyUpdate to a named export so types.ts, FeedItemRow, and helpers all reference one definition - Narrow logEditError return type to undefined and reuse vendor's ReturnType for serializeEditorContent so the shapes track upstream - Split tests across helpers.test.ts (helper internals), transformers test (badge mapping), and FeedItemRow tests (row-level wiring) --- src/common/types/annotations.js | 13 +- .../activity-feed-v2/FeedItemRow.tsx | 112 +++---------- .../__tests__/FeedItemRow.test.tsx | 155 +++++------------- .../__tests__/helpers.test.ts | 119 ++++++++++++++ .../__tests__/transformers.test.ts | 66 ++++++++ .../activity-feed-v2/helpers.ts | 65 ++++++++ .../activity-feed-v2/transformers.ts | 30 +++- .../content-sidebar/activity-feed-v2/types.ts | 18 +- 8 files changed, 364 insertions(+), 214 deletions(-) create mode 100644 src/elements/content-sidebar/activity-feed-v2/__tests__/helpers.test.ts create mode 100644 src/elements/content-sidebar/activity-feed-v2/helpers.ts diff --git a/src/common/types/annotations.js b/src/common/types/annotations.js index a07f4840dc..815277a699 100644 --- a/src/common/types/annotations.js +++ b/src/common/types/annotations.js @@ -23,6 +23,17 @@ export type Rect = { y: number, }; +export type TargetDrawing = { + location: Page, + type: 'drawing', +}; + +export type TargetHighlight = { + location: Page, + text?: string, + type: 'highlight', +}; + export type TargetRegion = { location: Page, shape?: Rect, @@ -36,7 +47,7 @@ export type TargetPoint = { y: number, }; -export type Target = TargetRegion | TargetPoint; +export type Target = TargetDrawing | TargetHighlight | TargetPoint | TargetRegion; export type AnnotationPermission = { can_delete?: boolean, diff --git a/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx b/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx index f72074e505..b4d82ed8ee 100644 --- a/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx +++ b/src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx @@ -7,22 +7,17 @@ import * as React from 'react'; import noop from 'lodash/noop'; import { ActivityFeed } from '@box/activity-feed'; -import { AnnotationBadgeType, serializeMentionMarkup } from '@box/threaded-annotations'; -import type { Annotation, AnnotationPermission, Target } from '../../../common/types/annotations'; +import type { Annotation, AnnotationPermission } from '../../../common/types/annotations'; import type { BoxCommentPermission, CommentFeedItemType, FeedItemStatus } from '../../../common/types/feed'; import type { TaskNew } from '../../../common/types/tasks'; -import type { - AnnotationBadgeTargetType, - TransformedCommentItem, - TransformedFeedItem, - UserSelectorProps, -} from './types'; +import { dispatchReplyEdit, logEditError, serializeEditorContent } from './helpers'; +import { annotationTargetToBadge } from './transformers'; -import { FEED_ITEM_TYPE_ANNOTATION, FEED_ITEM_TYPE_COMMENT } from '../../../constants'; +import type { OnReplyUpdate, TransformedFeedItem, UserSelectorProps } from './types'; -type EditorContent = Parameters[0]; +import { FEED_ITEM_TYPE_ANNOTATION, FEED_ITEM_TYPE_COMMENT } from '../../../constants'; type FeedItemRowProps = { currentUserId?: string; @@ -49,50 +44,13 @@ type FeedItemRowProps = { onError?: (() => void) | null, ) => void; onReplyCreate?: (parentId: string, parentType: CommentFeedItemType, text: string) => void; - onReplyUpdate?: (params: { - id: string; - onError?: () => void; - onSuccess?: () => void; - parentId: string; - permissions: BoxCommentPermission; - text: string; - }) => void; + onReplyUpdate?: OnReplyUpdate; onTaskDelete?: (task: TaskNew) => void; onTaskView?: (id: string, isCreator: boolean) => void; onVersionHistoryClick?: (version: { id: string; version_number: number }) => void; userSelectorProps: UserSelectorProps; }; -const annotationTargetToBadge = (target?: Target): AnnotationBadgeTargetType | undefined => { - if (!target) return undefined; - - const targetType = target.type as string; - const page = target.location?.value ?? 0; - - switch (targetType) { - case 'drawing': - return { page, type: AnnotationBadgeType.Drawing }; - case 'highlight': - return { highlightedText: '', page, type: AnnotationBadgeType.Highlight }; - case 'point': - return { page, type: AnnotationBadgeType.Point }; - case 'region': - return { page, type: AnnotationBadgeType.Region }; - default: - return undefined; - } -}; - -const serializeEditorContent = (content: unknown): { hasMention: boolean; text: string } | null => { - try { - return serializeMentionMarkup(content as EditorContent); - } catch (error) { - // eslint-disable-next-line no-console - console.error('ActivityFeedV2: failed to serialize editor content', error); - return null; - } -}; - const buildReplyPost = ( parentId: string, @@ -107,27 +65,6 @@ const buildReplyPost = onReplyCreate(parentId, parentType, serialized.text); }; -const findMessagePermissions = ( - messages: TransformedCommentItem['messages'], - id: string, -): BoxCommentPermission | undefined => { - const message = messages.find(m => m.id === id); - if (!message) return undefined; - const { canDelete, canEdit, canReply, canResolve } = message.permissions; - return { - can_delete: canDelete, - can_edit: canEdit, - can_reply: canReply, - can_resolve: canResolve, - }; -}; - -const logEditError = (error: unknown): string | undefined => { - // eslint-disable-next-line no-console - console.error('ActivityFeedV2: failed to save edit', error); - return undefined; -}; - const FeedItemRow = ({ currentUserId, isDisabled, @@ -166,13 +103,13 @@ const FeedItemRow = ({ onCommentUpdate?.(id, serialized.text, undefined, serialized.hasMention, permissions); return; } - const replyPermissions = findMessagePermissions(item.messages, id); - if (!replyPermissions) { - // eslint-disable-next-line no-console - console.error(`ActivityFeedV2: no permissions found for reply "${id}" in thread "${item.id}"`); - return; - } - onReplyUpdate?.({ id, parentId: item.id, permissions: replyPermissions, text: serialized.text }); + dispatchReplyEdit({ + id, + messages: item.messages, + onReplyUpdate, + parentId: item.id, + text: serialized.text, + }); }; return ( { if (isDisabled) return; onAnnotationDelete?.({ id, permissions }); @@ -215,13 +153,13 @@ const FeedItemRow = ({ onAnnotationEdit?.({ id, permissions, text: serialized.text }); return; } - const replyPermissions = findMessagePermissions(item.messages, id); - if (!replyPermissions) { - // eslint-disable-next-line no-console - console.error(`ActivityFeedV2: no permissions found for reply "${id}" in thread "${item.id}"`); - return; - } - onReplyUpdate?.({ id, parentId: item.id, permissions: replyPermissions, text: serialized.text }); + dispatchReplyEdit({ + id, + messages: item.messages, + onReplyUpdate, + parentId: item.id, + text: serialized.text, + }); }; return ( onAnnotationSelect?.(item.annotation)} onAvatarClick={noop} onCopyLink={ - onAnnotationCopyLink && item.annotation.file_version?.id - ? () => - onAnnotationCopyLink({ - annotationId: item.id, - fileVersionId: item.annotation.file_version.id, - }) + onAnnotationCopyLink && fileVersionId + ? () => onAnnotationCopyLink({ annotationId: item.id, fileVersionId }) : undefined } onDelete={handleDelete} diff --git a/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx b/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx index 02e1e610b0..1c97dbb592 100644 --- a/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx +++ b/src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx @@ -1,9 +1,12 @@ import * as React from 'react'; -import type { ThreadedAnnotationsPropsV2 } from '@box/threaded-annotations'; +import { AnnotationBadgeType } from '@box/threaded-annotations'; +import type { AnnotationBadgeTargetType, ThreadedAnnotationsPropsV2 } from '@box/threaded-annotations'; import { render, screen } from '../../../../test-utils/testing-library'; import FeedItemRow from '../FeedItemRow'; +import { dispatchReplyEdit, logEditError, serializeEditorContent } from '../helpers'; +import { annotationTargetToBadge } from '../transformers'; import type { TaskNew } from '../../../../common/types/tasks'; import type { @@ -37,18 +40,20 @@ jest.mock('@box/activity-feed', () => { return { ActivityFeed: { List: ActivityFeedList } }; }); -jest.mock('@box/threaded-annotations', () => ({ - AnnotationBadgeType: { - Drawing: 'drawing', - Frame: 'frame', - Highlight: 'highlight', - Point: 'point', - Region: 'region', - }, - serializeMentionMarkup: jest.fn(() => ({ hasMention: false, text: 'serialized-text' })), +jest.mock('../helpers', () => ({ + dispatchReplyEdit: jest.fn(), + logEditError: jest.fn(), + serializeEditorContent: jest.fn(), +})); + +jest.mock('../transformers', () => ({ + ...jest.requireActual('../transformers'), + annotationTargetToBadge: jest.fn(), })); -const mockedSerialize = jest.mocked(jest.requireMock('@box/threaded-annotations').serializeMentionMarkup); +const mockedSerializeEditorContent = jest.mocked(serializeEditorContent); +const mockedDispatchReplyEdit = jest.mocked(dispatchReplyEdit); +const mockedAnnotationTargetToBadge = jest.mocked(annotationTargetToBadge); const userSelectorProps: UserSelectorProps = { ariaRoleDescription: 'user selector', @@ -186,7 +191,7 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { lastThreadedAnnotationProps = {}; lastTaskProps = {}; lastVersionProps = {}; - mockedSerialize.mockReturnValue({ hasMention: false, text: 'serialized-text' }); + mockedSerializeEditorContent.mockReturnValue({ hasMention: false, text: 'serialized-text' }); }); afterEach(() => { @@ -263,7 +268,7 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { }); test('should route onEdit of root id to onCommentUpdate with serialized text and hasMention', () => { - mockedSerialize.mockReturnValue({ hasMention: true, text: 'edited-text' }); + mockedSerializeEditorContent.mockReturnValue({ hasMention: true, text: 'edited-text' }); const onCommentUpdate = jest.fn(); render(); @@ -278,8 +283,8 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { ); }); - test('should route onEdit of reply id to onReplyUpdate with reply permissions', () => { - mockedSerialize.mockReturnValue({ hasMention: false, text: 'edited-reply' }); + test('should delegate onEdit of a reply id to dispatchReplyEdit with messages, parent id, and reply text', () => { + mockedSerializeEditorContent.mockReturnValue({ hasMention: false, text: 'edited-reply' }); const onReplyUpdate = jest.fn(); const onCommentUpdate = jest.fn(); render( @@ -293,10 +298,11 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { lastThreadedAnnotationProps.onEdit?.('reply-1', { type: 'doc', content: [] }); - expect(onReplyUpdate).toHaveBeenCalledWith({ + expect(mockedDispatchReplyEdit).toHaveBeenCalledWith({ id: 'reply-1', + messages: mockComment.messages, + onReplyUpdate, parentId: 'comment-1', - permissions: { can_delete: true, can_edit: true, can_reply: false, can_resolve: false }, text: 'edited-reply', }); expect(onCommentUpdate).not.toHaveBeenCalled(); @@ -316,11 +322,8 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { expect(lastThreadedAnnotationProps.onCopyLink).toBeUndefined(); }); - test('should log and skip onEdit when serialize fails', () => { - const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined); - mockedSerialize.mockImplementation(() => { - throw new Error('bad content'); - }); + test('should skip onEdit when serializer returns null', () => { + mockedSerializeEditorContent.mockReturnValue(null); const onCommentUpdate = jest.fn(); const onReplyUpdate = jest.fn(); render( @@ -336,15 +339,10 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { expect(onCommentUpdate).not.toHaveBeenCalled(); expect(onReplyUpdate).not.toHaveBeenCalled(); - expect(consoleError).toHaveBeenCalledWith( - 'ActivityFeedV2: failed to serialize editor content', - expect.any(Error), - ); - consoleError.mockRestore(); }); test('should skip onEdit when serialized text is whitespace only', () => { - mockedSerialize.mockReturnValue({ hasMention: false, text: ' \n\t ' }); + mockedSerializeEditorContent.mockReturnValue({ hasMention: false, text: ' \n\t ' }); const onCommentUpdate = jest.fn(); const onReplyUpdate = jest.fn(); render( @@ -363,30 +361,9 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { expect(onReplyUpdate).not.toHaveBeenCalled(); }); - test('should log and skip onEdit when a reply id has no resolvable permissions', () => { - const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined); - const onReplyUpdate = jest.fn(); - const orphanReplyComment = { ...mockComment, messages: [mockComment.messages[0]] }; - render(); - - lastThreadedAnnotationProps.onEdit?.('reply-1', { type: 'doc', content: [] }); - - expect(onReplyUpdate).not.toHaveBeenCalled(); - expect(consoleError).toHaveBeenCalledWith( - 'ActivityFeedV2: no permissions found for reply "reply-1" in thread "comment-1"', - ); - consoleError.mockRestore(); - }); - - test('should log via onEditError so failed edits leave a trail', () => { - const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined); + test('should wire onEditError to the logEditError helper', () => { render(); - - const message = lastThreadedAnnotationProps.onEditError?.(new Error('save failed')); - - expect(message).toBeUndefined(); - expect(consoleError).toHaveBeenCalledWith('ActivityFeedV2: failed to save edit', expect.any(Error)); - consoleError.mockRestore(); + expect(lastThreadedAnnotationProps.onEditError).toBe(logEditError); }); }); @@ -461,65 +438,14 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { }); }); - test('should pass point badge for point annotation target', () => { - render(); - expect(lastThreadedAnnotationProps.annotationTarget).toEqual({ page: 3, type: 'point' }); - }); - - test('should pass region badge for region annotation target', () => { - const regionAnnotation: TransformedAnnotationItem = { - ...mockAnnotation, - annotation: { - ...mockAnnotation.annotation, - target: { - location: { type: 'page', value: 2 }, - shape: { height: 10, type: 'rect', width: 20, x: 5, y: 5 }, - type: 'region', - }, - } as TransformedAnnotationItem['annotation'], - }; - render(); - expect(lastThreadedAnnotationProps.annotationTarget).toEqual({ page: 2, type: 'region' }); - }); - - test('should pass drawing badge for drawing annotation target', () => { - const drawingAnnotation: TransformedAnnotationItem = { - ...mockAnnotation, - annotation: { - ...mockAnnotation.annotation, - target: { location: { type: 'page', value: 1 }, type: 'drawing' }, - } as TransformedAnnotationItem['annotation'], - }; - render(); - expect(lastThreadedAnnotationProps.annotationTarget).toEqual({ page: 1, type: 'drawing' }); - }); + test('should map the annotation target through annotationTargetToBadge and forward the result', () => { + const badge: AnnotationBadgeTargetType = { page: 7, type: AnnotationBadgeType.Drawing }; + mockedAnnotationTargetToBadge.mockReturnValue(badge); - test('should pass highlight badge with page number for highlight annotation target', () => { - const highlightAnnotation: TransformedAnnotationItem = { - ...mockAnnotation, - annotation: { - ...mockAnnotation.annotation, - target: { location: { type: 'page', value: 4 }, type: 'highlight' }, - } as TransformedAnnotationItem['annotation'], - }; - render(); - expect(lastThreadedAnnotationProps.annotationTarget).toEqual({ - highlightedText: '', - page: 4, - type: 'highlight', - }); - }); + render(); - test('should pass undefined badge for unknown annotation target type', () => { - const unknownAnnotation: TransformedAnnotationItem = { - ...mockAnnotation, - annotation: { - ...mockAnnotation.annotation, - target: { location: { type: 'page', value: 1 }, type: 'unknown' }, - } as TransformedAnnotationItem['annotation'], - }; - render(); - expect(lastThreadedAnnotationProps.annotationTarget).toBeUndefined(); + expect(annotationTargetToBadge).toHaveBeenCalledWith(mockAnnotation.annotation.target); + expect(lastThreadedAnnotationProps.annotationTarget).toBe(badge); }); test('should call onReplyCreate via onPost with annotation type', async () => { @@ -537,7 +463,7 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { }); test('should route onEdit of root id to onAnnotationEdit with serialized text', () => { - mockedSerialize.mockReturnValue({ hasMention: false, text: 'edited-annotation' }); + mockedSerializeEditorContent.mockReturnValue({ hasMention: false, text: 'edited-annotation' }); const onAnnotationEdit = jest.fn(); render(); @@ -550,8 +476,8 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { }); }); - test('should route onEdit of reply id to onReplyUpdate with reply permissions', () => { - mockedSerialize.mockReturnValue({ hasMention: false, text: 'edited-reply' }); + test('should delegate onEdit of a reply id to dispatchReplyEdit with messages, parent id, and reply text', () => { + mockedSerializeEditorContent.mockReturnValue({ hasMention: false, text: 'edited-reply' }); const onAnnotationEdit = jest.fn(); const onReplyUpdate = jest.fn(); render( @@ -565,10 +491,11 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { lastThreadedAnnotationProps.onEdit?.('annotation-reply-1', { type: 'doc', content: [] }); - expect(onReplyUpdate).toHaveBeenCalledWith({ + expect(mockedDispatchReplyEdit).toHaveBeenCalledWith({ id: 'annotation-reply-1', + messages: mockAnnotation.messages, + onReplyUpdate, parentId: 'annotation-1', - permissions: { can_delete: true, can_edit: true, can_reply: false, can_resolve: false }, text: 'edited-reply', }); expect(onAnnotationEdit).not.toHaveBeenCalled(); @@ -605,7 +532,7 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => { }); test('should skip onEdit when serialized text is whitespace only', () => { - mockedSerialize.mockReturnValue({ hasMention: false, text: ' \n\t ' }); + mockedSerializeEditorContent.mockReturnValue({ hasMention: false, text: ' \n\t ' }); const onAnnotationEdit = jest.fn(); const onReplyUpdate = jest.fn(); render( diff --git a/src/elements/content-sidebar/activity-feed-v2/__tests__/helpers.test.ts b/src/elements/content-sidebar/activity-feed-v2/__tests__/helpers.test.ts new file mode 100644 index 0000000000..a591d24cb3 --- /dev/null +++ b/src/elements/content-sidebar/activity-feed-v2/__tests__/helpers.test.ts @@ -0,0 +1,119 @@ +import { serializeMentionMarkup } from '@box/threaded-annotations'; + +import { dispatchReplyEdit, findMessagePermissions, logEditError, serializeEditorContent } from '../helpers'; + +import type { TransformedCommentItem } from '../types'; + +jest.mock('@box/threaded-annotations', () => ({ + serializeMentionMarkup: jest.fn(), +})); + +const mockedSerialize = jest.mocked(serializeMentionMarkup); + +const messages: TransformedCommentItem['messages'] = [ + { + author: { email: 'u@b.com', id: 1, name: 'User' }, + createdAt: 0, + id: 'root', + message: { type: 'doc', content: [] }, + permissions: { canDelete: true, canEdit: true, canReply: true, canResolve: true }, + }, + { + author: { email: 'u@b.com', id: 1, name: 'User' }, + createdAt: 0, + id: 'reply-1', + message: { type: 'doc', content: [] }, + permissions: { canDelete: true, canEdit: true, canReply: false, canResolve: false }, + }, +]; + +describe('elements/content-sidebar/activity-feed-v2/helpers', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + describe('serializeEditorContent()', () => { + beforeEach(() => { + mockedSerialize.mockReturnValue({ hasMention: false, text: 'serialized-text' }); + }); + + test('should return the serialized result on success', () => { + const content = { type: 'doc', content: [] }; + expect(serializeEditorContent(content)).toEqual({ hasMention: false, text: 'serialized-text' }); + expect(mockedSerialize).toHaveBeenCalledWith(content); + }); + + test('should log via console.error and return null when serialize throws', () => { + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined); + mockedSerialize.mockImplementation(() => { + throw new Error('bad content'); + }); + + expect(serializeEditorContent({})).toBeNull(); + expect(consoleError).toHaveBeenCalledWith( + 'ActivityFeedV2: failed to serialize editor content', + expect.any(Error), + ); + consoleError.mockRestore(); + }); + }); + + describe('findMessagePermissions()', () => { + test('should convert camelCase permissions to snake_case for the matched message', () => { + expect(findMessagePermissions(messages, 'reply-1')).toEqual({ + can_delete: true, + can_edit: true, + can_reply: false, + can_resolve: false, + }); + }); + + test('should return undefined when the id is not in the message list', () => { + expect(findMessagePermissions(messages, 'unknown')).toBeUndefined(); + }); + }); + + describe('logEditError()', () => { + test('should log via console.error and return undefined so the vendor shows its default message', () => { + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined); + const error = new Error('save failed'); + + expect(logEditError(error)).toBeUndefined(); + expect(consoleError).toHaveBeenCalledWith('ActivityFeedV2: failed to save edit', error); + consoleError.mockRestore(); + }); + }); + + describe('dispatchReplyEdit()', () => { + test('should call onReplyUpdate with parentId and snake_case reply permissions', () => { + const onReplyUpdate = jest.fn(); + dispatchReplyEdit({ id: 'reply-1', messages, onReplyUpdate, parentId: 'root', text: 'edited' }); + + expect(onReplyUpdate).toHaveBeenCalledWith({ + id: 'reply-1', + parentId: 'root', + permissions: { can_delete: true, can_edit: true, can_reply: false, can_resolve: false }, + text: 'edited', + }); + }); + + test('should log and skip the dispatch when reply permissions cannot be resolved', () => { + const consoleError = jest.spyOn(console, 'error').mockImplementation(() => undefined); + const onReplyUpdate = jest.fn(); + + dispatchReplyEdit({ id: 'orphan-id', messages, onReplyUpdate, parentId: 'root', text: 'edited' }); + + expect(onReplyUpdate).not.toHaveBeenCalled(); + expect(consoleError).toHaveBeenCalledWith( + 'ActivityFeedV2: no permissions found for reply "orphan-id" in thread "root"', + ); + consoleError.mockRestore(); + }); + + test('should not throw when onReplyUpdate is not provided', () => { + expect(() => + dispatchReplyEdit({ id: 'reply-1', messages, parentId: 'root', text: 'edited' }), + ).not.toThrow(); + }); + }); +}); diff --git a/src/elements/content-sidebar/activity-feed-v2/__tests__/transformers.test.ts b/src/elements/content-sidebar/activity-feed-v2/__tests__/transformers.test.ts index ed21ef692d..10e5f394b2 100644 --- a/src/elements/content-sidebar/activity-feed-v2/__tests__/transformers.test.ts +++ b/src/elements/content-sidebar/activity-feed-v2/__tests__/transformers.test.ts @@ -4,6 +4,7 @@ import type { BoxItemVersion } from '../../../../common/types/core'; import type { TaskNew } from '../../../../common/types/tasks'; import { + annotationTargetToBadge, textToDocumentNode, transformAnnotationToMessages, transformAppActivityToProps, @@ -525,4 +526,69 @@ describe('elements/content-sidebar/activity-feed-v2/transformers', () => { } }); }); + + describe('annotationTargetToBadge()', () => { + test('should return undefined when target is missing', () => { + expect(annotationTargetToBadge()).toBeUndefined(); + }); + + test('should return undefined for an unknown target type', () => { + const target = { location: { type: 'page', value: 1 }, type: 'unknown' }; + expect(annotationTargetToBadge(target as unknown as Annotation['target'])).toBeUndefined(); + }); + + test('should map drawing target to a drawing badge with the page number', () => { + const target = { location: { type: 'page', value: 1 }, type: 'drawing' }; + expect(annotationTargetToBadge(target as unknown as Annotation['target'])).toEqual({ + page: 1, + type: 'drawing', + }); + }); + + test('should map highlight target to a highlight badge with the highlighted text and the page number', () => { + const target = { location: { type: 'page', value: 4 }, text: 'selected excerpt', type: 'highlight' }; + expect(annotationTargetToBadge(target as unknown as Annotation['target'])).toEqual({ + highlightedText: 'selected excerpt', + page: 4, + type: 'highlight', + }); + }); + + test('should fall back to empty highlightedText when highlight target has no text', () => { + const target = { location: { type: 'page', value: 4 }, type: 'highlight' }; + expect(annotationTargetToBadge(target as unknown as Annotation['target'])).toEqual({ + highlightedText: '', + page: 4, + type: 'highlight', + }); + }); + + test('should map point target to a point badge with the page number', () => { + const target = { location: { type: 'page', value: 3 }, type: 'point', x: 0, y: 0 }; + expect(annotationTargetToBadge(target as unknown as Annotation['target'])).toEqual({ + page: 3, + type: 'point', + }); + }); + + test('should map region target to a region badge with the page number', () => { + const target = { + location: { type: 'page', value: 2 }, + shape: { height: 10, type: 'rect', width: 20, x: 5, y: 5 }, + type: 'region', + }; + expect(annotationTargetToBadge(target as unknown as Annotation['target'])).toEqual({ + page: 2, + type: 'region', + }); + }); + + test('should default page to 0 when location is missing', () => { + const target = { type: 'point', x: 0, y: 0 }; + expect(annotationTargetToBadge(target as unknown as Annotation['target'])).toEqual({ + page: 0, + type: 'point', + }); + }); + }); }); diff --git a/src/elements/content-sidebar/activity-feed-v2/helpers.ts b/src/elements/content-sidebar/activity-feed-v2/helpers.ts new file mode 100644 index 0000000000..4547b83f2e --- /dev/null +++ b/src/elements/content-sidebar/activity-feed-v2/helpers.ts @@ -0,0 +1,65 @@ +/** + * @file Helpers that bridge vendor activity-feed callbacks back to BUIE + * @author Box + */ + +import { serializeMentionMarkup } from '@box/threaded-annotations'; + +import type { BoxCommentPermission } from '../../../common/types/feed'; + +import type { OnReplyUpdate, TransformedCommentItem } from './types'; + +type EditorContent = Parameters[0]; + +export const serializeEditorContent = (content: unknown): ReturnType | null => { + try { + return serializeMentionMarkup(content as EditorContent); + } catch (error) { + // eslint-disable-next-line no-console + console.error('ActivityFeedV2: failed to serialize editor content', error); + return null; + } +}; + +export const findMessagePermissions = ( + messages: TransformedCommentItem['messages'], + id: string, +): BoxCommentPermission | undefined => { + const message = messages.find(m => m.id === id); + if (!message) return undefined; + const { canDelete, canEdit, canReply, canResolve } = message.permissions; + return { + can_delete: canDelete, + can_edit: canEdit, + can_reply: canReply, + can_resolve: canResolve, + }; +}; + +export const logEditError = (error: unknown): undefined => { + // eslint-disable-next-line no-console + console.error('ActivityFeedV2: failed to save edit', error); + return undefined; +}; + +export const dispatchReplyEdit = ({ + id, + messages, + onReplyUpdate, + parentId, + text, +}: { + id: string; + messages: TransformedCommentItem['messages']; + onReplyUpdate?: OnReplyUpdate; + parentId: string; + text: string; +}) => { + const permissions = findMessagePermissions(messages, id); + if (!permissions) { + // eslint-disable-next-line no-console + console.error(`ActivityFeedV2: no permissions found for reply "${id}" in thread "${parentId}"`); + return; + } + onReplyUpdate?.({ id, parentId, permissions, text }); +}; diff --git a/src/elements/content-sidebar/activity-feed-v2/transformers.ts b/src/elements/content-sidebar/activity-feed-v2/transformers.ts index 95f3191a65..ef64ec5785 100644 --- a/src/elements/content-sidebar/activity-feed-v2/transformers.ts +++ b/src/elements/content-sidebar/activity-feed-v2/transformers.ts @@ -7,6 +7,7 @@ */ import { TaskCompletionRule, TaskType } from '@box/activity-feed'; +import { AnnotationBadgeType } from '@box/threaded-annotations'; import type { DocumentNodeV2 as DocumentNode, MentionNodeV2 as MentionNode, @@ -17,12 +18,18 @@ import type { TextNodeV2 as TextNode, } from '@box/threaded-annotations'; -import type { Annotation } from '../../../common/types/annotations'; +import type { Annotation, Target } from '../../../common/types/annotations'; import type { AppActivityItem as BUIEAppActivityItem, Comment, FeedItem } from '../../../common/types/feed'; import type { BoxItemVersion, User } from '../../../common/types/core'; import type { TaskNew } from '../../../common/types/tasks'; -import type { AppActivityItemProps, TaskItemProps, TransformedFeedItem, VersionItemProps } from './types'; +import type { + AnnotationBadgeTargetType, + AppActivityItemProps, + TaskItemProps, + TransformedFeedItem, + VersionItemProps, +} from './types'; import { FEED_ITEM_TYPE_ANNOTATION, @@ -131,6 +138,25 @@ export const transformCommentToMessages = (comment: Comment): TextMessageType[] return [root, ...replies]; }; +export const annotationTargetToBadge = (target?: Target): AnnotationBadgeTargetType | undefined => { + if (!target) return undefined; + + const page = target.location?.value ?? 0; + + switch (target.type) { + case 'drawing': + return { page, type: AnnotationBadgeType.Drawing }; + case 'highlight': + return { highlightedText: target.text ?? '', page, type: AnnotationBadgeType.Highlight }; + case 'point': + return { page, type: AnnotationBadgeType.Point }; + case 'region': + return { page, type: AnnotationBadgeType.Region }; + default: + return undefined; + } +}; + export const transformAnnotationToMessages = (annotation: Annotation): TextMessageType[] => { const messageText = annotation.description?.message ?? ''; const root: TextMessageType = { diff --git a/src/elements/content-sidebar/activity-feed-v2/types.ts b/src/elements/content-sidebar/activity-feed-v2/types.ts index c67da576d5..5290d51705 100644 --- a/src/elements/content-sidebar/activity-feed-v2/types.ts +++ b/src/elements/content-sidebar/activity-feed-v2/types.ts @@ -28,6 +28,15 @@ export type UserSelectorProps = { loadingAriaLabel: string; }; +export type OnReplyUpdate = (params: { + id: string; + onError?: () => void; + onSuccess?: () => void; + parentId: string; + permissions: BoxCommentPermission; + text: string; +}) => void; + type ResolvedInfo = { isResolved: boolean; resolvedAt?: number; @@ -92,14 +101,7 @@ export type ActivityFeedV2Props = { onError?: (() => void) | null, ) => void; onReplyCreate?: (parentId: string, parentType: CommentFeedItemType, text: string) => void; - onReplyUpdate?: (params: { - id: string; - onError?: () => void; - onSuccess?: () => void; - parentId: string; - permissions: BoxCommentPermission; - text: string; - }) => void; + onReplyUpdate?: OnReplyUpdate; onTaskDelete?: (task: TaskNew) => void; onTaskView?: (id: string, isCreator: boolean) => void; onVersionHistoryClick?: (version: { id: string; version_number: number }) => void; From 9ac2af90b58ad350b565fbf17f23ff8e189ca0b3 Mon Sep 17 00:00:00 2001 From: Jackie Jou Date: Thu, 14 May 2026 23:06:31 -0700 Subject: [PATCH 6/7] feat(activity-feed-v2): Allow consumers to control filter state Expose the show-resolved and show-only-mentions-of-me filters as controlled-or-uncontrolled props on ActivityFeedV2, with ActivitySidebar plumbing the same props through to the row. The change handlers update local state only when the matching value prop is omitted, and always notify the consumer when provided. This keeps the controlled mode authoritative and makes the listener-only combination work as a notification channel. --- .../content-sidebar/ActivitySidebar.js | 12 ++ .../activity-feed-v2/ActivityFeedV2.tsx | 30 +++- .../__tests__/ActivityFeedV2.test.tsx | 140 +++++++++++++++++- .../content-sidebar/activity-feed-v2/types.ts | 4 + 4 files changed, 177 insertions(+), 9 deletions(-) diff --git a/src/elements/content-sidebar/ActivitySidebar.js b/src/elements/content-sidebar/ActivitySidebar.js index 1a606a61fe..541aa564e1 100644 --- a/src/elements/content-sidebar/ActivitySidebar.js +++ b/src/elements/content-sidebar/ActivitySidebar.js @@ -97,6 +97,8 @@ type ExternalProps = { onCommentCreate: Function, onCommentDelete: (comment: Comment) => any, onCommentUpdate: () => any, + onShowOnlyMentionsMeChange?: (checked: boolean) => void, + onShowResolvedChange?: (checked: boolean) => void, onTaskAssignmentUpdate: Function, onTaskCreate: Function, onTaskDelete: (id: string) => any, @@ -105,6 +107,8 @@ type ExternalProps = { routerDisabled?: boolean, /** When true, enables data fetching. When false, defers data fetching. Used to prioritize preview loading. */ shouldFetchSidebarData?: boolean, + showOnlyMentionsMe?: boolean, + showResolved?: boolean, } & ErrorContextProps & WithAnnotatorContextProps; @@ -1375,9 +1379,13 @@ class ActivitySidebar extends React.PureComponent { isDisabled = false, onAnnotationCopyLink, onCommentCopyLink, + onShowOnlyMentionsMeChange, + onShowResolvedChange, onVersionHistoryClick, getUserProfileUrl, onTaskView, + showOnlyMentionsMe, + showResolved, } = this.props; const { activityFeedError, approverSelectorContacts, contactsLoaded, mentionSelectorContacts } = this.state; const isNewThreadedRepliesEnabled = isFeatureEnabled(features, 'activityFeed.newThreadedReplies.enabled'); @@ -1418,9 +1426,13 @@ class ActivitySidebar extends React.PureComponent { onReplyUpdate={({ id, onError, onSuccess, parentId, permissions, text }) => this.updateReply(id, parentId, text, permissions, onSuccess, onError) } + onShowOnlyMentionsMeChange={onShowOnlyMentionsMeChange} + onShowResolvedChange={onShowResolvedChange} onTaskDelete={this.deleteTask} onTaskView={onTaskView} onVersionHistoryClick={onVersionHistoryClick} + showOnlyMentionsMe={showOnlyMentionsMe} + showResolved={showResolved} /> ); diff --git a/src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx b/src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx index 38435f43ba..ad78f8235f 100644 --- a/src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx +++ b/src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx @@ -49,9 +49,13 @@ const ActivityFeedV2 = ({ onCommentUpdate, onReplyCreate, onReplyUpdate, + onShowOnlyMentionsMeChange, + onShowResolvedChange, onTaskDelete, onTaskView, onVersionHistoryClick, + showOnlyMentionsMe: showOnlyMentionsMeProp, + showResolved: showResolvedProp, }: ActivityFeedV2Props) => { const intl = useIntl(); const scrollHandle = useActivityFeedScroll(); @@ -130,8 +134,20 @@ const ActivityFeedV2 = ({ [], ); - const [mentionMe, setMentionMe] = React.useState(false); - const [showResolved, setShowResolved] = React.useState(false); + const [localShowOnlyMentionsMe, setLocalShowOnlyMentionsMe] = React.useState(false); + const [localShowResolved, setLocalShowResolved] = React.useState(false); + const showOnlyMentionsMe = showOnlyMentionsMeProp ?? localShowOnlyMentionsMe; + const showResolved = showResolvedProp ?? localShowResolved; + + const handleShowOnlyMentionsMeChange = (checked: boolean) => { + if (showOnlyMentionsMeProp === undefined) setLocalShowOnlyMentionsMe(checked); + onShowOnlyMentionsMeChange?.(checked); + }; + const handleShowResolvedChange = (checked: boolean) => { + if (showResolvedProp === undefined) setLocalShowResolved(checked); + onShowResolvedChange?.(checked); + }; + const [isTaskFormOpen, setIsTaskFormOpen] = React.useState(false); const [taskType, setTaskType] = React.useState(TASK_TYPE_APPROVAL); const [taskError, setTaskError] = React.useState(null); @@ -157,7 +173,7 @@ const ActivityFeedV2 = ({ if ((item.type === 'comment' || item.type === 'annotation') && item.isResolved && !showResolved) { return false; } - if (mentionMe && currentUserId) { + if (showOnlyMentionsMe && currentUserId) { if (item.type === 'comment' || item.type === 'annotation') { const hasMention = item.messages.some(msg => msg.message?.content?.some( @@ -179,7 +195,7 @@ const ActivityFeedV2 = ({ } return true; }); - }, [currentUserId, mentionMe, showResolved, transformedItems]); + }, [currentUserId, showOnlyMentionsMe, showResolved, transformedItems]); React.useEffect(() => { const alreadyScrolledToThisEntry = scrolledEntryIdRef.current === activeFeedEntryId; @@ -227,12 +243,12 @@ const ActivityFeedV2 = ({ {hasTasks && ( diff --git a/src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx b/src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx index adb2059454..9b3aee1118 100644 --- a/src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx +++ b/src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx @@ -1,5 +1,6 @@ import * as React from 'react'; -import { render, screen } from '../../../../test-utils/testing-library'; + +import { act, render, screen } from '../../../../test-utils/testing-library'; import ActivityFeedV2 from '..'; import type { ActivityFeedV2Props } from '../ActivityFeedV2'; @@ -16,6 +17,10 @@ jest.mock('@box/threaded-annotations', () => ({ const mockScrollTo = jest.fn(() => true); +type FilterOptionProps = { checked?: boolean; onCheckedChange?: (checked: boolean) => void }; +let lastShowResolvedOptionProps: FilterOptionProps = {}; +let lastMentionMeOptionProps: FilterOptionProps = {}; + jest.mock('@box/activity-feed', () => { const actual = jest.requireActual('@box/activity-feed'); const ActivityFeedRoot = ({ children }: { children: React.ReactNode }) => ( @@ -33,7 +38,20 @@ jest.mock('@box/activity-feed', () => { ); ActivityFeedList.Version = (props: { id: string }) =>
Version
; const ActivityFeedEditor = () =>
Editor
; - const ActivityFeedHeader = () =>
Header
; + const ActivityFeedHeader = ({ children }: { children: React.ReactNode }) => ( +
{children}
+ ); + ActivityFeedHeader.Actions = ({ children }: { children: React.ReactNode }) =>
{children}
; + ActivityFeedHeader.FilterMenu = ({ children }: { children: React.ReactNode }) =>
{children}
; + ActivityFeedHeader.ShowResolvedOption = (props: FilterOptionProps) => { + lastShowResolvedOptionProps = props; + return
{String(props.checked)}
; + }; + ActivityFeedHeader.MentionMeOption = (props: FilterOptionProps) => { + lastMentionMeOptionProps = props; + return
{String(props.checked)}
; + }; + ActivityFeedHeader.TaskButton = () =>
; return { ...actual, @@ -133,6 +151,8 @@ const feedItems = [ describe('elements/content-sidebar/activity-feed-v2/ActivityFeedV2', () => { beforeEach(() => { mockScrollTo.mockReturnValue(true); + lastShowResolvedOptionProps = {}; + lastMentionMeOptionProps = {}; }); afterEach(() => { @@ -303,6 +323,122 @@ describe('elements/content-sidebar/activity-feed-v2/ActivityFeedV2', () => { expect(screen.getByTestId('activity-feed-root')).toBeVisible(); }); + describe('filter controls', () => { + test('should default showResolved and showOnlyMentionsMe to false in the filter menu', () => { + render(); + expect(lastShowResolvedOptionProps.checked).toBe(false); + expect(lastMentionMeOptionProps.checked).toBe(false); + }); + + test('should reflect the controlled showResolved prop in the filter menu', () => { + render( + , + ); + expect(lastShowResolvedOptionProps.checked).toBe(true); + }); + + test('should reflect the controlled showOnlyMentionsMe prop in the filter menu', () => { + render( + , + ); + expect(lastMentionMeOptionProps.checked).toBe(true); + }); + + test('should call onShowResolvedChange when the consumer toggles it', () => { + const onShowResolvedChange = jest.fn(); + render( + , + ); + + act(() => lastShowResolvedOptionProps.onCheckedChange?.(true)); + + expect(onShowResolvedChange).toHaveBeenCalledWith(true); + expect(onShowResolvedChange).toHaveBeenCalledTimes(1); + }); + + test('should call onShowOnlyMentionsMeChange when the consumer toggles it', () => { + const onShowOnlyMentionsMeChange = jest.fn(); + render( + , + ); + + act(() => lastMentionMeOptionProps.onCheckedChange?.(true)); + + expect(onShowOnlyMentionsMeChange).toHaveBeenCalledWith(true); + expect(onShowOnlyMentionsMeChange).toHaveBeenCalledTimes(1); + }); + + test('should manage filter state internally when no controlled props are provided', () => { + const resolvedComment = { ...mockComment, id: 'resolved-1', status: 'resolved' }; + render( + , + ); + expect(screen.queryByTestId('threaded-annotation-resolved-1')).not.toBeInTheDocument(); + + act(() => lastShowResolvedOptionProps.onCheckedChange?.(true)); + + expect(screen.getByTestId('threaded-annotation-resolved-1')).toBeVisible(); + }); + + test('should not update internal state when a controlled prop is provided', () => { + const resolvedComment = { ...mockComment, id: 'resolved-1', status: 'resolved' }; + const onShowResolvedChange = jest.fn(); + render( + , + ); + expect(screen.queryByTestId('threaded-annotation-resolved-1')).not.toBeInTheDocument(); + + act(() => lastShowResolvedOptionProps.onCheckedChange?.(true)); + + expect(onShowResolvedChange).toHaveBeenCalledWith(true); + expect(screen.queryByTestId('threaded-annotation-resolved-1')).not.toBeInTheDocument(); + }); + + test('should update internal state and notify the consumer when only a change handler is provided', () => { + const resolvedComment = { ...mockComment, id: 'resolved-1', status: 'resolved' }; + const onShowResolvedChange = jest.fn(); + render( + , + ); + expect(screen.queryByTestId('threaded-annotation-resolved-1')).not.toBeInTheDocument(); + + act(() => lastShowResolvedOptionProps.onCheckedChange?.(true)); + + expect(onShowResolvedChange).toHaveBeenCalledWith(true); + expect(screen.getByTestId('threaded-annotation-resolved-1')).toBeVisible(); + }); + }); + describe('scroll to end on mount', () => { test('should scroll to the last rendered items id when the tab opens', () => { render(); diff --git a/src/elements/content-sidebar/activity-feed-v2/types.ts b/src/elements/content-sidebar/activity-feed-v2/types.ts index 5290d51705..f7047fcc33 100644 --- a/src/elements/content-sidebar/activity-feed-v2/types.ts +++ b/src/elements/content-sidebar/activity-feed-v2/types.ts @@ -102,7 +102,11 @@ export type ActivityFeedV2Props = { ) => void; onReplyCreate?: (parentId: string, parentType: CommentFeedItemType, text: string) => void; onReplyUpdate?: OnReplyUpdate; + onShowOnlyMentionsMeChange?: (checked: boolean) => void; + onShowResolvedChange?: (checked: boolean) => void; onTaskDelete?: (task: TaskNew) => void; onTaskView?: (id: string, isCreator: boolean) => void; onVersionHistoryClick?: (version: { id: string; version_number: number }) => void; + showOnlyMentionsMe?: boolean; + showResolved?: boolean; }; From e2103c5d6c2760c4b9a8577153fdda6dc4391f7f Mon Sep 17 00:00:00 2001 From: Jackie Jou Date: Fri, 15 May 2026 00:08:17 -0700 Subject: [PATCH 7/7] feat(activity-feed-v2): Send shouldShowReplies when threaded V2 is on The UAA file activities endpoint only returns reply data when the request sets shouldShowReplies. Previously this gated on the v1 hasReplies prop alone, which meant ActivityFeedV2 received empty reply lists when the host opted into v2 without flipping the legacy hasReplies. Send shouldShowReplies when either hasReplies or threadedRepliesV2 is enabled. The legacy fetchRepliesForFeedItems expansion path still gates on hasReplies because v2 displays replies inline. --- .../content-sidebar/ActivitySidebar.js | 9 +- .../__tests__/ActivitySidebar.test.js | 84 +++++++++++++++++++ 2 files changed, 88 insertions(+), 5 deletions(-) diff --git a/src/elements/content-sidebar/ActivitySidebar.js b/src/elements/content-sidebar/ActivitySidebar.js index 541aa564e1..a102651f9b 100644 --- a/src/elements/content-sidebar/ActivitySidebar.js +++ b/src/elements/content-sidebar/ActivitySidebar.js @@ -765,15 +765,14 @@ class ActivitySidebar extends React.PureComponent { api, file, features, - hasReplies: shouldShowReplies, + hasReplies, hasTasks: shouldShowTasks, hasVersions: shouldShowVersions, } = this.props; + const isThreadedRepliesV2Enabled = isFeatureEnabled(features, 'activityFeed.threadedRepliesV2.enabled'); + const shouldShowReplies = hasReplies || isThreadedRepliesV2Enabled; const shouldFetchReplies = - shouldRefreshCache && - shouldShowReplies && - activeFeedEntryId && - activeFeedEntryType === FEED_ITEM_TYPE_COMMENT; + shouldRefreshCache && hasReplies && activeFeedEntryId && activeFeedEntryType === FEED_ITEM_TYPE_COMMENT; const shouldShowAppActivity = isFeatureEnabled(features, 'activityFeed.appActivity.enabled'); const shouldShowAnnotations = isFeatureEnabled(features, 'activityFeed.annotations.enabled'); const shouldUseUAA = isFeatureEnabled(features, 'activityFeed.uaaIntegration.enabled'); diff --git a/src/elements/content-sidebar/__tests__/ActivitySidebar.test.js b/src/elements/content-sidebar/__tests__/ActivitySidebar.test.js index 8a2ae3ee6f..0a8411e94c 100644 --- a/src/elements/content-sidebar/__tests__/ActivitySidebar.test.js +++ b/src/elements/content-sidebar/__tests__/ActivitySidebar.test.js @@ -796,6 +796,90 @@ describe('elements/content-sidebar/ActivitySidebar', () => { undefined, ); }); + + test('should set shouldShowReplies to true when threadedRepliesV2 is enabled even if hasReplies is false', () => { + wrapper = getWrapper({ + features: { + activityFeed: { + threadedRepliesV2: { enabled: true }, + }, + }, + hasReplies: false, + }); + instance = wrapper.instance(); + instance.errorCallback = jest.fn(); + instance.fetchFeedItemsErrorCallback = jest.fn(); + instance.fetchFeedItemsSuccessCallback = jest.fn(); + + instance.fetchFeedItems(); + + expect(feedAPI.feedItems).toHaveBeenCalledWith( + file, + false, + instance.fetchFeedItemsSuccessCallback, + instance.fetchFeedItemsErrorCallback, + instance.errorCallback, + expect.objectContaining({ shouldShowReplies: true }), + undefined, + ); + }); + + test('should set shouldShowReplies to true when both hasReplies and threadedRepliesV2 are enabled', () => { + wrapper = getWrapper({ + features: { + activityFeed: { + threadedRepliesV2: { enabled: true }, + }, + }, + hasReplies: true, + }); + instance = wrapper.instance(); + instance.errorCallback = jest.fn(); + instance.fetchFeedItemsErrorCallback = jest.fn(); + instance.fetchFeedItemsSuccessCallback = jest.fn(); + + instance.fetchFeedItems(); + + expect(feedAPI.feedItems).toHaveBeenCalledWith( + file, + false, + instance.fetchFeedItemsSuccessCallback, + instance.fetchFeedItemsErrorCallback, + instance.errorCallback, + expect.objectContaining({ shouldShowReplies: true }), + undefined, + ); + }); + + test('should not use fetchRepliesForFeedItems for the active comment when only threadedRepliesV2 is enabled', () => { + wrapper = getWrapper({ + activeFeedEntryId: '123', + activeFeedEntryType: 'comment', + features: { + activityFeed: { + threadedRepliesV2: { enabled: true }, + }, + }, + hasReplies: false, + }); + instance = wrapper.instance(); + instance.errorCallback = jest.fn(); + instance.fetchFeedItemsErrorCallback = jest.fn(); + instance.fetchFeedItemsSuccessCallback = jest.fn(); + instance.fetchRepliesForFeedItems = jest.fn(); + + instance.fetchFeedItems(true); + + expect(feedAPI.feedItems).toHaveBeenCalledWith( + file, + true, + instance.fetchFeedItemsSuccessCallback, + instance.fetchFeedItemsErrorCallback, + instance.errorCallback, + expect.objectContaining({ shouldShowReplies: true }), + undefined, + ); + }); }); describe('fetchFeedItemsSuccessCallback()', () => {