Enhance GraphQL Endpoints to Support Distinct Filtering for Issues and Releases #1154
Conversation
Summary by CodeRabbit
WalkthroughThis pull request updates GraphQL endpoints for GitHub issues and releases by introducing a new Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
🪧 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
Documentation and Community
|
rishyym0927
changed the title
![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 (5)
backend/apps/github/graphql/queries/issue.py (2)
13-17: Clean implementation of distinct parameterThe
distinctparameter is properly added to the GraphQL field definition with an appropriate default value ofFalse, making it backward compatible with existing queries.However, there are trailing whitespace issues on lines 14 and 15.
- recent_issues = graphene.List( - IssueNode, - limit=graphene.Int(default_value=15), - distinct=graphene.Boolean(default_value=False) - ) + recent_issues = graphene.List( + IssueNode, + limit=graphene.Int(default_value=15), + distinct=graphene.Boolean(default_value=False) + )🧰 Tools
🪛 Ruff (0.8.2)
14-14: Trailing whitespace
Remove trailing whitespace
(W291)
15-15: Trailing whitespace
Remove trailing whitespace
(W291)
19-26: Well-implemented distinct filtering logicThe resolver method has been updated to include the new
distinctparameter with a default value that maintains backward compatibility. The ordering strategy is well thought out - ordering by author_id and repository_id first ensures consistent results when applying the distinct filter.However, there are a few issues to fix:
- Trailing whitespace on line 21
- Line 24 exceeds the line length limit (118 > 99)
- The duplicate ordering in the distinct clause is unnecessary since you already ordered the query on line 21
- def resolve_recent_issues(root, info, limit=15, distinct=False): - """Resolve recent issue.""" - query = Issue.objects.order_by("author_id", "repository_id", "-created_at") - - if distinct: - query = query.order_by("author_id", "repository_id", "-created_at").distinct("author_id", "repository_id") - - return query[:limit] + def resolve_recent_issues(root, info, limit=15, distinct=False): + """Resolve recent issues with optional distinct filtering.""" + query = Issue.objects.order_by("author_id", "repository_id", "-created_at") + + if distinct: + query = query.distinct("author_id", "repository_id") + + return query[:limit]🧰 Tools
🪛 Ruff (0.8.2)
21-21: Trailing whitespace
Remove trailing whitespace
(W291)
24-24: Line too long (118 > 99)
(E501)
backend/apps/github/graphql/queries/release.py (1)
19-31: Good implementation of distinct filtering logicThe resolver method has been properly updated with:
- An improved docstring that clearly explains the new functionality
- Consistent ordering strategy (author_id, repository_id, -published_at)
- Proper distinct filtering when the parameter is enabled
However, there are trailing whitespace issues on lines 19, 25, and 28 that should be fixed.
- def resolve_recent_releases(root, info, limit=15, distinct=False): - """Resolve recent releases with optional distinct filtering.""" - query = Release.objects.filter( - is_draft=False, - is_pre_release=False, - published_at__isnull=False, - ).order_by("author_id", "repository_id", "-published_at") - - if distinct: - query = query.distinct("author_id", "repository_id") - - return query[:limit] + def resolve_recent_releases(root, info, limit=15, distinct=False): + """Resolve recent releases with optional distinct filtering.""" + query = Release.objects.filter( + is_draft=False, + is_pre_release=False, + published_at__isnull=False, + ).order_by("author_id", "repository_id", "-published_at") + + if distinct: + query = query.distinct("author_id", "repository_id") + + return query[:limit]🧰 Tools
🪛 Ruff (0.8.2)
19-19: Trailing whitespace
Remove trailing whitespace
(W291)
25-25: Trailing whitespace
Remove trailing whitespace
(W291)
28-28: Trailing whitespace
Remove trailing whitespace
(W291)
backend/apps/owasp/graphql/nodes/project.py (2)
71-78: Enhance docstring for consistencyThe method implementation looks good, but the docstring could be improved to match the style used in other resolvers and include parameter descriptions.
def resolve_recent_releases(self, info, distinct=False): - """Resolve recent releases with optional distinct filtering.""" + """Resolve recent releases with optional distinct filtering. + + Parameters: + - info: The GraphQL context and information. + - distinct (bool): Whether to return distinct releases based on author and repository. + + Returns: + - A list of recent releases, potentially filtered for distinct authors and repositories. + """ query = self.published_releases.order_by("author_id", "repository_id", "-published_at") if distinct: - query = query.distinct("author_id", "repository_id") + query = query.distinct("author_id", "repository_id") return query[:RECENT_RELEASES_LIMIT]🧰 Tools
🪛 Ruff (0.8.2)
76-76: Trailing whitespace
Remove trailing whitespace
(W291)
62-78: Consider adding a limit parameter to recent_releases resolverThe
resolve_recent_issuesmethod accepts a configurablelimitparameter, butresolve_recent_releasesuses a hardcoded constant. For API consistency, consider adding a similarlimitparameter to theresolve_recent_releasesmethod.- def resolve_recent_releases(self, info, distinct=False): + def resolve_recent_releases(self, info, limit=RECENT_RELEASES_LIMIT, distinct=False): """Resolve recent releases with optional distinct filtering.""" query = self.published_releases.order_by("author_id", "repository_id", "-published_at") if distinct: query = query.distinct("author_id", "repository_id") - return query[:RECENT_RELEASES_LIMIT] + return query[:limit]🧰 Tools
🪛 Ruff (0.8.2)
62-62: First argument of a method should be named
selfRename
roottoself(N805)
62-62: Trailing whitespace
Remove trailing whitespace
(W291)
64-64: Undefined name
Issue(F821)
64-64: Trailing whitespace
Remove trailing whitespace
(W291)
67-67: Trailing whitespace
Remove trailing whitespace
(W291)
76-76: Trailing whitespace
Remove trailing whitespace
(W291)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/apps/github/graphql/queries/issue.py(1 hunks)backend/apps/github/graphql/queries/release.py(1 hunks)backend/apps/owasp/graphql/nodes/project.py(2 hunks)frontend/src/api/queries/homeQueries.ts(3 hunks)frontend/src/pages/Home.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
frontend/src/pages/Home.tsx (1)
frontend/src/api/queries/homeQueries.ts (1) (1)
GET_MAIN_PAGE_DATA(3-81)
backend/apps/github/graphql/queries/release.py (1)
backend/apps/owasp/graphql/nodes/project.py (1) (1)
resolve_recent_releases(71-78)
backend/apps/owasp/graphql/nodes/project.py (2)
backend/apps/github/graphql/queries/issue.py (1) (1)
resolve_recent_issues(19-26)backend/apps/github/graphql/queries/release.py (1) (1)
resolve_recent_releases(19-30)
🪛 Ruff (0.8.2)
backend/apps/github/graphql/queries/issue.py
14-14: Trailing whitespace
Remove trailing whitespace
(W291)
15-15: Trailing whitespace
Remove trailing whitespace
(W291)
21-21: Trailing whitespace
Remove trailing whitespace
(W291)
24-24: Line too long (118 > 99)
(E501)
backend/apps/github/graphql/queries/release.py
19-19: Trailing whitespace
Remove trailing whitespace
(W291)
25-25: Trailing whitespace
Remove trailing whitespace
(W291)
28-28: Trailing whitespace
Remove trailing whitespace
(W291)
backend/apps/owasp/graphql/nodes/project.py
62-62: First argument of a method should be named self
Rename root to self
(N805)
62-62: Trailing whitespace
Remove trailing whitespace
(W291)
64-64: Undefined name Issue
(F821)
64-64: Trailing whitespace
Remove trailing whitespace
(W291)
67-67: Trailing whitespace
Remove trailing whitespace
(W291)
76-76: Trailing whitespace
Remove trailing whitespace
(W291)
🔇 Additional comments (7)
frontend/src/pages/Home.tsx (1)
37-39: Looks good - Added distinct parameter to query!The update to include
{ distinct: true }in theuseQueryhook properly implements the new feature to fetch distinct results for issues and releases. This change aligns perfectly with the PR objective of preventing consecutive items from the same author or project.frontend/src/api/queries/homeQueries.ts (3)
4-4: Correctly added distinct parameter to query definitionThe GraphQL query now accepts the Boolean parameter that will be used to control distinct filtering.
35-35: Properly implemented distinct parameter for recent issuesThe
distinctparameter is correctly passed to therecentIssuesquery, allowing the backend to filter unique issues by author and repository.
47-47: Properly implemented distinct parameter for recent releasesThe
distinctparameter is correctly passed to therecentReleasesquery, enabling filtering of unique releases by author and repository.backend/apps/github/graphql/queries/release.py (1)
13-17: Well-structured GraphQL field definitionThe addition of the
distinctparameter to therecent_releasesfield is implemented correctly with an appropriate default value to maintain backward compatibility.backend/apps/owasp/graphql/nodes/project.py (2)
22-25: LGTM! Good addition of distinct parameterThe introduction of a distinct parameter with a default value of False is a good enhancement, allowing clients to optionally request unique issues.
26-29: LGTM! Consistent implementation for recent_releasesThe implementation mirrors the approach used for recent_issues, maintaining a consistent API design across similar GraphQL fields.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/apps/owasp/graphql/nodes/project.py (2)
62-79: Improve docstring formatting and consider moving import statement.The resolver method looks good and addresses previous review comments by using
selfinstead ofrootand including the Issue import. However, there are still some docstring formatting issues and an import that should be at the top of the file.def resolve_recent_issues(self, info, limit=RECENT_ISSUES_LIMIT, distinct=False): """Resolve recent issues with optional distinct filtering. Parameters: - - info: The GraphQL context and information. - - limit (int): The maximum number of issues to return. - - distinct (bool): Whether to return distinct issues based on author and repository. + ---------- + info + The GraphQL context and information. + limit : int + The maximum number of issues to return. + distinct : bool + Whether to return distinct issues based on author and repository. Returns: - - A list of recent issues, potentially filtered for distinct authors and repositories. + ------- + list + A list of recent issues, potentially filtered for distinct authors and repositories. """ from apps.github.models import Issue query = Issue.objects.order_by("author_id", "repository_id", "-created_at")Also, consider moving the Issue import to the top of the file for better performance:
import graphene from apps.github.graphql.nodes.issue import IssueNode from apps.github.graphql.nodes.release import ReleaseNode from apps.github.graphql.nodes.repository import RepositoryNode +from apps.github.models import Issue from apps.owasp.graphql.nodes.common import GenericEntityNode from apps.owasp.models.project import Project🧰 Tools
🪛 Ruff (0.8.2)
62-62: Missing argument descriptions in the docstring for
resolve_recent_issues:distinct,info,limit(D417)
64-64: Blank line contains whitespace
Remove whitespace from blank line
(W293)
65-65: Missing dashed underline after section ("Parameters")
Add dashed line under "Parameters"
(D407)
65-65: Section name should end with a newline ("Parameters")
Add newline after "Parameters"
(D406)
69-69: Blank line contains whitespace
Remove whitespace from blank line
(W293)
70-70: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
70-70: Missing dashed underline after section ("Returns")
Add dashed line under "Returns"
(D407)
70-70: Section name should end with a newline ("Returns")
Add newline after "Returns"
(D406)
81-88: Improve docstring and fix trailing whitespace.The implementation of the resolver is good, but the docstring should be improved to match the style of other resolvers in the file, and there's a trailing whitespace that should be removed.
def resolve_recent_releases(self, info, distinct=False): - """Resolve recent releases with optional distinct filtering.""" + """Resolve recent releases with optional distinct filtering. + + Parameters: + ---------- + info + The GraphQL context and information. + distinct : bool + Whether to return distinct releases based on author and repository. + + Returns: + ------- + list + A list of recent releases, potentially filtered for distinct authors and repositories. + """ query = self.published_releases.order_by("author_id", "repository_id", "-published_at") if distinct: - query = query.distinct("author_id", "repository_id") + query = query.distinct("author_id", "repository_id") return query[:RECENT_RELEASES_LIMIT]🧰 Tools
🪛 Ruff (0.8.2)
86-86: Trailing whitespace
Remove trailing whitespace
(W291)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/apps/owasp/graphql/nodes/project.py(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
backend/apps/owasp/graphql/nodes/project.py (6)
backend/apps/github/graphql/nodes/issue.py (1) (1)
IssueNode(7-20)backend/apps/github/graphql/nodes/release.py (1) (1)
ReleaseNode(11-34)backend/apps/github/graphql/queries/issue.py (1) (1)
resolve_recent_issues(19-26)backend/apps/github/graphql/queries/release.py (1) (1)
resolve_recent_releases(19-30)backend/apps/github/models/repository.py (1) (1)
published_releases(140-146)backend/apps/owasp/models/project.py (1) (1)
published_releases(170-178)
🪛 Ruff (0.8.2)
backend/apps/owasp/graphql/nodes/project.py
62-62: Missing argument descriptions in the docstring for resolve_recent_issues: distinct, info, limit
(D417)
64-64: Blank line contains whitespace
Remove whitespace from blank line
(W293)
65-65: Missing dashed underline after section ("Parameters")
Add dashed line under "Parameters"
(D407)
65-65: Section name should end with a newline ("Parameters")
Add newline after "Parameters"
(D406)
69-69: Blank line contains whitespace
Remove whitespace from blank line
(W293)
70-70: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
70-70: Missing dashed underline after section ("Returns")
Add dashed line under "Returns"
(D407)
70-70: Section name should end with a newline ("Returns")
Add newline after "Returns"
(D406)
86-86: Trailing whitespace
Remove trailing whitespace
(W291)
🔇 Additional comments (3)
backend/apps/owasp/graphql/nodes/project.py (3)
22-25: Field enhancement with distinct filtering - good implementation.The distinct argument has been properly added to the GraphQL field definition with an appropriate default value of False, allowing clients to optionally request distinct results while maintaining backward compatibility for existing queries.
26-29: Field enhancement with distinct filtering - good implementation.Similar to the recent_issues field, the distinct parameter has been properly added to the recent_releases field definition with appropriate default value, maintaining consistent behavior across related fields.
62-88:❓ Verification inconclusive
Verify consistent implementation across resolvers.
The implementation of distinct filtering looks good, but there's a small inconsistency between the two resolver methods. In
resolve_recent_issuesthe ordering is applied before the distinct filtering, while inresolve_recent_releasesit isn't explicitly reapplied in the distinct case.Run the following script to check how distinct is implemented in similar resolvers:
🏁 Script executed:
#!/bin/bash # Description: Check implementation of distinct filtering in other resolvers # Search for distinct implementation patterns in other Python files echo "Checking how distinct is implemented in other resolvers:" rg -A 5 -B 5 "distinct\(.*\)" --glob "*.py" # Check specifically for any inconsistencies in ordering when using distinct echo -e "\nChecking for patterns that include both order_by and distinct:" rg -A 3 -B 3 "order_by.*distinct|distinct.*order_by" --glob "*.py"Length of output: 4806
Consistent Ordering for Distinct Filtering Required
Both resolver methods in this file currently apply ordering once—prior to checking the
distinctflag. In contrast, elsewhere in the codebase (for example, inbackend/apps/github/graphql/queries/issue.py) the query is explicitly re-ordered inside theif distinct:block before calling.distinct(...). This discrepancy might affect query behavior under certain database conditions.
OWASP Resolvers (
project.py):
resolve_recent_issuesandresolve_recent_releasesboth callorder_by(...)before the conditional distinct filtering.GitHub Resolver (
queries/issue.py):
- The distinct branch re-applies the ordering with
query.order_by("author_id", "repository_id", "-created_at").distinct("author_id", "repository_id").Please verify if the current approach in the OWASP resolvers is intentional. If consistency across resolvers is desired, consider aligning the pattern (by either reordering within the distinct block or confirming that a single ordering call is sufficient).
🧰 Tools
🪛 Ruff (0.8.2)
62-62: Missing argument descriptions in the docstring for
resolve_recent_issues:distinct,info,limit(D417)
64-64: Blank line contains whitespace
Remove whitespace from blank line
(W293)
65-65: Missing dashed underline after section ("Parameters")
Add dashed line under "Parameters"
(D407)
65-65: Section name should end with a newline ("Parameters")
Add newline after "Parameters"
(D406)
69-69: Blank line contains whitespace
Remove whitespace from blank line
(W293)
70-70: Missing blank line after last section ("Returns")
Add blank line after "Returns"
(D413)
70-70: Missing dashed underline after section ("Returns")
Add dashed line under "Returns"
(D407)
70-70: Section name should end with a newline ("Returns")
Add newline after "Returns"
(D406)
86-86: Trailing whitespace
Remove trailing whitespace
(W291)
There was a problem hiding this comment.
Please make your code ready for review according to https://github.com/OWASP/Nest/blob/main/CONTRIBUTING.md#code-quality-checks
Sure !! I'll test it accordingly !! |
Any updates on this? The issues' deadline was Mar, 18 |
Yess sorry for the delay !! Im working on its test |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
backend/apps/github/graphql/queries/issue.py (3)
25-27: Line exceeds maximum length and consider adding database compatibility note.The implementation of distinct filtering looks good functionally, but there are a couple of improvements to consider:
- The line exceeds the maximum length of 99 characters (currently 124)
- The
distinctmethod with field names is PostgreSQL-specific functionalityif distinct: - queryset = queryset.order_by("author_id", "repository_id", "-created_at").distinct("author_id", "repository_id") + # Note: distinct on specific fields is PostgreSQL-specific + queryset = queryset.order_by( + "author_id", "repository_id", "-created_at" + ).distinct("author_id", "repository_id")🧰 Tools
🪛 Ruff (0.8.2)
26-26: Line too long (124 > 99)
(E501)
18-18: Consider enhancing the docstring.The current docstring "Resolve recent issues" could be enhanced to include information about the new
distinctparameter.def resolve_recent_issues(root, info, limit=15, distinct=False, login=None): - """Resolve recent issues.""" + """ + Resolve recent issues. + + Args: + limit: Maximum number of issues to return + distinct: When True, returns only one issue per author and repository + login: Filter issues by author login + """
11-16: Consider adding description to the new parameter.For better GraphQL schema documentation, consider adding a description to the
distinctparameter.recent_issues = graphene.List( IssueNode, limit=graphene.Int(default_value=15), - distinct=graphene.Boolean(default_value=False), + distinct=graphene.Boolean( + default_value=False, + description="When True, returns only one issue per author and repository" + ), login=graphene.String(required=False), )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/apps/github/graphql/queries/issue.py(1 hunks)frontend/src/api/queries/homeQueries.ts(3 hunks)frontend/src/pages/Home.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/pages/Home.tsx
- frontend/src/api/queries/homeQueries.ts
🧰 Additional context used
🪛 Ruff (0.8.2)
backend/apps/github/graphql/queries/issue.py
26-26: Line too long (124 > 99)
(E501)
🔇 Additional comments (2)
backend/apps/github/graphql/queries/issue.py (2)
13-14: LGTM: Default value updated and new parameter added correctly.The update to the
limitdefault value (from 6 to 15) and the addition of the newdistinctparameter with the appropriate default value is implemented correctly. This aligns with the PR objectives to support distinct filtering.
18-18: Function signature properly updated.The resolver function signature has been appropriately updated to match the new parameter in the GraphQL field definition.
|
…d Releases (OWASP#1154) * recent release fix * Recent Release Fix * Update backend/apps/owasp/graphql/nodes/project.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Updated * Update code --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Arkadii Yakovets <[email protected]> Co-authored-by: Arkadii Yakovets <[email protected]>
Resolves #1074
This PR enhances the GraphQL endpoints to support a distinct parameter, ensuring that issues and releases return unique results based on their authors and associated projects. This prevents multiple items from the same author or project from appearing consecutively in the results.
Changes: