fix(activity-feed-v2): forward hasActiveFilters and harden UAA parser#4606
Merged
mergify[bot] merged 2 commits intoJun 4, 2026
Merged
Conversation
- Rename useEnhancedActivities to shouldUseEnhancedActivities for naming consistency with shouldUseUAA and the shouldShow* options in the same feedItems options bag. - Add a missing-source guard to the FILE_ACTIVITY_TYPE_TASK parser case so entries lacking the task source key are dropped rather than producing a phantom feed item via spreading undefined. Matches the existing comment and annotation guards. - Add Feed.test.js to CODEOWNERS so it is co-owned alongside Feed.js.
Forward hasActiveFilters to ActivityFeed.Header.FilterMenu, derived from showOnlyMentionsMe or showResolved, so the activity feed filter trigger renders its pressed state when a filter is applied. Capture the FilterMenu prop in the ActivityFeedV2 test mock and exercise the truthy combinations of showOnlyMentionsMe and showResolved.
Contributor
WalkthroughThis PR renames Feed API options from ChangesFeed API Refactor & Filter Menu State
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/api/Feed.js (1)
577-593:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
useEnhancedActivitiesas a deprecated alias for now.
feedItems()takes a plain options bag, so any caller still sendinguseEnhancedActivitieswill now silently fall back to legacy comment/annotation activity types. Because this is a public contract rename, that becomes a behavioral regression instead of a loud failure.♻️ Suggested compatibility shim
{ shouldShowAnnotations = false, shouldShowAppActivity = false, shouldShowReplies = false, shouldShowTasks = true, shouldShowVersions = true, - shouldUseEnhancedActivities = false, + shouldUseEnhancedActivities: shouldUseEnhancedActivitiesOption, + useEnhancedActivities = false, shouldUseUAA = false, }: { shouldShowAnnotations?: boolean, shouldShowAppActivity?: boolean, shouldShowReplies?: boolean, shouldShowTasks?: boolean, shouldShowVersions?: boolean, - shouldUseEnhancedActivities?: boolean, + shouldUseEnhancedActivities?: boolean, + useEnhancedActivities?: boolean, shouldUseUAA?: boolean, } = {}, logAPIParity?: Function, ): void { + const shouldUseEnhancedActivities = shouldUseEnhancedActivitiesOption ?? useEnhancedActivities; const { id, permissions = {} } = file;🤖 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/api/Feed.js` around lines 577 - 593, The options destructuring in feedItems (in src/api/Feed.js) removed the deprecated alias useEnhancedActivities causing callers to silently get legacy behavior; restore compatibility by accepting useEnhancedActivities in the same parameter bag and map it into shouldUseEnhancedActivities (e.g., in the feedItems function parameter or immediately after destructuring, set shouldUseEnhancedActivities = shouldUseEnhancedActivities || useEnhancedActivities or use nullish coalescing so incoming useEnhancedActivities flips the new flag), keeping the original shouldUseEnhancedActivities name as the canonical flag.src/elements/content-sidebar/__tests__/ActivitySidebar.test.js (1)
865-872:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the threaded-replies-v2
feedItemscontract assertions specific.These two cases now only pin
shouldShowRepliesandshouldUseEnhancedActivities, so a regression in the same branch could still pass ifshouldUseUAAflips or another option silently drops. Please keep asserting the rest of the options bag here, or at least include the defaulted flags that must remain unchanged on the v2 path.Suggested tightening
- expect.objectContaining({ shouldShowReplies: true, shouldUseEnhancedActivities: true }), + expect.objectContaining({ + shouldShowReplies: true, + shouldUseEnhancedActivities: true, + shouldUseUAA: false, + }),Also applies to: 892-899
🤖 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` around lines 865 - 872, The test's call assertion for feedAPI.feedItems (using feedAPI.feedItems, instance.fetchFeedItemsSuccessCallback, instance.fetchFeedItemsErrorCallback, instance.errorCallback) only pins shouldShowReplies and shouldUseEnhancedActivities; tighten it to assert the full options bag expected for threaded-replies-v2 (include shouldUseUAA plus other defaulted flags that must not change) rather than a partial expect.objectContaining, and apply the same tightening to the duplicate assertion around the other occurrence (the block around lines ~892-899) so both v2-path assertions verify the complete options object.
🤖 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.
Outside diff comments:
In `@src/api/Feed.js`:
- Around line 577-593: The options destructuring in feedItems (in
src/api/Feed.js) removed the deprecated alias useEnhancedActivities causing
callers to silently get legacy behavior; restore compatibility by accepting
useEnhancedActivities in the same parameter bag and map it into
shouldUseEnhancedActivities (e.g., in the feedItems function parameter or
immediately after destructuring, set shouldUseEnhancedActivities =
shouldUseEnhancedActivities || useEnhancedActivities or use nullish coalescing
so incoming useEnhancedActivities flips the new flag), keeping the original
shouldUseEnhancedActivities name as the canonical flag.
In `@src/elements/content-sidebar/__tests__/ActivitySidebar.test.js`:
- Around line 865-872: The test's call assertion for feedAPI.feedItems (using
feedAPI.feedItems, instance.fetchFeedItemsSuccessCallback,
instance.fetchFeedItemsErrorCallback, instance.errorCallback) only pins
shouldShowReplies and shouldUseEnhancedActivities; tighten it to assert the full
options bag expected for threaded-replies-v2 (include shouldUseUAA plus other
defaulted flags that must not change) rather than a partial
expect.objectContaining, and apply the same tightening to the duplicate
assertion around the other occurrence (the block around lines ~892-899) so both
v2-path assertions verify the complete options object.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7d2a31b9-a697-4d96-9a3f-b793c3f9f305
📒 Files selected for processing (7)
CODEOWNERSsrc/api/Feed.jssrc/api/__tests__/Feed.test.jssrc/elements/content-sidebar/ActivitySidebar.jssrc/elements/content-sidebar/__tests__/ActivitySidebar.test.jssrc/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsxsrc/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx
ahorowitz123
approved these changes
Jun 4, 2026
tjuanitas
approved these changes
Jun 4, 2026
Contributor
Merge Queue Status
This pull request spent 13 seconds in the queue, including 2 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
hasActiveFilterstoActivityFeed.Header.FilterMenu,derived from
showOnlyMentionsMe || showResolved, so the activityfeed filter trigger renders its pressed state when a filter is
applied.
useEnhancedActivitiestoshouldUseEnhancedActivitiesfornaming consistency with
shouldUseUAAand theshouldShow*optionsin the same
feedItemsoptions bag.FILE_ACTIVITY_TYPE_TASKparsercase so an entry whose source key is absent is dropped rather than
producing a phantom feed item by spreading
undefined. Matches theexisting comment and annotation guards.
Feed.test.jstoCODEOWNERSso it is co-owned alongsideFeed.js.Test plan
yarn test --watchAll=false --testPathPattern="(activity-feed-v2|api/__tests__/Feed|content-sidebar/__tests__/ActivitySidebar)"yarn flowreports no errors"Show resolved" renders the filter trigger in its pressed state
the unpressed state regardless of filter selections
entry instead of producing a blank row in the feed
Summary by CodeRabbit
Bug Fixes
New Features