feat(activity-feed-v2): replace stub with real adapter#4533
Merged
mergify[bot] merged 12 commits intoMay 6, 2026
Conversation
…pter Wire up ActivityFeedV2 with real compound components from @box/activity-feed behind the threadedRepliesV2 feature flag. Add data transformers, FeedItemRow renderer, CSS isolation, and tighten prop types.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a complete ActivityFeedV2 implementation: new Box UI dev/peer deps and Jest transform exceptions, typed adapter types and transformers, FeedItemRow and ActivityFeedV2 components with scoped styles, wiring into ActivitySidebar behind a feature flag, plus extensive tests and mocks. ChangesActivityFeedV2 Adapter Implementation
Sequence DiagramsequenceDiagram
actor User
participant Sidebar as ActivitySidebar
participant Adapter as ActivityFeedV2
participant Transformer as transformers
participant Feed as ActivityFeed(UI)
participant Row as FeedItemRow
User->>Sidebar: open sidebar / interact
Sidebar->>Adapter: mount with props (feedItems, callbacks, flags)
Adapter->>Transformer: transformFeedItem() per item
Transformer-->>Adapter: TransformedFeedItem[]
Adapter->>Adapter: apply filters (showResolved, mentionMe)
Adapter->>Feed: render ActivityFeed.Root with filtered items
Feed->>Row: render FeedItemRow for each item
User->>Feed: actions (reply, resolve, delete, view)
Feed->>Row: emit event (onPost/onResolve/onDelete/onView)
Row->>Adapter: forward handler invocation
Adapter->>Sidebar: call provided callbacks (onCommentCreate, onAnnotationDelete, onTaskView, etc.)
Sidebar-->>User: update UI/state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx (1)
107-119: ⚡ Quick winReplace
as unknown as neverwith proper type imports from@box/activity-feed.
neveris the bottom type — no runtime value can be assigned to it. Using'ALL_ASSIGNEES' as unknown as never(and similarly forstatusandtaskType) is semantically incorrect and completely bypasses TypeScript's type safety, equivalent toas any. If the library exposes enum types forTaskCompletionRule,TaskStatus, andTaskType, import and use them directly so future type changes in@box/activity-feedare caught at compile time.♻️ Suggested approach
-import type { +import type { TransformedCommentItem, TransformedAnnotationItem, TransformedFeedItem, UserSelectorProps, } from '../types'; +// Import the actual enum types from `@box/activity-feed` (adjust paths as needed) +import type { TaskCompletionRule, TaskStatus, TaskType } from '@box/activity-feed'; const mockTask: TransformedFeedItem = { id: 'task-1', props: { assignees: [], author: { id: 'user-1', name: 'Creator' }, - completionRule: 'ALL_ASSIGNEES' as unknown as never, + completionRule: 'ALL_ASSIGNEES' as TaskCompletionRule, createdAt: 0, description: 'Review', id: 'task-1', permissions: { canCreateTaskCollaborator: false, canCreateTaskLink: false, canDelete: true, canUpdate: true }, - status: 'NOT_STARTED' as unknown as never, - taskType: 'GENERAL' as unknown as never, + status: 'NOT_STARTED' as TaskStatus, + taskType: 'GENERAL' as TaskType, }, type: 'task', };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx` around lines 107 - 119, The test uses unsafe casts ('ALL_ASSIGNEES' as unknown as never) for completionRule, status, and taskType which defeats TypeScript checks; import the correct enum/type symbols (e.g., TaskCompletionRule, TaskStatus, TaskType) from `@box/activity-feed` and replace the casts by using the corresponding enum members (e.g., TaskCompletionRule.ALL_ASSIGNEES) for the props: completionRule, status, and taskType so the test aligns with library types and preserves compile-time safety.src/elements/content-sidebar/__tests__/ActivitySidebar.test.js (1)
11-11: ⚡ Quick winMock is correct, but three new code paths in
ActivitySidebar.jslack test coverage.
getMentionAsync— no unit test covers its success path (should resolve withcollaborators.entries) or its reject path (should callthis.errorCallbackthen reject).
updateCommentwhenisThreadedRepliesV2EnabledistruebuthasRepliesisfalse— the newhasReplies || isThreadedRepliesV2Enabledbranch routes toupdateThreadedComment, but the existingupdateCommenttest suite only varieshasReplies.V2 render path — no test passes
features: { activityFeed: { threadedRepliesV2: { enabled: true } } }to verify the<div data-testid="activity-feed-adapter-v2" />stub is actually rendered.🧪 Suggested test additions (sketch)
+ describe('getMentionAsync()', () => { + test('should call getCollaboratorsWithQuery and resolve with entries', async () => { + const wrapper = getWrapper(); + const instance = wrapper.instance(); + fileCollaboratorsAPI.getCollaboratorsWithQuery = jest + .fn() + .mockImplementationOnce((_id, successCb) => successCb(collaborators)); + + const result = await instance.getMentionAsync('foo'); + expect(fileCollaboratorsAPI.getCollaboratorsWithQuery).toHaveBeenCalledWith( + file.id, + expect.any(Function), + expect.any(Function), + 'foo', + ); + expect(result).toEqual(collaborators.entries); + }); + + test('should call errorCallback and reject on API error', async () => { + const wrapper = getWrapper(); + const instance = wrapper.instance(); + const error = new Error('oops'); + instance.errorCallback = jest.fn(); + fileCollaboratorsAPI.getCollaboratorsWithQuery = jest + .fn() + .mockImplementationOnce((_id, _ok, errorCb) => errorCb(error, 'code', {})); + + await expect(instance.getMentionAsync('foo')).rejects.toEqual(error); + expect(instance.errorCallback).toHaveBeenCalledWith(error, 'code', {}); + }); + }); describe('updateComment()', () => { + test('should call updateThreadedComment when isThreadedRepliesV2Enabled is true and hasReplies is false', () => { + const wrapper = getWrapper({ + hasReplies: false, + features: { activityFeed: { threadedRepliesV2: { enabled: true } } }, + }); + const instance = wrapper.instance(); + instance.fetchFeedItems = jest.fn(); + + wrapper.instance().updateComment('123', 'hello', undefined, false, { can_edit: true, can_delete: true }); + + expect(api.getFeedAPI().updateThreadedComment).toBeCalledWith( + file, '123', 'hello', undefined, + { can_edit: true, can_delete: true }, + expect.any(Function), + expect.any(Function), + ); + }); describe('render()', () => { + test('should render ActivityFeedV2 adapter when threadedRepliesV2 is enabled', () => { + const wrapper = getWrapper({ + features: { activityFeed: { threadedRepliesV2: { enabled: true } } }, + elementId: 'sidebar', + }); + expect(wrapper.find('[data-testid="bcs-content"]')).toHaveLength(1); + expect(wrapper.find('[data-testid="activity-feed-adapter-v2"]')).toHaveLength(1); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/elements/content-sidebar/__tests__/ActivitySidebar.test.js` at line 11, Add unit tests to cover three new code paths in ActivitySidebar: (1) test getMentionAsync's success and failure by mocking collaborators to resolve with collaborators.entries and asserting the promise resolves, and mock a rejection to assert this.errorCallback is called and the promise rejects; (2) add an updateComment test where isThreadedRepliesV2Enabled is true and hasReplies is false to assert updateThreadedComment is called (spy/mocks on updateThreadedComment) and the correct branch executes; (3) add a render test that passes features: { activityFeed: { threadedRepliesV2: { enabled: true } } } into ActivitySidebar props and assert the mocked activity-feed-v2 adapter (data-testid="activity-feed-adapter-v2") is rendered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 134-135: Remove the entries for "@box/collaboration-popover" and
"@box/readable-time" from the peerDependencies section (they are the unique
symbols to look for) if they are not directly imported anywhere in the codebase;
scan for any direct imports/usages of those package names and, if none exist,
delete those peerDependency entries so downstream consumers are not forced to
install them, ensuring the packages remain available transitively via
"@box/activity-feed" in dependencies or devDependencies as appropriate and run
the install/build to verify nothing breaks.
In `@src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx`:
- Around line 105-113: Update userSelectorProps so ariaRoleDescription and
loadingAriaLabel use semantically appropriate intl messages rather than
messages.sidebarActivityTitle: change ariaRoleDescription to use a message like
messages.userMentionAriaRole (e.g., "Mention a user") and change
loadingAriaLabel to use a message like messages.loadingUsersAriaLabel (e.g.,
"Loading users..."). Locate the userSelectorProps object in ActivityFeedV2.tsx
and replace the two intl.formatMessage calls accordingly, adding the new keys to
the messages bundle if they don’t already exist.
In `@src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx`:
- Around line 196-208: The code passes item.props (TaskItemProps) to
onTaskDelete by casting to TaskNew in FeedItemRow; instead modify the
transformer (transformFeedItem in transformers.ts) and types
(TransformedFeedItem in types.ts) to preserve the original TaskNew object on the
transformed item (e.g., add an original or rawTask field), then update
FeedItemRow to call onTaskDelete with that preserved TaskNew (use
item.originalTask or similar) rather than casting item.props; ensure
ActivityFeed.List.Task still receives item.props (TaskItemProps) and only
deletion/view callbacks use the preserved original TaskNew.
- Around line 55-66: In FeedItemRow.tsx update the switch case for 'highlight'
so it doesn't hardcode highlightedText to '', but instead reads the text from
the target object if present (e.g., check target.text, target.selector?.exact,
or any TargetHighlight field your domain uses) and pass that value into the
returned object along with type: AnnotationBadgeType.Highlight; fall back to an
empty string only if no text field is available. This ensures the
highlightedText property is populated from the target data rather than always
empty.
In `@src/elements/content-sidebar/activity-feed-v2/transformers.ts`:
- Around line 221-251: In the FEED_ITEM_TYPE_COMMENT and
FEED_ITEM_TYPE_ANNOTATION branches replace direct use of comment.permissions and
annotation.permissions with the toPermissions() conversion so permissions cannot
be undefined at runtime; locate the blocks that cast item to Comment/Annotation
(the case for FEED_ITEM_TYPE_COMMENT and FEED_ITEM_TYPE_ANNOTATION that call
transformCommentToMessages/transformAnnotationToMessages) and set permissions:
toPermissions(comment.permissions) and permissions:
toPermissions(annotation.permissions) (or surface the toPermissions output type
instead) and remove reliance on the unsafe double cast to ensure
TransformedCommentItem.permissions and TransformedAnnotationItem.permissions are
always populated.
In `@src/elements/content-sidebar/ActivitySidebar.js`:
- Around line 995-1008: getMentionAsync currently calls
api.getCollaboratorsWithQuery on every keystroke causing excessive network
calls; either implement a debounced promise wrapper and replace getMentionAsync
with a debounced version that returns a Promise (so callers still receive
results), or debounce the invocation at the call site in ActivityFeedV2 where
getMentionAsync is passed to the mention component; update references to
getMentionAsync and ActivityFeedV2 to use the debounced-promise helper (or wrap
the call in lodash.debounce at ActivityFeedV2 and ensure the return
value/promise is preserved) and keep error handling via this.errorCallback
unchanged.
---
Nitpick comments:
In `@src/elements/content-sidebar/__tests__/ActivitySidebar.test.js`:
- Line 11: Add unit tests to cover three new code paths in ActivitySidebar: (1)
test getMentionAsync's success and failure by mocking collaborators to resolve
with collaborators.entries and asserting the promise resolves, and mock a
rejection to assert this.errorCallback is called and the promise rejects; (2)
add an updateComment test where isThreadedRepliesV2Enabled is true and
hasReplies is false to assert updateThreadedComment is called (spy/mocks on
updateThreadedComment) and the correct branch executes; (3) add a render test
that passes features: { activityFeed: { threadedRepliesV2: { enabled: true } } }
into ActivitySidebar props and assert the mocked activity-feed-v2 adapter
(data-testid="activity-feed-adapter-v2") is rendered.
In
`@src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx`:
- Around line 107-119: The test uses unsafe casts ('ALL_ASSIGNEES' as unknown as
never) for completionRule, status, and taskType which defeats TypeScript checks;
import the correct enum/type symbols (e.g., TaskCompletionRule, TaskStatus,
TaskType) from `@box/activity-feed` and replace the casts by using the
corresponding enum members (e.g., TaskCompletionRule.ALL_ASSIGNEES) for the
props: completionRule, status, and taskType so the test aligns with library
types and preserves compile-time safety.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6d838f97-8181-491c-ba87-3090c2e2dedb
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
package.jsonscripts/jest/jest.config.jssrc/elements/content-sidebar/ActivitySidebar.jssrc/elements/content-sidebar/__tests__/ActivitySidebar.test.jssrc/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.scsssrc/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsxsrc/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsxsrc/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsxsrc/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsxsrc/elements/content-sidebar/activity-feed-v2/__tests__/transformers.test.tssrc/elements/content-sidebar/activity-feed-v2/transformers.tssrc/elements/content-sidebar/activity-feed-v2/types.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/elements/content-sidebar/activity-feed-v2/transformers.ts (1)
231-233: ⚡ Quick winAdd
modified_by?to theCommenttype instead of casting.The double-cast
(comment as unknown as { modified_by?: { name?: string } })is needed only because theCommenttype is missing themodified_byfield. TheAnnotationtype already carries it (line 248 accessesannotation.modified_by?.namewithout any cast), so this asymmetry is a type-definition gap rather than a runtime difference.✏️ Suggested fix
In
src/common/types/feed.ts(or whereverCommentis declared), add the optional field:export type Comment = { // …existing fields… + modified_by?: User; // … };Then remove the unsafe cast in
transformFeedItem:- resolvedBy: commentIsResolved - ? (comment as unknown as { modified_by?: { name?: string } }).modified_by?.name - : undefined, + resolvedBy: commentIsResolved ? comment.modified_by?.name : undefined,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/elements/content-sidebar/activity-feed-v2/transformers.ts` around lines 231 - 233, Add the optional modified_by?: { name?: string } field to the Comment type definition (to match Annotation) and then remove the unsafe cast in transformFeedItem where resolvedBy is assigned from comment (currently using (comment as unknown as { modified_by?: { name?: string } }).modified_by?.name); update resolvedBy to read comment.modified_by?.name directly and ensure Comment and Annotation types are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx`:
- Around line 161-165: The onEdit handler in FeedItemRow is calling
onAnnotationEdit with text: '' which will overwrite the annotation message;
change the call so it does not pass an empty string—either make the text
parameter optional (update FeedItemRowProps and handler signatures such that
text?: string) or pass the existing annotation text from item.annotation (e.g.,
item.annotation.message) when invoking onAnnotationEdit, ensuring the handler
handleAnnotationEdit in ActivitySidebar.js receives undefined for UI-only edits
instead of an empty string.
---
Nitpick comments:
In `@src/elements/content-sidebar/activity-feed-v2/transformers.ts`:
- Around line 231-233: Add the optional modified_by?: { name?: string } field to
the Comment type definition (to match Annotation) and then remove the unsafe
cast in transformFeedItem where resolvedBy is assigned from comment (currently
using (comment as unknown as { modified_by?: { name?: string }
}).modified_by?.name); update resolvedBy to read comment.modified_by?.name
directly and ensure Comment and Annotation types are consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: df6decc2-ce84-4dfd-a51e-e11577cb4ebd
📒 Files selected for processing (6)
src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsxsrc/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsxsrc/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsxsrc/elements/content-sidebar/activity-feed-v2/__tests__/transformers.test.tssrc/elements/content-sidebar/activity-feed-v2/transformers.tssrc/elements/content-sidebar/activity-feed-v2/types.ts
✅ Files skipped from review due to trivial changes (2)
- src/elements/content-sidebar/activity-feed-v2/types.ts
- src/elements/content-sidebar/activity-feed-v2/tests/transformers.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/elements/content-sidebar/activity-feed-v2/tests/FeedItemRow.test.tsx
- src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx
…ty edit text
- Debounce getMentionAsync in ActivitySidebar so mention lookups don't fire
on every keystroke; reuse DEFAULT_COLLAB_DEBOUNCE and supersede in-flight
promises with empty results.
- Default Comment/Annotation permissions to {} in transformFeedItem so the
non-optional TransformedFeedItem.permissions field can't be undefined at
runtime.
- Stop passing text: '' to onAnnotationEdit (would blank the annotation
message); make text optional in the handler signature.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx (1)
474-502: ⚡ Quick win
onPost(reply creation) is absent from both "callbacks not provided" guard blocks, and the async case requires a different assertion pattern.The guard tests verify that unmounted callbacks don't throw, but neither the comment block (lines 475–487) nor the annotation block (lines 489–501) exercises
onPost. SinceonPostcallsserializeMentionMarkupbefore forwarding toonReplyCreate, an unguardedonReplyCreate?.()call will silently succeed, but an absent guard likeonReplyCreate()would throw aTypeErrorin production when no handler is wired.Beyond the coverage gap, the existing sync pattern can't validate an async
onPost.expect(() => onPost(content)).not.toThrow()returns before the inner promise settles, so a rejected promise goes undetected. The correct assertion form is:await expect(onPost({ type: 'doc', content: [] })).resolves.not.toThrow();Similarly,
onEdit(which firesonCommentUpdate/onAnnotationEdit) andonUnresolve(annotation-specific) are also absent from the annotation guard test.✅ Proposed additions to the guard tests
test('should not throw when comment callbacks are not provided', () => { render(<FeedItemRow {...defaultProps} item={mockComment} />); const onDelete = lastThreadedAnnotationProps.onDelete as (id: string) => void; const onResolve = lastThreadedAnnotationProps.onResolve as (id: string) => void; const onUnresolve = lastThreadedAnnotationProps.onUnresolve as (id: string) => void; const onThreadDelete = lastThreadedAnnotationProps.onThreadDelete as () => void; + const onEdit = lastThreadedAnnotationProps.onEdit as (id: string) => void; + const onPost = lastThreadedAnnotationProps.onPost as (content: unknown) => Promise<void>; expect(() => onDelete('c1')).not.toThrow(); expect(() => onResolve('c1')).not.toThrow(); expect(() => onUnresolve('c1')).not.toThrow(); expect(() => onThreadDelete()).not.toThrow(); + expect(() => onEdit('c1')).not.toThrow(); }); + test('should not throw when onReplyCreate is not provided and onPost fires for comment', async () => { + render(<FeedItemRow {...defaultProps} item={mockComment} />); + const onPost = lastThreadedAnnotationProps.onPost as (content: unknown) => Promise<void>; + await expect(onPost({ type: 'doc', content: [] })).resolves.not.toThrow(); + }); test('should not throw when annotation callbacks are not provided', () => { render(<FeedItemRow {...defaultProps} item={mockAnnotation} />); const onDelete = lastThreadedAnnotationProps.onDelete as (id: string) => void; const onResolve = lastThreadedAnnotationProps.onResolve as (id: string) => void; + const onUnresolve = lastThreadedAnnotationProps.onUnresolve as (id: string) => void; const onAnnotationBadgeClick = lastThreadedAnnotationProps.onAnnotationBadgeClick as () => void; const onThreadDelete = lastThreadedAnnotationProps.onThreadDelete as () => void; + const onEdit = lastThreadedAnnotationProps.onEdit as (id: string) => void; expect(() => onDelete('a1')).not.toThrow(); expect(() => onResolve('a1')).not.toThrow(); + expect(() => onUnresolve('a1')).not.toThrow(); expect(() => onAnnotationBadgeClick()).not.toThrow(); expect(() => onThreadDelete()).not.toThrow(); + expect(() => onEdit('a1')).not.toThrow(); }); + test('should not throw when onReplyCreate is not provided and onPost fires for annotation', async () => { + render(<FeedItemRow {...defaultProps} item={mockAnnotation} />); + const onPost = lastThreadedAnnotationProps.onPost as (content: unknown) => Promise<void>; + await expect(onPost({ type: 'doc', content: [] })).resolves.not.toThrow(); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx` around lines 474 - 502, Add coverage for missing async and sync callbacks in the "callbacks not provided" tests: when rendering FeedItemRow with mockComment and mockAnnotation (as already done), also extract lastThreadedAnnotationProps.onPost and assert it as an async handler with await expect(onPost({ type: 'doc', content: [] })).resolves.not.toThrow(); likewise add checks for onEdit (which triggers onCommentUpdate/onAnnotationEdit) and annotation-specific onUnresolve using the async .resolves.not.toThrow() pattern; ensure you still validate existing onDelete/onResolve/onThreadDelete/onAnnotationBadgeClick with the existing non-throw assertions, and reference serializeMentionMarkup/onReplyCreate only in comments if needed to explain why onPost must be awaited.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/elements/content-sidebar/activity-feed-v2/transformers.ts`:
- Around line 199-209: transformVersionToProps may return undefined for
authorName when all user sources and version.uploader_display_name are missing;
update the returned authorName to default to an empty string so it always
returns a string (for example, change the expression used to set authorName in
transformVersionToProps to use ?? ''), ensuring VersionItemProps.authorName
never propagates undefined.
In `@src/elements/content-sidebar/ActivitySidebar.js`:
- Around line 999-1034: The issue is that in-flight API responses from
superseded requests can resolve/reject a newer promise because only the pending
resolver is swapped; fix by adding a generation counter (e.g.,
this.mentionGeneration) that you increment in getMentionAsync, capture into a
local const (e.g., myGen) when calling debouncedFetchMentionCollaborators, and
in the success/error callbacks inside debouncedFetchMentionCollaborators only
call pendingMentionResolve/pendingMentionReject and clear them if myGen ===
this.mentionGeneration; otherwise ignore the callback (no-op) so stale responses
don't corrupt the active promise; keep references to
pendingMentionResolve/pendingMentionReject and the debounced call sites
(debouncedFetchMentionCollaborators, getMentionAsync) when implementing this
guard.
---
Nitpick comments:
In
`@src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx`:
- Around line 474-502: Add coverage for missing async and sync callbacks in the
"callbacks not provided" tests: when rendering FeedItemRow with mockComment and
mockAnnotation (as already done), also extract
lastThreadedAnnotationProps.onPost and assert it as an async handler with await
expect(onPost({ type: 'doc', content: [] })).resolves.not.toThrow(); likewise
add checks for onEdit (which triggers onCommentUpdate/onAnnotationEdit) and
annotation-specific onUnresolve using the async .resolves.not.toThrow() pattern;
ensure you still validate existing
onDelete/onResolve/onThreadDelete/onAnnotationBadgeClick with the existing
non-throw assertions, and reference serializeMentionMarkup/onReplyCreate only in
comments if needed to explain why onPost must be awaited.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 97b70c23-d4a7-4053-8d56-75a04c1dbdfe
📒 Files selected for processing (5)
src/elements/content-sidebar/ActivitySidebar.jssrc/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsxsrc/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsxsrc/elements/content-sidebar/activity-feed-v2/transformers.tssrc/elements/content-sidebar/activity-feed-v2/types.ts
✅ Files skipped from review due to trivial changes (1)
- src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/elements/content-sidebar/activity-feed-v2/types.ts
Use ElementsXhrError for the mention reject type (flow fix) and add a generation counter so in-flight responses from superseded requests cant resolve or reject the current promise with stale data.
reneshen0328
reviewed
May 5, 2026
reneshen0328
reviewed
May 5, 2026
reneshen0328
reviewed
May 5, 2026
Remove the onEdit handler for comments and annotations: threaded-annotations
V2 has no inline edit UI yet, and the existing wiring routed into the
submit-path (updateThreadedComment / updateAnnotation) which throws when
text and status are both undefined. Drop the onAnnotationEdit prop across
FeedItemRow, ActivityFeedV2, and ActivitySidebar's V2 render block.
Rewire onVersionClick to consume the V2 callback's { id, versionNumber }
args and remap to the V1 { id, version_number } shape that downstream
onVersionHistoryClick consumers expect.
reneshen0328
reviewed
May 5, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/elements/content-sidebar/ActivitySidebar.js (1)
480-498:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse one threaded-comment API switch across create, update, and delete.
This change updates only
updateComment()to followactivityFeed.threadedRepliesV2.enabled, whilecreateComment()anddeleteComment()still key offhasReplies. If those flags ever drift, the V2 UI will mix threaded and legacy endpoints in the same session.A single
shouldUseThreadedCommentApi = hasReplies || isFeatureEnabled(features, 'activityFeed.threadedRepliesV2.enabled')check reused by all three mutation methods would keep the behavior consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/elements/content-sidebar/ActivitySidebar.js` around lines 480 - 498, The code currently gates createComment() and deleteComment() on hasReplies while updateComment() uses isFeatureEnabled(features, 'activityFeed.threadedRepliesV2.enabled'), causing inconsistent API usage; introduce a single computed flag like shouldUseThreadedCommentApi = hasReplies || isFeatureEnabled(features, 'activityFeed.threadedRepliesV2.enabled') (or similar) at top of ActivitySidebar and replace the existing per-method checks so createComment(), updateComment(), and deleteComment() all consult shouldUseThreadedCommentApi; update references in the successCallback/errorCallback block and in the methods createComment, updateComment, deleteComment to use this unified variable.
♻️ Duplicate comments (1)
src/elements/content-sidebar/ActivitySidebar.js (1)
1004-1018:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard stale mention-request errors before calling
errorCallback.The generation check protects the promise resolution path, but stale requests still invoke
this.errorCallback(...)first. That means a superseded lookup can still surface a global error even though its result is intentionally ignored.Suggested fix
(error, code, contextInfo) => { - this.errorCallback(error, code, contextInfo); - if (generation === this.mentionGeneration && this.pendingMentionReject) { + if (generation === this.mentionGeneration && this.pendingMentionReject) { + this.errorCallback(error, code, contextInfo); this.pendingMentionReject(error); this.pendingMentionResolve = null; this.pendingMentionReject = null; } },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/elements/content-sidebar/ActivitySidebar.js` around lines 1004 - 1018, In debouncedFetchMentionCollaborators, avoid calling this.errorCallback for responses from stale generations: check that generation === this.mentionGeneration before invoking this.errorCallback and before calling this.pendingMentionReject; if the generation is stale, return/ignore the response so only the current-generation request can trigger errorCallback or pendingMentionReject (keep the existing checks for pendingMentionResolve/pendingMentionReject and reference debouncedFetchMentionCollaborators, mentionGeneration, pendingMentionResolve, pendingMentionReject, and errorCallback).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx`:
- Around line 102-131: The threaded comment/annotation component
ActivityFeed.List.ThreadedAnnotation in FeedItemRow.tsx is still wiring mutation
callbacks (onDelete, onPost, onResolve, onThreadDelete, onUnresolve) even when
the row is read-only, so update those prop bindings to honor isDisabled: if
item.isDisabled (or the row-level isDisabled flag) is true, pass noop handlers
or undefined instead of the mutation callbacks; otherwise pass the existing
closures that call onCommentDelete/onReplyCreate/onCommentUpdate. Apply the
identical guard pattern to the annotation branch (the other ThreadedAnnotation
usage around the 137-180 area) so both branches consistently disable all
mutation callbacks when isDisabled is set.
In `@src/elements/content-sidebar/ActivitySidebar.js`:
- Around line 1364-1395: The V2 branch omits passing inline error state and
callbacks to ActivityFeedV2: update ActivitySidebar so ActivityFeedV2 receives
the same error props the legacy renderer used (pass this.state.activityFeedError
and this.state.currentUserError and the fetchFeedItemsErrorCallback handler),
and ensure any prop names match ActivityFeedV2's API (e.g., activityFeedError /
currentUserError or onFetchError); this preserves inline error rendering and
keeps the global error callback behavior while switching to ActivityFeedV2.
---
Outside diff comments:
In `@src/elements/content-sidebar/ActivitySidebar.js`:
- Around line 480-498: The code currently gates createComment() and
deleteComment() on hasReplies while updateComment() uses
isFeatureEnabled(features, 'activityFeed.threadedRepliesV2.enabled'), causing
inconsistent API usage; introduce a single computed flag like
shouldUseThreadedCommentApi = hasReplies || isFeatureEnabled(features,
'activityFeed.threadedRepliesV2.enabled') (or similar) at top of ActivitySidebar
and replace the existing per-method checks so createComment(), updateComment(),
and deleteComment() all consult shouldUseThreadedCommentApi; update references
in the successCallback/errorCallback block and in the methods createComment,
updateComment, deleteComment to use this unified variable.
---
Duplicate comments:
In `@src/elements/content-sidebar/ActivitySidebar.js`:
- Around line 1004-1018: In debouncedFetchMentionCollaborators, avoid calling
this.errorCallback for responses from stale generations: check that generation
=== this.mentionGeneration before invoking this.errorCallback and before calling
this.pendingMentionReject; if the generation is stale, return/ignore the
response so only the current-generation request can trigger errorCallback or
pendingMentionReject (keep the existing checks for
pendingMentionResolve/pendingMentionReject and reference
debouncedFetchMentionCollaborators, mentionGeneration, pendingMentionResolve,
pendingMentionReject, and errorCallback).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ef3623d-89e0-4e00-b93a-799d392d74c4
📒 Files selected for processing (5)
src/elements/content-sidebar/ActivitySidebar.jssrc/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsxsrc/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsxsrc/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsxsrc/elements/content-sidebar/activity-feed-v2/types.ts
reneshen0328
reviewed
May 5, 2026
reneshen0328
reviewed
May 5, 2026
reneshen0328
reviewed
May 5, 2026
reneshen0328
reviewed
May 5, 2026
reneshen0328
reviewed
May 5, 2026
reneshen0328
reviewed
May 5, 2026
reneshen0328
reviewed
May 5, 2026
reneshen0328
reviewed
May 5, 2026
reneshen0328
reviewed
May 5, 2026
reneshen0328
reviewed
May 5, 2026
reneshen0328
reviewed
May 5, 2026
reneshen0328
reviewed
May 5, 2026
reneshen0328
reviewed
May 5, 2026
- Bump peer dep minimums to match @box/activity-feed@1.17.0 requirements (@box/blueprint-web ^14.18.0, @box/blueprint-web-assets ^4.115.4, @box/collaboration-popover ^1.61.5, @box/readable-time ^1.40.5, @box/user-selector ^1.75.5). Run yarn-deduplicate to collapse @tanstack/react-virtual to a single 3.13.24. - Cancel pending debounce and bump mention generation on ActivitySidebar unmount, and move errorCallback inside the generation check so superseded mention requests do not fire onError on a destroyed component. - Drop the broken sync getAvatarUrl wrapper in mentionContext: the vendor expects sync (id) => string but our BUIE prop is async, so the wrapper always returned empty. Leave the optional prop undefined until a caching layer exists. - Rename toPermissions() param perms to permissions, and expand the UAA acronym in file headers (public repo).
@box/blueprint-web 14.18.0 emits new CSS module class hashes; markup is otherwise unchanged.
Switch ActivityFeed list-item mocks from plain divs with data-testid to semantic article elements with aria-label, so tests can use getByRole instead of getByTestId per project testing conventions. Add afterEach(jest.clearAllMocks) in both test files so mock call counts don't leak between tests.
…ng mention on unmount - Honor isDisabled in FeedItemRow comment/annotation branches so delete, resolve, unresolve, and reply callbacks no-op in read-only mode, matching how the editor and task button already behave - Resolve any pending getMentionAsync promise with [] in componentWillUnmount so awaiters do not hang after the sidebar closes - Narrow captured-prop test types via Partial<ThreadedAnnotationsPropsV2> / Partial<TaskItemProps> / Partial<VersionItemProps> so the tests drop per-callback (as ...) => void casts
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
jmcbgaston
reviewed
May 6, 2026
jmcbgaston
reviewed
May 6, 2026
jmcbgaston
reviewed
May 6, 2026
jmcbgaston
reviewed
May 6, 2026
jmcbgaston
reviewed
May 6, 2026
jmcbgaston
reviewed
May 6, 2026
jmcbgaston
reviewed
May 6, 2026
jmcbgaston
reviewed
May 6, 2026
… empty posts - Key fetchAvatarUrls cache by contact.value (string id) instead of contact.id (Number(id) || 0 funneled non-numeric ids to 0) - Replace misused noActivityCommentPrompt and loading messages on the mention user selector with mentionUserSelectorRoleDescription and mentionUserSelectorLoading so screen readers announce the control's actual role and loading state - Trim-and-early-return empty or whitespace-only content in handleCommentPost and buildReplyPost to avoid the feed API's missing-item-text error path; tighten the try block so consumer callback throws propagate - Scope the ul > li padding rule to the top-level feed list by wrapping ActivityFeed.List in a display:contents div; prevents bleed into nested reply lists
jmcbgaston
approved these changes
May 6, 2026
reneshen0328
approved these changes
May 6, 2026
Contributor
Merge Queue Status
This pull request spent 12 minutes 10 seconds in the queue, including 11 minutes 54 seconds running CI. Required conditions to merge
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ActivityFeedV2stub with real@box/activity-feedcompound components, gated behindactivityFeed.threadedRepliesV2.enabledfeature flag@box/activity-feedcomponent propsFeedItemRowcomponent to render individual feed items with full callback wiring (create, delete, edit, resolve, reply)ObjectwithAnnotationPermission/Record, replaceFunctionwith specific signaturesActivitySidebarto pass real props and callbacks toActivityFeedV2in the v2 code pathTest plan
ActivityFeedV2.test.tsx,FeedItemRow.test.tsx,transformers.test.tsthreadedRepliesV2flag, verify feed renders comments, annotations, tasks, versions, app activitiesSummary by CodeRabbit
New Features
Style / Accessibility
Tests
Chores