feat(policies): Enhance policies dashboard with comprehensive overvie…#84
feat(policies): Enhance policies dashboard with comprehensive overvie…#84carhartlewis merged 4 commits intomainfrom
Conversation
…w and navigation - Add new policies dashboard with status and assignee charts - Implement dynamic routing for policies with improved navigation - Create reusable chart components using D3.js for data visualization - Update localization and secondary menu for better user experience - Refactor policies module structure with new routes and components - Add server-side caching for policies data - Improve type safety and error handling in policy-related actions
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update introduces new data visualization libraries and adjusts navigation across policies and risk dashboards. The policies section now features an expanded menu and revised routing with caching for policy overviews, while risk pages display updated components with refined layouts. Chart components, table links, and localized strings have been enhanced. Additional UI components and Tailwind configurations now support new tooltip and back-button functionality, with updated color schemes defined in CSS and package exports. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant Server
participant Database
User->>Client: Request Policies Overview page
Client->>Server: Send request with locale & session info
Server->>Database: Execute getPoliciesOverview (with caching)
Database-->>Server: Return policy counts/status
alt No organizationId
Server-->>Client: Redirect to onboarding/root
else
alt Total policies = 0
Server-->>Client: Redirect to "/policies/all"
else
Server-->>Client: Render PoliciesOverview layout with status & assignee charts
end
end
sequenceDiagram
participant User
participant Client
participant Server
participant DataAPI
User->>Client: Request Risk Detail page
Client->>Server: Request risk data
Server->>DataAPI: Query risk info & user risk stats (using RisksAssignee)
DataAPI-->>Server: Return risk data
Server-->>Client: Render risk page with updated back button (/risk/register)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 3
🧹 Nitpick comments (16)
apps/app/src/app/[locale]/(app)/(dashboard)/risk/[riskId]/layout.tsx (1)
59-59: Added trailing comma for consistent code formatting.Minor formatting improvement with the added trailing comma in the array parameter.
apps/app/src/app/[locale]/(app)/(dashboard)/risk/(overview)/page.tsx (1)
3-4: Remove the unused import.The
RisksByAssigneeimport on line 4 is no longer used since it has been replaced by the newRisksAssigneecomponent.import { RiskOverview } from "@/components/risks/charts/risk-overview"; import { RisksAssignee } from "@/components/risks/charts/risks-assignee"; -import { RisksByAssignee } from "@/components/risks/charts/risks-by-assignee"; import { getI18n } from "@/locales/server";apps/app/src/components/policies/charts/policies-status.tsx (1)
52-53: Verify color variable usage for archived policies.The "archived" status is using the "pending" color variable (
--chart-pending), which might be confusing. Consider using the dedicated--chart-archivedvariable instead for consistency.name: t("policies.status.archived"), value: statusCounts.archived, - color: "var(--chart-pending)", - colorClass: "bg-[var(--chart-pending)]", + color: "var(--chart-archived)", + colorClass: "bg-[var(--chart-archived)]",apps/app/src/components/risks/charts/risks-by-department.tsx (1)
6-6: Consider making department list more dynamic or configuration-driven.While the ALL_DEPARTMENTS constant works for the current use case, hardcoding department names might make future additions or changes more difficult to maintain.
Consider one of these approaches:
- Move this to a central configuration file
- Make it dynamic based on API data
- Add a comment explaining why these specific departments are hardcoded
-const ALL_DEPARTMENTS = ["none", "admin", "gov", "hr", "it", "itsm", "qms"]; +// Core departments supported by the application +const ALL_DEPARTMENTS = ["none", "admin", "gov", "hr", "it", "itsm", "qms"];packages/ui/src/components/chart-tooltip.tsx (1)
45-114: Comprehensive event handling for tooltip triggers.The TooltipTrigger component properly handles different pointer events with appropriate differentiation between mouse and touch events. The ref forwarding implementation is robust.
Consider making the auto-dismiss timeout configurable or providing a visual indication that the tooltip will auto-dismiss:
- setTimeout(() => { - context.setTooltip(undefined); - }, 2000); + // Auto-dismiss tooltip after 2 seconds on touch devices + const autoHideTimeout = setTimeout(() => { + context.setTooltip(undefined); + }, 2000); + + return () => { + clearTimeout(autoHideTimeout); + };apps/app/src/components/risks/charts/status-chart.tsx (1)
51-60: Type assertion may be unnecessary.The type assertion on line 53 might be redundant if the config's label is already typed as a string.
Consider reviewing if the type assertion is necessary:
- name: config[key as keyof typeof config].label as string, + name: config[key as keyof typeof config].label,apps/app/src/components/risks/charts/donut-chart.tsx (3)
23-26: EnsureshowLabelsprop is used or remove it.Although
showLabelsis declared in the props, it does not appear to be used within the component. If you plan to toggle label rendering, implement the necessary conditional checks. Otherwise, remove the unused prop to keep the component clean.
76-91: Improve uniqueness of gradient and clip path IDs.The values for
clipIdandgradientIdonly use the slice index. If multiple charts are rendered on the same page or if you have slices with identical indexes, there could be ID collisions. Incorporate a more unique string (e.g., chart ID or a random UUID) to avoid conflicts.
98-104: Revisit centroid offset logic.Manually adjusting the centroid by adding or subtracting 10 might not reliably position labels for various slice sizes. Consider using a more robust label collision-avoidance technique or rely on automatic label placement solutions from D3 to ensure consistent label positioning across different data sets.
apps/app/src/components/risks/charts/risks-assignee.tsx (3)
23-28: Ensure color consistency across UI.Currently, the
riskStatusColorsobject (lines 23–28) uses color codes that differ from the ones used in the legend (lines 69–93). Consider consolidating or reusing color definitions to keep your UI consistent and avoid confusion.
48-49: Validate re-sorting approach for large data sets.Sorting in place with
stats.sort((...) => ...)works fine, but for larger data sets, you might consider returning a new sorted array to avoid possibly mutating shared data. This is a minor design consideration but can improve predictability in certain scenarios.
103-210: Examine bar chart calculations when including the gap.When the gap is added to
totalWidth, each bar width is computed as(d.value / totalWidth) * 100. For multiple statuses, small rounding errors may produce a final set of bars that don’t sum neatly to 100%. Consider adjusting or rounding the final bar to fill any remainder, or reduce the gap from the total width.apps/app/src/app/[locale]/(app)/(dashboard)/policies/(overview)/page.tsx (1)
45-88: Evaluate caching strategy inunstable_cache.While it improves performance by caching DB calls, any new policy or status change may not reflect instantly. Ensure you have a suitable revalidation or eviction policy for your use case so you do not serve stale data to users.
apps/app/src/components/risks/charts/department-chart.tsx (1)
8-16: Nitpick: Ensure these dynamic Tailwind classes are safelisted or handled properly.
Becausebg-[${COLORS.xyz}]can be purged by Tailwind if not explicitly safelisted, confirm your build pipeline accounts for dynamic classes. Otherwise, these classes might not be included in production builds.apps/app/src/components/policies/charts/policies-by-assignee.tsx (2)
22-26: Inline color classes could be centralized.
Though this works, consider moving these color definitions to a shared constant or Tailwind config for consistency and reuse.
132-196: Bar chart logic is effective but slightly imperative.
The incrementalcumulativeWidthapproach works, though it might be refactored using a reduce or callback chain for clarity. Nonetheless, it’s perfectly valid as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
apps/app/languine.lockis excluded by!**/*.lockapps/web/public/og.pngis excluded by!**/*.pngbun.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (32)
apps/app/package.json(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/(overview)/layout.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/(overview)/page.tsx(2 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/hooks/usePolicy.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/layout.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/page.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/[policyId]/layout.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/risk/(overview)/page.tsx(2 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/risk/[riskId]/layout.tsx(2 hunks)apps/app/src/components/policies/charts/policies-by-assignee.tsx(3 hunks)apps/app/src/components/policies/charts/policies-status.tsx(1 hunks)apps/app/src/components/risks/charts/department-chart.tsx(1 hunks)apps/app/src/components/risks/charts/donut-chart.tsx(1 hunks)apps/app/src/components/risks/charts/risks-assignee.tsx(1 hunks)apps/app/src/components/risks/charts/risks-by-department.tsx(2 hunks)apps/app/src/components/risks/charts/status-chart.tsx(4 hunks)apps/app/src/components/tables/policies/columns.tsx(1 hunks)apps/app/src/components/tables/policies/data-table.tsx(0 hunks)apps/app/src/components/tables/risk-register/columns.tsx(1 hunks)apps/app/src/components/tables/risk-register/data-table.tsx(0 hunks)apps/app/src/constants/colors.ts(1 hunks)apps/app/src/locales/en.ts(1 hunks)apps/app/src/locales/es.ts(1 hunks)apps/app/src/locales/fr.ts(1 hunks)apps/app/src/locales/no.ts(1 hunks)apps/app/src/locales/pt.ts(1 hunks)apps/web/src/app/components/cta.tsx(0 hunks)packages/ui/package.json(1 hunks)packages/ui/src/components/chart-tooltip.tsx(1 hunks)packages/ui/src/components/secondary-menu.tsx(3 hunks)packages/ui/src/globals.css(3 hunks)packages/ui/tailwind.config.ts(1 hunks)
💤 Files with no reviewable changes (3)
- apps/app/src/components/tables/policies/data-table.tsx
- apps/app/src/components/tables/risk-register/data-table.tsx
- apps/web/src/app/components/cta.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ui/src/globals.css
[error] 78-78: Expected a color but instead found '#'.
Expected a color here.
Ensure the color is specified in a valid hexadecimal format. Examples: #000, #000f, #ffffff, #ffffffff
(parse)
[error] 79-79: Expected a color but instead found '#'.
Expected a color here.
Ensure the color is specified in a valid hexadecimal format. Examples: #000, #000f, #ffffff, #ffffffff
(parse)
🔇 Additional comments (66)
apps/app/src/components/tables/risk-register/columns.tsx (1)
82-82: Badge variant updated to "marketing"The badge variant has been changed to "marketing" for department display, which provides a clearer visual distinction. This aligns with the overall enhancement of the policies dashboard mentioned in the PR objectives.
Could you confirm that this visual change is consistent with the design system and matches the intended appearance for department badges?
apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/hooks/usePolicy.ts (1)
6-6: Removed unused importThe
UpdatePolicyInputtype has been removed from imports, which is good as it's not being used in this file. TheupdatePolicyContentfunction usesPartial<PolicyDetails>instead.apps/app/package.json (1)
35-35:Details
❓ Verification inconclusive
Added D3.js dependencies for data visualization
The addition of D3.js and its type definitions is appropriate for implementing the charts mentioned in the PR objectives. These dependencies will support the new policies dashboard with status and assignee charts.
Also applies to: 42-42
🏁 Script executed:
#!/bin/bash # Description: Verify D3.js version compatibility and check for any security advisories # Check if the version is the latest available npm view d3 version # Check for known security vulnerabilities npm audit --package=d3@7.9.0Length of output: 407
Attention: Verify D3.js Dependency Update and Security Audit Process
The addition of D3.js along with its type definitions (as seen on lines 35 and 42 of
apps/app/package.json) is appropriate for supporting the new policies dashboard charts. The reported D3 version ("7.9.0") aligns with our needs. However, the vulnerability audit command failed because no lockfile exists. Please ensure that either a lockfile is generated (for example, usingnpm i --package-lock-only) before runningnpm auditor perform a manual security verification of the dependency.apps/app/src/components/tables/policies/columns.tsx (1)
35-35:Details
❓ Verification inconclusive
Updated URL structure for policy links
The link route has been updated to include the "all" segment, which aligns with the dynamic routing updates mentioned in the PR objectives. This change supports the refactored structure of the policies module.
🏁 Script executed:
#!/bin/bash # Description: Check for route definitions that match the new policy URL pattern # Search for route definitions matching the new policies/all pattern rg -p "policies\/all" --glob="*.{tsx,ts}" -g "!*columns.tsx" # Check if there are any lingering old path references that might need updating rg -p "policies\/\$\{id\}" --glob="*.{tsx,ts}" -g "!*columns.tsx"Length of output: 309
Updated URL structure for policy links
The
<Link>component inapps/app/src/components/tables/policies/columns.tsxnow uses the updated route/policies/all/${id}. This change aligns with the new dynamic routing approach in the policies module. However, the initial regex commands used for cross-file verification encountered escaping errors, so please verify manually that:
- The new URL structure (
policies/all) is consistently used across the repository.- No lingering references to an outdated pattern (such as misconstructed paths involving
${id}without the "all" segment) remain.For your convenience, here’s an updated verification script with corrected regex escaping:
#!/bin/bash # Verify occurrences of the updated policy URL structure, excluding columns.tsx echo "Verifying updated URLs (policies/all) in the codebase:" rg 'policies/all' --glob "*.{tsx,ts}" -g "!*columns.tsx" # Verify absence of outdated references involving dynamic policy IDs outside the updated pattern echo "Verifying absence of old dynamic route references (e.g., policies/${id}) in the codebase:" rg 'policies/\${id}' --glob "*.{tsx,ts}" -g "!*columns.tsx"Please run these commands and confirm that no outdated routes remain.
apps/app/src/app/[locale]/(app)/(dashboard)/policies/(overview)/layout.tsx (1)
13-17: Layout now supports navigation to two policy pages.The SecondaryMenu has been enhanced with a more comprehensive navigation structure, supporting both a dashboard overview and an "all policies" view. This implementation aligns with the PR objective of improving the policies dashboard with a comprehensive overview and enhancing navigation within the policies section.
apps/app/src/locales/pt.ts (1)
251-255: Localization enhanced with policy subpages.The addition of the
sub_pagesobject with translations for "overview" and "edit_policy" properly extends the localization structure to support the new navigation paths in the policy section. This change is consistent with the PR objective of updating localization to improve the user experience.apps/app/src/constants/colors.ts (1)
1-6: Standardized color scheme for broader application use.The export has been renamed from (likely)
RISK_COLORStoCOLORSand the color values have been updated, indicating a more generalized use of these colors across the application. This change supports the implementation of the reusable chart components mentioned in the PR objectives.packages/ui/package.json (1)
93-94: Added chart-tooltip component export.The addition of the chart-tooltip component to the exports is well-aligned with the PR objective of creating reusable chart components for data visualization. This will allow the component to be easily imported and used across the application.
apps/app/src/locales/en.ts (1)
282-285: Localization structure properly implemented for policy sub-pages.The addition of the
sub_pagesobject with translations for "Overview" and "Edit Policy" enhances the internationalization support for the new policies dashboard navigation structure.packages/ui/tailwind.config.ts (1)
49-54: Chart colors well-defined for consistent visualization.The new chart color palette is properly structured with status-specific colors (closed, pending, open, archived) using HSL values from CSS variables. This will ensure consistent styling across the status charts in the policies dashboard.
apps/app/src/app/[locale]/(app)/(dashboard)/risk/[riskId]/layout.tsx (1)
32-34: Back navigation improvement with explicit href definition.Replacing the generic
isChildprop with the more specificshowBackButtonandbackButtonHrefprops improves the navigation experience and makes the component usage more intuitive.apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/layout.tsx (1)
1-23: New layout implementation follows best practices.The layout component is well-structured, providing a consistent container with navigation through the SecondaryMenu component. The implementation correctly:
- Uses async/await pattern with getI18n for localization
- Implements a clean component structure with proper typing
- Provides navigation between dashboard and all policies views
This aligns well with the PR's objective to enhance navigation within the policies section.
apps/app/src/locales/fr.ts (1)
251-255: The translation keys and structure look well-organized.The addition of the
sub_pagesobject with "Aperçu" and "Modifier la politique" translations provides proper localization for the new policy navigation structure. The trailing comma on line 251 was appropriately added to maintain clean syntax.apps/app/src/locales/es.ts (1)
251-255: Consistent translations across language files.The Spanish translations for the new policy sub-pages are correctly implemented and match the structure of the French translation file, maintaining consistency across the application's localization system.
apps/app/src/locales/no.ts (1)
251-255: Translation structure maintained consistently.The Norwegian translations for the sub-pages properly follow the same pattern as the other language files, ensuring a uniform localization experience. The structure and naming are consistent with the application's localization standards.
apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/[policyId]/layout.tsx (3)
35-36: Back navigation enhances user experience.The addition of the back button functionality provides users with an intuitive way to navigate back to the policy list. This is a good UX improvement.
39-45: Improved menu structure with localized labels.The updated menu structure with absolute paths and localized labels enhances navigation consistency and maintainability. Using translation functions instead of hardcoded text improves the internationalization of the application.
The paths are now constructed using template literals with the policy ID, which provides clear, predictable routing throughout the policies section.
32-48: Verify that Title component removal doesn't affect accessibility.While the SecondaryMenu improvements look good, I notice that the Title component was removed. Ensure that the page still has an appropriate heading structure for accessibility and user orientation within the policies section.
Make sure there's a heading element elsewhere in the component tree that identifies the current policy for users, particularly for screen readers.
apps/app/src/app/[locale]/(app)/(dashboard)/risk/(overview)/page.tsx (1)
42-42: LGTM! Component replacement looks good.The new
RisksAssigneecomponent replaces the previousRisksByAssigneecomponent while maintaining the same prop structure.packages/ui/src/globals.css (2)
37-40: LGTM! New color variables for chart status.These new CSS variables provide consistent colors for different policy and risk statuses across the application.
92-95: LGTM! Consistent chart colors in dark mode.The chart status colors are consistently defined in dark mode, ensuring visual coherence across themes.
packages/ui/src/components/secondary-menu.tsx (3)
22-24: LGTM! Well-defined props for back button functionality.The new optional properties enhance the component's flexibility by allowing for back navigation.
27-33: LGTM! Clean implementation with sensible defaults.The updated component signature includes the new props with appropriate default values for the back button.
60-70: LGTM! Well-implemented back button with clear visual cues.The conditional rendering for the back button is clean and provides a good user experience with the arrow icon and label.
apps/app/src/components/policies/charts/policies-status.tsx (3)
15-20: LGTM! Clear and well-defined props interface.The Props interface clearly defines all the required data for displaying policy statuses.
30-54: LGTM! Well-structured data mapping for the chart.Good organization of status counts and mapping to chart data with appropriate colors and internationalization.
57-75: LGTM! Clean component rendering with good layout structure.The component uses appropriate card structure and layout for displaying the pie chart and status indicators.
apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/page.tsx (3)
1-7: Good module organization with clear dependencies.The imports are well-structured, separating authentication, component, internationalization, and Next.js-specific imports.
9-27: Well-implemented authentication and redirection flow.The function properly handles authentication, checks for the required organization ID, and implements appropriate redirection. The column headers are fetched server-side before rendering the component, which is good practice for performance.
29-42: Appropriate metadata generation with internationalization support.The metadata function is well-implemented with proper locale handling and internationalization. This ensures correct page titles across different languages.
apps/app/src/components/risks/charts/risks-by-department.tsx (3)
15-25: Improved data handling with comprehensive department coverage.The new approach ensures all departments are represented in the chart, even if they have no associated risks. The sorting ensures departments with higher values appear first, improving data visualization.
27-36: Enhanced UI with consistent card structure.Wrapping the chart in a Card component with proper header and content structure improves UI consistency across the application.
39-51:Details
❓ Verification inconclusive
Effective use of caching for performance optimization.
Using
unstable_cacheis appropriate for this data fetching operation and will improve performance. The cache key and tags are well-defined for proper cache invalidation.
🌐 Web query:
Is Next.js unstable_cache still experimental in the latest version?💡 Result:
As of Next.js 15 (latest version),
unstable_cacheremains an experimental API, though it's now considered a legacy approach being replaced by the newuse cachedirective[1][2][4]. Here's the current state:
- Status of unstable_cache
- Still marked as "unstable" in official documentation[1]
- Considered a legacy API in v15[4]
- Will eventually be replaced by
use cacheonce stabilized[1]
- The replacement: use cache
- New experimental directive introduced in v15[2][4]
- Handles more data types (React components, JSX) vs JSON-only in unstable_cache[2]
- Requires canary channel and experimental flags:
// next.config.ts experimental: { useCache: true // or dynamicIO: true }- Offers better integration with React's rendering model[2][10]
Key differences
| Feature | unstable_cache | use cache |
|-----------------------|----------------|--------------------|
| Data types | JSON only | Any serializable |
| Component caching | No | Yes |
| Cache key management | Manual | Automatic |
| React integration | Limited | Deep |Recommendation
- For new projects: Consider experimenting with
use cachein canary builds[4][9]- For production: Stick with
unstable_cachetemporarily but prepare for migration[1][7]- Both APIs remain experimental as of v15.0.x[3][8]
The documentation explicitly states: "This API will be replaced by
use cachewhen it reaches stability"[1]. However, neither caching solution is fully stable yet - they're both part of Next.js' ongoing caching improvements[4][10].Citations:
- 1: https://nextjs.org/docs/app/api-reference/functions/unstable_cache
- 2: https://nextjs.org/docs/app/api-reference/directives/use-cache
- 3: vercel/next.js#72101
- 4: https://nextjs.org/blog/our-journey-with-caching
- 5: https://nextjs.org/docs/app/building-your-application/data-fetching/fetching
- 6: https://buymeacoffee.com/codev_/implementing-caching-next-js-unstable-cache
- 7: https://blog.logrocket.com/caching-next-js-unstable-cache/
- 8: vercel/next.js#60336
- 9: https://www.youtube.com/watch?v=jK8NLWZIUfA
- 10: https://www.nextjscourse.dev/courses/nextjs-course/use-cache-deep-dive
Attention: Verify Experimental API Usage in Caching Implementation
The current implementation of caching withunstable_cacheis effective for optimizing data fetching. However, as of Next.js 15,unstable_cacheremains marked as experimental and is considered legacy—its eventual replacement being theuse cachedirective. The cache key and tags are well-defined for invalidation, but please verify that relying on this experimental API aligns with your long-term maintenance strategy. If you plan to scale or update the project further, consider evaluating the migration path touse cacheto ensure stability in production.
- File:
apps/app/src/components/risks/charts/risks-by-department.tsx- Lines: 39-51
packages/ui/src/components/chart-tooltip.tsx (3)
1-26: Well-structured tooltip context implementation with proper error handling.The tooltip context and hook implementation follow best practices with clear error messages when used incorrectly.
115-178: Responsive tooltip positioning with viewport boundary awareness.The tooltip content component intelligently positions itself based on available viewport space and adapts to mobile devices. Using createPortal to render the tooltip at the document level is the correct approach.
183-183: Consistent component naming for client components.Exporting the Tooltip as ClientTooltip clearly indicates it's a client-side component, following good naming conventions.
apps/app/src/components/risks/charts/status-chart.tsx (3)
5-7: Updated imports for new chart component architecture.The imports have been properly updated to use the new DonutChart component, improving modularity.
19-36: Improved color configuration using CSS variables.Using CSS variables for chart colors is a better approach for theming and consistency across the application.
74-84: Simplified chart rendering with the new DonutChart component.The rendering has been simplified by using the custom DonutChart component, which improves code readability and maintainability.
apps/app/src/components/risks/charts/donut-chart.tsx (1)
127-131: Confirm whether values are truly percentages.Appending the “%” character might be misleading if the data is not normalized as a percentage. If you want actual values displayed, remove the “%” suffix or ensure the data is normalized beforehand.
apps/app/src/app/[locale]/(app)/(dashboard)/policies/(overview)/page.tsx (3)
21-22: Confirm user session checks.You are only checking
session?.user?.organizationId. Ifsession.useris undefined or partial, the code still attempts to accessorganizationId. Ensure the session object is always defined or handle the scenario in which session data might be missing or malformed.
25-29: Anticipate undefined or partial overview.
getPoliciesOverviewmight returnnullor undefined if something fails in the DB call. Ensure thatoverviewhas the structure you expect before relying on its properties. Additionally, confirm that redirecting to/policies/allaligns with your UX requirements.
31-42: Kudos for simplifying the layout.Splitting the logic into
PoliciesStatusandPoliciesByAssigneecomponents makes it cleaner to maintain and reason about. This modular approach is good for testing and readability.apps/app/src/components/risks/charts/department-chart.tsx (15)
3-5: Imports look good.
No issues with these imports; using named imports from D3 is a suitable approach for custom chart rendering.
18-21: Interface definition approved.
Defining a simple interface for chart data is straightforward and clear.
24-24: Prop update recognized.
Switching thedataprop toDepartmentData[]is consistent with the new interface.
28-28: Sorting ensures appropriate bar ordering.
This descending sort is correct for displaying highest values first.
30-35: Band scale usage looks appropriate.
UsingscaleBandfor the y-axis with a small padding is a good choice for bar spacing.
36-38: Linear scale for x-axis is valid.
Ensuring the domain starts at 0 and goes up to the max value is a standard pattern for horizontal bar charts.
40-43: Margins are clearly defined.
No concerns here; custom properties for margin are a tidy approach.
44-46: Utility key functions are straightforward.
These helper functions reduce redundancy and improve clarity.
48-54: Fallback color is a good safeguard.
If a department name doesn't match one of the predefined colors, defaulting to gray is a reasonable fallback.
57-68: Container layout looks fine.
Leveraging relative positioning and custom properties for margins is consistent and should ensure layout flexibility.
69-95: Bar rendering logic is valid.
Absolute positioning for the bars, including a minimum width for zero-value bars, appears intentional and clear.
96-122: Grid lines are well-implemented.
Using an SVG overlay for the grid is a neat way to keep the layout consistent with the div-based bars.
124-135: X-axis tick display is concise.
Formatting ticks withformat(",d")is helpful for readability; no issues noted.
138-152: Department labels positioning is logical.
Anchoring the labels usingyScale.bandwidth()for vertical centering is correct.
153-154: ClientTooltip wrapper is consistent.
Wrapping the chart inClientTooltipis a clean approach; no concerns here.apps/app/src/components/policies/charts/policies-by-assignee.tsx (8)
1-1: Import of React & CSSProperties is fine.
If you’re on Next.js 13+ with the newest JSX transform, a named import of React might not be strictly necessary, but it’s still harmless.
16-19: Allowing undefined was handled properly.
Usingnumber | undefinedensures the logic can gracefully handle missing data, especially with the?? 0fallback in sorting.
38-47: Aggregates are safe.
Filtering theOrganizationPolicyarray is straightforward and dynamic checks are suitable for counting published, draft, and archived.
50-50: Robust sorting approach.
Using nullish coalescing in the sort ensures undefined values don't break the ordering.
58-66: User info fallback is nice.
Providing a fallback label "Unknown User" prevents crashes for null names.
68-89: Status legend clarifies distribution.
Clear labeling for published, draft, and archived policies is beneficial for user understanding.
98-131: Initial data array build is logical.
Conditionally pushing objects for non-zero policy statuses helps keep the chart minimal.
207-213: Query usage approved.
SelectingstatusunderOrganizationPolicyis succinct and ensures minimal data retrieval.
| --chart-4: ##af57db; | ||
| --chart-5: ##e23670; |
There was a problem hiding this comment.
Fix double hash in hex color values.
There's a syntax error in the hex color values - they have an extra # character which will prevent the colors from being applied correctly.
--chart-4: ##af57db;
--chart-5: ##e23670;
++chart-4: #af57db;
++chart-5: #e23670;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --chart-4: ##af57db; | |
| --chart-5: ##e23670; | |
| --chart-4: #af57db; | |
| --chart-5: #e23670; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 78-78: Expected a color but instead found '#'.
Expected a color here.
Ensure the color is specified in a valid hexadecimal format. Examples: #000, #000f, #ffffff, #ffffffff
(parse)
[error] 79-79: Expected a color but instead found '#'.
Expected a color here.
Ensure the color is specified in a valid hexadecimal format. Examples: #000, #000f, #ffffff, #ffffffff
(parse)
| async function userData(organizationId: string) { | ||
| return await db.user.findMany({ | ||
| where: { | ||
| organizationId, | ||
| Risk: { | ||
| some: {}, | ||
| }, | ||
| }, | ||
| select: { | ||
| id: true, | ||
| name: true, | ||
| image: true, | ||
| Risk: { | ||
| select: { | ||
| status: true, | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Capture potential database errors in userData.
The function userData can throw exceptions or experience DB connectivity issues. Wrap the query in a try/catch block or implement robust error handling to maintain stability if something goes wrong.
| export async function RisksAssignee({ organizationId }: Props) { | ||
| const t = await getI18n(); | ||
| const userStats = await userData(organizationId); | ||
|
|
||
| const stats: UserRiskStats[] = userStats.map((user) => ({ | ||
| user: { | ||
| id: user.id, | ||
| name: user.name, | ||
| image: user.image, | ||
| }, | ||
| totalRisks: user.Risk.length, | ||
| openRisks: user.Risk.filter((risk) => risk.status === "open").length, | ||
| pendingRisks: user.Risk.filter((risk) => risk.status === "pending").length, | ||
| closedRisks: user.Risk.filter((risk) => risk.status === "closed").length, | ||
| archivedRisks: user.Risk.filter((risk) => risk.status === "archived") | ||
| .length, | ||
| })); |
There was a problem hiding this comment.
Handle potential empty or error states in RisksAssignee.
This component directly calls userData and then maps the results. If userData fails or returns an empty array, the component gracefully skips rendering user stats but does not handle the error scenario. Consider adding a fallback UI or error boundary for better resilience.
+ if (!userStats || userStats.length === 0) {
+ return <p className="text-sm text-muted-foreground">No data available.</p>;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function RisksAssignee({ organizationId }: Props) { | |
| const t = await getI18n(); | |
| const userStats = await userData(organizationId); | |
| const stats: UserRiskStats[] = userStats.map((user) => ({ | |
| user: { | |
| id: user.id, | |
| name: user.name, | |
| image: user.image, | |
| }, | |
| totalRisks: user.Risk.length, | |
| openRisks: user.Risk.filter((risk) => risk.status === "open").length, | |
| pendingRisks: user.Risk.filter((risk) => risk.status === "pending").length, | |
| closedRisks: user.Risk.filter((risk) => risk.status === "closed").length, | |
| archivedRisks: user.Risk.filter((risk) => risk.status === "archived") | |
| .length, | |
| })); | |
| export async function RisksAssignee({ organizationId }: Props) { | |
| const t = await getI18n(); | |
| const userStats = await userData(organizationId); | |
| if (!userStats || userStats.length === 0) { | |
| return <p className="text-sm text-muted-foreground">No data available.</p>; | |
| } | |
| const stats: UserRiskStats[] = userStats.map((user) => ({ | |
| user: { | |
| id: user.id, | |
| name: user.name, | |
| image: user.image, | |
| }, | |
| totalRisks: user.Risk.length, | |
| openRisks: user.Risk.filter((risk) => risk.status === "open").length, | |
| pendingRisks: user.Risk.filter((risk) => risk.status === "pending").length, | |
| closedRisks: user.Risk.filter((risk) => risk.status === "closed").length, | |
| archivedRisks: user.Risk.filter((risk) => risk.status === "archived").length, | |
| })); | |
| // ... rest of the component logic | |
| } |
2 participants
…w and navigation
Summary by CodeRabbit
New Features
PoliciesStatuscomponent for visualizing policy statuses with a pie chart.DonutChartandRisksAssigneecomponents for improved risk data visualization.Enhancements