Skip to content

fix(activity-feed-v2): Show Edited indicator and fix version mapping#4570

Merged
mergify[bot] merged 1 commit into
box:masterfrom
jackiejou:fix/activity-feed-v2-edited-indicator-and-version-mapping
May 20, 2026
Merged

fix(activity-feed-v2): Show Edited indicator and fix version mapping#4570
mergify[bot] merged 1 commit into
box:masterfrom
jackiejou:fix/activity-feed-v2-edited-indicator-and-version-mapping

Conversation

@jackiejou
Copy link
Copy Markdown
Contributor

@jackiejou jackiejou commented May 20, 2026

Summary

Three fixes to the new ActivityFeedV2 transformer layer so it matches v1 behavior:

  • Edited indicator: populate TextMessageType.updatedAt from modified_at when it differs from created_at. After editing a comment or annotation, threaded-annotations now renders the "Edited" label.
  • Mention authorId: thread the message author id through textToDocumentNode so each mention node carries authorId: comment.created_by.id (or annotation creator) instead of an empty string. This restores the data-author-id attribute on rendered mention elements.
  • Version mapping: derive the version action from version_promoted / restored_at / trashed_at flags before falling back to action_type, matching v1's selectors/version.js. Restore v1 priority for the version author (restored_by -> trashed_by -> promoted_by -> modified_by); the previous v2 ordering preferred modified_by and so showed the wrong actor for promote/restore/delete events.

Test plan

  • yarn test --watchAll=false --testPathPattern="activity-feed-v2" (136/136 pass, 16 new tests)
  • Edit a root annotation in the v2 sidebar; verify "Edited" label appears
  • Edit a root comment in the v2 sidebar; verify "Edited" label appears
  • Edit a reply; verify "Edited" label appears
  • Promote a version; verify the actor shown matches the user who promoted (not the original uploader)
  • Inspect a rendered mention; verify data-author-id is the comment/annotation author id

Summary by CodeRabbit

  • New Features

    • Activity feed now displays mention author attribution in comments and annotations.
    • Added edited timestamp tracking for comments and annotations.
    • Improved version action tracking and type derivation.
  • Tests

    • Updated test coverage for mention parsing with author attribution and edit timestamps.
    • Expanded version action mapping test cases.

Review Change Stack

- Populate TextMessageType.updatedAt from modified_at when it differs
  from created_at so threaded-annotations renders the Edited indicator
  after a successful edit.
- Thread the message author id through textToDocumentNode so each
  mention node carries the correct authorId instead of an empty string.
- Derive version action from version_promoted, restored_at and
  trashed_at flags before falling back to action_type, matching v1
  behavior. Restore v1 priority for the version author (restored_by
  -> trashed_by -> promoted_by -> modified_by).
@jackiejou jackiejou requested review from a team as code owners May 20, 2026 02:08
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

Walkthrough

The PR extends the activity feed transformers to support mention author attribution and message modification timestamps. Text parsing now accepts and propagates an authorId parameter into mention nodes. Comments and annotations include optional updatedAt derived from creation/modification timestamps. Version transformation logic is refactored to prioritize action-specific user IDs and derive action types from version flags when action_type is missing.

Changes

Activity Feed Transformer Enhancements

Layer / File(s) Summary
Mention author attribution in document parsing
src/elements/content-sidebar/activity-feed-v2/transformers.ts, src/elements/content-sidebar/activity-feed-v2/__tests__/transformers.test.ts
parseLine and textToDocumentNode now accept and inject authorId into mention node attributes. Tests for single-line, multi-line, and multi-mention cases are updated to pass authorId and verify propagation into mention node attrs.
Message update timestamps and comment/annotation integration
src/elements/content-sidebar/activity-feed-v2/transformers.ts, src/elements/content-sidebar/activity-feed-v2/__tests__/transformers.test.ts
New toUpdatedAt helper computes optional unix-ms timestamps, returning undefined when unedited. Comment and annotation transformations now pass authorId to document parsing and populate updatedAt. Tests verify updatedAt is undefined when unedited and matches modified_at when edited, and that mention nodes within comments/annotations receive the correct creator authorId.
Version action and user derivation refactoring
src/elements/content-sidebar/activity-feed-v2/transformers.ts, src/elements/content-sidebar/activity-feed-v2/__tests__/transformers.test.ts
Replaced mapVersionActionType with helpers mapActionTypeString, getVersionAction, and getVersionUser. transformVersionToProps now derives actionType from promotion/restoration/trash flags with fallback mapping, and selects author from `promoted_by

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

ready-to-merge

Suggested reviewers

  • ahorowitz123
  • JChan106
  • kduncanhsu

Poem

🐰 Mentions now know who speaks their words,
Updates logged when changes are heard,
Actions derived from their ancient flags,
Authors tracked through the feed's journeys and tags,
No more empty voices in the talk.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the three main changes: edited indicator, mention authorId, and version mapping fixes in the activity-feed-v2 transformer.
Description check ✅ Passed The description is comprehensive and well-structured, covering all three fixes with clear explanations and a detailed test plan, though it differs from the template's focus on merge procedures.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/elements/content-sidebar/activity-feed-v2/__tests__/transformers.test.ts (1)

427-455: ⚡ Quick win

Add an explicit restored_by precedence regression test.

The new ordering puts restored_by ahead of the other version user fields, but the suite only locks in promotion precedence and a trashed fallback. A small restore case would guard the highest-priority branch against future reorder regressions.

Suggested test shape
+        test('should prefer restored_by over other version users', () => {
+            const version = {
+                ...mockVersion,
+                action_type: undefined,
+                modified_by: { id: '300', name: 'Uploader', type: 'user' },
+                promoted_by: { id: '500', name: 'Promoter', type: 'user' },
+                restored_at: '2024-04-02T00:00:00Z',
+                restored_by: { id: '600', name: 'Restorer', type: 'user' },
+            };
+            const result = transformVersionToProps(version as unknown as BoxItemVersion);
+            expect(result.authorName).toBe('Restorer');
+            expect(result.actionType).toBe('restore');
+        });
🤖 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__/transformers.test.ts`
around lines 427 - 455, Add a unit test in the same suite to assert restored_by
takes precedence for author and yields a restore action: create a version object
based on mockVersion with restored_at set (or action_type: 'restored'), include
restored_by: { id: '700', name: 'Restorer', type: 'user' } plus other user
fields like modified_by and uploaded_by with different names, call
transformVersionToProps(version as unknown as BoxItemVersion) and expect
authorName toBe 'Restorer' and actionType toBe 'restore' to lock in restored_by
precedence in transformVersionToProps.
🤖 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.

Nitpick comments:
In
`@src/elements/content-sidebar/activity-feed-v2/__tests__/transformers.test.ts`:
- Around line 427-455: Add a unit test in the same suite to assert restored_by
takes precedence for author and yields a restore action: create a version object
based on mockVersion with restored_at set (or action_type: 'restored'), include
restored_by: { id: '700', name: 'Restorer', type: 'user' } plus other user
fields like modified_by and uploaded_by with different names, call
transformVersionToProps(version as unknown as BoxItemVersion) and expect
authorName toBe 'Restorer' and actionType toBe 'restore' to lock in restored_by
precedence in transformVersionToProps.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fd316ca9-9f51-442a-b070-f0c505206f11

📥 Commits

Reviewing files that changed from the base of the PR and between fa9ccea and 42e5a4e.

📒 Files selected for processing (2)
  • src/elements/content-sidebar/activity-feed-v2/__tests__/transformers.test.ts
  • src/elements/content-sidebar/activity-feed-v2/transformers.ts

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 20, 2026

Merge Queue Status

  • Entered queue2026-05-20 22:30 UTC · Rule: Automatic strict merge
  • Checks skipped · PR is already up-to-date
  • Merged2026-05-20 22:31 UTC · at 42e5a4ec7a8c2ad4ec39e79c171ae790459cbcaf · squash

This pull request spent 43 seconds in the queue, including 6 seconds running CI.

Required conditions to merge

@mergify mergify Bot merged commit 60486da into box:master May 20, 2026
10 of 11 checks passed
@mergify mergify Bot removed the queued label May 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants