[WEB-4355] fix: activity additional property handling #7273
Conversation
WalkthroughThis update introduces the "added" verb to several activity and notification types, refactors type definitions for clarity, and centralizes notification content rendering logic via shared utility functions. It also adds new React components and exports for issue detail pages, with re-exports for both CE and EE codebases to unify component usage. Changes
Sequence Diagram(s)sequenceDiagram
participant IssueActivityItem
participant IssuePageActivity
IssueActivityItem->>IssuePageActivity: Render when activity field is "page"
sequenceDiagram
participant NotificationContent
participant ContentUtils
NotificationContent->>ContentUtils: renderAdditionalAction(field, verb)
NotificationContent->>ContentUtils: renderAdditionalValue(field, new, old)
NotificationContent->>ContentUtils: shouldShowConnector(field)
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
gakshita
changed the title
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
web/ce/components/issues/issue-details/page.tsx (1)
1-8: Stub currently renders nothing – add TODO or minimal fallback UI.Returning an empty fragment gives no visual cue and makes debugging hard.
At minimum add a comment or a skeleton placeholder so it’s obvious in the UI that this activity type is pending implementation.web/ce/components/workspace-notifications/notification-card/content.ts (1)
14-17: Consider making the exclusion list configurable.The hardcoded field list could become stale as new notification fields are added. Consider extracting this to a configuration constant or making it more maintainable.
+const FIELDS_WITHOUT_CONNECTOR = [ + "comment", + "archived_at", + "None", + "assignees", + "labels", + "start_date", + "target_date", + "parent" +] as const; + export const shouldShowConnector = (notificationField: string | undefined) => - !["comment", "archived_at", "None", "assignees", "labels", "start_date", "target_date", "parent"].includes( + !FIELDS_WITHOUT_CONNECTOR.includes( notificationField || "" );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/types/src/activity.d.ts(2 hunks)packages/types/src/workspace-notifications.d.ts(1 hunks)web/ce/components/issues/issue-details/index.ts(1 hunks)web/ce/components/issues/issue-details/page.tsx(1 hunks)web/ce/components/workspace-notifications/index.ts(1 hunks)web/ce/components/workspace-notifications/notification-card/content.ts(1 hunks)web/core/components/issues/issue-detail/issue-activity/activity/activity-list.tsx(2 hunks)web/core/components/workspace-notifications/sidebar/notification-card/content.tsx(3 hunks)web/ee/components/issues/issue-details/index.ts(1 hunks)web/ee/components/issues/issue-details/page.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
web/core/components/issues/issue-detail/issue-activity/activity/activity-list.tsx (1)
web/ce/components/issues/issue-details/page.tsx (1)
IssuePageActivity(8-8)
web/core/components/workspace-notifications/sidebar/notification-card/content.tsx (1)
web/ce/components/workspace-notifications/notification-card/content.ts (3)
renderAdditionalAction(3-6)renderAdditionalValue(8-12)shouldShowConnector(14-17)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (12)
web/ee/components/issues/issue-details/index.ts (1)
7-7: Export added correctly – no issues.The re-export aligns EE with CE; build surface stays consistent.
web/ce/components/issues/issue-details/index.ts (1)
8-8: Export list now includespage– looks good.Keeps CE and EE in sync; no problems spotted.
web/ce/components/workspace-notifications/index.ts (1)
3-3: New export may clash ifcontentalready re-exported elsewhere.Double-check other index files under
workspace-notificationsto ensure./notification-card/contentisn’t already surfaced, avoiding duplicate symbol warnings in consumers.web/ee/components/issues/issue-details/page.tsx (1)
1-1: Path alias relies on tsconfig – confirm it resolves.
export * from "ce/components/...";assumes a “ce” path alias is configured for EE builds. Verify tsconfig paths or webpack aliases include it; otherwise relative import (../../ce/...) is safer.web/core/components/issues/issue-detail/issue-activity/activity/activity-list.tsx (2)
8-12: LGTM - Import structure follows existing patterns.The import addition is well-structured and consistent with the existing import pattern.
96-97: ```shell
#!/bin/bashSearch for IssuePageActivity definition and usage across the repo
rg -n "IssuePageActivity" -C 3
</details> <details> <summary>packages/types/src/workspace-notifications.d.ts (1)</summary> `31-31`: **LGTM - Consistent type extension.** The addition of the "added" verb maintains consistency with the corresponding change in `activity.d.ts` and supports the new functionality. </details> <details> <summary>web/core/components/workspace-notifications/sidebar/notification-card/content.tsx (2)</summary> `65-65`: **LGTM - Good refactoring to centralize logic.** The refactoring to use helper functions is a good practice that centralizes the rendering logic and reduces code duplication. Also applies to: 82-82, 91-91 --- `2-6`: ```shell #!/bin/bash sed -n '1,200p' web/ce/components/workspace-notifications/notification-card/content.tspackages/types/src/activity.d.ts (2)
30-30: LGTM - Consistent type system extension.The addition of "added" to
TBaseActivityVerbsmaintains consistency with the notification types and supports the new functionality.
1-1: LGTM - Formatting improvements.The formatting changes improve readability without altering functionality.
Also applies to: 19-19, 23-26
web/ce/components/workspace-notifications/notification-card/content.ts (1)
3-6: LGTM - Well-structured action rendering logic.The function correctly handles field formatting and verb integration for notification actions.
Description
Summary by CodeRabbit
New Features
Improvements
Exports