[WEB-2879] chore sub issue analytics improvements #6275
Conversation
WalkthroughThis pull request introduces comprehensive changes to the issue and epic tracking system, focusing on enhancing type definitions and analytics capabilities. The modifications span multiple files across the project, introducing new TypeScript types for epic analytics, updating helper functions, and extending component functionality to support more dynamic issue and epic management. The changes provide a more flexible and structured approach to tracking issue states within epics. Changes
Sequence DiagramsequenceDiagram
participant User
participant IssueComponent
participant EpicAnalytics
participant IssueStore
User->>IssueComponent: Interact with Issue
IssueComponent->>IssueStore: Update Issue State
IssueStore->>EpicAnalytics: Trigger Analytics Update
EpicAnalytics-->>IssueStore: Update Analytics Counts
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
web/core/components/issues/issue-detail-widgets/sub-issues/helper.tsx (2)
39-39: Analytics abstraction
const { updateAnalytics } = updateEpicAnalytics();wraps analytics updates in a dedicated function, facilitating future enhancements or modifications to analytics logic without impacting the main hook.Consider gracefully handling scenarios where analytics may be disabled or deferred based on environment settings.
81-81: Error handling for addition
Similarly, customizing error text when adding issues or sub-issues reinforces clarity.Consider including the parent issue ID or other details in the toast for easier debugging if needed.
web/ce/helpers/epic-analytics.ts (1)
3-15: Stubbed-out analytics function
updateAnalyticsis currently unimplemented. Adding a TODO or explanation can prevent confusion and highlight where future logic is expected.+ // TODO: Implement analytics logic here once the backend or event ingestion mechanism is ready.web/core/components/issues/sub-issues/issue-list-item.tsx (1)
60-61: Consider consolidating theuseIssueDetailcalls.
Currently, the hook is called twice in succession to destructure different properties. A single invocation that destructures both sets of properties can avoid any repeated side effects or overhead.} = useIssueDetail(issueServiceType); + const { + toggleCreateIssueModal, + toggleDeleteIssueModal, + ... + } = issueDetailHook; - const { toggleCreateIssueModal, toggleDeleteIssueModal } = useIssueDetail(issueServiceType);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/types/src/epics.d.ts(1 hunks)packages/types/src/index.d.ts(1 hunks)web/ce/helpers/epic-analytics.ts(1 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/helper.tsx(7 hunks)web/core/components/issues/sub-issues/issue-list-item.tsx(4 hunks)web/core/components/issues/sub-issues/properties.tsx(1 hunks)web/core/store/issue/helpers/base-issues.store.ts(3 hunks)web/ee/helpers/epic-analytics.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- web/ee/helpers/epic-analytics.ts
🔇 Additional comments (22)
web/core/components/issues/issue-detail-widgets/sub-issues/helper.tsx (9)
3-3: Using useParams and usePathname for route-based data retrieval
Importing the useParams and usePathname hooks from next/navigation is a clean approach for extracting URL parameters and the current path, which helps keep the component logic more encapsulated.
10-12: Enhanced importing of project and analytics hooks
Bringing in useEventTracker, useIssueDetail, useProjectState, and updateEpicAnalytics effectively consolidates store access, user tracking, and analytics capabilities, aligning with the PR objective of improving sub-issue analytics.
25-28: Deriving epicId from route parameters
Using const { epicId: epicIdParam } = useParams(); ensures that the epic context is accurately derived from the current route. This offers greater flexibility than hardcoding or explicitly passing an epic ID prop.
36-38: Introducing project state
Retrieving getStateById() from useProjectState() is a solid approach for referencing and manipulating state-based information, which is important for sub-issue involvement in different states.
54-54: Contextual success message
Differentiating between "Issue" and "Epic" links in the toast message is a minor but user-friendly detail.
65-65: Consistent error notifications
Dynamically reporting errors for sub-issues vs. issues helps maintain accurate and contextual user feedback.
75-75: Sub-issue creation feedback
Providing specific success messages for "Sub-issues" or "Issues" clarifies the outcome for users.
98-120: Epic analytics updates in updateSubIssue
Decrementing old state counts and incrementing new state counts is critical for maintaining accurate epic analytics. This logic effectively captures both parent changes and state changes.
Please verify this works as expected under rapid or sequential state changes to ensure no missed analytics updates.
157-166: Removal-based analytics updates
Decrementing the old state count upon sub-issue removal in an epic context avoids data mismatch. Good job ensuring the epic analytics remain reliable.
web/ce/helpers/epic-analytics.ts (1)
1-2: Importing the epic analytics group type
Importing TEpicAnalyticsGroup from @plane/types nicely centralizes typing for epic analytics.
packages/types/src/epics.d.ts (2)
1-7: Union type for categorizing epic issue states
TEpicAnalyticsGroup enumerates relevant states (backlog, unstarted, etc.) and helps maintain strict type checking for analytics operations.
9-16: Defining a comprehensive analytics object
TEpicAnalytics provides a clear, extensible schema for capturing the count of issues in each state. This is well structured for future analytics usage.
packages/types/src/index.d.ts (1)
39-39: Making epic types globally accessible
export * from "./epics"; ensures that the newly created epic analytics types are readily available across the entire codebase.
web/core/components/issues/sub-issues/properties.tsx (2)
20-20: Extended props for service type
Adding issueServiceType to IIssueProperty is a small but impactful change, allowing different issue services to leverage the same component logic seamlessly.
24-24: Customizing the store hook
Passing issueServiceType into useIssueDetail(issueServiceType) is a neat approach for hooking into the correct store context, especially when dealing with epics, sub-issues, or standard issues.
web/core/components/issues/sub-issues/issue-list-item.tsx (4)
6-6: Use of EIssueServiceType import is appropriate.
No issues found with this new import; it is properly referenced and aligns well with the rest of the codebase.
54-54: Defaulting issueServiceType is a good practice.
Establishing a default value for issueServiceType ensures backward compatibility whenever this prop is not provided.
168-168: Prop forwarding looks consistent.
Passing issueServiceType into <IssueProperty> helps keep the component flexible for different issue types.
214-214: Dynamic text based on issueServiceType.
Conditionally labeling the remove action is an effective way to prevent confusion between removing standard issues vs. parent issues.
web/core/store/issue/helpers/base-issues.store.ts (3)
16-16: Additional import to manage service type.
Including EIssueServiceType for properly handling the different issue services is a logical addition.
265-265: IssueService instantiation with serviceType.
This update aligns the store's logic with the new serviceType constructor parameter to distinguish between multiple issue-service contexts.
206-211: Constructor now accepts a serviceType parameter.
The extended signature accommodates multiple issue-service contexts. Ensure that all instantiations of BaseIssuesStore pass the appropriate serviceType or rely on the default.
To confirm correct usage, run:
| const { removeSubIssue } = useIssueDetail(issueServiceType); | ||
| const { getStateById } = useProjectState(); | ||
| const { peekIssue: epicPeekIssue } = useIssueDetail(EIssueServiceType.EPICS); | ||
| // const { updateEpicAnalytics } = useIssueTypes(); |
There was a problem hiding this comment.
Please remove unused comments
Description
This PR addresses a bug related to sub-issue analytic mutation.
References
[WEB-2879]
Summary by CodeRabbit
New Features
IssueListItemandIssuePropertycomponents to support dynamic handling of different issue types.Bug Fixes
Documentation
Refactor