Chore(UI): Fix activity feed tab bugs - #30879
Conversation
…activity feed tab
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
github-actions
Bot
added
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| </Space> | ||
|
|
||
| <span> | ||
| <span data-testid="left-panel-all-count"> |
There was a problem hiding this comment.
💡 Bug: All badge mixes server total with client-loaded count
The left-panel "All" badge is computed as conversationCount + activityEvents.length, where conversationCount is a server-side total (from getFeedCount) but activityEvents.length is the number of client-loaded activities, which fetchEntityActivity caps at limit: 50. Meanwhile the entity-tab header total uses activityCount = paging.total (the true server total). For entities with more than 50 activity events, or when conversations paginate beyond the loaded entityThread, the badge, the header total, and the rendered list will disagree — the exact invariant the new Playwright test 'All badge, header and rendered list agree on the count' asserts (it only passes because the test entity has one activity). Consider deriving the badge from the same server counts used for the header (e.g. conversationCount + activityCount) so all three stay consistent regardless of pagination.
Use the server-side activityCount instead of the client-loaded activityEvents.length so the badge matches the header total.:
getCountBadge(
(countData?.data?.conversationCount ?? 0) +
(countData?.data?.activityCount ?? 0),
'',
activeTab === ActivityFeedTabs.ALL
)
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
|
Code Review 👍 Approved with suggestions 0 resolved / 1 findingsFixes activity feed tab bugs by marking change-event activities as read-only and unskipping Playwright tests. Consider addressing the badge count mismatch where the 'All' badge mixes server totals with client-loaded counts. 💡 Bug: All badge mixes server total with client-loaded count📄 openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedTab/ActivityFeedTab.component.tsx:752-760 📄 openmetadata-ui/src/main/resources/ui/src/utils/FeedUtilsPure.ts:600-613 The left-panel "All" badge is computed as Use the server-side activityCount instead of the client-loaded activityEvents.length so the badge matches the header total.🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source |
There was a problem hiding this comment.
Pull request overview
This PR fixes regressions in the Activity Feed UI introduced during the task redesign work by (1) making activity change-events read-only (no replies/editor), and (2) correctly rendering/counting both conversation threads and activity change-events together on the “All” tab. It also adds/updates Jest and Playwright coverage to prevent these regressions from reappearing.
Changes:
- Correct feed counting by separating conversation thread counts (feed store) from activity change-event counts (activity stream) and summing them appropriately.
- Update Activity Feed UI behavior so activity change-events are read-only and conversation threads remain fully interactive.
- Add regression-focused Jest + Playwright tests for merged rendering, read-only behavior, selection determinism, and count consistency.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/utils/FeedUtilsPure.ts | Updates feed counting logic to include conversation + activity + tasks, and propagates activityCount. |
| openmetadata-ui/src/main/resources/ui/src/utils/FeedUtilsPure.getFeedCounts.test.ts | Adds unit tests asserting conversation/activity counts are not conflated and totals are correct. |
| openmetadata-ui/src/main/resources/ui/src/interface/feed.interface.ts | Extends FeedCounts with activityCount. |
| openmetadata-ui/src/main/resources/ui/src/constants/entity.constants.ts | Initializes activityCount in FEED_COUNT_INITIAL_DATA. |
| openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedTab/ActivityFeedTab.component.tsx | Gates activity fetch by tab, fixes selection clearing, and adjusts “All” badge to include conversations + activities. |
| openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedTab/ActivityFeedTab.component.test.tsx | Adds tests for tab-gated activity fetching and correct “All” badge summation. |
| openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/DummyTestComponent.tsx | Updates provider test helpers to reflect read-only activities and reaction sync behavior. |
| openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/ActivityFeedProviderContext.interface.ts | Removes activity-thread/comment APIs from provider context to enforce read-only activities. |
| openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/ActivityFeedProvider.tsx | Removes activity-thread/comment flow; keeps selected activity in sync on reaction updates. |
| openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/ActivityFeedProvider.test.tsx | Replaces activity-comment tests with reaction sync and read-only selection tests. |
| openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedPanel/ActivityPanelBody.tsx | Removes editor/comment UI for activity events (read-only). |
| openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedList/ActivityFeedListV1New.component.tsx | Merges activity + conversation lists, sorts by timestamp, and makes auto-selection deterministic. |
| openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedList/ActivityFeedListV1New.component.test.tsx | Adds tests for merged rendering, ordering, empty states, and auto-selection behavior. |
| openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedCardNew/ActivityFeedcardNew.component.tsx | Ensures activities never render reply UI and only conversation threads can be replied to. |
| openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedCardNew/ActivityFeedcardNew.component.test.tsx | Adds tests verifying editor appears only for conversation threads (not activities). |
| openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeed.spec.ts | Re-enables mention notification test and adds regression suite for merged list + read-only activity behavior. |
Suppressed comments (2)
openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedList/ActivityFeedListV1New.component.tsx:180
componentsVisibilityis accepted byActivityFeedListV1Newbut isn't forwarded toFeedPanelBodyV1Newfor feed (conversation) items either, so callers passing it won't see any effect. Forward it consistently so the flag can be consumed downstream if/when implemented.
<FeedPanelBodyV1New
feed={item.feed}
handlePanelResize={handlePanelResize}
hidePopover={hidePopover}
isActive={activeFeedId === item.id}
openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedProvider/DummyTestComponent.tsx:233
- This dummy test component introduces a hardcoded, user-visible button label (
react). UI code in this repo is expected to avoid hardcoded user-facing strings (use i18n keys or remove the visible label if it's only needed for tests).
}>
react
</button>
| <FeedPanelBodyV1New | ||
| activity={activity} | ||
| activity={item.activity} | ||
| handlePanelResize={handlePanelResize} | ||
| hidePopover={hidePopover} | ||
| isActive={activeFeedId === activity.id} | ||
| isActive={activeFeedId === item.id} |
| <button | ||
| data-testid="select-activity" | ||
| onClick={() => setActiveActivity(activityEvents[0])}> | ||
| select | ||
| </button> |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/ActivityFeedTab/ActivityFeedTab.component.tsx:277
getFeedCount()returns an array and can legitimately be empty. Accessingres[0].conversationCount/res[0].mentionCountwill throw when there are no feed items for the user. Also, the API can return multiple entries (per field), so picking index 0 can undercount.
Consider defensively summing across the response and handling the empty-array case (similar to FeedUtilsPure.getFeedCounts).
if (isUserEntity) {
// Also get feed counts for conversations and mentions
const res = await getFeedCount(getEntityUserLink(fqn));
setCountData((prev) => ({
...prev,
| export interface FeedCounts { | ||
| conversationCount: number; | ||
| activityCount: number; | ||
| totalTasksCount: number; |
Jest test CoverageUI tests summary
|
|
✅ Playwright Results — workflow succeededValidated commit ✅ 566 passed · ❌ 0 failed · 🟡 1 flaky · ⏭️ 3 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 54m 27s ⏱️ Max setup 3m 3s · max shard execution 14m 58s · max shard-job elapsed before upload 18m 38s · reporting 5s 🌐 202.65 requests/attempt · 2.81 app boots/UI scenario · 14.25% common-shard skew Optimization targets still in progress:
🟡 1 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
This pull request addresses regressions in the Activity Feed component and its tests, primarily ensuring that activity change-events are treated as read-only and cannot be replied to, while conversation threads retain full interactivity. It also introduces a new test suite for these behaviors and cleans up related code and test logic.
Activity Feed: Read-only Activities & Regression Tests
Read-only activity events:
ActivityFeedCardNewso that change-event activities are always rendered as read-only: replies are disabled, the comment editor is hidden, and only conversation threads can be replied to. This logic is enforced in both the UI and theonSavehandler. [1] [2] [3]Test coverage:
ActivityFeedcardNew.component.test.tsxthat verifies conversation threads show the comment editor, while activity events do not.Test improvements and fixes:
Minor code cleanup: