[WEB-2743] chore: restrict sub-issue to have different project id than parent #5981
Conversation
WalkthroughThe changes involve the addition of a new prop, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
anmolsinghbhatia
requested review from
SatishGandham,
rahulramesha and
sriramveeraghanta
![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
🧹 Outside diff range and nitpick comments (4)
web/core/components/issues/issue-modal/modal.tsx (1)
30-30: Add JSDoc documentation for the new prop.The new prop's purpose should be documented to improve maintainability and developer experience.
+ /** When true, disables project selection in the issue form. Used to enforce sub-issues remain in the same project as their parent. */ isProjectSelectionDisabled?: boolean;web/core/components/issues/issue-modal/draft-issue-layout.tsx (1)
Line range hint
83-95: Refactor the cleanup logic for better maintainability.The cleanup logic for
changesMadecould be improved:
- The unconditional deletion of
project_idseems incorrect and contradicts the PR's purpose of maintaining project relationships.- The multiple conditions could be consolidated into a helper function.
Consider refactoring like this:
- Object.entries(changesMade).forEach(([key, value]) => { - const issueKey = key as keyof TIssue; - if (value === null || value === undefined || value === "") delete changesMade[issueKey]; - if (typeof value === "object" && isEmpty(value)) delete changesMade[issueKey]; - if (Array.isArray(value) && value.length === 0) delete changesMade[issueKey]; - if (issueKey === "project_id") delete changesMade.project_id; - if (issueKey === "priority" && value && value === "none") delete changesMade.priority; - if ( - issueKey === "description_html" && - changesMade.description_html && - isEmptyHtmlString(changesMade.description_html, ["img"]) - ) - delete changesMade.description_html; - }); + const shouldRemoveValue = (key: keyof TIssue, value: any) => { + if (value === null || value === undefined || value === "") return true; + if (typeof value === "object" && isEmpty(value)) return true; + if (Array.isArray(value) && value.length === 0) return true; + if (key === "priority" && value === "none") return true; + if (key === "description_html" && isEmptyHtmlString(value, ["img"])) return true; + return false; + }; + + Object.entries(changesMade).forEach(([key, value]) => { + const issueKey = key as keyof TIssue; + if (shouldRemoveValue(issueKey, value)) delete changesMade[issueKey]; + });web/core/components/issues/issue-modal/base.tsx (1)
Line range hint
40-388: Consider enhancing error handling for project selection restrictions.While the implementation of
isProjectSelectionDisabledis clean, consider adding explicit error handling or user feedback when project selection is attempted while disabled. This would improve the user experience by clearly communicating why the project cannot be changed.Consider:
- Adding a tooltip or helper text explaining why project selection is disabled
- Implementing a validation message if project selection is attempted programmatically
- Adding error boundary handling for edge cases
web/core/components/issues/issue-modal/form.tsx (1)
341-341: Consider improving readability of the disabled condition.While the logic is correct, the condition could be more readable by extracting the complex boolean expression into a descriptive variable.
Consider this refactor:
- disabled={!!data?.id || !!data?.sourceIssueId || isProjectSelectionDisabled} + disabled={ + const isExistingIssue = !!data?.id; + const hasSourceIssue = !!data?.sourceIssueId; + return isExistingIssue || hasSourceIssue || isProjectSelectionDisabled; + }
📜 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(1 hunks)web/core/components/issues/issue-modal/base.tsx(3 hunks)web/core/components/issues/issue-modal/draft-issue-layout.tsx(3 hunks)web/core/components/issues/issue-modal/form.tsx(3 hunks)web/core/components/issues/issue-modal/modal.tsx(1 hunks)
🔇 Additional comments (12)
web/core/components/issues/issue-modal/modal.tsx (1)
30-30: Verify prop handling in child components.
The implementation looks good. Let's verify that child components properly handle this prop.
✅ Verification successful
Prop is properly handled throughout the component hierarchy
The verification shows that the isProjectSelectionDisabled prop is:
- Correctly passed down through the component hierarchy from
CreateUpdateIssueModalthroughCreateUpdateIssueModalBase,DraftIssueLayout, andIssueFormRoot - Properly implemented in
IssueProjectSelectcomponent where it's used to disable project selection - Correctly defaulted to
falsewhen not provided - Used appropriately to disable project selection along with other conditions (
!!data?.id || !!data?.sourceIssueId || isProjectSelectionDisabled)
The implementation is consistent and follows best practices for prop handling in React components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the proper handling of isProjectSelectionDisabled prop in child components
# Test 1: Check CreateUpdateIssueModalBase implementation
echo "Checking CreateUpdateIssueModalBase implementation..."
ast-grep --pattern 'interface $interface {
$$$
isProjectSelectionDisabled?: boolean;
$$$
}'
# Test 2: Check for prop usage in project selection logic
echo "Checking project selection logic..."
rg -A 5 "isProjectSelectionDisabled" --type tsx
# Test 3: Look for any disabled state handling in project selection components
echo "Checking disabled state handling..."
rg -A 5 "disabled.*project.*selection|project.*selection.*disabled" --type tsx
Length of output: 563
Script:
#!/bin/bash
# Let's try a different approach with correct file extensions
# Check for interfaces and types that include isProjectSelectionDisabled
echo "Checking interfaces and types..."
rg -t ts -t tsx "interface.*\{[^}]*isProjectSelectionDisabled.*\}|type.*\{[^}]*isProjectSelectionDisabled.*\}" -A 5
# Check for usage of isProjectSelectionDisabled prop
echo "Checking prop usage..."
rg -t ts -t tsx "isProjectSelectionDisabled" -A 5
# Look for project selection related components
echo "Checking project selection components..."
rg -t ts -t tsx "ProjectSelect|project.*selection" -A 5
Length of output: 530
Script:
#!/bin/bash
# Let's try without file type restrictions since ripgrep might not recognize tsx extension
# Check for interfaces and prop usage
echo "Checking interfaces and prop usage..."
rg "isProjectSelectionDisabled" -A 5
# Look for project selection related components
echo "Checking project selection components..."
rg "ProjectSelect|project.*selection" -A 5
# Check the specific file content
echo "Checking the modal file content..."
cat web/core/components/issues/issue-modal/modal.tsx
Length of output: 17610
web/core/components/issues/issue-detail-widgets/issue-detail-widget-modals.tsx (1)
154-154: LGTM: Project selection restriction implemented correctly.
The addition of isProjectSelectionDisabled prop aligns with the PR objective to restrict sub-issues to the same project as their parent issue.
web/core/components/issues/issue-modal/draft-issue-layout.tsx (4)
41-41: LGTM: Well-defined prop addition.
The new optional boolean prop isProjectSelectionDisabled is appropriately typed and named, clearly indicating its purpose for controlling project selection.
62-62: LGTM: Appropriate default value.
Setting isProjectSelectionDisabled to false by default maintains backward compatibility for existing usage.
184-184: LGTM: Proper prop forwarding.
The isProjectSelectionDisabled prop is correctly passed down to the IssueFormRoot component.
Line range hint 41-184: Verify complete implementation of project selection restriction.
While the changes implement project selection restriction through the isProjectSelectionDisabled prop, let's verify the complete implementation:
✅ Verification successful
Based on the search results, I can now provide a final response about the project selection restriction implementation:
Project selection restriction is properly implemented for sub-issues
The implementation correctly handles project selection restriction through:
-
The
isProjectSelectionDisabledprop is properly propagated through the component hierarchy:- Defined in the modal interfaces (
IssuesModalProps,IssueFormProps,DraftIssueProps) - Passed down to the form components
- Applied to the project selection UI with
disabled={!!data?.id || !!data?.sourceIssueId || isProjectSelectionDisabled}
- Defined in the modal interfaces (
-
The parent-child issue relationship maintains project context:
- Parent project details are tracked (
parentIssue.project_id) - Sub-issues are properly scoped to their parent's project through the UI and operations
- Project validation is enforced at both UI and data operation levels
- Parent project details are tracked (
The implementation ensures that sub-issues remain in the same project as their parent through UI restrictions and proper data handling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other potential places where project selection needs to be restricted
# and verify the enforcement of the restriction.
# Test 1: Look for other project selection components that might need the restriction
echo "Searching for other project selection components..."
rg -l "projectId|project_id" --type tsx
# Test 2: Check for validation logic around project selection
echo "Checking for project validation logic..."
rg "project.*validation|validateProject" --type tsx
# Test 3: Look for API calls that might need project validation
echo "Checking API calls related to project selection..."
rg "createIssue|updateIssue" --type tsx
Length of output: 536
Script:
#!/bin/bash
# Let's try without file type restriction since tsx might not be recognized
# Look for project selection components and validation
echo "Searching for project selection components and validation..."
rg -l "projectId|project_id"
echo "\nChecking project selection UI components..."
ast-grep --pattern 'const ProjectSelect = $_'
echo "\nChecking for project validation logic..."
rg "validateProject|project.*validation" -A 3
echo "\nChecking issue creation/update handlers..."
rg "createIssue|updateIssue" -A 3
Length of output: 169134
Script:
#!/bin/bash
# Let's check for project selection validation and restrictions
echo "Checking project selection validation..."
rg "isProjectSelectionDisabled|project.*validation" -A 3 web/core/components/issues/issue-modal/
echo "\nChecking sub-issue project validation..."
rg "parent.*project|sub.*issue.*project" -A 3 web/core/components/issues/
echo "\nChecking project selection component usage..."
ast-grep --pattern 'ProjectSelect'
Length of output: 10132
web/core/components/issues/issue-modal/base.tsx (3)
40-40: LGTM: Clean prop declaration for project selection control.
The new prop isProjectSelectionDisabled with its default value of false is well-implemented and aligns with the PR objective to restrict project selection for sub-issues.
365-365: LGTM: Proper prop forwarding to DraftIssueLayout.
The isProjectSelectionDisabled prop is correctly passed down to the DraftIssueLayout component, maintaining the project selection restriction logic.
388-388: LGTM: Proper prop forwarding to IssueFormRoot with verification needed.
The isProjectSelectionDisabled prop is correctly passed down to the IssueFormRoot component. However, let's verify the project selection restriction logic.
Let's verify the implementation of project selection restriction in the child components:
✅ Verification successful
Let me gather more information about the project selection implementation.
Let me try one more search to locate the project selection component and its implementation.
Let me check the actual implementation of the project-select component since we found its location.
Project selection restriction is properly implemented and enforced
The verification confirms that:
- The
isProjectSelectionDisabledprop is correctly passed through the component hierarchy - The
IssueProjectSelectcomponent properly implements the disabled state using theProjectDropdowncomponent - The disabled state is enforced in multiple scenarios:
- When there's an existing issue ID
- When there's a source issue ID
- When
isProjectSelectionDisabledis explicitly set to true
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the project selection restriction implementation in child components
# Check DraftIssueLayout implementation
echo "Checking DraftIssueLayout implementation..."
rg -A 5 "isProjectSelectionDisabled.*DraftIssueLayout" web/core/components/
# Check IssueFormRoot implementation
echo "Checking IssueFormRoot implementation..."
rg -A 5 "isProjectSelectionDisabled.*IssueFormRoot" web/core/components/
# Check if there are any project selection UI elements that might bypass this restriction
echo "Checking for potential bypass points..."
rg -A 5 "projectSelect|project-select|selectProject" web/core/components/
Length of output: 971
Script:
#!/bin/bash
# Check the implementation of project-select component
echo "Checking project-select component implementation..."
rg -A 10 "isProjectSelectionDisabled" web/core/components/issues/issue-modal/components/project-select
# Check IssueFormRoot for project selection handling
echo "Checking IssueFormRoot for project selection..."
ast-grep --pattern 'interface IssueFormRootProps {
$$$
}'
# Check the actual component implementation
echo "Checking project-select component usage..."
ast-grep --pattern 'const ProjectSelect = ({ $$ }) => {
$$$
}'
Length of output: 649
Script:
#!/bin/bash
# Find the project selection component file
echo "Finding project selection component file..."
fd "project-select" web/core/components/
# Check implementation in the issue modal directory
echo "Checking implementation in issue modal components..."
rg -A 10 "isProjectSelectionDisabled" web/core/components/issues/issue-modal/
# Look for ProjectSelect component definition
echo "Looking for ProjectSelect component..."
ast-grep --pattern 'export const ProjectSelect = {
$$$
}'
Length of output: 8806
Script:
#!/bin/bash
# Check the ProjectSelect component implementation
echo "Checking ProjectSelect component implementation..."
cat web/core/components/issues/issue-modal/components/project-select.tsx
# Look for disabled prop usage
echo "Looking for disabled prop usage in ProjectSelect..."
rg -A 5 "disabled.*ProjectSelect" web/core/components/issues/issue-modal/components/project-select.tsx
Length of output: 2212
web/core/components/issues/issue-modal/form.tsx (3)
74-74: LGTM: Clear and well-typed interface addition.
The new optional prop isProjectSelectionDisabled is appropriately typed and named, maintaining backward compatibility.
97-97: LGTM: Appropriate default value.
Setting isProjectSelectionDisabled to false by default maintains backward compatibility and existing behavior.
341-341: Verify parent components pass correct value for sub-issues.
While the implementation looks correct, we should verify that parent components correctly set isProjectSelectionDisabled when creating/editing sub-issues.
✅ Verification successful
Project selection is correctly disabled for sub-issues
The implementation correctly handles project selection for sub-issues. Here's the verification:
-
The
isProjectSelectionDisabledprop is properly passed down through the component hierarchy:- From
CreateUpdateIssueModalBasetoIssueFormRoot - From
DraftIssueLayouttoIssueFormRoot
- From
-
The project selection field in
IssueFormRootis correctly disabled when:
disabled={!!data?.id || !!data?.sourceIssueId || isProjectSelectionDisabled}- When creating sub-issues, the parent components set
isProjectSelectionDisabledto ensure sub-issues are created in the same project as their parent:- The sub-issues store logic enforces this by managing parent-child relationships
- The UI properly disables project selection when adding/creating sub-issues
The implementation aligns with the PR objectives by ensuring sub-issues maintain the same project ID as their parent issue.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find components that import and use IssueFormRoot to verify they pass isProjectSelectionDisabled correctly
# Search for files importing IssueFormRoot
echo "Files importing IssueFormRoot:"
rg --type typescript --type typescriptreact "import.*IssueFormRoot.*from.*issue-modal/form" -l
# Search for usage of IssueFormRoot with isProjectSelectionDisabled prop
echo -e "\nUsage of IssueFormRoot with isProjectSelectionDisabled:"
rg --type typescript --type typescriptreact "IssueFormRoot.*isProjectSelectionDisabled"
Length of output: 454
Script:
#!/bin/bash
# Let's try a different approach using just the file extension patterns
# Search for files importing IssueFormRoot
echo "Files importing IssueFormRoot:"
rg "import.*IssueFormRoot.*from.*issue-modal/form" --glob "*.{ts,tsx}" -l
# Search for usage of IssueFormRoot with isProjectSelectionDisabled prop
echo -e "\nUsage of IssueFormRoot with isProjectSelectionDisabled:"
rg "IssueFormRoot.*isProjectSelectionDisabled" --glob "*.{ts,tsx}" -A 2
Length of output: 347
Script:
#!/bin/bash
# Let's try a broader search to find where IssueFormRoot is used
echo "Files containing IssueFormRoot:"
rg "IssueFormRoot" --glob "*.{ts,tsx}" -l
echo -e "\nUsage context of IssueFormRoot:"
rg "IssueFormRoot" --glob "*.{ts,tsx}" -B 2 -A 2
# Also search for the form component itself to understand its exports
echo -e "\nContent of form.tsx:"
rg "export.*IssueFormRoot" --glob "form.tsx" -B 2 -A 2
Length of output: 4730
Script:
#!/bin/bash
# Let's check how IssueFormRoot is used in base.tsx and draft-issue-layout.tsx, focusing on props related to sub-issues
echo "Content of base.tsx showing IssueFormRoot usage with props:"
rg -B 10 -A 10 "<IssueFormRoot" web/core/components/issues/issue-modal/base.tsx
echo -e "\nContent of draft-issue-layout.tsx showing IssueFormRoot usage with props:"
rg -B 10 -A 10 "<IssueFormRoot" web/core/components/issues/issue-modal/draft-issue-layout.tsx
# Let's also check if there's any logic related to sub-issues and project selection
echo -e "\nChecking for sub-issue related logic:"
rg "sourceIssueId|parentIssue|isProjectSelectionDisabled" --glob "*.{ts,tsx}" -B 2 -A 2
Length of output: 92899
Changes:
This PR includes following changes:
Reference:
[WEB-2743]
Media:
Summary by CodeRabbit
Release Notes
New Features
isProjectSelectionDisabled, across various modal components to enhance control over project selection.DraftIssueLayoutandIssueFormRootcomponents.Bug Fixes
Documentation
isProjectSelectionDisabledproperty in relevant components.