[WEB-2000] fix: issue link edit modal and mutation fix#5172
[WEB-2000] fix: issue link edit modal and mutation fix#5172SatishGandham merged 1 commit intopreviewfrom
Conversation
WalkthroughThe recent changes streamline the state management of issue linking within the application. Key improvements include the integration of MobX's Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant Store
User->>UI: Open Issue Link Modal
UI->>Store: setIssueLinkData(linkDetail)
UI->>Store: Fetch linkDetail
Store-->>UI: Return linkDetail
UI->>User: Display Modal with linkDetail
User->>UI: Close Modal
UI->>Store: setIssueLinkData(null)
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- web/core/components/issues/issue-detail-widgets/issue-detail-widget-modals.tsx (2 hunks)
- web/core/components/issues/issue-detail/links/create-update-link-modal.tsx (4 hunks)
- web/core/components/issues/issue-detail/links/link-detail.tsx (3 hunks)
- web/core/components/issues/issue-detail/links/link-item.tsx (3 hunks)
- web/core/store/issue/issue-details/root.store.ts (6 hunks)
Additional context used
Biome
web/core/store/issue/issue-details/root.store.ts
[error] 241-241: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
Additional comments not posted (17)
web/core/components/issues/issue-detail/links/link-item.tsx (5)
3-4: Approved: Import ofobserver.The import of
observerfrommobx-reactis necessary for MobX reactivity.
14-14: Approved: Import ofTLinkOperationsModal.The import of
TLinkOperationsModalis necessary for type-checking thelinkOperationsprop.
22-22: Approved: Wrapping component withobserver.Wrapping the component with
observerenhances its reactivity to MobX observable state changes.
28-29: Approved: Simplified state management for modal visibility.The modal's open state is now managed by invoking the
toggleIssueLinkModalStorefunction and setting the issue link data usingsetIssueLinkDatawith thelinkDetaildirectly.
35-37: Approved: AddedtoggleIssueLinkModalfunction.The
toggleIssueLinkModalfunction simplifies the logic for toggling the modal and setting the issue link data.web/core/components/issues/issue-detail/links/link-detail.tsx (3)
15-15: Approved: Import ofTLinkOperationsModal.The import of
TLinkOperationsModalis necessary for type-checking thelinkOperationsprop.
30-30: Approved: Simplified state management for modal visibility.The modal's open state is now managed by invoking the
toggleIssueLinkModalStorefunction and setting the issue link data usingsetIssueLinkDatawith thelinkDetaildirectly.
33-39: Approved: AddedtoggleIssueLinkModalfunction.The
toggleIssueLinkModalfunction simplifies the logic for toggling the modal and setting the issue link data.web/core/components/issues/issue-detail-widgets/issue-detail-widget-modals.tsx (2)
28-28: Approved: AddedsetIssueLinkDatato destructured props.This addition allows the component to manage state related to issue link data more effectively.
92-92: Approved: UpdatedhandleIssueLinkModalOnClosefunction.The
handleIssueLinkModalOnClosefunction now includes a call tosetIssueLinkData(null), ensuring that when the issue link modal is closed, any existing issue link data is reset.web/core/components/issues/issue-detail/links/create-update-link-modal.tsx (4)
9-9: LGTM!The import of
useIssueDetailis necessary for the new state management approach.
32-32: LGTM!The removal of
preloadedDatafrom props is consistent with the new state management approach.
44-45: LGTM!The use of
useIssueDetailhook aligns with the new state management approach and simplifies state handling.
47-47: LGTM!The change to the
onClosefunction simplifies the closing behavior by directly managing the state through the hook.web/core/store/issue/issue-details/root.store.ts (3)
62-62: LGTM!The addition of
issueLinkDataobservable is necessary for storing link-related information for issues.
80-80: LGTM!The addition of
setIssueLinkDataaction is necessary for updating theissueLinkDatastate.
112-112: LGTM!The addition of
issueLinkDataobservable to theIssueDetailclass is consistent with the interface and necessary for storing link-related information for issues.
| this.openWidgets = this.openWidgets.filter((s) => s !== state); | ||
| else this.openWidgets = [state, ...this.openWidgets]; | ||
| }; | ||
| setIssueLinkData = (issueLinkData: TIssueLink | null) => (this.issueLinkData = issueLinkData); |
There was a problem hiding this comment.
Avoid assignment within an expression.
The assignment within the expression is confusing. Consider refactoring to improve readability.
- setIssueLinkData = (issueLinkData: TIssueLink | null) => (this.issueLinkData = issueLinkData);
+ setIssueLinkData = (issueLinkData: TIssueLink | null) => {
+ this.issueLinkData = issueLinkData;
+ };Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setIssueLinkData = (issueLinkData: TIssueLink | null) => (this.issueLinkData = issueLinkData); | |
| setIssueLinkData = (issueLinkData: TIssueLink | null) => { | |
| this.issueLinkData = issueLinkData; | |
| }; |
Tools
Biome
[error] 241-241: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
Problem Statement:
When the user tries to update an existing issue link, preloadData is not populating.
Solution:
The issue was caused by multiple instances of the modal. I removed the duplicate instances and made the necessary adjustments to fix this.
Issue link: [WEB-2000]
Media: