[WEB-2907] chore: issue store updated and code refactor #6279
Conversation
anmolsinghbhatia
added
🐛bug
WalkthroughThe pull request introduces a comprehensive enhancement to issue management by adding an Changes
Suggested Reviewers
Poem
Suggested Labels
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 (15)
web/core/components/issues/issue-layouts/quick-add/form/kanban.tsx (1)
15-15: Placeholder text updated based onisEpic.
This dynamic placeholder is helpful. Consider if any i18n or additional customization might be required.web/core/components/issues/issue-layouts/quick-add/form/gantt.tsx (2)
21-23: Good approach for dynamic placeholders and validation messages.
UsingisEpicto switch between "Epic" and "Issue" is a clean pattern. Consider centralizing strings in a constants file if you anticipate multiple similar dynamic expressions across the codebase.
29-29: Informative guidance for user input.
Displaying a dynamic message is beneficial for user experience. If you plan to localize the app, ensure this string is also covered.web/ce/components/issues/quick-add/root.tsx (1)
37-37: Properly pullingisEpicfrom props.
The destructuring pattern is clear. Consider providing a default value (e.g.,false) to avoid potential undefined instances if this prop is used across multiple apps or libraries.web/core/store/issue/issue.store.ts (1)
42-42: Initializing the service within the constructor.
This approach is fine; if you require specialized configuration in the future, consider injecting dependencies instead for improved testability.web/core/components/issues/issue-layouts/calendar/quick-add-issue-actions.tsx (2)
31-31: Destructuring default forisEpic.
Defaulting tofalseis a good move, ensuring no unintended truthy usage. Keep consistent defaults across other similar props.
126-127: Conditional menu items.
This approach for toggling "New Epic" vs. "New Issue" and adding the "Add existing issue" only if!isEpicis intuitive. Use caution if you eventually need “Add existing epic.”web/ce/store/issue/issue-details/activity.store.ts (1)
100-100: Check for null/undefined comment object
Before callingcurrentStore.comment.getCommentsByIssueId(issueId), consider ensuringcurrentStore.commentis defined to prevent a runtime error ifcommentis missing or uninitialized.- const comments = currentStore.comment.getCommentsByIssueId(issueId) || []; + const comments = currentStore?.comment?.getCommentsByIssueId(issueId) || [];web/core/store/issue/root.store.ts (2)
70-70: Document the newepicDetailproperty.Consider adding a brief code comment to explain how
epicDetailis expected to be used or how it interacts with other issue-related properties. This clarification helps maintain consistency for future contributors.
138-138: Ensure parallel definitions betweenissueDetailandepicDetail.Both properties are logically similar, so it might be useful to maintain a uniform approach to naming, documentation, and usage patterns for clarity. Optionally, confirm that any watchers, reactions, or additional initializations required for
epicDetailare mirrored fromissueDetail.web/core/components/issues/issue-layouts/kanban/kanban-group.tsx (1)
288-288: Overlay styling for epics.You're passing
isEpicto<GroupDragOverlay />. Consider customizing the overlay style or adding a distinct visual marker to differentiate epic items if that’s part of the product requirements.web/core/components/issues/issue-layouts/quick-add/form/list.tsx (2)
22-22: Validation message alignment.The required field message matches the placeholder logic. Keep the user’s mental model consistent by ensuring any longer disclaimers or tooltips also reflect the correct type.
28-28: Improve user guidance for adding multiple epics or issues.The dynamic text is helpful. If this pattern is repeated elsewhere (e.g., Kanban Quick Add), ensure consistency across all quick-add flows.
web/core/components/issues/create-issue-toast-action-items.tsx (2)
30-30: Consider extracting the URL pattern to avoid duplicationThe URL pattern is duplicated between the
issueLinkvariable and thehrefattribute below. Consider extracting it into a utility function or constant to maintain DRY principles.+const getItemUrl = (workspaceSlug: string, projectId: string, issueId: string, isEpic: boolean) => + `${workspaceSlug}/projects/${projectId}/${isEpic ? "epics" : "issues"}/${issueId}`; -const issueLink = `${workspaceSlug}/projects/${projectId}/${isEpic ? "epics" : "issues"}/${issueId}`; +const issueLink = getItemUrl(workspaceSlug, projectId, issueId, isEpic);
52-52: Simplify the template literalThe template literal can be simplified as it's only concatenating static strings.
-{`View ${isEpic ? "epic" : "issue"}`} +{`View ${isEpic ? "epic" : "issue"}`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
web/ce/components/issues/quick-add/root.tsx(2 hunks)web/ce/store/issue/issue-details/activity.store.ts(2 hunks)web/core/components/issues/create-issue-toast-action-items.tsx(3 hunks)web/core/components/issues/issue-layouts/calendar/quick-add-issue-actions.tsx(2 hunks)web/core/components/issues/issue-layouts/kanban/default.tsx(1 hunks)web/core/components/issues/issue-layouts/kanban/kanban-group.tsx(3 hunks)web/core/components/issues/issue-layouts/list/default.tsx(1 hunks)web/core/components/issues/issue-layouts/quick-add/form/calendar.tsx(2 hunks)web/core/components/issues/issue-layouts/quick-add/form/gantt.tsx(2 hunks)web/core/components/issues/issue-layouts/quick-add/form/kanban.tsx(2 hunks)web/core/components/issues/issue-layouts/quick-add/form/list.tsx(2 hunks)web/core/components/issues/issue-layouts/quick-add/form/spreadsheet.tsx(2 hunks)web/core/components/issues/issue-layouts/quick-add/root.tsx(6 hunks)web/core/components/issues/issue-layouts/utils.tsx(2 hunks)web/core/hooks/store/use-issue-detail.ts(1 hunks)web/core/store/issue/issue.store.ts(4 hunks)web/core/store/issue/root.store.ts(3 hunks)web/core/store/root.store.ts(0 hunks)
💤 Files with no reviewable changes (1)
- web/core/store/root.store.ts
🔇 Additional comments (40)
web/core/hooks/store/use-issue-detail.ts (1)
12-12: Consider graceful fallback for epicDetail.
While the logic is straightforward, if epicDetail is ever undefined, returning it could cause runtime issues. As a precaution, consider providing a fallback or verifying that this property is set.
web/core/components/issues/issue-layouts/quick-add/form/kanban.tsx (2)
6-6: New prop isEpic is well introduced.
The introduction of isEpic helps differentiate epic vs. standard issue creation.
23-23: User guidance text improved.
Using conditional text to instruct the user is a clear UX enhancement. Good job!
web/core/components/issues/issue-layouts/quick-add/form/spreadsheet.tsx (4)
6-6: isEpic property introduction is consistent.
This maintains parity with the other quick-add forms.
19-19: Dynamic placeholder logic.
Nicely done. This keeps the form user-friendly for both issues and epics.
21-21: Required message is now context-aware.
This is a good detail to guide users accurately.
27-27: Instruction text clarifies epic vs. issue creation.
Enhances user understanding of the form’s functionality.
web/core/components/issues/issue-layouts/quick-add/form/calendar.tsx (4)
6-6: Addition of isEpic to props.
This is consistent with other quick-add forms.
10-12: Conditional animation logic for isOpen.
Looks good. No concurrency or performance concerns noted.
23-23: Dynamic placeholder text.
Great for distinguishing epics from standard issues at a glance.
25-25: Context-aware required message.
This ensures the user is prompted correctly for epics vs. issues.
web/core/components/issues/issue-layouts/quick-add/form/gantt.tsx (1)
7-7: Great usage of destructuring for new prop.
The isEpic prop is properly extracted here, ensuring that the conditionals in subsequent lines work seamlessly.
web/ce/components/issues/quick-add/root.tsx (2)
33-33: New boolean prop complements the existing type definitions.
Declaring isEpic here is straightforward and consistent. Ensure no references require the prop to be optional when it’s defined as required.
74-74: Prop forwarding ensures layout-specific handling.
Passing isEpic to the layout form fosters a clean separation of concerns. This fosters easy expansions if new flags are introduced in the future (e.g., isBug, isFeature).
web/core/store/issue/issue.store.ts (3)
6-6: Ensuring type safety for TIssue usage.
Although this line is annotated, it appears standard. No issues are apparent here.
33-33: Constructor simplification.
Removing the serviceType indicates a shift toward a default service usage. Verify if any legacy code references serviceType or if you need polymorphism in the future.
88-90: Selective update of persistent layer.
By excluding epics from updatePersistentLayer, you preserve them from further persistence changes. Double-check that this is the intended logic and won’t result in inconsistent data for epics.
web/core/components/issues/issue-layouts/calendar/quick-add-issue-actions.tsx (3)
27-27: Optional isEpic prop broadens usage scenarios.
Having this prop allows smoother transitions between epic-related and standard issue flows.
122-122: User-friendly button label.
Changing the label to "New Epic" assists user understanding. Great for immediate clarity on which entity is being created.
131-131: isEpic forwarded correctly.
Propagating isEpic through the quick-add flow helps maintain consistent logic.
web/ce/store/issue/issue-details/activity.store.ts (2)
96-97: Ensure store properties are defined before use
It appears that this.serviceType correctly selects epicDetail or issueDetail, but please confirm that both this.store.issue.epicDetail and this.store.issue.issueDetail are always defined to avoid potential undefined errors.
113-113: Validate comment retrieval logic
You already check if comment is falsy after retrieval. This logic is solid for handling a nonexistent comment scenario; no further action needed.
web/core/components/issues/issue-layouts/quick-add/root.tsx (5)
9-9: Imported new constants
Importing updated constants EIssueLayoutTypes and EIssueServiceType is consistent with the rest of the PR. This looks good.
33-37: “isEpic” property additions
The addition of isEpic fields across form types (TQuickAddIssueForm, TQuickAddIssueButton, TQuickAddIssueRoot) effectively distinguishes epic creation from regular issues. Implementation aligns well with the rest of the codebase.
Also applies to: 50-50
67-67: Default value for “isEpic”
Defaulting the prop to false is a safe choice to avoid breaking existing logic when the property is not provided.
116-119: Conditional messaging for epic creation
The logic for conditionally displaying “epic” or “issue” during creation and success messages is clear and user-friendly.
Also applies to: 125-125
173-173: Dynamic UI elements for epics
Your conditional rendering of epics vs. issues (button labels, text) provides a smooth user experience. No issues found.
Also applies to: 177-177, 185-185
web/core/components/issues/issue-layouts/list/default.tsx (1)
87-87: Indicating “isEpic” for grouped issues
Passing the isEpic prop into getGroupByColumns ensures correct grouping logic for epics. This approach is consistent with other layout components.
web/core/components/issues/issue-layouts/kanban/default.tsx (1)
101-101: Kanban grouping for epics
Passing isEpic into the Kanban columns aligns with your epic/issue distinction. This is a good, minimal change that maintains consistency.
web/core/store/issue/root.store.ts (1)
226-229: Confirm the instantiation sequence and the default service type.
this.issues = new IssueStore();omits the service type, relying on theIssueStoredefault. Ensure that the default aligns with the intended usage.- Using separate service types for
issueDetail(ISSUES) andepicDetail(EPICS) is a solid approach to keep them scoped. Confirm all references in the rest of the codebase.
web/core/components/issues/issue-layouts/kanban/kanban-group.tsx (2)
85-85: Set default isEpic to false.
Providing a default of false is a practical choice, preventing undefined checks. Ensure that all calling components properly handle isEpic when they want to differentiate epic vs. non-epic items.
316-316: Enable or disable the quick-add form for epics.
This line effectively extends epic creation to the quick-add form. If there are differences in validation or permissible fields for epics vs. issues, ensure those are enforced here or in downstream logic.
web/core/components/issues/issue-layouts/utils.tsx (3)
71-71: Type extension for isEpic property.
Introducing an optional isEpic is consistent with the rest of the PR’s enhancements. Confirm that all references to TGetGroupByColumns properly handle the new property if omitted.
81-81: Default parameter usage for isEpic.
Providing isEpic = false achieves well-defined behavior. Confirm that no existing code paths rely on this property being undefined.
88-88: Conditionally naming "All Epics" vs. "All Issues".
The conditional naming is straightforward and effective. Ensure that the rest of the UI, including pagination and search, also correctly references "Epics" if isEpic is set.
web/core/components/issues/issue-layouts/quick-add/form/list.tsx (2)
6-6: Name alignment with epic vs. issue.
isEpic is being passed as part of the props. This is consistent with the rest of the PR. Make sure at the call site that isEpic is set accurately.
20-20: Contextual placeholder text.
Using "Epic Title" vs. "Issue Title" enhances clarity. This subtle UI distinction is beneficial for users. Good approach.
web/core/components/issues/create-issue-toast-action-items.tsx (3)
13-13: LGTM: Type definition is well-structured
The optional isEpic boolean property is correctly added to the type definition.
17-17: LGTM: Props destructuring with sensible defaults
The isEpic prop is correctly destructured with a default value of false, ensuring backward compatibility.
47-47: Use the suggested URL utility here as well
If you implement the URL utility function suggested above, you can also use it here:
-href={`/${workspaceSlug}/projects/${projectId}/${isEpic ? "epics" : "issues"}/${issueId}/`}
+href={`/${getItemUrl(workspaceSlug, projectId, issueId, isEpic)}/`}
Description
This PR includes following changes:
Type of Change
References
[WEB-2907]
Summary by CodeRabbit
New Features
isEpicproperty across various components to differentiate between standard issues and epics, enhancing user interactions.Bug Fixes
Chores