Implement resolver for a list of project health metrics #1676
Conversation
Summary by CodeRabbit
WalkthroughThis change introduces a new GraphQL query field for retrieving project health metrics with filtering and pagination support. It adds a Strawberry-Django filter class for Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (6)
🧰 Additional context used🧠 Learnings (6)📓 Common learningsbackend/apps/owasp/graphql/queries/project_health_metrics.py (1)backend/apps/owasp/graphql/nodes/project_health_metrics.py (1)backend/apps/owasp/graphql/filters/project_health_metrics.py (1)backend/tests/apps/owasp/graphql/filters/project_health_metrics_test.py (1)backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py (1)🧬 Code Graph Analysis (5)backend/apps/owasp/graphql/queries/project_health_metrics.py (3)
backend/apps/owasp/graphql/nodes/project_health_metrics.py (1)
backend/apps/owasp/graphql/filters/project_health_metrics.py (2)
backend/tests/apps/owasp/graphql/filters/project_health_metrics_test.py (2)
backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py (3)
🪛 Pylint (3.3.7)backend/apps/owasp/graphql/queries/project_health_metrics.py[error] 4-4: Unable to import 'strawberry_django' (E0401) backend/apps/owasp/graphql/nodes/project_health_metrics.py[error] 71-71: Instance of 'ProjectHealthMetricsNode' has no 'project' member (E1101) backend/apps/owasp/graphql/filters/project_health_metrics.py[error] 3-3: Unable to import 'strawberry' (E0401) [error] 4-4: Unable to import 'strawberry_django' (E0401) [error] 5-5: Unable to import 'django.db.models' (E0401) [warning] 20-20: Unused argument 'prefix' (W0613) [refactor] 12-12: Too few public methods (1/2) (R0903) backend/tests/apps/owasp/graphql/filters/project_health_metrics_test.py[error] 17-17: Class 'ProjectHealthMetricsFilter' has no 'strawberry_definition' member (E1101) [warning] 25-25: Comparing against a callable, did you omit the parenthesis? (W0143) backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py[error] 27-27: Class 'ProjectHealthMetricsQuery' has no 'strawberry_definition' member (E1101) [error] 42-42: Class 'ProjectHealthMetricsQuery' has no 'strawberry_definition' member (E1101) ⏰ Context from checks skipped due to timeout of 90000ms (4)
🔇 Additional comments (11)
✨ 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. 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py (1)
125-140: Simplify the project health metrics test.The test logic is straightforward but could be cleaner.
Consider testing the actual field access pattern:
def test_resolve_project_health_metrics(self): """Test resolving project health metrics.""" - metrics = [ - ProjectHealthMetrics( - score=85.0, - stars_count=1000, - forks_count=200, - ) - ] - query = ProjectHealthMetricsQuery(project_health_metrics=metrics) - result = query.project_health_metrics - assert isinstance(result, list) - assert len(result) == 1 - assert result[0].stars_count == 1000 - assert result[0].score == 85.0 + query = ProjectHealthMetricsQuery() + # Test that the field exists and has correct type annotation + assert hasattr(query, 'project_health_metrics') + field = next(f for f in query.__strawberry_definition__.fields if f.name == 'project_health_metrics') + assert field.filters is not None # Verify filtering is enabled
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
backend/apps/owasp/graphql/filters/project_health_metrics.py(1 hunks)backend/apps/owasp/graphql/nodes/health_stats.py(1 hunks)backend/apps/owasp/graphql/nodes/project_health_metrics.py(3 hunks)backend/apps/owasp/graphql/queries/__init__.py(2 hunks)backend/apps/owasp/graphql/queries/project_health_metrics.py(1 hunks)backend/apps/owasp/models/project_health_metrics.py(2 hunks)backend/tests/apps/owasp/graphql/nodes/health_stats_test.py(1 hunks)backend/tests/apps/owasp/graphql/nodes/project_health_metrics_test.py(2 hunks)backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
backend/apps/owasp/graphql/queries/__init__.py (1)
backend/apps/owasp/graphql/queries/project_health_metrics.py (1)
ProjectHealthMetricsQuery(15-31)
backend/apps/owasp/graphql/nodes/project_health_metrics.py (1)
backend/apps/owasp/graphql/filters/project_health_metrics.py (1)
ProjectHealthMetricsFilter(12-22)
backend/tests/apps/owasp/graphql/nodes/health_stats_test.py (2)
backend/apps/owasp/graphql/queries/project_health_metrics.py (1)
health_stats(24-31)backend/apps/owasp/graphql/nodes/health_stats.py (1)
HealthStatsNode(7-20)
backend/apps/owasp/graphql/filters/project_health_metrics.py (2)
backend/apps/owasp/models/enums/project.py (1)
ProjectLevel(27-34)backend/apps/owasp/models/project_health_metrics.py (1)
ProjectHealthMetrics(13-198)
backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py (3)
backend/apps/owasp/graphql/queries/project_health_metrics.py (2)
health_stats(24-31)ProjectHealthMetricsQuery(15-31)backend/apps/owasp/graphql/nodes/health_stats.py (1)
HealthStatsNode(7-20)backend/apps/owasp/models/project_health_metrics.py (1)
ProjectHealthMetrics(13-198)
backend/apps/owasp/models/project_health_metrics.py (2)
backend/apps/owasp/graphql/queries/project_health_metrics.py (1)
health_stats(24-31)backend/apps/owasp/graphql/nodes/health_stats.py (1)
HealthStatsNode(7-20)
🪛 Pylint (3.3.7)
backend/apps/owasp/graphql/nodes/project_health_metrics.py
[error] 71-71: Instance of 'ProjectHealthMetricsNode' has no 'project' member
(E1101)
backend/apps/owasp/graphql/queries/project_health_metrics.py
[refactor] 15-15: Too few public methods (1/2)
(R0903)
backend/tests/apps/owasp/graphql/nodes/health_stats_test.py
[error] 13-13: Class 'HealthStatsNode' has no 'strawberry_definition' member
(E1101)
[error] 33-33: Class 'HealthStatsNode' has no 'strawberry_definition' member
(E1101)
backend/apps/owasp/graphql/nodes/health_stats.py
[refactor] 7-7: Too few public methods (0/2)
(R0903)
backend/apps/owasp/graphql/filters/project_health_metrics.py
[refactor] 12-12: Too few public methods (1/2)
(R0903)
backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py
[error] 29-29: Class 'ProjectHealthMetricsQuery' has no 'strawberry_definition' member
(E1101)
[error] 44-44: Class 'ProjectHealthMetricsQuery' has no 'strawberry_definition' member
(E1101)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run frontend unit tests
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
🔇 Additional comments (9)
backend/apps/owasp/graphql/nodes/health_stats.py (1)
6-20: LGTM! Well-structured GraphQL type definition.The
HealthStatsNodefollows GraphQL conventions and provides a comprehensive set of fields for aggregated health statistics. All field types are appropriately chosen for their purposes.backend/apps/owasp/graphql/queries/__init__.py (1)
8-8: LGTM! Clean integration of the new query class.The addition of
ProjectHealthMetricsQueryfollows the established pattern and correctly integrates the new health metrics querying capabilities into the main OWASP GraphQL schema.Also applies to: 20-20
backend/tests/apps/owasp/graphql/nodes/project_health_metrics_test.py (1)
33-33: LGTM! Appropriate test coverage for the new field.The test updates correctly include the new
project_namefield in both the field names verification and type checking, ensuring proper test coverage for the enhancement.Also applies to: 69-69
backend/apps/owasp/graphql/nodes/project_health_metrics.py (2)
8-8: LGTM! Proper integration of filtering and pagination capabilities.The addition of
ProjectHealthMetricsFilterand pagination follows Strawberry-Django best practices and enhances the GraphQL API with essential querying capabilities.Also applies to: 27-28
68-71: LGTM! Clean field resolver implementation.The
project_namefield resolver correctly accesses the related project's name. The static analysis warning about missing 'project' member is incorrect sinceProjectHealthMetricsNodeis based on theProjectHealthMetricsmodel which has aprojectforeign key relationship.backend/tests/apps/owasp/graphql/nodes/health_stats_test.py (1)
1-61: Well-structured comprehensive test suite.The test implementation effectively validates the HealthStatsNode GraphQL definition and field types using a clean parametrized approach. The helper method
_get_field_by_nameprovides good abstraction for field lookup.backend/apps/owasp/graphql/queries/project_health_metrics.py (1)
14-32: Clean GraphQL query implementation.The query class properly exposes both filtered list queries and aggregated statistics with appropriate type annotations and documentation.
backend/apps/owasp/models/project_health_metrics.py (1)
141-156: LGTM - Efficient latest metrics retrieval.The subquery approach correctly retrieves the most recent health metrics per project using the outer reference pattern.
backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py (1)
17-49: Good field definition validation.The parametrized tests effectively verify that the GraphQL query class has the correct field definitions and types.
backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py
Outdated
Show resolved
Hide resolved
ahmedxgouda
force-pushed
the
dashboard/internal-metrics-resolver
branch
from
53eae73 to
d2ac5ef
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
backend/apps/owasp/graphql/filters/project_health_metrics.py (1)
17-22: Add error handling for invalid project level values.The custom
levelfilter lacks error handling when converting the string value toProjectLevelenum. If an invalid value is passed,ProjectLevel(value)will raise aValueError.backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py (2)
88-88: Fix incorrect query constructor usage.Same issue as above - GraphQL query classes shouldn't be instantiated with field values.
- query = ProjectHealthMetricsQuery(project_health_metrics=metrics) + query = ProjectHealthMetricsQuery()
66-66: Fix incorrect query constructor usage.GraphQL query classes shouldn't be instantiated with field values. The
project_health_metricsparameter should be removed from the constructor.- query = ProjectHealthMetricsQuery(project_health_metrics=[]) + query = ProjectHealthMetricsQuery()
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/apps/owasp/graphql/filters/project_health_metrics.py(1 hunks)backend/apps/owasp/graphql/nodes/project_health_metrics.py(3 hunks)backend/apps/owasp/graphql/queries/project_health_metrics.py(1 hunks)backend/tests/apps/owasp/graphql/nodes/project_health_metrics_test.py(2 hunks)backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/tests/apps/owasp/graphql/nodes/project_health_metrics_test.py
🧰 Additional context used
🧬 Code Graph Analysis (2)
backend/apps/owasp/graphql/nodes/project_health_metrics.py (1)
backend/apps/owasp/graphql/filters/project_health_metrics.py (1)
ProjectHealthMetricsFilter(12-22)
backend/apps/owasp/graphql/queries/project_health_metrics.py (4)
backend/apps/owasp/graphql/nodes/health_stats.py (1)
HealthStatsNode(7-20)backend/apps/owasp/graphql/filters/project_health_metrics.py (1)
ProjectHealthMetricsFilter(12-22)backend/apps/owasp/graphql/nodes/project_health_metrics.py (1)
ProjectHealthMetricsNode(30-76)backend/apps/owasp/models/project_health_metrics.py (2)
ProjectHealthMetrics(13-198)get_overall_stats(158-198)
🪛 Pylint (3.3.7)
backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py
[error] 27-27: Class 'ProjectHealthMetricsQuery' has no 'strawberry_definition' member
(E1101)
[error] 42-42: Class 'ProjectHealthMetricsQuery' has no 'strawberry_definition' member
(E1101)
backend/apps/owasp/graphql/filters/project_health_metrics.py
[refactor] 12-12: Too few public methods (1/2)
(R0903)
backend/apps/owasp/graphql/nodes/project_health_metrics.py
[error] 71-71: Instance of 'ProjectHealthMetricsNode' has no 'project' member
(E1101)
backend/apps/owasp/graphql/queries/project_health_metrics.py
[refactor] 15-15: Too few public methods (1/2)
(R0903)
🔇 Additional comments (8)
backend/apps/owasp/graphql/filters/project_health_metrics.py (1)
11-16: LGTM: Clean filter implementation.The filter class structure and auto-typing for the score field are implemented correctly following Strawberry-Django patterns.
backend/apps/owasp/graphql/nodes/project_health_metrics.py (2)
27-28: LGTM: Proper filtering and pagination integration.The addition of filtering and pagination capabilities follows Strawberry-Django best practices and integrates well with the existing node structure.
68-71: Relationship Verified:projectForeignKey Exists
- The
ProjectHealthMetricsmodel definesproject = models.ForeignKey("owasp.Project", on_delete=models.CASCADE, related_name="health_metrics")inbackend/apps/owasp/models/project_health_metrics.py:28-32.
No further action required.backend/apps/owasp/graphql/queries/project_health_metrics.py (2)
18-21: LGTM: Proper field configuration.The
project_health_metricsfield is correctly configured with filtering support and appropriate description.
23-31: LGTM: Clean resolver implementation.The
health_statsresolver correctly delegates to the model's static method and includes proper type annotations and documentation.backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py (3)
15-29: LGTM: Comprehensive field definition testing.The parametrized tests for checking Strawberry field definitions are well-structured and cover all expected fields.
31-46: LGTM: Proper type validation testing.The field configuration tests correctly validate the expected types for each GraphQL field.
48-69: LGTM: Improved mocking strategy.The test now properly mocks the
get_overall_statsmethod and validates the expected behavior. This addresses the issues mentioned in previous reviews.
ahmedxgouda
force-pushed
the
dashboard/internal-metrics-resolver
branch
from
d2ac5ef to
b035e75
Compare
ahmedxgouda
force-pushed
the
dashboard/internal-metrics-resolver
branch
from
76d1312 to
7ed3923
Compare
|
2 participants
Resolves #1666