Repository navigation
Implement the presentation of dashboard metrics list with filters - #1718
Conversation
Summary by CodeRabbit
WalkthroughThis change implements a new project health metrics dashboard list in the frontend, including a paginated, filterable, and sortable metrics page with supporting components, GraphQL queries, and types. Backend changes add ordering and filtering to the health metrics API, expose new fields, and update tests. New end-to-end and unit tests validate the dashboard's functionality. Changes
Estimated code review effort3 (~45 minutes) Possibly related PRs
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)
⏰ 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). (4)
✨ 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: 10
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
backend/apps/owasp/graphql/nodes/project_health_metrics.py(1 hunks)backend/apps/owasp/graphql/queries/project_health_metrics.py(1 hunks)frontend/src/app/projects/dashboard/layout.tsx(1 hunks)frontend/src/app/projects/dashboard/metrics/page.tsx(1 hunks)frontend/src/app/projects/dashboard/page.tsx(1 hunks)frontend/src/components/MetricsCard.tsx(1 hunks)frontend/src/components/ProjectsDashboardNavBar.tsx(1 hunks)frontend/src/server/queries/projectsHealthDashboardQueries.ts(1 hunks)frontend/src/types/healthMetrics.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
backend/apps/owasp/graphql/nodes/project_health_metrics.py (1)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1676
File: backend/apps/owasp/graphql/filters/project_health_metrics.py:17-22
Timestamp: 2025-06-29T00:41:32.198Z
Learning: In the OWASP Nest codebase, when implementing GraphQL filters that convert string values to enums (like ProjectLevel), do not catch ValueError exceptions for invalid values. Let the errors propagate to provide proper error responses to GraphQL clients rather than silently ignoring invalid input.
frontend/src/app/projects/dashboard/layout.tsx (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/components/ProjectsDashboardNavBar.tsx (1)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1703
File: frontend/src/components/BarChart.tsx:33-46
Timestamp: 2025-07-03T03:08:03.290Z
Learning: In the OWASP Nest project's BarChart component (frontend/src/components/BarChart.tsx), the days and requirements arrays are guaranteed to always have the same length in their use cases, so input validation for array length matching is not needed.
frontend/src/components/MetricsCard.tsx (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/app/projects/dashboard/metrics/page.tsx (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/types/healthMetrics.ts (3)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/GradientRadialChart.tsx:113-113
Timestamp: 2025-06-21T12:22:01.889Z
Learning: In the OWASP Nest project, health metrics requirements (like lastCommitDaysRequirement, lastReleaseDaysRequirement) should never be 0. A requirement value of 0 is considered invalid and should result in displaying 0 on the radial chart.
frontend/src/app/projects/dashboard/page.tsx (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/server/queries/projectsHealthDashboardQueries.ts (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
🧬 Code Graph Analysis (3)
frontend/src/components/MetricsCard.tsx (1)
frontend/src/types/healthMetrics.ts (1)
HealthMetricsProps(26-42)
frontend/src/app/projects/dashboard/metrics/page.tsx (2)
frontend/src/types/healthMetrics.ts (2)
HealthMetricsProps(26-42)HealthMetricsFilter(44-52)frontend/src/server/queries/projectsHealthDashboardQueries.ts (1)
GET_PROJECT_HEALTH_METRICS_LIST(20-35)
frontend/src/app/projects/dashboard/page.tsx (1)
frontend/src/server/queries/projectsHealthDashboardQueries.ts (1)
GET_PROJECT_HEALTH_STATS(3-19)
🪛 Biome (1.9.4)
frontend/src/app/projects/dashboard/layout.tsx
[error] 10-10: The elements with the following roles can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
[error] 10-10: Using the role attribute 'main' on the 'main' element is redundant, because it is implied by the semantic 'main' element.
Unsafe fix: Remove the role attribute.
(lint/a11y/noRedundantRoles)
🪛 GitHub Actions: Run CI/CD
frontend/src/types/healthMetrics.ts
[error] 49-49: CSpell: Unknown word 'lte' detected during spelling check.
⏰ 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). (2)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
🔇 Additional comments (8)
frontend/src/components/ProjectsDashboardNavBar.tsx (1)
1-55: LGTM! Well-implemented navigation component.The component follows React and Next.js best practices with proper TypeScript typing, accessibility attributes, and responsive design. The code is clean, maintainable, and includes appropriate client-side directive usage.
backend/apps/owasp/graphql/nodes/project_health_metrics.py (1)
29-29: LGTM! Proper relay node inheritance for pagination support.The change to inherit from
strawberry.relay.Nodecorrectly enables GraphQL relay-style pagination, aligning with the corresponding query-level pagination changes.backend/apps/owasp/graphql/queries/project_health_metrics.py (1)
19-19: LGTM! Pagination enabled at query level.The addition of
pagination=Trueto the field decorator correctly enables pagination support for the project health metrics query, complementing the node-level changes.frontend/src/app/projects/dashboard/layout.tsx (1)
1-17: LGTM! Well-structured layout component.The component provides a clean, responsive layout structure for the dashboard with proper semantic HTML and accessibility considerations.
frontend/src/server/queries/projectsHealthDashboardQueries.ts (2)
3-19: LGTM! Well-structured GraphQL query for aggregated statistics.The query structure is clean and includes all necessary fields for displaying project health dashboard statistics.
20-35: LGTM! Proper use of variables for pagination and filtering.The query correctly uses variables for filters and pagination, enabling flexible data fetching. The field selection matches the component requirements.
frontend/src/types/healthMetrics.ts (2)
26-42: LGTM! Type updates properly support the new dashboard functionality.The additions of optional fields like
id,contributorsCount, andprojectNamealign well with the GraphQL queries and component requirements. Making several fields optional provides flexibility for different use cases.
44-52: LGTM! Well-structured filter type for GraphQL queries.The
HealthMetricsFiltertype properly supports both score range filtering and level filtering, matching the GraphQL query structure and component filtering needs.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
cspell/custom-dict.txt(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
⏰ 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). (4)
- GitHub Check: Run CI Denendencies Scan
- GitHub Check: Run Code Scan
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
frontend/src/components/MetricsCard.tsx (3)
8-8: Handle optionalidfield to prevent navigation issues.The
idfield is optional inHealthMetricsPropsbut used directly for navigation. This could cause issues ifidis undefined.
13-13: Handle optionalprojectNamefield gracefully.The
projectNamefield is optional inHealthMetricsPropsbut used directly, which could display "undefined" text.
22-22: Handle optionalcontributorsCountfield gracefully.The
contributorsCountfield is optional inHealthMetricsPropsbut used directly, which could display "undefined" text.frontend/src/app/projects/dashboard/metrics/page.tsx (3)
25-25: Consider making pagination limit configurable.The hard-coded pagination limit of 10 reduces flexibility. Consider making it configurable or using a constant.
53-59: Fix potential race condition in filter handling.The refetch call uses the old
filtersstate instead of the new value being set, which could lead to inconsistent filtering behavior due to React's asynchronous state updates.
76-80: Add handling for empty metrics state.Consider adding a message or placeholder when no metrics are available to improve user experience.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
frontend/src/app/projects/dashboard/metrics/page.tsx(1 hunks)frontend/src/components/MetricsCard.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/app/projects/dashboard/metrics/page.tsx (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/components/MetricsCard.tsx (4)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Learnt from: codic-yeeshu
PR: OWASP/Nest#1444
File: frontend/src/components/NavDropDown.tsx:0-0
Timestamp: 2025-04-30T13:41:20.846Z
Learning: Use React's useId() hook rather than manually generating random strings when creating accessibility identifiers for UI components. This creates stable, unique IDs without causing hydration mismatches.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/GradientRadialChart.tsx:113-113
Timestamp: 2025-06-21T12:22:01.889Z
Learning: In the OWASP Nest project, health metrics requirements (like lastCommitDaysRequirement, lastReleaseDaysRequirement) should never be 0. A requirement value of 0 is considered invalid and should result in displaying 0 on the radial chart.
⏰ 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). (4)
- GitHub Check: Run Code Scan
- GitHub Check: Run CI Denendencies Scan
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
🔇 Additional comments (2)
frontend/src/components/MetricsCard.tsx (1)
31-34: Score color logic is well-implemented.The conditional color coding for scores (green ≥75, orange 50-74, red <50) provides clear visual feedback and follows good UX practices for health metrics display.
frontend/src/app/projects/dashboard/metrics/page.tsx (1)
68-75: Grid layout implementation is well-structured.The grid headers match the layout used in
MetricsCard, providing consistent column alignment. The responsive grid system with fractional units appropriately allocates space for different data types.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
frontend/src/app/projects/dashboard/metrics/page.tsx (4)
19-19: Remove redundant manual loading state management.The manual
isLoadingstate is unnecessary since Apollo Client'suseQueryalready provides aloadingstate. This creates redundant state management and potential inconsistencies.
30-39: Simplify useEffect by removing manual loading state updates.The
useEffectshould focus only on data and error handling, not loading state management since Apollo Client handles this automatically.useEffect(() => { if (data) { setMetrics(data.projectHealthMetrics) - setIsLoading(false) } if (graphQLRequestError) { handleAppError(graphQLRequestError) - setIsLoading(false) } }, [data, graphQLRequestError])
40-42: Use Apollo Client's loading state instead of manual state.Replace the manual
isLoadingstate with Apollo Client'sloadingstate for consistency and to eliminate redundancy.- if (isLoading) { + if (loading) { return <LoadingSpinner /> }
20-29: Missing loading state from useQuery destructuring.The
loadingstate should be destructured fromuseQueryto replace the manual loading state management.const { data, + loading, error: graphQLRequestError, refetch, } = useQuery(GET_PROJECT_HEALTH_METRICS_LIST, { variables: { filters, pagination: { offset: 0, limit: PAGINATION_LIMIT }, }, })
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
frontend/src/app/projects/dashboard/metrics/page.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/app/projects/dashboard/metrics/page.tsx (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
⏰ 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). (4)
- GitHub Check: Run frontend checks
- GitHub Check: Run pre-commit checks
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
frontend/src/types/healthMetrics.ts (1)
49-49: CSpell false positive for standard GraphQL term.The
lte(less than or equal) term is a standard GraphQL comparison operator and should be added to the CSpell dictionary to prevent false positives.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
backend/apps/owasp/graphql/ordering/project_health_metrics.py(1 hunks)backend/apps/owasp/graphql/queries/project_health_metrics.py(1 hunks)backend/tests/apps/owasp/graphql/ordering/project_health_metrics_test.py(1 hunks)frontend/src/app/projects/dashboard/metrics/page.tsx(1 hunks)frontend/src/server/queries/projectsHealthDashboardQueries.ts(1 hunks)frontend/src/types/healthMetrics.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/app/projects/dashboard/metrics/page.tsx (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/server/queries/projectsHealthDashboardQueries.ts (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/types/healthMetrics.ts (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
backend/apps/owasp/graphql/queries/project_health_metrics.py (1)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1676
File: backend/apps/owasp/graphql/filters/project_health_metrics.py:17-22
Timestamp: 2025-06-29T00:41:32.198Z
Learning: In the OWASP Nest codebase, when implementing GraphQL filters that convert string values to enums (like ProjectLevel), do not catch ValueError exceptions for invalid values. Let the errors propagate to provide proper error responses to GraphQL clients rather than silently ignoring invalid input.
🧬 Code Graph Analysis (2)
backend/tests/apps/owasp/graphql/ordering/project_health_metrics_test.py (3)
backend/apps/owasp/graphql/queries/project_health_metrics.py (1)
project_health_metrics(23-40)backend/apps/owasp/graphql/ordering/project_health_metrics.py (1)
ProjectHealthMetricsOrder(10-16)backend/apps/owasp/graphql/nodes/committee.py (3)
forks_count(25-27)stars_count(40-42)contributors_count(15-17)
backend/apps/owasp/graphql/queries/project_health_metrics.py (3)
backend/apps/owasp/graphql/ordering/project_health_metrics.py (1)
ProjectHealthMetricsOrder(10-16)backend/apps/owasp/models/project_health_metrics.py (2)
ProjectHealthMetrics(16-210)get_latest_health_metrics(145-158)backend/apps/owasp/graphql/filters/project_health_metrics.py (1)
ProjectHealthMetricsFilter(12-22)
⏰ 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). (2)
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (9)
backend/tests/apps/owasp/graphql/ordering/project_health_metrics_test.py (1)
1-30: LGTM! Comprehensive test coverage for ordering functionality.The test suite effectively validates the ProjectHealthMetricsOrder class with good coverage of:
- Strawberry GraphQL definition verification
- Field validation against expected ordering options
- Instance creation with different ordering directions
The tests follow pytest best practices and provide confidence in the ordering implementation.
backend/apps/owasp/graphql/ordering/project_health_metrics.py (1)
1-17: LGTM! Clean ordering implementation following framework conventions.The ordering class correctly:
- Uses the
@order_typedecorator for automatic GraphQL type generation- Defines the expected fields for project health metrics ordering
- Leverages
autotype inference for proper GraphQL scalar typesThis implementation aligns well with the test coverage and GraphQL query requirements.
frontend/src/server/queries/projectsHealthDashboardQueries.ts (1)
1-37: LGTM! Well-structured GraphQL queries supporting dashboard functionality.The queries are properly implemented with:
- Correct GraphQL syntax and variable definitions
- Comprehensive field selection for both stats and metrics list
- Proper support for filtering, pagination, and ordering parameters
- Alignment with backend schema and frontend requirements
These queries effectively bridge the frontend dashboard components with the backend GraphQL API.
frontend/src/types/healthMetrics.ts (2)
26-42: LGTM! Enhanced type definitions support new dashboard features.The additions to
HealthMetricsPropscorrectly include the new fields (id,contributorsCount,projectName) that align with the backend GraphQL schema and dashboard requirements.
44-67: LGTM! Well-designed filter and ordering types.The new
HealthMetricsFilterandHealthMetricsOrderingtypes provide:
- Comprehensive score filtering with standard comparison operators (gt, gte, lt, lte)
- Level-based filtering support
- Structured ordering options for all relevant metrics fields
These types effectively support the dashboard's filtering and sorting functionality.
frontend/src/app/projects/dashboard/metrics/page.tsx (2)
17-47: LGTM! Well-structured component with proper state management.The component correctly:
- Uses Apollo Client's useQuery hook for data fetching
- Implements proper error handling with handleAppError
- Manages loading states appropriately
- Uses a configurable pagination limit constant
The overall architecture is solid for a dashboard metrics page.
116-132: LGTM! Well-implemented grid layout with proper empty state handling.The component correctly:
- Uses a responsive grid layout for the metrics display
- Provides clear column headers
- Handles empty states with user-friendly messaging
- Maps through metrics with proper key handling
The UI implementation is clean and user-friendly.
backend/apps/owasp/graphql/queries/project_health_metrics.py (2)
9-9: LGTM - Import addition aligns with new functionality.The import for
ProjectHealthMetricsOrderis correctly added to support the new ordering capabilities.
17-28: LGTM - Well-structured resolver method signature.The decorator and method signature are properly implemented:
- Correct use of
@strawberry_django.fielddecorator with all necessary parameters- Proper parameter types and optional ordering
- Good documentation format
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
frontend/src/app/projects/dashboard/metrics/page.tsx (2)
30-30: Manual loading state management is still redundant.This duplicates a previous review comment that hasn't been addressed. Apollo Client's
useQueryalready provides aloadingstate, making the manualisLoadingstate anduseEffectunnecessary.Apply this diff to use Apollo's loading state:
- const [isLoading, setIsLoading] = useState<boolean>(true) const { data, + loading, error: graphQLRequestError, refetch, } = useQuery(GET_PROJECT_HEALTH_METRICS_LIST, { variables: { filters, pagination: { offset: 0, limit: PAGINATION_LIMIT }, ordering: Object.values(ordering), }, }) useEffect(() => { if (data) { setMetrics(data.projectHealthMetrics) - setIsLoading(false) } if (graphQLRequestError) { handleAppError(graphQLRequestError) - setIsLoading(false) } }, [data, graphQLRequestError]) - if (isLoading) { + if (loading) { return <LoadingSpinner /> }Also applies to: 42-54
103-115: Complex ordering logic still needs simplification.This duplicates a previous review comment that hasn't been addressed. The dynamic property construction using string interpolation and bracket notation reduces type safety and maintainability.
Apply this diff to improve type safety:
onPress={() => { const [key, order] = option.key.split('_') - const newOrdering: HealthMetricsOrdering = { - ...ordering, - } - newOrdering[`${key}Ordering`] = { [key]: order } + const orderingMap: Record<string, keyof HealthMetricsOrdering> = { + score: 'scoreOrdering', + starsCount: 'starsCountOrdering', + forksCount: 'forksCountOrdering', + contributorsCount: 'contributorsCountOrdering', + } + const newOrdering: HealthMetricsOrdering = { + ...ordering, + [orderingMap[key]]: { [key]: order as 'ASC' | 'DESC' } + } setOrdering(newOrdering) refetch({ filters, pagination: { offset: 0, limit: PAGINATION_LIMIT }, ordering: Object.values(newOrdering), }) }}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
frontend/src/app/projects/dashboard/metrics/page.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/app/projects/dashboard/metrics/page.tsx (4)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1703
File: frontend/src/components/BarChart.tsx:33-46
Timestamp: 2025-07-03T03:08:03.290Z
Learning: In the OWASP Nest project's BarChart component (frontend/src/components/BarChart.tsx), the days and requirements arrays are guaranteed to always have the same length in their use cases, so input validation for array length matching is not needed.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1714
File: frontend/src/components/ProjectTypeDashboardCard.tsx:8-12
Timestamp: 2025-07-08T17:07:50.957Z
Learning: In the OWASP/Nest project, union types for component props are not necessary when they would require creating separate type definitions. The project prefers inline prop type definitions even for props with specific string values, maintaining consistency with the single-use component prop pattern.
⏰ 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). (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
backend/apps/owasp/graphql/queries/project_health_metrics.py(2 hunks)backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py(2 hunks)frontend/src/app/projects/dashboard/metrics/page.tsx(1 hunks)frontend/src/components/ProjectsDashboardDropDown.tsx(1 hunks)frontend/src/server/queries/projectsHealthDashboardQueries.ts(1 hunks)frontend/src/types/DropDownSectionProps.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/app/projects/dashboard/metrics/page.tsx (5)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1718
File: frontend/src/app/projects/dashboard/metrics/page.tsx:96-96
Timestamp: 2025-07-09T08:37:10.232Z
Learning: In the OWASP Nest project's MetricsPage component (frontend/src/app/projects/dashboard/metrics/page.tsx), the sorting dropdown intentionally uses selectionMode="multiple" to allow users to select multiple sorting criteria simultaneously. This enables complex sorting scenarios where users can sort by multiple fields in sequence.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1703
File: frontend/src/components/BarChart.tsx:33-46
Timestamp: 2025-07-03T03:08:03.290Z
Learning: In the OWASP Nest project's BarChart component (frontend/src/components/BarChart.tsx), the days and requirements arrays are guaranteed to always have the same length in their use cases, so input validation for array length matching is not needed.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1714
File: frontend/src/components/ProjectTypeDashboardCard.tsx:8-12
Timestamp: 2025-07-08T17:07:50.957Z
Learning: In the OWASP/Nest project, union types for component props are not necessary when they would require creating separate type definitions. The project prefers inline prop type definitions even for props with specific string values, maintaining consistency with the single-use component prop pattern.
backend/apps/owasp/graphql/queries/project_health_metrics.py (3)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1718
File: backend/apps/owasp/graphql/queries/project_health_metrics.py:0-0
Timestamp: 2025-07-09T08:41:58.105Z
Learning: In strawberry_django, when using @strawberry_django.field decorator with filters, pagination, and ordering parameters, the framework automatically applies these to the QuerySet returned by the resolver method. Manual application using strawberry_django.filters.apply() is only needed for custom implementations, not when using the standard decorator approach.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1676
File: backend/apps/owasp/graphql/filters/project_health_metrics.py:17-22
Timestamp: 2025-06-29T00:41:32.198Z
Learning: In the OWASP Nest codebase, when implementing GraphQL filters that convert string values to enums (like ProjectLevel), do not catch ValueError exceptions for invalid values. Let the errors propagate to provide proper error responses to GraphQL clients rather than silently ignoring invalid input.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1675
File: backend/apps/owasp/graphql/queries/project_health_metrics.py:13-21
Timestamp: 2025-07-03T03:47:05.436Z
Learning: The OWASP Nest project does not use async operations anywhere in the codebase and doesn't have the required database driver for async operations. All database methods and GraphQL resolvers should remain synchronous to maintain consistency with the existing architecture.
frontend/src/server/queries/projectsHealthDashboardQueries.ts (2)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/components/ProjectsDashboardDropDown.tsx (1)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1718
File: frontend/src/app/projects/dashboard/metrics/page.tsx:96-96
Timestamp: 2025-07-09T08:37:10.232Z
Learning: In the OWASP Nest project's MetricsPage component (frontend/src/app/projects/dashboard/metrics/page.tsx), the sorting dropdown intentionally uses selectionMode="multiple" to allow users to select multiple sorting criteria simultaneously. This enables complex sorting scenarios where users can sort by multiple fields in sequence.
🧬 Code Graph Analysis (3)
frontend/src/app/projects/dashboard/metrics/page.tsx (4)
frontend/src/types/healthMetrics.ts (3)
HealthMetricsProps(26-42)HealthMetricsFilter(44-52)HealthMetricsOrdering(54-67)frontend/src/server/queries/projectsHealthDashboardQueries.ts (1)
GET_PROJECT_HEALTH_METRICS_LIST(20-37)frontend/src/app/global-error.tsx (1)
handleAppError(66-86)frontend/src/types/DropDownSectionProps.ts (1)
DropDownSectionProps(1-7)
frontend/src/components/ProjectsDashboardDropDown.tsx (1)
frontend/src/types/DropDownSectionProps.ts (1)
DropDownSectionProps(1-7)
backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py (2)
backend/apps/owasp/graphql/queries/project_health_metrics.py (4)
ProjectHealthMetricsQuery(14-60)project_health_stats(43-50)project_health_metrics(23-40)project_health_metrics_distinct_length(53-60)backend/apps/owasp/graphql/nodes/project_health_metrics.py (1)
ProjectHealthMetricsNode(29-75)
⏰ 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). (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run backend tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (14)
frontend/src/types/DropDownSectionProps.ts (1)
1-7: LGTM!The type definition is well-structured and provides a clear interface for dropdown sections.
frontend/src/server/queries/projectsHealthDashboardQueries.ts (1)
1-37: Well-structured GraphQL queries!The queries are properly defined with appropriate parameters and field selections that align with the backend schema changes.
backend/tests/apps/owasp/graphql/queries/project_health_metrics_test.py (3)
66-66: Test updates align well with the refactored query structure.The removal of the empty list parameter correctly reflects the new query implementation.
71-99: Comprehensive test coverage for the enhanced resolver.The test properly validates the new method signature with filters, pagination, and ordering parameters.
100-109: Good addition of distinct length test.The test correctly verifies the new
project_health_metrics_distinct_lengthresolver functionality.frontend/src/components/ProjectsDashboardDropDown.tsx (1)
14-41: Well-designed reusable dropdown component!The component is properly structured with clear props interface and clean implementation. Good use of the @heroui/react library components.
frontend/src/app/projects/dashboard/metrics/page.tsx (5)
1-37: Good setup with proper state management and Apollo Client usage.Excellent use of a constant for pagination limit and proper destructuring of Apollo Client's query results including the loading state.
39-47: Proper data synchronization and error handling.Good use of useEffect to sync Apollo query results with local state and appropriate error handling.
49-132: Well-organized configuration for dropdowns.Good separation of dropdown sections and filter mappings. The inclusion of a reset option is a nice UX touch.
220-232: Excellent UI state handling.Great implementation with proper loading state, empty state message, and data rendering. This addresses the UX concerns from previous reviews.
233-261: Well-implemented pagination with proper cache updates.The pagination component correctly handles page changes and updates the Apollo cache appropriately through the updateQuery callback.
backend/apps/owasp/graphql/queries/project_health_metrics.py (3)
9-9: LGTM: Proper import for ordering support.The import of
ProjectHealthMetricsOrderis correctly added to support the new ordering functionality.
17-40: LGTM: Excellent refactor with proper pagination and ordering support.The refactor from a simple field declaration to a method resolver with
@strawberry_django.fielddecorator is well-implemented. The decorator correctly specifiespagination=Trueandordering=ProjectHealthMetricsOrder, and the method signature includes all necessary parameters. Based on the strawberry_django framework behavior, the filters, pagination, and ordering will be automatically applied to the QuerySet returned byProjectHealthMetrics.get_latest_health_metrics().The docstring is comprehensive and follows good documentation practices.
52-60: LGTM: Clean implementation of distinct count field.The new field provides a straightforward way to get the count of distinct project health metrics by calling
.count()on the result ofget_latest_health_metrics(). The implementation is clean and the docstring is well-written.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
frontend/src/app/projects/dashboard/metrics/page.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/app/projects/dashboard/metrics/page.tsx (4)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1718
File: frontend/src/app/projects/dashboard/metrics/page.tsx:96-96
Timestamp: 2025-07-09T08:37:10.241Z
Learning: In the OWASP Nest project's MetricsPage component (frontend/src/app/projects/dashboard/metrics/page.tsx), the sorting dropdown intentionally uses selectionMode="multiple" to allow users to select multiple sorting criteria simultaneously. This enables complex sorting scenarios where users can sort by multiple fields in sequence.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1703
File: frontend/src/components/BarChart.tsx:33-46
Timestamp: 2025-07-03T03:08:03.290Z
Learning: In the OWASP Nest project's BarChart component (frontend/src/components/BarChart.tsx), the days and requirements arrays are guaranteed to always have the same length in their use cases, so input validation for array length matching is not needed.
🧬 Code Graph Analysis (1)
frontend/src/app/projects/dashboard/metrics/page.tsx (4)
frontend/src/types/healthMetrics.ts (3)
HealthMetricsProps(26-42)HealthMetricsFilter(44-52)HealthMetricsOrdering(54-67)frontend/src/server/queries/projectsHealthDashboardQueries.ts (1)
GET_PROJECT_HEALTH_METRICS_LIST(20-37)frontend/src/app/global-error.tsx (1)
handleAppError(66-86)frontend/src/types/DropDownSectionProps.ts (1)
DropDownSectionProps(1-7)
⏰ 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). (4)
- GitHub Check: Run Code Scan
- GitHub Check: Run CI Denendencies Scan
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (8)
frontend/src/app/projects/dashboard/metrics/page.tsx (8)
1-16: LGTM! Clean imports and setup.The imports are well-organized and the PAGINATION_LIMIT constant extraction follows the feedback from previous reviews.
17-39: Good state management and Apollo integration.The state management is comprehensive and the Apollo useQuery hook is properly configured with all necessary variables.
41-49: Proper data handling and error management.The useEffect properly handles both successful data responses and GraphQL errors using the global error handler.
51-103: Well-structured dropdown configuration.The sections are logically organized and provide a good user experience with clear categorization of filters and sorting options.
162-164: Simple and effective pagination helper.The getCurrentPage calculation is straightforward and correctly handles the offset-to-page conversion.
166-211: Excellent header implementation with proper dropdown integration.The header layout with filtering and sorting dropdowns is well-implemented, properly handling state updates and refetching data when selections change.
212-232: Good grid layout and empty state handling.The grid headers provide clear context and the empty state message improves user experience when no metrics match the current filters.
233-264: Robust pagination implementation.The pagination component properly handles page changes using
fetchMoreand maintains filters/ordering state correctly across page transitions.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
frontend/src/app/projects/dashboard/metrics/page.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/app/projects/dashboard/metrics/page.tsx (4)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1718
File: frontend/src/app/projects/dashboard/metrics/page.tsx:96-96
Timestamp: 2025-07-09T08:37:10.241Z
Learning: In the OWASP Nest project's MetricsPage component (frontend/src/app/projects/dashboard/metrics/page.tsx), the sorting dropdown intentionally uses selectionMode="multiple" to allow users to select multiple sorting criteria simultaneously. This enables complex sorting scenarios where users can sort by multiple fields in sequence.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1703
File: frontend/src/components/BarChart.tsx:33-46
Timestamp: 2025-07-03T03:08:03.290Z
Learning: In the OWASP Nest project's BarChart component (frontend/src/components/BarChart.tsx), the days and requirements arrays are guaranteed to always have the same length in their use cases, so input validation for array length matching is not needed.
🧬 Code Graph Analysis (1)
frontend/src/app/projects/dashboard/metrics/page.tsx (4)
frontend/src/types/healthMetrics.ts (3)
HealthMetricsProps(26-42)HealthMetricsFilter(44-52)HealthMetricsOrdering(54-67)frontend/src/server/queries/projectsHealthDashboardQueries.ts (1)
GET_PROJECT_HEALTH_METRICS_LIST(20-37)frontend/src/app/global-error.tsx (1)
handleAppError(66-86)frontend/src/types/DropDownSectionProps.ts (1)
DropDownSectionProps(1-7)
🪛 Biome (1.9.4)
frontend/src/app/projects/dashboard/metrics/page.tsx
[error] 167-167: expected , but instead found const
Remove const
(parse)
[error] 167-167: expected : but instead found getCurrentPage
Remove getCurrentPage
(parse)
[error] 171-171: expected , but instead found return
Remove return
(parse)
[error] 172-172: Expected a parameter but instead found '<'.
Expected a parameter here.
(parse)
[error] 172-172: Expected a type parameter but instead found '>'.
Expected a type parameter here.
(parse)
[error] 173-173: Expected a parenthesis '(' but instead found '<'.
Expected a parenthesis '(' here.
(parse)
[error] 173-173: expected , but instead found className
Remove className
(parse)
[error] 174-174: Expected a parenthesis '(' but instead found '<'.
Expected a parenthesis '(' here.
(parse)
[error] 174-174: expected , but instead found className
Remove className
(parse)
[error] 174-174: Expected a parenthesis '(' but instead found 'Project'.
Expected a parenthesis '(' here.
(parse)
[error] 174-174: expected , but instead found Health
Remove Health
(parse)
[error] 174-174: expected , but instead found Metrics
Remove Metrics
(parse)
[error] 174-174: Expected a type parameter but instead found '/'.
Expected a type parameter here.
(parse)
[error] 174-174: expected , but instead found h1
Remove h1
(parse)
[error] 175-175: Expected a parenthesis '(' but instead found '<'.
Expected a parenthesis '(' here.
(parse)
[error] 175-175: expected , but instead found className
Remove className
(parse)
[error] 176-176: Expected a parenthesis '(' but instead found '<'.
Expected a parenthesis '(' here.
(parse)
[error] 177-177: expected , but instead found buttonDisplayName
Remove buttonDisplayName
(parse)
[error] 178-178: expected , but instead found icon
Remove icon
(parse)
[error] 179-179: expected , but instead found sections
Remove sections
(parse)
[error] 180-180: expected , but instead found selectionMode
Remove selectionMode
(parse)
[error] 181-181: expected , but instead found selectedKeys
Remove selectedKeys
(parse)
[error] 182-182: expected , but instead found onAction
Remove onAction
(parse)
[error] 182-182: ';' expected'
An explicit or implicit semicolon is expected here...
(parse)
[error] 184-184: ';' expected'
An explicit or implicit semicolon is expected here...
(parse)
[error] 184-184: ';' expected'
An explicit or implicit semicolon is expected here...
(parse)
[error] 184-184: Expected a property, or a signature but instead found ','.
Expected a property, or a signature here.
(parse)
[error] 184-184: expected , but instead found :
Remove :
(parse)
[error] 184-184: expected , but instead found PAGINATION_LIMIT
Remove PAGINATION_LIMIT
(parse)
[error] 184-184: expected , but instead found }
Remove }
(parse)
[error] 185-185: expected , but instead found setPagination
Remove setPagination
(parse)
[error] 185-185: expected , but instead found (
Remove (
(parse)
[error] 185-185: expected , but instead found newPagination
Remove newPagination
(parse)
[error] 185-185: expected , but instead found )
Remove )
(parse)
[error] 186-186: expected , but instead found setFilters
Remove setFilters
(parse)
[error] 186-186: expected , but instead found (
Remove (
(parse)
[error] 186-186: expected , but instead found filtersMapping
Remove filtersMapping
(parse)
[error] 186-186: expected , but instead found [
Remove [
(parse)
[error] 186-186: expected , but instead found key
Remove key
(parse)
[error] 186-186: expected , but instead found ]
Remove ]
(parse)
[error] 187-187: expected , but instead found setActiveFilterKey
Remove setActiveFilterKey
(parse)
[error] 187-187: expected , but instead found (
Remove (
(parse)
[error] 187-187: expected , but instead found key
Remove key
(parse)
[error] 187-187: expected , but instead found )
Remove )
(parse)
[error] 188-188: expected , but instead found await
Remove await
(parse)
[error] 188-188: expected , but instead found refetch
Remove refetch
(parse)
[error] 188-188: expected , but instead found (
Remove (
(parse)
[error] 189-189: expected , but instead found filters
Remove filters
(parse)
[error] 189-189: expected , but instead found :
Remove :
(parse)
[error] 189-189: expected , but instead found filtersMapping
Remove filtersMapping
(parse)
[error] 189-189: expected , but instead found [
Remove [
(parse)
[error] 189-189: expected , but instead found key
Remove key
(parse)
[error] 189-189: expected , but instead found ]
Remove ]
(parse)
[error] 190-190: expected , but instead found :
Remove :
(parse)
[error] 190-190: expected , but instead found newPagination
Remove newPagination
(parse)
[error] 191-191: expected , but instead found :
Remove :
(parse)
[error] 191-191: expected , but instead found Object
Remove Object
(parse)
[error] 191-191: expected , but instead found .
Remove .
(parse)
[error] 191-191: expected , but instead found values
Remove values
(parse)
[error] 191-191: expected , but instead found (
Remove (
(parse)
[error] 191-191: expected , but instead found ordering
Remove ordering
(parse)
[error] 191-191: expected , but instead found )
Remove )
(parse)
[error] 192-192: Expected a type parameter but instead found '}'.
Expected a type parameter here.
(parse)
[error] 192-192: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 193-198: Expected a statement but instead found '}
/>
<ProjectsDashboardDropDown
buttonDisplayName="Sort By"
icon='.
Expected a statement here.
(parse)
[error] 201-201: expected : but instead found }
Remove }
(parse)
[error] 202-202: Expected a function body but instead found '=>'.
Expected a function body here.
(parse)
[error] 204-204: expected , but instead found const
Remove const
(parse)
[error] 204-204: expected : but instead found newPagination
Remove newPagination
(parse)
[error] 205-205: expected , but instead found setPagination
Remove setPagination
(parse)
[error] 206-206: Expected a function body but instead found 'setOrdering'.
Expected a function body here.
(parse)
[error] 206-206: expected , but instead found [
Remove [
(parse)
[error] 207-207: Expected a function body but instead found 'setActiveOrderingKey'.
Expected a function body here.
(parse)
[error] 208-208: Expected a function body but instead found 'await'.
Expected a function body here.
(parse)
[error] 208-208: expected , but instead found refetch
Remove refetch
(parse)
[error] 211-211: expected , but instead found .
Remove .
(parse)
[error] 211-211: expected , but instead found )
Remove )
(parse)
[error] 211-211: Expected a function body but instead found ','.
Expected a function body here.
(parse)
[error] 212-212: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 267-267: Expected an expression but instead found '<'.
Expected an expression here.
(parse)
[error] 211-211: Do not shadow the global "Object" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
[error] 187-187: Shouldn't redeclare 'key'. Consider to delete it or rename it.
'key' is defined here:
(lint/suspicious/noRedeclare)
[error] 189-189: Shouldn't redeclare 'key'. Consider to delete it or rename it.
'key' is defined here:
(lint/suspicious/noRedeclare)
[error] 190-190: Shouldn't redeclare 'newPagination'. Consider to delete it or rename it.
'newPagination' is defined here:
(lint/suspicious/noRedeclare)
[error] 198-198: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
(lint/complexity/noUselessLoneBlockStatements)
[error] 225-237: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
(lint/complexity/noUselessLoneBlockStatements)
⏰ 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). (4)
- GitHub Check: Run pre-commit checks
- GitHub Check: Run frontend checks
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
8e0c025 to
a15b0b0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
frontend/src/app/projects/dashboard/metrics/page.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
frontend/src/app/projects/dashboard/metrics/page.tsx (4)
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1718
File: frontend/src/app/projects/dashboard/metrics/page.tsx:96-96
Timestamp: 2025-07-09T08:37:10.241Z
Learning: In the OWASP Nest project's MetricsPage component (frontend/src/app/projects/dashboard/metrics/page.tsx), the sorting dropdown intentionally uses selectionMode="multiple" to allow users to select multiple sorting criteria simultaneously. This enables complex sorting scenarios where users can sort by multiple fields in sequence.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1703
File: frontend/src/components/BarChart.tsx:33-46
Timestamp: 2025-07-03T03:08:03.290Z
Learning: In the OWASP Nest project's BarChart component (frontend/src/components/BarChart.tsx), the days and requirements arrays are guaranteed to always have the same length in their use cases, so input validation for array length matching is not needed.
🧬 Code Graph Analysis (1)
frontend/src/app/projects/dashboard/metrics/page.tsx (4)
frontend/src/types/healthMetrics.ts (3)
HealthMetricsProps(26-42)HealthMetricsFilter(44-52)HealthMetricsOrdering(54-67)frontend/src/server/queries/projectsHealthDashboardQueries.ts (1)
GET_PROJECT_HEALTH_METRICS_LIST(20-37)frontend/src/app/global-error.tsx (1)
handleAppError(66-86)frontend/src/types/DropDownSectionProps.ts (1)
DropDownSectionProps(1-7)
⏰ 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). (4)
- GitHub Check: Run CI Denendencies Scan
- GitHub Check: Run Code Scan
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (1)
frontend/src/app/projects/dashboard/metrics/page.tsx (1)
1-266: Excellent implementation of the metrics dashboard with comprehensive filtering and sorting!The component is well-structured and follows React best practices. I can see that most issues from previous reviews have been addressed:
✅ Pagination limit is now properly extracted to a constant
✅ Apollo Client's loading state is used instead of manual state management
✅ Race conditions in filter handling have been resolved
✅ Empty state handling is implemented
✅ Visual indication of active filters and sorting is working
✅ Ordering property keys have been correctedThe current implementation provides a robust dashboard with:
- Proper state management for metrics, filters, pagination, and ordering
- Clean separation of configuration objects for dropdowns
- Appropriate error handling using the global error handler
- Efficient pagination using Apollo's
fetchMore- Good user experience with loading states and empty state messages
The code is production-ready and handles all the key requirements for the metrics dashboard feature.
| const filtersMapping = { | ||
| incubator: { | ||
| level: 'incubator', | ||
| }, | ||
| lab: { | ||
| level: 'lab', | ||
| }, | ||
| production: { | ||
| level: 'production', | ||
| }, | ||
| flagship: { | ||
| level: 'flagship', | ||
| }, | ||
| healthy: { | ||
| score: { | ||
| gte: 75, | ||
| }, | ||
| }, | ||
| warning: { | ||
| score: { | ||
| gte: 50, | ||
| lt: 75, | ||
| }, | ||
| }, | ||
| critical: { | ||
| score: { | ||
| lt: 50, | ||
| }, | ||
| }, | ||
| reset: {}, | ||
| } | ||
| const orderingMapping = { | ||
| scoreDESC: { | ||
| scoreOrdering: { score: 'DESC' }, | ||
| }, | ||
| scoreASC: { | ||
| scoreOrdering: { score: 'ASC' }, | ||
| }, | ||
| starsCountDESC: { | ||
| starsCountOrdering: { starsCount: 'DESC' }, | ||
| }, | ||
| starsCountASC: { | ||
| starsCountOrdering: { starsCount: 'ASC' }, | ||
| }, | ||
| forksCountDESC: { | ||
| forksCountOrdering: { forksCount: 'DESC' }, | ||
| }, | ||
| forksCountASC: { | ||
| forksCountOrdering: { forksCount: 'ASC' }, | ||
| }, | ||
| contributorsCountDESC: { | ||
| contributorsCountOrdering: { contributorsCount: 'DESC' }, | ||
| }, | ||
| contributorsCountASC: { | ||
| contributorsCountOrdering: { contributorsCount: 'ASC' }, | ||
| }, | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider improving type safety for mapping objects.
While the current implementation works correctly, the filtersMapping and orderingMapping objects use string indexing which could be enhanced with proper TypeScript typing for better type safety and IDE support.
You could define types for the mapping keys:
type FilterKey = 'incubator' | 'lab' | 'production' | 'flagship' | 'healthy' | 'warning' | 'critical' | 'reset'
type OrderingKey = 'scoreDESC' | 'scoreASC' | 'starsCountDESC' | 'starsCountASC' | 'forksCountDESC' | 'forksCountASC' | 'contributorsCountDESC' | 'contributorsCountASC'
const filtersMapping: Record<FilterKey, HealthMetricsFilter> = {
// ... existing mapping
}
const orderingMapping: Record<OrderingKey, HealthMetricsOrdering> = {
// ... existing mapping
}🤖 Prompt for AI Agents
In frontend/src/app/projects/dashboard/metrics/page.tsx around lines 104 to 160,
the filtersMapping and orderingMapping objects currently use untyped string keys
which reduces type safety and IDE support. Define explicit TypeScript union
types for the keys of each mapping (e.g., FilterKey and OrderingKey) and then
type the mapping objects using Record with these key types and the appropriate
value types (HealthMetricsFilter and HealthMetricsOrdering). This will enforce
valid keys and improve code maintainability and autocompletion.
|
I was resolving conflicts, of course I would not dismiss your approval :) @kasya |
It seems you overthink it a bit, the workflow is fine and all understand whats and whys, don't sweat it @ahmedxgouda it's all good 👍 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/tests/apps/owasp/api/internal/ordering/project_health_metrics_test.py (1)
21-26: Consider testing both ASC and DESC ordering options.The current test only validates DESC ordering. Consider adding a parameterized test or additional test case to verify ASC ordering as well to ensure complete coverage of the ordering functionality.
- def test_order_by(self): - """Test ordering by score.""" - order_instance = ProjectHealthMetricsOrder( - score="DESC", - ) - assert order_instance.score == "DESC" + @pytest.mark.parametrize("order_direction", ["ASC", "DESC"]) + def test_order_by(self, order_direction): + """Test ordering by score in both directions.""" + order_instance = ProjectHealthMetricsOrder( + score=order_direction, + ) + assert order_instance.score == order_direction
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/apps/owasp/api/internal/nodes/project_health_metrics.py(2 hunks)backend/apps/owasp/api/internal/ordering/project_health_metrics.py(1 hunks)backend/apps/owasp/api/internal/queries/project_health_metrics.py(2 hunks)backend/tests/apps/owasp/api/internal/nodes/project_health_metrics_test.py(2 hunks)backend/tests/apps/owasp/api/internal/ordering/project_health_metrics_test.py(1 hunks)backend/tests/apps/owasp/api/internal/queries/project_health_metrics_test.py(2 hunks)
🧠 Learnings (2)
📓 Common learnings
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1718
File: frontend/src/app/projects/dashboard/metrics/page.tsx:96-96
Timestamp: 2025-07-09T08:37:10.241Z
Learning: In the OWASP Nest project's MetricsPage component (frontend/src/app/projects/dashboard/metrics/page.tsx), the sorting dropdown intentionally uses selectionMode="multiple" to allow users to select multiple sorting criteria simultaneously. This enables complex sorting scenarios where users can sort by multiple fields in sequence.
backend/apps/owasp/api/internal/nodes/project_health_metrics.py (3)
Learnt from: Rajgupta36
PR: #1717
File: backend/apps/mentorship/graphql/queries/module.py:39-50
Timestamp: 2025-07-11T15:57:56.648Z
Learning: In the OWASP Nest mentorship GraphQL queries, Strawberry GraphQL automatically converts between Django Module instances and ModuleNode types, so methods can return Module instances directly without manual conversion even when typed as ModuleNode.
Learnt from: Rajgupta36
PR: #1717
File: backend/apps/mentorship/graphql/mutations/program.py:166-166
Timestamp: 2025-07-13T05:55:46.436Z
Learning: In the OWASP Nest mentorship GraphQL mutations, Strawberry GraphQL automatically converts between Django Program instances and ProgramNode types, so mutations can return Program instances directly without manual conversion even when typed as ProgramNode, similar to the Module/ModuleNode pattern.
Learnt from: ahmedxgouda
PR: #1676
File: backend/apps/owasp/graphql/filters/project_health_metrics.py:17-22
Timestamp: 2025-06-29T00:41:32.198Z
Learning: In the OWASP Nest codebase, when implementing GraphQL filters that convert string values to enums (like ProjectLevel), do not catch ValueError exceptions for invalid values. Let the errors propagate to provide proper error responses to GraphQL clients rather than silently ignoring invalid input.
🧬 Code Graph Analysis (2)
backend/tests/apps/owasp/api/internal/queries/project_health_metrics_test.py (2)
backend/apps/owasp/api/internal/queries/project_health_metrics.py (4)
ProjectHealthMetricsQuery(14-70)project_health_stats(43-50)project_health_metrics(23-40)project_health_metrics_distinct_length(53-70)backend/apps/owasp/api/internal/nodes/project_health_metrics.py (1)
ProjectHealthMetricsNode(31-97)
backend/tests/apps/owasp/api/internal/ordering/project_health_metrics_test.py (1)
backend/apps/owasp/api/internal/queries/project_health_metrics.py (1)
project_health_metrics(23-40)
✅ Files skipped from review due to trivial changes (1)
- backend/apps/owasp/api/internal/ordering/project_health_metrics.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Learnt from: ahmedxgouda
PR: OWASP/Nest#1718
File: frontend/src/app/projects/dashboard/metrics/page.tsx:96-96
Timestamp: 2025-07-09T08:37:10.241Z
Learning: In the OWASP Nest project's MetricsPage component (frontend/src/app/projects/dashboard/metrics/page.tsx), the sorting dropdown intentionally uses selectionMode="multiple" to allow users to select multiple sorting criteria simultaneously. This enables complex sorting scenarios where users can sort by multiple fields in sequence.
backend/apps/owasp/api/internal/nodes/project_health_metrics.py (3)
Learnt from: Rajgupta36
PR: #1717
File: backend/apps/mentorship/graphql/queries/module.py:39-50
Timestamp: 2025-07-11T15:57:56.648Z
Learning: In the OWASP Nest mentorship GraphQL queries, Strawberry GraphQL automatically converts between Django Module instances and ModuleNode types, so methods can return Module instances directly without manual conversion even when typed as ModuleNode.
Learnt from: Rajgupta36
PR: #1717
File: backend/apps/mentorship/graphql/mutations/program.py:166-166
Timestamp: 2025-07-13T05:55:46.436Z
Learning: In the OWASP Nest mentorship GraphQL mutations, Strawberry GraphQL automatically converts between Django Program instances and ProgramNode types, so mutations can return Program instances directly without manual conversion even when typed as ProgramNode, similar to the Module/ModuleNode pattern.
Learnt from: ahmedxgouda
PR: #1676
File: backend/apps/owasp/graphql/filters/project_health_metrics.py:17-22
Timestamp: 2025-06-29T00:41:32.198Z
Learning: In the OWASP Nest codebase, when implementing GraphQL filters that convert string values to enums (like ProjectLevel), do not catch ValueError exceptions for invalid values. Let the errors propagate to provide proper error responses to GraphQL clients rather than silently ignoring invalid input.
🧬 Code Graph Analysis (2)
backend/tests/apps/owasp/api/internal/queries/project_health_metrics_test.py (2)
backend/apps/owasp/api/internal/queries/project_health_metrics.py (4)
ProjectHealthMetricsQuery(14-70)project_health_stats(43-50)project_health_metrics(23-40)project_health_metrics_distinct_length(53-70)backend/apps/owasp/api/internal/nodes/project_health_metrics.py (1)
ProjectHealthMetricsNode(31-97)
backend/tests/apps/owasp/api/internal/ordering/project_health_metrics_test.py (1)
backend/apps/owasp/api/internal/queries/project_health_metrics.py (1)
project_health_metrics(23-40)
⏰ 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). (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (7)
backend/tests/apps/owasp/api/internal/nodes/project_health_metrics_test.py (1)
33-33: LGTM! Test coverage updated correctly for the new field.The addition of
"project_key"to both the expected field names validation and the parameterized type test ensures proper coverage of the new field with the correct expected type (str).Also applies to: 72-72
backend/apps/owasp/api/internal/nodes/project_health_metrics.py (1)
31-31: LGTM! Proper relay Node inheritance.The change to inherit from
strawberry.relay.Nodeis the correct approach for GraphQL relay-compliant nodes, replacing the previouspagination=Truedecorator argument.backend/tests/apps/owasp/api/internal/queries/project_health_metrics_test.py (3)
67-67: LGTM! Correct instantiation update.The removal of the empty list parameter in the
ProjectHealthMetricsQuery()instantiation aligns with the updated constructor signature.
72-102: LGTM! Comprehensive test coverage for the updated method.The test properly mocks the
get_latest_health_metricsmethod and validates the new method signature with explicitfilters=None,pagination=None, andordering=Noneparameters. The assertions correctly verify both the return type and content.
103-112: No changes needed for get_latest_health_metrics mock
get_latest_health_metrics()returns a Django QuerySet with a.count()method, somock_get_latest_metrics.return_value.count.return_value = 42already correctly stubs the count call. You can ignore the proposed diff.
Likely an incorrect or invalid review comment.
backend/apps/owasp/api/internal/queries/project_health_metrics.py (2)
52-70: LGTM! Proper implementation of distinct length with filter support.The
project_health_metrics_distinct_lengthmethod correctly applies filters when provided and returns the count. The implementation follows the expected pattern and properly handles the optional filters parameter.
17-40: No manual filter/ordering logic needed—strawberry_django handles it automatically.The
@strawberry_django.fielddecorator withfilters=,pagination=, andordering=will automatically apply those parameters to theQuerySetreturned by the resolver. SinceProjectHealthMetrics.get_latest_health_metrics()returns aQuerySet, you don’t need to callstrawberry_django.filters.apply()or manually handle ordering/pagination in your method.No changes required here.
|
arkid15r
left a comment
arkid15r
left a comment
There was a problem hiding this comment.
LGTM,
Thank you @ahmedxgouda
Resolves #1701
Screencast.from.2025-07-13.18-11-14.mp4