Conversation
…ssue relation response
WalkthroughThis pull request introduces enhancements to issue serializers and components related to issue relations, focusing on expanding the data representation for issues. The changes primarily involve adding new fields like Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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: 1
🧹 Nitpick comments (4)
web/core/components/issues/issue-detail-widgets/relations/content.tsx (3)
152-152: Simplify condition by removing redundant double-negationThe use of
!!is unnecessary here because the ternary operator already evaluates the condition as a boolean. Removing the double-negation improves code readability.Apply this diff to simplify the condition:
- const deleteOperation = !!issueCrudState.delete.issue?.is_epic + const deleteOperation = issueCrudState.delete.issue?.is_epic🧰 Tools
🪛 Biome (1.9.4)
[error] 152-152: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
163-163: Simplify condition by removing redundant double-negationSimilarly, the double-negation
!!in this conditional expression is unnecessary. Removing it will make the code cleaner and more straightforward.Apply this diff to simplify the condition:
- {!!issueCrudState?.update?.issue?.is_epic ? ( + {issueCrudState?.update?.issue?.is_epic ? (🧰 Tools
🪛 Biome (1.9.4)
[error] 163-163: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
162-188: Remove unnecessary Fragment to streamline JSXThe Fragment (
<>and</>) wrapping this section is not needed because it contains only one child element. Eliminating the Fragment simplifies the JSX structure.Apply this diff to remove the unnecessary Fragment:
- <> {issueCrudState?.update?.issue?.is_epic ? ( <CreateUpdateEpicModal isOpen={issueCrudState?.update?.toggle} onClose={() => { handleIssueCrudState("update", null, null); toggleCreateIssueModal(false); }} data={issueCrudState?.update?.issue ?? undefined} onSubmit={async (_issue: TIssue) => { await epicOperations.update(workspaceSlug, projectId, _issue.id, _issue); }} /> ) : ( <CreateUpdateIssueModal isOpen={issueCrudState?.update?.toggle} onClose={() => { handleIssueCrudState("update", null, null); toggleCreateIssueModal(false); }} data={issueCrudState?.update?.issue ?? undefined} onSubmit={async (_issue: TIssue) => { await issueOperations.update(workspaceSlug, projectId, _issue.id, _issue); }} /> )} - </>🧰 Tools
🪛 Biome (1.9.4)
[error] 163-163: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
[error] 162-188: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
web/core/components/issues/relations/issue-list-item.tsx (1)
58-58: Simplify condition by removing redundant double-negationThe double-negation
!!is unnecessary since the ternary operator will coerce the condition to a boolean. Removing it enhances code readability.Apply this diff to simplify the condition:
- const issueOperations = useRelationOperations(!!issue?.is_epic ? EIssueServiceType.EPICS : EIssueServiceType.ISSUES); + const issueOperations = useRelationOperations(issue?.is_epic ? EIssueServiceType.EPICS : EIssueServiceType.ISSUES);🧰 Tools
🪛 Biome (1.9.4)
[error] 58-58: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apiserver/plane/app/serializers/issue.py(2 hunks)web/core/components/issues/issue-detail-widgets/relations/content.tsx(4 hunks)web/core/components/issues/issue-detail-widgets/relations/helper.tsx(4 hunks)web/core/components/issues/relations/issue-list-item.tsx(3 hunks)web/core/components/issues/relations/issue-list.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- web/core/components/issues/relations/issue-list.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
web/core/components/issues/relations/issue-list-item.tsx
[error] 58-58: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
web/core/components/issues/issue-detail-widgets/relations/content.tsx
[error] 152-152: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 163-163: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 162-188: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint-apiserver
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
web/core/components/issues/issue-detail-widgets/relations/helper.tsx (1)
26-27: Dynamic entity names enhance user feedbackIntroducing the
entityNamevariable to customize messages based onissueServiceTypeimproves clarity and provides a better user experience.apiserver/plane/app/serializers/issue.py (1)
319-323: Fix inconsistency inassignee_idsfield implementation.The same issue exists here as in
IssueRelationSerializer. The field is marked aswrite_only=Truebut lacks the necessary implementation.
| assignee_ids = serializers.ListField( | ||
| child=serializers.PrimaryKeyRelatedField(queryset=User.objects.all()), | ||
| write_only=True, | ||
| required=False, | ||
| ) |
There was a problem hiding this comment.
Fix inconsistency in assignee_ids field implementation.
The assignee_ids field is marked as write_only=True but:
- It's not sourced from
related_issuelike other fields - There are no create/update methods to handle the write operations
Consider one of these fixes:
- assignee_ids = serializers.ListField(
- child=serializers.PrimaryKeyRelatedField(queryset=User.objects.all()),
- write_only=True,
- required=False,
- )
+ # Option 1: Make it read-only like other fields
+ assignee_ids = serializers.ListField(
+ child=serializers.UUIDField(),
+ source="related_issue.assignees.all",
+ read_only=True,
+ )
+
+ # Option 2: Add create/update methods to handle write operations
+ assignee_ids = serializers.ListField(
+ child=serializers.PrimaryKeyRelatedField(queryset=User.objects.all()),
+ write_only=True,
+ required=False,
+ )
+
+ def create(self, validated_data):
+ assignees = validated_data.pop("assignee_ids", None)
+ instance = super().create(validated_data)
+ if assignees is not None:
+ instance.related_issue.assignees.set(assignees)
+ return instanceCommittable suggestion skipped: line range outside the PR's diff.
4 participants
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes