fix: issue serializer to remove deleted labels and assignees #6241
Conversation
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
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 (1)
apiserver/plane/api/serializers/issue.py (1)
240-270: Consider optimizing database queries using select_related/prefetch_related.While the implementation correctly handles deleted labels and assignees, we could optimize the database queries when retrieving related objects. Consider using Django's
select_relatedorprefetch_relatedto reduce the number of database queries:- User.objects.filter( - pk__in=IssueAssignee.objects.filter(issue=instance).values_list( - "assignee_id", flat=True - ) - ) + User.objects.filter( + pk__in=IssueAssignee.objects.filter(issue=instance).values_list( + "assignee_id", flat=True + ) + ).select_related('profile') # If User has related fields you're accessing - Label.objects.filter( - pk__in=IssueLabel.objects.filter(issue=instance).values_list( - "label_id", flat=True - ) - ) + Label.objects.filter( + pk__in=IssueLabel.objects.filter(issue=instance).values_list( + "label_id", flat=True + ) + ).select_related('project') # If Label has related fields you're accessing
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apiserver/plane/api/serializers/issue.py(1 hunks)
🔇 Additional comments (2)
apiserver/plane/api/serializers/issue.py (2)
256-263: LGTM! Improved label handling to properly exclude deleted labels.
The changes correctly retrieve labels through the IssueLabel model instead of direct field access, ensuring deleted labels are properly excluded.
Let's verify the handling of deleted labels:
Also applies to: 265-270
✅ Verification successful
Let's examine the actual model and relationship definitions to verify the label handling approach.
LGTM! The label handling implementation is correct and safe
The code inspection confirms:
Issuemodel doesn't have a direct relationship to labelsIssueLabelis the proper junction model with cascading delete- The serializer correctly uses
IssueLabelfor retrieving labels, ensuring deleted labels are excluded - Label validation is properly scoped to the project context
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the code properly handles deleted labels
# by checking for any direct label relationships in the codebase
# Check for any direct label field declarations that might bypass IssueLabel
ast-grep --pattern 'class Issue:
$$$
label = $_
$$$'
# Check for any direct label relationship queries that might bypass IssueLabel
rg -l "label.*=.*Label" --type py
Length of output: 692
Script:
#!/bin/bash
# Check Issue model definition for label relationships
rg -A 5 "class Issue\(" apiserver/plane/db/models/issue.py
# Check IssueLabel model definition
rg -A 5 "class IssueLabel\(" apiserver/plane/db/models/issue.py
# Check for any direct label access in the serializer
rg -B 2 -A 2 "labels.*=.*Label" apiserver/plane/api/serializers/issue.py
Length of output: 1456
240-244: LGTM! Improved assignee handling to properly exclude deleted assignees.
The changes correctly retrieve assignees through the IssueAssignee model instead of direct field access, ensuring deleted assignees are properly excluded.
Let's verify the handling of deleted assignees:
Also applies to: 249-252
✅ Verification successful
The changes correctly handle assignee relationships through IssueAssignee model
The code review is correct. The verification confirms that:
- The changes in
apiserver/plane/api/serializers/issue.pyproperly use theIssueAssigneemodel to manage assignee relationships. - Other files that reference assignees either:
- Use the same
IssueAssigneemodel for managing relationships - Only read assignee data through serializers for display purposes
- Use the same
- The background task in
issue_activities_task.pyonly uses direct User queries for activity tracking, which is appropriate as it's just recording historical events.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the code properly handles deleted assignees
# by checking for any direct assignee relationships in the codebase
# Check for any direct assignee field declarations that might bypass IssueAssignee
ast-grep --pattern 'class Issue:
$$$
assignee = $_
$$$'
# Check for any direct assignee relationship queries that might bypass IssueAssignee
rg -l "assignee.*=.*User" --type py
Length of output: 402
Script:
#!/bin/bash
# Let's examine the actual usage of assignee relationships in these files
# to ensure they're using IssueAssignee model properly
# Check the content of files that might have assignee relationships
for file in apiserver/plane/space/serializer/issue.py \
apiserver/plane/space/serializer/intake.py \
apiserver/plane/bgtasks/issue_activities_task.py \
apiserver/plane/app/serializers/issue.py \
apiserver/plane/api/serializers/issue.py \
apiserver/plane/app/serializers/intake.py \
apiserver/plane/app/serializers/project.py; do
echo "=== $file ==="
rg "assignee.*=.*User|class.*Issue.*:|def.*assignee" -A 5 -B 2 "$file"
echo
done
# Also check for IssueAssignee model definition and usage
ast-grep --pattern 'class IssueAssignee:
$$$'
Length of output: 22360
Description
This PR fixes removes deleted labels and assignees from an issue.
Type of Change
Test Scenarios
References
Closes #6235
Summary by CodeRabbit
New Features
Bug Fixes
Chores