refactor!: move compliance overview to a dedicated recap page #1444
Conversation
WalkthroughThis pull request introduces a new "Recap" feature for compliance assessments. The changes involve adding a new navigation item, creating a dedicated recap page, and modifying existing analytics pages. The implementation includes updating translation files, adding a new server-side load function, and creating a Svelte component to display project compliance information. The changes represent a significant restructuring of how compliance assessment data is presented to users. Changes
Sequence DiagramsequenceDiagram
participant User
participant NavBar
participant RecapPage
participant API
User->>NavBar: Click "Recap" navigation item
NavBar->>RecapPage: Navigate to /recap
RecapPage->>API: Fetch project compliance data
API-->>RecapPage: Return project analytics
RecapPage->>RecapPage: Process and render compliance data
RecapPage->>User: Display project compliance overview
Possibly related PRs
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
frontend/src/routes/(app)/(internal)/recap/+page.server.ts (5)
6-13: Ensure consistent naming and documentation for status constants.This array of requirement statuses is clear, but consider adding a brief comment indicating its usage or referencing it in other modules for better maintainability.
22-25: Clarify or add a JSDoc for RequirementAssessmentDonutItem interface.To enhance readability and self-documentation, consider adding a short JSDoc comment describing the interface properties and how they're used.
34-95: Check HTTP response status before parsing JSON.Currently, the code directly proceeds to use
res.json()without confirmingres.ok. Consider adding a check forres.okto handle potential 4xx or 5xx errors more explicitly. This will improve reliability and clarity of error handling.const projects: ProjectAnalytics[] = await fetch(`${BASE_API_URL}/projects/`) - .then((res) => res.json()) + .then(async (res) => { + if (!res.ok) { + throw new Error(`Failed to fetch projects: ${res.status} ${res.statusText}`); + } + return res.json(); + }) .then(async (projects) => { ...
130-133: Standardize the displayed percentage format.Currently, percentages display as a string with a single decimal or 0. Consider always showing a single decimal place (e.g., "0.0") for consistency.
- percentage: totalValue > 0 ? ((item.value / totalValue) * 100).toFixed(1) : '0' + percentage: totalValue > 0 ? ((item.value / totalValue) * 100).toFixed(1) : '0.0'
140-145: Consider returning additional metadata.If further analytics or metrics may be needed downstream (e.g., per-framework compliance levels), extend the return object to include more fields. This can help avoid repeated fetches.
frontend/src/routes/(app)/(internal)/recap/+page.svelte (3)
11-18: Reuse the requirement statuses across the client and server.The
REQUIREMENT_ASSESSMENT_STATUSarray is defined in both client and server code. Consider centralizing it for consistency across layers if possible.
66-81: Handle potential negative or undefined scores gracefully.While the condition checks
>= 0, any future code changes or data anomalies could lead to unexpected values. Consider how to handle negative or null scores more explicitly.
88-108: Consider a dedicated component for compliance assessment cards.This section includes repeated logic for displaying compliance data, edit/export links, etc. Extracting it into a smaller component can improve maintainability and readability.
frontend/src/routes/(app)/(internal)/analytics/+page.svelte (1)
507-510: Enhance accessibility and clean up the implementation.The link to the recap section could be improved:
- Remove the empty div as it serves no purpose
- Add aria-label for better accessibility
- Use CSS variables for hover color to maintain consistency
- <span class="text-xl font-extrabold" - ><a href="/recap" class="hover:text-purple-500">{m.sectionMoved()}</a></span - > - <div class="flex flex-col space-y-2"></div> + <span class="text-xl font-extrabold"> + <a + href="/recap" + class="hover:text-primary-500" + aria-label={m.sectionMoved()} + > + {m.sectionMoved()} + </a> + </span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
frontend/messages/en.json(1 hunks)frontend/messages/fr.json(1 hunks)frontend/src/lib/components/SideBar/navData.ts(1 hunks)frontend/src/routes/(app)/(internal)/analytics/+page.server.ts(0 hunks)frontend/src/routes/(app)/(internal)/analytics/+page.svelte(2 hunks)frontend/src/routes/(app)/(internal)/recap/+layout.svelte(1 hunks)frontend/src/routes/(app)/(internal)/recap/+page.server.ts(1 hunks)frontend/src/routes/(app)/(internal)/recap/+page.svelte(1 hunks)
💤 Files with no reviewable changes (1)
- frontend/src/routes/(app)/(internal)/analytics/+page.server.ts
✅ Files skipped from review due to trivial changes (3)
- frontend/src/routes/(app)/(internal)/recap/+layout.svelte
- frontend/messages/fr.json
- frontend/messages/en.json
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: migrations-check (3.12)
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
frontend/src/routes/(app)/(internal)/recap/+page.server.ts (1)
96-137: Guard against missing or undefined compliance_assessments.While the fetch logic initializes
project.compliance_assessments, there’s a remote risk that the property could be missing or corrupted. If you intend to handle partial or empty project data gracefully, you may want to verify existence before iterating.frontend/src/routes/(app)/(internal)/recap/+page.svelte (1)
37-45: Double-check sorting logic for statuses.Sorting by array index is fine, but ensure that the order of statuses in the UI aligns with user expectations or business logic (e.g., grouping “compliant” vs. “non_compliant” side by side).
frontend/src/lib/components/SideBar/navData.ts (1)
198-203: Ensure permission alignment with your compliance recap.You’ve added
'view_complianceassessment'as the required permission for “recap.” Verify that this permission is the correct scope for reading or summarizing compliance data. Consider introducing a more specific permission if you plan to restrict certain recap details.frontend/src/routes/(app)/(internal)/analytics/+page.svelte (1)
15-15: LGTM! Import changes align with the compliance overview move.The addition of
tableSourceMapperand removal of unused imports keeps the code clean.
ab-smith
changed the title
Summary by CodeRabbit
Release Notes
New Features
User Interface Changes
Localization
Performance