[WEB-4187] fix: related search issues #7628
Conversation
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
WalkthroughThe issue search filter now appends the current issue’s ID to related issue IDs and performs a single exclusion of all those IDs when the issue exists. If the issue is not found, no exclusions are applied. No public interfaces or surrounding logic were changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant S as SearchIssuesView
participant R as IssueRepository
C->>S: Request issues with "exclude related to issue_id"
S->>R: Fetch issue by issue_id
alt Issue exists
S->>R: Get related_issue_ids
Note right of S: Append current issue_id to exclusion set
S->>R: Query issues exclude pk in [related_issue_ids + issue_id]
R-->>S: Filtered results
else Issue not found
S->>R: Query issues without exclusions
R-->>S: Unfiltered results
end
S-->>C: Return issues
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/plane/app/views/search/issue.py (1)
74-78: Bug: possible AttributeError when issue is None; also prefer parent_id over parent
if issue.parent:will raise whenissueis None because it’s outside theif issue:guard. Nest the check and useparent_idto avoid an extra fetch.- if issue: - issues = issues.filter(~Q(pk=issue_id), parent__isnull=True) - if issue.parent: - issues = issues.filter(~Q(pk=issue.parent_id)) + if issue: + issues = issues.filter(~Q(pk=issue_id), parent__isnull=True) + if issue.parent_id: + issues = issues.filter(~Q(pk=issue.parent_id))
🧹 Nitpick comments (4)
apps/api/plane/app/views/search/issue.py (4)
54-66: Guard related-issue query behind existence check; dedupe IDs; avoid None; use issue.pk for type fidelityCurrently, the related-issue query runs even when the issue doesn’t exist, potentially matching NULL relations and doing unnecessary work. Flattening may include None and duplicates, and
issue_idis appended before verifying existence. Refactor to only compute when the issue is found, dedupe with a set, drop None, and useissue.pkto preserve DB field type.- issue = Issue.issue_objects.filter(pk=issue_id).first() - related_issue_ids = ( - IssueRelation.objects.filter(Q(related_issue=issue) | Q(issue=issue)) - .values_list("issue_id", "related_issue_id") - .distinct() - ) - - related_issue_ids = [item for sublist in related_issue_ids for item in sublist] - related_issue_ids.append(issue_id) - - if issue: - issues = issues.exclude(pk__in=related_issue_ids) + issue = Issue.issue_objects.filter(pk=issue_id).first() + if issue: + pairs = ( + IssueRelation.objects.filter(Q(related_issue=issue) | Q(issue=issue)) + .values_list("issue_id", "related_issue_id") + .distinct() + ) + related_issue_ids = {x for pair in pairs for x in pair if x is not None} + related_issue_ids.add(issue.pk) + issues = issues.exclude(pk__in=list(related_issue_ids))
41-45: Prefer exclude with ORs; use parent_id to avoid extra fetch and handle None cleanlyThe current triple negation filter works, but
excludeis clearer and usingparent_idavoids fetching the parent object. Also, building the OR expression handles the case whereparent_idis None without relying on~Q(pk=None).- if issue: - issues = issues.filter( - ~Q(pk=issue_id), ~Q(pk=issue.parent_id), ~Q(parent_id=issue_id) - ) + if issue: + excludes = Q(pk=issue_id) | Q(parent_id=issue_id) + if issue.parent_id: + excludes |= Q(pk=issue.parent_id) + issues = issues.exclude(excludes)
55-63: Alternative: DB-only exclusion without Python-side flatteningIf the relation set can grow large, consider keeping this in the DB with a union subquery (supported on Postgres/Django). This avoids pulling IDs into Python and generating very long IN lists.
- related_issue_ids = ( - IssueRelation.objects.filter(Q(related_issue=issue) | Q(issue=issue)) - .values_list("issue_id", "related_issue_id") - .distinct() - ) - related_issue_ids = [item for sublist in related_issue_ids for item in sublist] - related_issue_ids.append(issue_id) - if issue: - issues = issues.exclude(pk__in=related_issue_ids) + if issue: + qs_issue_ids = IssueRelation.objects.filter(issue=issue).values_list("related_issue_id", flat=True) + qs_related_ids = IssueRelation.objects.filter(related_issue=issue).values_list("issue_id", flat=True) + # Union the two subqueries and exclude the current issue separately + issues = issues.exclude(pk__in=qs_issue_ids.union(qs_related_ids)).exclude(pk=issue.pk)Note: Keep this only if your DB is Postgres (Django supports UNION on same-model QuerySets there).
105-167: Add regression tests for new exclusion behaviorGiven the search and exclusion logic is subtle, please add tests that verify:
- When issue_relation=true and the issue exists, results exclude: (a) the issue itself and (b) all its related issues.
- When the issue does not exist, no exclusions are applied.
- Duplicated relations (A↔B and B↔A) do not cause errors and do not affect results beyond exclusion.
- Parent-related exclusions do not raise if the issue has no parent.
I can draft pytest tests targeting the view with factory-generated Issues and IssueRelations. Want me to push a test module?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/api/plane/app/views/search/issue.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
Description
This PR will fix bugs related to search issues.
Type of Change
Summary by CodeRabbit