-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[WEB-2715] fix: draft issue type update outside click #5902
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes in this pull request focus on the Changes
Possibly related PRs
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
web/core/components/issues/issue-modal/base.tsx (3)
Line range hint
144-150: Consider adding visual feedback for draft auto-saveWhen changes are auto-saved as draft on modal close, users might benefit from a visual indication. Consider showing a toast notification to inform users their changes were preserved.
if (changesMade && saveAsDraft && !data) { handleCreateIssue(changesMade, true); + setToast({ + type: TOAST_TYPE.SUCCESS, + title: "Draft Saved", + message: "Your changes have been saved as a draft." + }); }
Line range hint
235-246: Enhance error tracking for better debuggingConsider including more detailed error information in the event tracking to aid debugging.
} catch (error) { setToast({ type: TOAST_TYPE.ERROR, title: "Error!", - message: `${is_draft_issue ? "Draft issue" : "Issue"} could not be created. Please try again.`, + message: `${is_draft_issue ? "Draft issue" : "Issue"} could not be created: ${error.message || 'Unknown error'}`, }); captureIssueEvent({ eventName: ISSUE_CREATED, payload: { ...payload, - state: "FAILED" + state: "FAILED", + error: error.message }, path: pathname, }); throw error; }
Line range hint
89-108: Consider more thorough state cleanupThe cleanup function only resets the description state. Consider resetting all component-level states in the cleanup function to prevent potential stale states.
useEffect(() => { // ... existing code ... - return () => setDescription(undefined); + return () => { + setDescription(undefined); + setChangesMade(null); + setCreateMore(false); + setUploadedAssetIds([]); + }; }, [data, projectId, isOpen, activeProjectId]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- web/core/components/issues/issue-modal/base.tsx (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
web/core/components/issues/issue-modal/base.tsx (2)
207-207: LGTM: Parameter rename improves consistencyThe rename from
isDrafttois_draft_issuealigns with the codebase's naming convention and ensures consistent draft state handling.
Line range hint
144-150: Verify consistent draft handling across codebaseLet's verify that the draft issue handling is consistent across the codebase.
Also applies to: 207-207
✅ Verification successful
Based on the search results, I can now generate the final response as I have sufficient information about the draft handling implementation across the codebase.
Draft issue handling is consistently implemented across the codebase
The codebase shows a well-structured and consistent implementation of draft issue handling:
- The
isDraftflag is consistently used across components and properly propagated through the component hierarchy- The
is_draft_issueparameter is consistently used in create/update operations- Draft-specific UI states and messages are properly handled
- Draft-specific storage and API calls are properly segregated
The implementation in the reviewed code aligns with the rest of the codebase's patterns for handling draft issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent draft issue handling across the codebase # Check for any inconsistent draft parameter naming echo "Checking for inconsistent draft parameter naming..." rg -g '*.{ts,tsx}' -A 2 -B 2 'isDraft|is_draft_issue' # Check for potential places where draft handling might be needed echo "Checking for other modal close handlers that might need draft handling..." ast-grep --pattern 'handleClose() { $$$ }'Length of output: 27542
rahulramesha
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anmolsinghbhatia approving this since the name was already there but please use camelCase going forward.
Changes:
This PR addresses an issue where the custom properties data would be lost when clicking outside the draft issue form. Previously, the correct custom properties endpoint was not being triggered when the user filled out the "create issue" form and click outside. I've made the necessary changes to ensure the endpoint is now properly hit, resolving the issue.
Reference:
[WEB-2715]
Summary by CodeRabbit
New Features
Bug Fixes
Refactor