Lewis/policies - #88
Conversation
…racking and overview - Add new policy status "needs_review" to support more granular policy lifecycle management - Implement policy overview and update forms with detailed editing capabilities - Add server actions for updating policy details and form information - Enhance policy dashboard with new status tracking and visualization - Update database schema to support policy owner, department, and review date - Improve filter and search functionality for policies - Add localization support for new policy management features
WalkthroughThis pull request implements several enhancements across the application. Dependencies in the AWS SDK are updated, and new actions with corresponding validation schemas are added for updating policy forms and overviews. UI components in dashboards and forms have been modified for improved routing, style, and functionality. Caching mechanisms have been integrated for policy and user data retrieval. Risk and policies charts are updated with new status types including “needs_review,” and localization files in multiple languages are enhanced. Database migrations and Prisma schema modifications add new columns, relationships, and enum values to support these changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ClientForm
participant AuthActionClient
participant Database
participant CacheManager
User->>ClientForm: Submit policy update form data
ClientForm->>AuthActionClient: Invoke updatePolicyFormAction
AuthActionClient->>Database: Retrieve current policy data
Database-->>AuthActionClient: Return policy details
AuthActionClient->>AuthActionClient: Validate credentials & check status change
AuthActionClient->>Database: Update policy record with new data
Database-->>AuthActionClient: Confirm update
AuthActionClient->>CacheManager: Trigger cache revalidation
AuthActionClient-->>ClientForm: Return success response
sequenceDiagram
participant Browser
participant PolicyDetailsPage
participant Database
participant Cache
Browser->>PolicyDetailsPage: Request Policy Details (policyId, organizationId)
PolicyDetailsPage->>Database: Call getPolicy(policyId, organizationId)
Database-->>PolicyDetailsPage: Return policy data
PolicyDetailsPage->>Database: Call getUsers(organizationId)
Database-->>PolicyDetailsPage: Return list of users
alt Policy exists
PolicyDetailsPage->>Browser: Render PolicyOverview with policy & users
else Policy missing
PolicyDetailsPage->>Browser: Redirect to "/policies"
end
Poem
✨ Finishing Touches
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: 6
🔭 Outside diff range comments (4)
apps/app/src/components/risks/charts/donut-chart.tsx (2)
15-15: 🛠️ Refactor suggestionRemove the unused
showLabelspropThe
showLabelsprop is defined and defaulted totrue, but it's not being used in the component since the label rendering code has been removed. Props that aren't used should be removed to avoid confusion.interface DonutChartProps { data: ChartItem[]; colors?: string[]; - showLabels?: boolean; } export function DonutChart({ data, colors = ["#7e4cfe", "#895cfc", "#956bff", "#a37fff", "#b291fd", "#b597ff"], - showLabels = true, }: DonutChartProps) {Also applies to: 21-21
90-100: 🛠️ Refactor suggestionRemove unused centroid calculations in the render loop
The code calculates centroids and adjusts them based on angles, but these calculations aren't used anywhere in the component since the label rendering code has been removed. Consider removing these calculations to improve code clarity and performance.
{/* Slices */} {arcs.map((d, i) => { const angle = computeAngle(d); - const centroid = arcLabel.centroid(d); - if (d.endAngle > Math.PI) { - centroid[0] += 10; - centroid[1] += 10; - } else { - centroid[0] -= 10; - centroid[1] -= 0; - } return (apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/types/index.ts (1)
3-14:⚠️ Potential issueUpdate policySchema to include 'needs_review' status
The policySchema still uses an enum that only includes "draft", "published", and "archived" statuses, but you've added a 'needs_review' status in the database. This schema should be updated to include the new status.
export const policySchema = z.object({ id: z.string(), - status: z.enum(["draft", "published", "archived"]), + status: z.enum(["draft", "published", "archived", "needs_review"]), createdAt: z.date(), updatedAt: z.date(), policy: z.object({ id: z.string(), name: z.string(), description: z.string().nullable(), slug: z.string(), }), });apps/app/src/components/policies/charts/policies-by-assignee.tsx (1)
143-143:⚠️ Potential issueMissing implementation of
needs_reviewstatus in RiskBarChartThe RiskBarChart component's data array includes blocks for
published,draft, andarchivedstatuses, but doesn't include a block for the newly addedneeds_reviewstatus. This will cause policies with the "needs_review" status to be excluded from the visualization, creating an inconsistency with the numbers displayed in the legend.Add this code block after line 143:
...(stat.archivedPolicies && stat.archivedPolicies > 0 ? [ { key: "archived", value: stat.archivedPolicies, color: policyStatus.archived, label: t("common.status.archived"), }, ] : []), + ...(stat.needsReviewPolicies && stat.needsReviewPolicies > 0 + ? [ + { + key: "needs_review", + value: stat.needsReviewPolicies, + color: policyStatus.needs_review, + label: t("common.status.needs_review"), + }, + ] + : []),
🧹 Nitpick comments (23)
apps/app/src/components/risks/charts/donut-chart.tsx (1)
45-48: Clean up unused label-related variables and calculationsThe variables
labelRadius,arcLabel, andminAngleare defined but not used anywhere in the component since the label rendering code has been removed. Consider removing these variables to clean up the code.- const labelRadius = radius * 0.825; - const arcLabel = arc<PieArcDatum<ChartItem>>() - .innerRadius(labelRadius) - .outerRadius(labelRadius); const arcs = pieLayout(data); // Calculate the angle for each slice function computeAngle(d: PieArcDatum<ChartItem>) { return ((d.endAngle - d.startAngle) * 180) / Math.PI; } - // Minimum angle to display text - const minAngle = 20; // Adjust this value as neededAlso applies to: 58-58
apps/app/src/actions/schema.ts (2)
288-292: Add validation constraints to theupdatePolicyOverviewSchemafields.While the schema correctly defines the structure for policy overview updates, it lacks validation constraints (like min/max length requirements) that are present in other schemas within this file.
Consider adding validation constraints similar to other schemas:
export const updatePolicyOverviewSchema = z.object({ - id: z.string(), - title: z.string(), - description: z.string(), + id: z.string().min(1, "ID is required"), + title: z.string().min(1, "Title is required").max(100, "Title should be at most 100 characters"), + description: z.string().min(1, "Description is required").max(255, "Description should be at most 255 characters"), });
294-301: Add validation error messages and consider consistent naming conventions.The schema lacks error messages for validation failures, and uses inconsistent naming conventions.
Add error messages and consider using camelCase consistently:
export const updatePolicyFormSchema = z.object({ - id: z.string(), - status: z.nativeEnum(PolicyStatus), - ownerId: z.string(), - department: z.nativeEnum(Departments), - review_frequency: z.nativeEnum(Frequency), - review_date: z.date(), + id: z.string().min(1, { message: "Policy ID is required" }), + status: z.nativeEnum(PolicyStatus, { required_error: "Policy status is required" }), + ownerId: z.string({ required_error: "You must assign an owner to the policy" }), + department: z.nativeEnum(Departments, { required_error: "Department is required" }), + reviewFrequency: z.nativeEnum(Frequency, { required_error: "Review frequency is required" }), + reviewDate: z.date({ required_error: "Review date is required" }), });Alternatively, if snake_case is preferred for these fields to match existing data models:
- review_frequency: z.nativeEnum(Frequency), - review_date: z.date(), + review_frequency: z.nativeEnum(Frequency, { required_error: "Review frequency is required" }), + review_date: z.date({ required_error: "Review date is required" }),apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/data-table/columns.tsx (1)
15-15: Remove unused type import.The
StatusTypetype is imported but not used in this file.-import { StatusPolicies, type StatusType } from "@/components/status-policies"; +import { StatusPolicies } from "@/components/status-policies";apps/app/src/components/risks/charts/risks-by-department.tsx (1)
27-41: Consider documenting the department filtering logic.The implementation to show departments with risks and up to 2 empty departments when fewer than 4 departments have risks is a good enhancement, but the rationale behind this specific logic isn't clear.
Add a comment explaining the business logic:
// Separate departments with values > 0 and departments with values = 0 const departmentsWithValues = data.filter((dept) => dept.value > 0); const departmentsWithoutValues = data.filter((dept) => dept.value === 0); // Determine which departments to show let departmentsToShow = [...departmentsWithValues]; +// Business rule: Show all departments with risks, plus up to 2 empty departments +// when we have fewer than 4 departments with risks. This ensures the chart shows +// relevant information without being dominated by empty departments. if (departmentsWithValues.length < 4 && departmentsWithoutValues.length > 0) { departmentsToShow = [ ...departmentsWithValues, ...departmentsWithoutValues.slice(0, 2), ]; }apps/app/src/components/policies/sheets/policy-overview-sheet.tsx (1)
35-64: Good responsive implementation for desktop view.The Sheet implementation for desktop provides a great user experience with:
- Clear header structure
- Descriptive title and explanation
- Accessible close button
- Scrollable content area for longer forms
Minor note: The
{" "}on line 52 appears unnecessary and can be removed.- </div>{" "} + </div>apps/app/src/app/[locale]/(app)/(dashboard)/risk/(overview)/page.tsx (1)
35-35: Architecture improvement: Simplified data flow.The refactoring removes the need to pass data explicitly through props, letting the
RiskOverviewcomponent fetch its own data. This improves component independence and reduces unnecessary database queries.This architectural change promotes better separation of concerns where child components manage their own data dependencies.
Also applies to: 48-52
apps/app/src/components/risks/charts/risks-assignee.tsx (1)
25-28: Improved maintainability: CSS variables for consistent colors.Replacing hardcoded color values with CSS variables enhances maintainability and ensures consistent theming across the application.
Consider documenting these CSS variables in a central location for better developer onboarding.
Also applies to: 71-71, 77-77, 83-83, 90-90
apps/app/src/actions/policies/update-policy-form-action.ts (3)
47-50: Consider providing more descriptive errors.Currently, if
user.idoruser.organizationIdis missing, the error message is "Invalid user input." Clarifying which field is missing can aid in debugging and usage. For example, throw a more specific error iforganizationIdis absent, or ifuser.idis undefined.
73-86: Transaction-based update might be safer.Updating the policy in isolation is fine; however, if multiple related updates are needed in the future, or if other records must be updated at the same time (e.g., logs, notifications), consider wrapping them in a database transaction for atomicity.
88-101: Improve error handling workflow.Revalidating paths and tags on successful update is great. However, if a revalidation call fails, the end user won’t be notified. Likewise, the catch block only returns a generic failure object. Consider adding logs, metrics, or specialized error messages that help identify the root cause for front-end or operational users.
apps/app/src/components/risks/charts/risks-by-assignee.tsx (1)
170-193: Add error handling or logging for database calls.In the
userDatafunction, there's no error handling if thedb.user.findManycall fails. You might consider wrapping the call in a try/catch block or returning more contextual information in the event of an error, especially ifunstable_cachelogs are insufficient.const userData = unstable_cache( - async (organizationId: string) => { - return await db.user.findMany({ - ... - }); - }, + async (organizationId: string) => { + try { + return await db.user.findMany({ + ... + }); + } catch (error) { + console.error("Failed to fetch user data by organizationId:", error); + throw new Error("Unable to fetch user data."); + } + }, ["users-with-risks"], { tags: ["risks", "users"] }, );apps/app/src/actions/policies/update-policy-overview-action.ts (1)
45-55: Consider using a transaction for the database update.The update operation is correctly structured, but for data consistency and to prevent partial updates in case of failure, consider using a database transaction.
- await db.organizationPolicy.update({ - where: { id }, - data: { - policy: { - update: { - name: title, - description, - }, - }, - }, - }); + await db.$transaction(async (tx) => { + await tx.organizationPolicy.update({ + where: { id }, + data: { + policy: { + update: { + name: title, + description, + }, + }, + }, + }); + });apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/[policyId]/page.tsx (2)
55-67: Consider adding error handling to cached policy fetch.The
getPolicyfunction could benefit from try/catch error handling to gracefully manage database query failures.const getPolicy = unstable_cache( async (policyId: string, organizationId: string) => { + try { const policy = await db.organizationPolicy.findUnique({ where: { id: policyId, organizationId }, include: { policy: true, }, }); return policy; + } catch (error) { + console.error("Error fetching policy:", error); + return null; + } }, ["policy-details"], { tags: ["policies", "policy-details"] }, );
69-78: Add error handling to getUsers function.Similar to the policy fetching function, the
getUsersfunction should include error handling to prevent unhandled exceptions.const getUsers = unstable_cache( async (organizationId: string) => { + try { const users = await db.user.findMany({ where: { organizationId: organizationId }, }); return users; + } catch (error) { + console.error("Error fetching users:", error); + return []; + } }, ["users-cache"], );apps/app/src/components/forms/policies/update-policy-form.tsx (1)
93-98: Inconsistent translation usage in form labels.The description field label is hardcoded as "Description" instead of using a translation key, and the placeholder uses a risk-related translation key.
- <FormLabel>Description</FormLabel> + <FormLabel>{t("policies.overview.form.description")}</FormLabel> <FormControl> <Textarea {...field} className="mt-3 min-h-[80px]" - placeholder={t("risk.form.risk_description_description")} + placeholder={t("policies.overview.form.description_placeholder")} />apps/app/src/components/forms/policies/policy-overview.tsx (2)
140-153: Inconsistent translation usage for status field.The form label and placeholder are using risk-related translation keys instead of policy-specific ones.
<FormItem> - <FormLabel>{t("risk.form.risk_status")}</FormLabel> + <FormLabel>{t("policies.overview.form.status")}</FormLabel> <FormControl> <Select value={field.value} onValueChange={field.onChange}> <SelectTrigger> <SelectValue - placeholder={t("risk.form.risk_status_placeholder")} + placeholder={t("policies.overview.form.status_placeholder")} >
198-212: Inconsistent translation usage for department field.The form label and placeholder are using risk-related translation keys instead of policy-specific ones.
<FormItem> - <FormLabel>{t("risk.form.risk_department")}</FormLabel> + <FormLabel>{t("policies.overview.form.department")}</FormLabel> <FormControl> <Select {...field} value={field.value} onValueChange={field.onChange} > <SelectTrigger> <SelectValue - placeholder={t("risk.form.risk_department_placeholder")} + placeholder={t("policies.overview.form.department_placeholder")} />apps/app/src/components/tables/policies/filter-toolbar.tsx (1)
162-162: User ID fallback could be improved.The current implementation uses the logical OR operator which could result in an empty string for undefined IDs. Consider using the nullish coalescing operator for clearer intent.
- <SelectItem key={user.id} value={user?.id || ""}> + <SelectItem key={user.id} value={user?.id ?? ""}>apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/actions/get-policies.ts (1)
20-118: Cached function logic for retrieving policies.
This block handles combined queries (search, status), sorting, and pagination. The approach to building theorderByClauseobject is clear. Ensure that any newly added fields (e.g.,ownerId) are also retrieved here if needed.Consider providing typed definitions for the
orderByClauseto avoid any runtime confusion:- let orderByClause: any = { updatedAt: 'desc' }; + let orderByClause: Prisma.OrganizationPolicyOrderByWithRelationInput = { + updatedAt: "desc" + };apps/app/src/components/risks/charts/department-chart.tsx (1)
140-145: Tooltip data attribution.
Attachingdata-tipis an elegant way to provide quick info. Just be sure any styling or positioning matches the desired UX.If you prefer advanced customization, consider integrating a more dynamic tooltip component for richer interactions.
apps/app/src/components/risks/charts/status-chart.tsx (2)
7-12: Consider including 'needs_review' status color mapping if relevant.
If this project adds a "needs_review" status, you may want to include it here to ensure proper styling instead of falling back to the default gray color.
89-110: Provide a guaranteed color for all known statuses.
Your fallbackbg-gray-400is useful, but consider expandingSTATUS_COLORSfor any known statuses, e.g., “needs_review,” to provide a distinct color.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
apps/app/languine.lockis excluded by!**/*.lockbun.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (44)
apps/app/package.json(1 hunks)apps/app/src/actions/policies/update-policy-form-action.ts(1 hunks)apps/app/src/actions/policies/update-policy-overview-action.ts(1 hunks)apps/app/src/actions/schema.ts(2 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Components/data-table/data-table.tsx(2 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceList.tsx(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/data-table/EvidenceListTable.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/data-table/columns.tsx(3 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/(overview)/page.tsx(3 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/actions/get-policies.ts(3 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/components/PoliciesList.tsx(4 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/hooks/usePolicies.ts(2 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/page.tsx(3 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/types/index.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/[policyId]/page.tsx(4 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/risk/(overview)/page.tsx(2 hunks)apps/app/src/components/forms/create-organization-form.tsx(2 hunks)apps/app/src/components/forms/policies/policy-overview.tsx(1 hunks)apps/app/src/components/forms/policies/update-policy-form.tsx(1 hunks)apps/app/src/components/policies/charts/policies-by-assignee.tsx(5 hunks)apps/app/src/components/policies/charts/policies-status.tsx(3 hunks)apps/app/src/components/policies/policy-overview.tsx(1 hunks)apps/app/src/components/policies/sheets/policy-overview-sheet.tsx(1 hunks)apps/app/src/components/risks/charts/department-chart.tsx(6 hunks)apps/app/src/components/risks/charts/donut-chart.tsx(1 hunks)apps/app/src/components/risks/charts/risk-overview.tsx(1 hunks)apps/app/src/components/risks/charts/risks-assignee.tsx(4 hunks)apps/app/src/components/risks/charts/risks-by-assignee.tsx(2 hunks)apps/app/src/components/risks/charts/risks-by-department.tsx(1 hunks)apps/app/src/components/risks/charts/risks-by-status.tsx(2 hunks)apps/app/src/components/risks/charts/status-chart.tsx(1 hunks)apps/app/src/components/risks/risk-overview.tsx(1 hunks)apps/app/src/components/tables/policies/filter-toolbar.tsx(5 hunks)apps/app/src/locales/en.ts(2 hunks)apps/app/src/locales/es.ts(3 hunks)apps/app/src/locales/fr.ts(3 hunks)apps/app/src/locales/no.ts(3 hunks)apps/app/src/locales/pt.ts(3 hunks)packages/db/prisma/migrations/20250228163636_org_policy_assignee_and_departments/migration.sql(1 hunks)packages/db/prisma/migrations/20250228170410_policy_status_added_needs_review/migration.sql(1 hunks)packages/db/prisma/migrations/20250228170748_policy_added_review_date/migration.sql(1 hunks)packages/db/prisma/schema.prisma(3 hunks)packages/ui/src/components/chart-tooltip.tsx(2 hunks)packages/ui/src/globals.css(1 hunks)
💤 Files with no reviewable changes (1)
- apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/EvidenceList.tsx
✅ Files skipped from review due to trivial changes (2)
- apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/data-table/EvidenceListTable.tsx
- apps/app/src/components/forms/create-organization-form.tsx
🔇 Additional comments (94)
apps/app/src/components/risks/charts/donut-chart.tsx (2)
4-4: Great cleanup of unused importsRemoving the unused
TooltipContentimport helps keep the codebase clean and maintainable.
53-55:Details
❌ Incorrect review comment
Evaluate if
computeAnglefunction is still neededThe
computeAnglefunction is only used to compute the angle for each slice, but it's not clear if this is still needed after removing the label rendering code. Ifangleis only used for the removed labels, consider removing this function as well.
🏁 Script executed:
#!/bin/bash # Check if computeAngle or angle variable is used elsewhere in the component rg -n "\bangle\b" apps/app/src/components/risks/charts/donut-chart.tsxLength of output: 197
The
computeAnglefunction remains necessary
Our search shows that the function is still used at line 92 to compute theanglefor each slice, so it should not be removed.
- Verified that
computeAngleis invoked in the code (line 92).- The suggestion to remove the function is not applicable as its output is still in use.
Likely an incorrect or invalid review comment.
packages/db/prisma/migrations/20250228170748_policy_added_review_date/migration.sql (1)
1-2: LGTM - Clean migration for review date trackingThe migration adds a nullable "reviewDate" column to the "OrganizationPolicy" table, which aligns well with the new "needs_review" status being added to the system. This will allow tracking when policies need to be reviewed.
packages/db/prisma/migrations/20250228170410_policy_status_added_needs_review/migration.sql (1)
1-2: LGTM - Well-structured enum updateThe migration properly adds the 'needs_review' value to the "PolicyStatus" enum, which aligns with the addition of the reviewDate field and UI enhancements. This is a clean approach to extending enum values.
apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/types/index.ts (1)
19-20: LGTM - Enhanced filtering capabilitiesAdding optional ownerId and sort fields to the policies input schema improves the API's flexibility for filtering and sorting policies.
apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/hooks/usePolicies.ts (2)
38-39: Good addition of filtering parameters.The addition of the
ownerIdandsortparameters allows for more flexible policy data retrieval, aligning with the enhanced filtering capabilities in the UI.
49-50: Correctly updated SWR cache key and fetch parameters.The SWR cache key array and fetch function parameters have been properly updated to include the new filtering options. This ensures proper cache invalidation when these parameters change.
apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Components/data-table/data-table.tsx (2)
45-45: Updated URL structure for policy navigation.The URL path has been updated to reflect the new routing structure that includes the "all" segment, which aligns with the broader routing changes in the application.
76-77: Added trailing comma for consistent code style.This is a minor style improvement that helps maintain consistency in the codebase and prevents merge conflicts when lines are added later.
packages/db/prisma/migrations/20250228163636_org_policy_assignee_and_departments/migration.sql (2)
1-4: Good schema enhancement for policy ownership and categorization.The addition of
departmentandownerIdfields to theOrganizationPolicytable provides valuable metadata for policy management, allowing for better organization and accountability tracking.
5-7: Well-defined foreign key constraint.The foreign key constraint is correctly configured with appropriate ON DELETE and ON UPDATE actions. Setting
ownerIdto NULL on user deletion prevents orphaned records while preserving policy data.apps/app/package.json (1)
15-16:Details
✅ Verification successful
Appropriate AWS SDK dependency updates.
The addition of
@aws-sdk/client-s3and version update of@aws-sdk/s3-request-presignerensure consistent AWS SDK versions. This is good practice for maintainability and compatibility between related AWS packages.
🏁 Script executed:
#!/bin/bash # Verify if these are the latest AWS SDK versions npm view @aws-sdk/client-s3 version --json npm view @aws-sdk/s3-request-presigner version --jsonLength of output: 120
AWS SDK Dependency Versions Confirmed
Both
@aws-sdk/client-s3and@aws-sdk/s3-request-presignerare confirmed to be at version3.758.0, ensuring consistent dependency versions. This alignment supports maintainability and compatibility across AWS SDK packages, and no further changes are necessary.
- Verified
@aws-sdk/client-s3version:3.758.0- Verified
@aws-sdk/s3-request-presignerversion:3.758.0apps/app/src/app/[locale]/(app)/(dashboard)/evidence/Components/data-table/columns.tsx (2)
50-52: LGTM! StatusPolicies component implementation.The code successfully replaces the conditional icon rendering with a more maintainable component-based approach.
107-110: Minor formatting fix properly applied.The comma addition improves code clarity.
apps/app/src/components/risks/risk-overview.tsx (1)
4-4: Import cleanup correctly implemented.The import statement has been simplified to only include the types that are actually used in the component.
apps/app/src/components/risks/charts/risks-by-department.tsx (1)
48-48:Details
✅ Verification successful
Verify the DepartmentChart props interface includes showEmptyDepartments.
Make sure the
DepartmentChartcomponent properly defines theshowEmptyDepartmentsprop in its interface.
🏁 Script executed:
#!/bin/bash # Check if the DepartmentChart component defines the showEmptyDepartments prop rg -A 20 -B 5 "interface.*Props" ./apps/app/src/components/risks/charts/department-chart.tsxLength of output: 841
Verified: The DepartmentChart component properly defines the
showEmptyDepartmentsprop.
- The interface
DepartmentChartPropsinapps/app/src/components/risks/charts/department-chart.tsxincludes the optionalshowEmptyDepartmentsproperty.- The component’s default value for this prop is correctly set to
true.No further changes are required.
apps/app/src/components/policies/charts/policies-status.tsx (4)
13-13: Interface extension looks good!The addition of the
needsReviewPoliciesproperty to the Props interface is well-structured and maintains consistency with the existing properties.
21-21: Correctly implemented new parameter and status mapping.The inclusion of the new parameter in the component function signature and its mapping in the statusCounts object follows the established pattern.
Also applies to: 29-29
51-56: Good implementation of the "needs_review" status in the chart data.The new status is properly added to the data array with the appropriate translation key, value reference, and distinctive styling using the destructive color from the design system, which is semantically appropriate for content needing review.
66-66: Simplified grid layout.The removal of the
2xl:grid 2xl:grid-cols-3classes changes how the content displays on extra-large screens. Ensure this was intentional and won't negatively impact the layout.apps/app/src/components/risks/charts/risks-by-status.tsx (3)
3-4: Appropriate imports for the enhanced component.The addition of
unstable_cachefor performance optimization and Card components for better UI structure are good improvements.
18-27: Improved UI structure with proper Card layout.Wrapping the StatusChart in a Card with proper header elements improves the visual hierarchy and consistency with other components in the application.
30-40: Good implementation of caching for performance optimization.Converting the function to use
unstable_cachewith appropriate keys and tags will significantly improve performance for this data-fetching operation.Note that
unstable_cacheis marked as unstable in Next.js, which means its API might change in future versions. This implementation follows current best practices, but be aware of potential future changes to this API.apps/app/src/components/policies/sheets/policy-overview-sheet.tsx (1)
20-33: Well-structured component with URL-based state management.Good implementation of the PolicyOverviewSheet component with:
- Proper TypeScript typing
- URL query state management using nuqs
- Clean handling of open state changes
This approach enables deep linking and proper browser history navigation.
apps/app/src/components/policies/policy-overview.tsx (4)
14-23: Well-defined component with proper TypeScript typing.The PolicyOverview component is well structured with:
- Clear props definition
- Proper typing using combined types from the database schema
- Appropriate initialization of internationalization and query state
This provides good type safety and maintainability.
24-44: Good use of Alert component for policy information display.The Alert component provides a clear visual hierarchy for the policy name and description. The edit button is well positioned and follows good UX practices by opening a dedicated editing interface rather than inline editing.
46-57: Well-structured card for policy overview content.The card implementation follows UI best practices with:
- Clear header and title
- Proper component composition using UpdatePolicyOverview
- Clean integration with the users data for form functionality
59-59: Good integration of the sheet component.The PolicyOverviewSheet is properly included with the necessary policy prop, creating a cohesive user experience for viewing and editing policy information.
apps/app/src/app/[locale]/(app)/(dashboard)/policies/(overview)/page.tsx (2)
39-39: Added support for tracking policies that need review.The addition of the
needsReviewPoliciesprop to thePoliciesStatuscomponent enhances the UI to display policies requiring review, which is a great improvement for user experience.
54-54: LGTM: Properly implemented "needs_review" status count.The implementation correctly fetches and tracks policies with "needs_review" status following the same pattern as other status types, maintaining consistency in the codebase.
Also applies to: 79-84, 92-92
apps/app/src/components/risks/charts/risk-overview.tsx (1)
4-6: LGTM: Interface simplified correctly.The
RiskOverviewPropsinterface has been properly simplified to only includeorganizationId, which aligns with the architectural changes in the parent component.Also applies to: 8-8
apps/app/src/components/risks/charts/risks-assignee.tsx (1)
5-5: Performance improvement: Implemented caching for user data.Adding
unstable_cachefor theuserDatafunction will reduce database query load and improve performance for repeated requests with the same organization ID.The caching implementation is well-structured with appropriate cache keys and tags.
Also applies to: 214-237
apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/page.tsx (1)
48-67: Excellent use of caching for performance optimization!The implementation of
getUserswithunstable_cacheis well-structured. Using cache tags and a reasonable cache configuration will help improve performance by reducing redundant database queries.packages/ui/src/components/chart-tooltip.tsx (1)
167-175: Great implementation of proper ref handling!The ref handling follows React best practices by correctly managing both the component's internal ref and the forwarded ref, maintaining backward compatibility while enhancing functionality.
apps/app/src/locales/pt.ts (1)
154-160: Well-structured localization additionsThe new translations for frequency options and policy overview elements are comprehensive and consistent with the existing localization patterns.
Also applies to: 256-256, 303-316
apps/app/src/actions/policies/update-policy-form-action.ts (2)
33-42: Good separation of concerns with schema and metadata.Defining the schema (
updatePolicyFormSchema) and specifying metadata for tracking events is clear and maintainable. Keep this pattern for other actions for consistency.
63-71: Validate policy existence before updating status.If the policy does not exist (
currentPolicyisnull), the status update logic still runs. While this may not cause an error due to the optional chaining operator (?.), consider throwing an error or at least logging a warning when attempting to publish a nonexistent policy.apps/app/src/components/risks/charts/risks-by-assignee.tsx (1)
7-7: Importing unstable_cache is correct for caching.Using
unstable_cachefromnext/cachehelps reduce repeated database calls. Marking this as approved.apps/app/src/locales/en.ts (2)
291-291: Renaming “Policy Status” to “Policy by Status”.This renaming is consistent with the rest of the localized strings and reflects a more descriptive usage. No concerns.
299-311: Localization keys for updating policies look good.The
overview.formentries are descriptive and aligned with the new policy update functionality. Good job ensuring placeholders and success/failure messages are localized.apps/app/src/actions/policies/update-policy-overview-action.ts (6)
1-8: Good import structure and server directive.The code appropriately uses the "use server" directive and imports all necessary dependencies for the server action. The schema import indicates proper validation will be used.
10-19: Well-structured server action with good metadata.The action is properly set up with schema validation and includes metadata for tracking. This follows good practices for server actions in Next.js.
20-28: Proper authorization check implementation.The code correctly extracts the parsed input and user context, then validates that a user exists before proceeding. This is good security practice to prevent unauthorized access.
30-43: Database query has appropriate scoping and error handling.The query correctly filters by both the policy ID and the user's organization ID, preventing cross-organization data access. The policy existence check provides clear error messaging.
57-60: Comprehensive path revalidation.Good practice to revalidate all relevant policy paths to ensure the UI is updated with the latest data after a successful update.
61-70: Proper error handling structure.The code properly catches any errors during the database operation and returns a user-friendly error message. This follows good error handling practices.
apps/app/src/locales/fr.ts (3)
153-160: New frequency translations added correctly.The new
frequencyobject includes all necessary time period translations in French. This aligns with the localization changes in other language files.
256-257: Updated policy status label to match other localization files.The translation for
policy_statushas been updated from "État de la politique" to "Politique par Statut", maintaining consistency with other language files.
302-316: New policy overview form translations added.The new
overviewobject adds all necessary translations for the policy update forms, including field labels, placeholders, and success/error messages. This supports the new policy overview update functionality.apps/app/src/locales/es.ts (3)
153-160: New frequency translations added correctly in Spanish.The frequency object with daily, weekly, monthly, quarterly, and yearly translations has been properly added to the Spanish localization file.
256-257: Updated policy status label to maintain consistency.The translation for
policy_statushas been updated from "Estado de la Política" to "Política por Estado", maintaining consistency with the updated translations in other language files.
302-316: Complete policy overview form translations added in Spanish.The new
overviewsection includes all necessary Spanish translations for the policy update functionality, properly structured with appropriate form fields and messages.apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/[policyId]/page.tsx (3)
1-8: Good import structure with caching utilities.The imports correctly include the new components and utilities needed, especially the
unstable_cachefrom Next.js for optimizing data fetching.
26-31: Good data fetching and error handling.The code correctly retrieves the policy and users data using cached functions, and includes a proper redirect when the policy isn't found.
33-37: Clean component rendering structure.The component now renders the PolicyOverview with the required props instead of a placeholder, maintaining a clean and semantic structure.
apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/components/PoliciesList.tsx (4)
20-29: Props update correctly incorporates user data.The addition of the
Usertype import and updating thePoliciesListPropsinterface to include users is a good enhancement.
31-37: Good implementation of sort parameter.The function signature update properly handles the new
usersparameter, and the addition of thesortparameter is properly extracted from search parameters.
59-59: Filter condition properly updated for sorting.The
hasFilterscondition has been correctly updated to include the sort parameter.
70-71: Users prop correctly passed to FilterToolbar.The
FilterToolbarcomponent correctly receives the users data in both instances.Also applies to: 80-82
apps/app/src/components/forms/policies/update-policy-form.tsx (3)
26-43: Form action handler properly set up with success and error handling.The component correctly initializes the
updatePolicyaction with appropriate success and error handlers, including toast notifications.
44-59: Form initialization with defaultValues looks good.The form is properly set up with the zodResolver and the default values match the policy properties.
106-117: Submit button implementation looks good.The button properly handles the loading state during form submission.
apps/app/src/components/forms/policies/policy-overview.tsx (1)
267-267: Review date selection constraint might be too restrictive.The Calendar component is set to disable dates that are today or in the past. Consider if this is the intended behavior, as users might want to set the review date to today.
<Calendar mode="single" selected={field.value} onSelect={field.onChange} - disabled={(date) => date <= new Date()} + disabled={(date) => date < new Date()} // Allow today's date initialFocus />apps/app/src/components/tables/policies/filter-toolbar.tsx (4)
19-20: Type update from custom shape to next-auth User.Updating the users prop type to use the standard User type from next-auth is a good improvement for type consistency.
Also applies to: 24-24
30-31: Good implementation of debounced search input.The addition of a debounced search input with proper useEffect hooks improves the user experience by reducing unnecessary API calls.
Also applies to: 57-70
50-54: Sort functionality properly implemented.The addition of the sort query state parameter and updating handleReset to clear it is well implemented.
Also applies to: 77-80
144-146: Status filter updated with new needs_review option.The addition of the needs_review status option to the dropdown is appropriate given the schema updates.
apps/app/src/locales/no.ts (3)
153-160: Ensure consistent translation keys and usage.
The newly introducedfrequencyobject is clear, and each key has an accurate Norwegian translation. Everything appears consistent with the rest of the file.
256-256: Confirm localized wording alignment.
Changing"Retningslinjestatus"to"Policy etter status"seems consistent with the English version. Just verify that other references to policy statuses use similar terminology for clarity.
302-316: New overview localization appears coherent.
The addition of theoverviewobject undersaved_erroris well-structured, providing relevant keys for updating policies. No apparent grammatical issues.packages/db/prisma/schema.prisma (3)
73-73: Validate the new user-to-policy relationship.
AddingOrganizationPolicy OrganizationPolicy[]in theUsermodel enables tracking multiple policies per user. Ensure all references to this array are updated throughout the codebase.
742-745: Optional fields in OrganizationPolicy model look consistent.
IntroducingownerId,owner,department, andreviewDateas nullable fields provides flexibility for policy ownership and departmental context. Confirm that these optional fields are properly handled in queries and APIs.
1017-1018: New policy status added to the enum.
Theneeds_reviewenum value makes sense for policies awaiting further action. Verify that all relevant application logic is updated to handle this new status correctly.apps/app/src/app/[locale]/(app)/(dashboard)/policies/all/(overview)/actions/get-policies.ts (3)
6-7: Cache import for performance optimization.
Importingunstable_cachefrom "next/cache" can help improve performance. Validate that this function behavior meets all concurrency and invalidation needs.
8-18: Cache key generator for consistent policy retrieval.
ThegeneratePoliciesCacheKeyfunction is straightforward and includes all relevant parameters (organizationId, search, status, page, per_page, and sort). Good approach for ensuring consistent cache hits.
130-148: Refined action logic.
Delegating the database query togetCachedPolicieshelps keep the action concise. Verifying the user’sorganizationIdensures correct scoping. Error handling is also appropriate.apps/app/src/components/risks/charts/department-chart.tsx (12)
7-15: Use of CSS variables for bar colors.
Switching to CSS variable-based classes is consistent and makes themes easier to manage. Good approach to maintain a uniform color palette across different departments.
24-24: Optional prop for showing empty departments.
IntroducingshowEmptyDepartmentsadds welcome flexibility, letting callers decide whether to include zero-value entries.
27-30: Default prop setting.
The default value oftrueforshowEmptyDepartmentsensures the chart remains inclusive by default. This is a sensible choice.
31-35: Conditional filtering logic.
Filtering out zero-value departments whenshowEmptyDepartmentsis false is straightforward. Confirm that upstream logic can handle the omission of certain departments.
39-45: Early return when no departments present.
Showing a friendly message instead of an empty chart is user-friendly. Good fallback for zero-length arrays.
48-56: Adjustable chart height logic.
Dynamically computing the chart height prevents overlap on large data sets while preserving a reasonable minimum. Looks good.
58-59: Single pass to computemaxValue.
Usingmax()once is efficient and readably straightforward.
67-68: Linear scale domain.
Mapping from[0, maxValue]to[0, 100]is consistent with the final visualization. Ensure bar widths reflect large values proportionately.
87-100: Generating dynamic tick values.
Smart approach for generating a minimal set of ticks. For large values, letting D3 handle the intervals keeps the axis neat.
126-133: Accurate bar positioning.
Center-aligning bars by using the band’s midpoint is visually precise. The constant bar height ratio is a good approach for consistent proportions.
154-172: Rendering grid lines for reference.
Drawing dashed lines for each tick value is a nice visual aid. The minimal stroke keeps the chart from looking too busy.
175-185: Tick label formatting.
Conditionally formatting integers vs. floats helps maintain readability. Usingformat(",")is nice for integer grouping.apps/app/src/components/risks/charts/status-chart.tsx (6)
3-5: Imports look good.
No concerns since these imports are well-organized and necessary for the new chart functionality.
14-17: Interfaces appear well-defined.
TheStatusDataandStatusChartPropsinterfaces are concise and accurately typed, promoting clarity and easier maintenance.Also applies to: 20-20
23-44: Ensurenameis validated or non-empty.
Although capitalizing status names is helpful, confirm that upstream data always provides a valid string. Consider adding checks for unexpected or emptynamevalues to avoid potential runtime errors.
46-69: Sorting and empty-data handling look correct.
The descending sort for status values and the fallback display when no items exist both improve usability.
72-88: Scales and helper key functions are straightforward.
Using D3’sscaleBandandscaleLinearkeeps the layout flexible for different data. The helper functions also improve readability.
113-215: Overall chart rendering logic is robust.
The usage of absolute positioning for bars, SVG for grid lines, and separate tick labels is a neat approach. Keep an eye on potential overlaps on very narrow viewports.
- Correct error toast message for policy update to use policy-specific translation - Update drawer title to use policy-specific translation key - Improve localization consistency in policy management UI
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/app/src/components/forms/policies/policy-overview.tsx (2)
142-148: Update translation domain for status labels.You’re using
t("risk.form.risk_status")andt("risk.form.risk_status_placeholder")for a policy-related field. Consider switching to a policy-specific translation key (e.g.t("policies.overview.form.policy_status")).
202-213: Align department translation key with policy domain.Similar to the status field, this label references
risk.form.risk_departmentwhen it should reflect the policy domain, such aspolicies.overview.form.department.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/app/src/components/forms/policies/policy-overview.tsx(1 hunks)apps/app/src/components/policies/sheets/policy-overview-sheet.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/app/src/components/policies/sheets/policy-overview-sheet.tsx
🔇 Additional comments (4)
apps/app/src/components/forms/policies/policy-overview.tsx (4)
61-68: Looks great!Your success and error handlers for
updatePolicyFormwith toast notifications are properly set up. No issues here.
113-131: Avoid submitting the form on dropdown open.At line 116,
onOpenChange={() => form.handleSubmit(onSubmit)}triggers form submission when the select is opened or closed. Typically, form submission should occur after the user selects a new value. Consider moving this logic toonValueChangeif your intent is to auto-save only after picking a new owner.
267-267: Confirm disabling past dates.You currently disable all dates earlier than or equal to
new Date(). If you need to allow backdated reviews, this might be too restrictive. Otherwise, this approach is valid.
51-293: Overall solid implementation!Aside from the minor translation key and event handling concerns, your form structure, validation, and submission flow are well-organized and easy to follow.
…) (#3462) adm-zip < 0.6.0 allows a crafted ZIP to trigger a 4GB memory allocation (DoS). This is reachable: apps/api/src/questionnaire/utils/content-extractor.ts runs `new AdmZip(fileBuffer)` on user-uploaded questionnaire files (xlsx/docx are ZIP containers). Bumps the direct dependency to ^0.6.0 (patched) and updates bun.lock. Our usage (new AdmZip, getEntry, getData, addFile, toBuffer) is unchanged in 0.6.0. Supersedes Dependabot PR #3451, which bumped package.json but left bun.lock out of sync. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
# [3.106.0](v3.105.0...v3.106.0) (2026-07-22) ### Bug Fixes * **auth:** attribute API-key mutations to the key's creator, not the org owner ([#3472](#3472)) ([206ed96](206ed96)), closes [hi#risk](https://github.com/hi/issues/risk) * **deps:** bump adm-zip 0.5.18 -> 0.6.0 in apps/api (Dependabot [#88](https://github.com/trycompai/comp/issues/88)/[#89](https://github.com/trycompai/comp/issues/89)) ([#3462](#3462)) ([300f2a1](300f2a1)), closes [#3451](#3451) * **deps:** override tar to ^7.5.19 to clear node-tar Dependabot alerts ([#94](https://github.com/trycompai/comp/issues/94)-[#104](https://github.com/trycompai/comp/issues/104)) ([#3466](#3466)) ([8ab5709](8ab5709)) * **deps:** patch engine.io ([#93](#93)) and body-parser ([#92](#92)) Dependabot alerts ([#3464](#3464)) ([94c33b1](94c33b1)) * **isms:** harden internal-audit validation and edge cases from deploy review ([#3473](#3473)) ([c6c7379](c6c7379)) * **policies:** create draft version on policy regenerate instead of overwriting published ([#3471](#3471)) ([ff31dbd](ff31dbd)) * **policies:** delete detached PDF objects when regenerating a draft ([#3474](#3474)) ([ecd1bd0](ecd1bd0)) * **policies:** rename CreateVersionDto to avoid swagger collision with automations ([#3469](#3469)) ([2d5290a](2d5290a)) ### Features * **isms:** internal audit programme, plan and report — clause 9.2 (CS-724) ([#3468](#3468)) ([42e5ebd](42e5ebd)), closes [hi#impact](https://github.com/hi/issues/impact)
2 participants
Summary by CodeRabbit
New Features
Style
Documentation