Com 20 cloud tests - #100
carhartlewis wants to merge 35 commits into
Conversation
…into COM-20-cloud-tests
…into COM-20-cloud-tests
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
WalkthroughThe update introduces several new features and improvements across the codebase. A new test registration schema and associated server actions facilitate employee creation and test record retrieval. Multiple React components, hooks, and pages have been added for displaying test details, listing tests, and managing pagination and filtering. Additionally, new TypeScript types and localization sections for cloud tests have been introduced in various languages. On the integrations front, AWS Security Hub functionality is now incorporated with new configuration files, a logo component, and a script for findings retrieval. Database migrations and Prisma schema updates support integration result storage. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ActionAPI as RegisterTestAction
participant SchemaValidator as Zod Validator
participant AuthService
participant Database
Client->>ActionAPI: Send registration request with employee data
ActionAPI->>SchemaValidator: Validate input using registerTestSchema
SchemaValidator-->>ActionAPI: Return validated data / error
ActionAPI->>AuthService: Verify user's organization membership
alt Not Authorized
AuthService-->>ActionAPI: Return authorization error
ActionAPI-->>Client: Send error response
else Authorized
ActionAPI->>Database: Create new employee record
Database-->>ActionAPI: Confirm creation
ActionAPI->>Database: Upsert default tasks and assign to employee
Database-->>ActionAPI: Confirm task assignments
ActionAPI-->>Client: Send success response
end
sequenceDiagram
participant Script
participant AWS as AWS Security Hub
participant Logger
Script->>Logger: Log start of findings retrieval
Script->>AWS: Call fetchSecurityFindings with AWS credentials
AWS-->>Script: Return fetched findings (supporting pagination)
Script->>Script: Group findings by severity
Script->>Logger: Log counts and details for CRITICAL/HIGH findings
Script->>Logger: Log completion message and exit
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: 35
🧹 Nitpick comments (65)
apps/app/src/app/[locale]/(app)/(dashboard)/tests/types/search-params.ts (2)
7-12: Consider adding validation for date-related parameters.The search parameters are well-structured with appropriate parsers, but consider adding validation or formatting requirements for the
startandendparameters if they're expected to be dates or follow a specific format.You could enhance the type safety by using a date parser with validation:
- start: parseAsString, - end: parseAsString, + start: parseAsString.withConstraint((value) => { + // Add validation logic for date format + return /^\d{4}-\d{2}-\d{2}$/.test(value) || "Invalid date format (YYYY-MM-DD required)"; + }), + end: parseAsString.withConstraint((value) => { + // Add validation logic for date format + return /^\d{4}-\d{2}-\d{2}$/.test(value) || "Invalid date format (YYYY-MM-DD required)"; + }),Or if you're using a different date format, adjust the regex pattern accordingly.
1-12: Add JSDoc comments to explain purpose of parameters.Consider adding JSDoc comments to explain what each parameter is used for, especially for future developers who might need to maintain this code.
+/** + * Search parameters cache for tests listing + * @property {string} q - Search query for filtering tests + * @property {number} page - Current page number (zero-based) + * @property {string} start - Start date for filtering tests + * @property {string} end - End date for filtering tests + */ export const searchParamsCache = createSearchParamsCache({ q: parseAsString, page: parseAsInteger.withDefault(0), start: parseAsString, end: parseAsString, });apps/app/src/app/[locale]/(app)/(dashboard)/tests/components/TestsListSkeleton.tsx (2)
8-14: Consider generating skeleton rows programmatically to reduce repetition.The multiple skeleton rows follow an identical pattern with only height differences. This repetition could be simplified.
- <div className="flex flex-col gap-0.5"> - <div className="flex items-center gap-0.5 h-[53px]"> - <Skeleton className="h-full w-2/5" /> - <Skeleton className="h-full w-1/5" /> - <Skeleton className="h-full w-1/5" /> - <Skeleton className="h-full w-1/5" /> - </div> + <div className="flex flex-col gap-0.5"> + {[ + { height: "h-[53px]" }, + { height: "h-[37px]" }, + { height: "h-[37px]" }, + { height: "h-[37px]" } + ].map((row, index) => ( + <div key={index} className={`flex items-center gap-0.5 ${row.height}`}> + <Skeleton className="h-full w-2/5" /> + <Skeleton className="h-full w-1/5" /> + <Skeleton className="h-full w-1/5" /> + <Skeleton className="h-full w-1/5" /> + </div> + ))}
15-36: Add accessibility attributes to skeletons for better screen reader support.Skeleton components should have appropriate ARIA attributes to improve accessibility for screen reader users.
Add an appropriate
aria-labelto the container to indicate this is a loading state:- <div className="relative overflow-hidden"> + <div className="relative overflow-hidden" aria-label="Loading tests" role="status">apps/app/src/components/tables/tests/columns.tsx (1)
3-11: Consider enhancing the TestType interface with JSDoc comments and more specific types.The interface is well-structured but could benefit from additional documentation and more specific types.
/** * Represents a test record in the application */ export interface TestType { /** Unique identifier for the test */ id: string; - severity: string | null; + /** Indicates the severity level of the test result - e.g., 'high', 'medium', 'low' */ + severity: 'high' | 'medium' | 'low' | null; /** Result status of the test */ - result: string; + result: 'pass' | 'fail' | 'warning' | 'info' | string; /** Title or name of the test */ title: string; /** Provider of the test, e.g., 'aws', 'gcp' */ provider: string; /** When the test was created */ createdAt: Date; - assignedUser: null; + /** User assigned to the test, null if unassigned */ + assignedUser: null | { id: string; name: string }; }packages/integrations/src/aws/assets/logo.tsx (1)
1-51: Make the logo component more flexible by accepting size props.The component has hardcoded dimensions, which limits its reusability.
- export const Logo = () => { + export const Logo = ({ width = 131.417, height = 50 }) => { return ( <svg xmlns="http://www.w3.org/2000/svg" version="1.1" id="Layer_1" xmlnsXlink="http://www.w3.org/1999/xlink" x="0px" y="0px" - width={131.417} - height={50} + width={width} + height={height} viewBox="0 0 500 182" style={{ enableBackground: "new 0 0 304 182" }} xmlSpace="preserve">apps/app/src/components/tables/tests/server-columns.tsx (1)
6-14: Add TypeScript interface for return type to improve type safety.Adding a return type would enhance code maintainability and provide better type checking.
+ interface ColumnHeaders { + severity: string; + result: string; + title: string; + provider: string; + status: string; + createdAt: string; + assignedUser: string; + } - export async function getServerColumnHeaders() { + export async function getServerColumnHeaders(): Promise<ColumnHeaders> { const t = await getI18n(); return { severity: t("tests.table.severity"), result: t("tests.table.result"), title: t("tests.table.title"), provider: t("tests.table.provider"), status: t("tests.table.status"), createdAt: t("tests.table.createdAt"), assignedUser: t("tests.table.assignedUser"), }; }apps/app/src/app/[locale]/(app)/(dashboard)/tests/layout.tsx (1)
1-18: Well-structured layout component for the tests section.The layout component is properly implemented with:
- Correct async function definition
- Proper use of getI18n for localization
- Appropriate secondary menu setup
- Consistent styling with other layouts
The implementation follows the project's patterns for route-specific layouts.
Consider future expandability of the secondary menu.
Currently, there's only one item in the SecondaryMenu. If there will be more test-related sections in the future, this layout is ready to accommodate them.
apps/app/src/jobs/tasks/integration/utils/task-email-notification.tsx (2)
5-16: Consider using Date type for task.dueDate instead of string.For better type safety and consistency, consider using the Date type for the dueDate field rather than string. This would make it clearer what format is expected and ensure proper date handling.
interface Props { owner: { id: string; fullName?: string; email: string; organizationId: string; }; task: { recordId: string; - dueDate: string; + dueDate: Date; }; }
18-49: Well-implemented email notification with proper error handling.The function is well-structured with:
- Clear separation of email rendering and notification triggering
- Proper error handling with logging and re-throwing
- Good use of the TaskReminderEmail component
The overall implementation follows best practices for notification handling.
Enhance fallback name for better user experience.
The current fallback of "there" when a name isn't provided works, but consider a more personalized approach:
<TaskReminderEmail email={owner.email} - name={owner.fullName ?? "there"} + name={owner.fullName ?? owner.email.split('@')[0] ?? "User"} dueDate={task.dueDate} recordId={task.recordId} />This extracts the username part of the email as a more personalized fallback before using a generic term.
packages/integrations/src/aws/config.ts (2)
31-38: Session token shouldn't be required for standard AWS authentication.The session token is marked as required, but it's only needed for temporary credentials (as correctly noted in the comment on line 22). For standard API keys, it should be optional.
{ id: "session_token", label: "AWS session token", description: "The API session token AWS account", type: "text", - required: true, + required: false, value: "", },
18-30: Consider adding password type for the secret access key field.The secret access key should use a password field type to mask the sensitive information during input.
{ id: "secret_access_key", label: "AWS secret access key", description: "The API secret access key for your AWS account", - type: "text", + type: "password", required: true, value: "", },packages/integrations/src/aws/src/index.ts (4)
1-7: Useimport typefor type-only imports.Several imports are only used as types, which should be imported with the
import typesyntax.import { SecurityHubClient, - GetFindingsCommand, - SecurityHubClientConfig, - GetFindingsCommandInput, - GetFindingsCommandOutput + GetFindingsCommand } from "@aws-sdk/client-securityhub"; +import type { + SecurityHubClientConfig, + GetFindingsCommandInput, + GetFindingsCommandOutput +} from "@aws-sdk/client-securityhub";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-7: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
38-41: Useconstinstead ofletfor variables that aren't reassigned.
commandandallFindingsare never reassigned and should useconstinstead oflet.- let command = new GetFindingsCommand(params); - let response: GetFindingsCommandOutput = await securityHubClient.send(command); + const command = new GetFindingsCommand(params); + const response: GetFindingsCommandOutput = await securityHubClient.send(command); - let allFindings: any[] = response.Findings || []; + const allFindings: SecurityFinding[] = response.Findings || [];🧰 Tools
🪛 Biome (1.9.4)
[error] 38-38: This let declares a variable that is only assigned once.
'command' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 41-41: This let declares a variable that is only assigned once.
'allFindings' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
42-54: Add a timeout or maximum iterations to the pagination loop.The pagination loop may run indefinitely if there are too many findings. Consider adding a limit to the number of pages retrieved or a timeout mechanism.
let nextToken = response.NextToken; + let pageCount = 1; + const MAX_PAGES = 50; // Adjust based on your requirements // 3. Loop to paginate through all results if there are more than 100 findings - while (nextToken) { + while (nextToken && pageCount < MAX_PAGES) { const nextPageParams: GetFindingsCommandInput = { ...params, NextToken: nextToken }; response = await securityHubClient.send(new GetFindingsCommand(nextPageParams)); if (response.Findings) { allFindings.push(...response.Findings); } nextToken = response.NextToken; + pageCount++; } + + if (nextToken && pageCount >= MAX_PAGES) { + console.warn(`Retrieved maximum of ${MAX_PAGES} pages. There might be more findings available.`); + }
56-60: Improve error handling with more specific error information.The current error handling logs a generic message. Add more context and consider different types of AWS errors.
- console.log(`Retrieved ${allFindings.length} findings`); + console.info(`Retrieved ${allFindings.length} findings from AWS Security Hub`); return allFindings; } catch (error) { - console.error("Error fetching Security Hub findings:", error); + if (error.name === 'AccessDeniedException') { + console.error("Access denied when fetching Security Hub findings. Check IAM permissions:", error); + } else if (error.name === 'ThrottlingException') { + console.error("AWS API rate limit exceeded when fetching Security Hub findings:", error); + } else { + console.error("Error fetching Security Hub findings:", error); + } throw error; }apps/app/src/locales/no.ts (1)
718-796: Test translation keys look good, with one small improvement needed.The Norwegian translations for the tests section are well structured and comprehensive, covering all necessary UI elements.
One minor improvement: the description for the session token in line 34 has an incomplete sentence, which should be reflected in this translation section as well.
For consistency with your updated AWS integration description, consider updating the related auth_config description in the tests section:
auth_config: { label: "Autentiseringskonfigurasjon", - placeholder: "Skriv inn JSON-autentiseringskonfigurasjon" + placeholder: "Skriv inn JSON-autentiseringskonfigurasjon (inkludert midlertidige legitimasjon hvis nødvendig)" }apps/app/src/components/tables/tests/loading.tsx (1)
9-9: Move static array outside the component to avoid recreation on each render.The static array is created every time the component renders. Move it outside the component to improve performance.
import { Suspense } from "react"; import { DataTableHeader } from "./data-table-header"; - const data = [...Array(10)].map((_, i) => ({ id: i.toString() })); + // Create static array only once + const SKELETON_ROWS = [...Array(10)].map((_, i) => ({ id: i.toString() })); export function Loading({apps/app/src/app/[locale]/(app)/(dashboard)/tests/hooks/useTest.ts (4)
10-14: Simplify conditional logic by removing unnecessary else clause.The
elseclause can be omitted since the previous branch already returns a value in the success case.if (response.success) { return response.data; - } else { - throw response.error; } + throw response.error;🧰 Tools
🪛 Biome (1.9.4)
[error] 12-14: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
16-19: Ensure error type safety with more precise type checking.The current error handling checks if the error object has a 'message' property, but this doesn't guarantee it conforms to the full
AppErrortype specification. Consider implementing more robust type checking.if (error && typeof error === 'object' && 'message' in error) { - throw error as AppError; + // Ensure the error conforms to AppError interface + const appError: AppError = { + code: 'UNEXPECTED_ERROR', + message: (error as {message: string}).message + }; + throw appError; }
23-31: Consider adding a means to refresh data on demand.The current SWR configuration disables automatic revalidation on focus and reconnect. While this can reduce unnecessary network requests, it may also lead to stale data. Consider exposing the
mutatefunction more explicitly in your API to allow consumers to trigger refreshes when needed.
25-25: Add retry mechanism for network failures.The current implementation doesn't specify retry behavior for failed requests. Adding retry options would improve resilience against temporary network issues.
useSWR<Test, AppError>( testId ? ["cloud-test-details", testId] : null, () => fetchTest(testId), { revalidateOnFocus: false, revalidateOnReconnect: false, + onErrorRetry: (error, key, config, revalidate, { retryCount }) => { + // Only retry on network errors, up to 3 times + if (error.code === 'NETWORK_ERROR' && retryCount < 3) { + // Retry after 5 seconds + setTimeout(() => revalidate({ retryCount }), 5000); + } + }, } )apps/app/src/app/[locale]/(app)/(dashboard)/tests/hooks/useTests.ts (2)
13-35: Refactor repetitive error handling to reduce duplication.The error handling logic is repeated three times with similar structure. Consider creating a helper function to reduce code duplication.
+ /** Helper function to create and throw an AppError */ + function throwAppError(code: string, message: string): never { + const error: AppError = { + code, + message, + }; + throw error; + } async function fetchTests(input: TestsInput): Promise<TestsResponse> { const result = await getTests(input); if (!result) { - const error: AppError = { - code: "UNEXPECTED_ERROR", - message: "An unexpected error occurred", - }; - throw error; + throwAppError("UNEXPECTED_ERROR", "An unexpected error occurred"); } if (result.serverError) { - const error: AppError = { - code: "UNEXPECTED_ERROR", - message: result.serverError || "An unexpected error occurred", - }; - throw error; + throwAppError("UNEXPECTED_ERROR", result.serverError || "An unexpected error occurred"); } if (!result.data) { - const error: AppError = { - code: "UNEXPECTED_ERROR", - message: "No data returned from server", - }; - throw error; + throwAppError("UNEXPECTED_ERROR", "No data returned from server"); } return result.data.data as TestsResponse; }
44-46: Enhance provider validation with a type constant.The provider validation could be improved by using a constant for valid providers, which would make the code more maintainable.
+ // Define valid cloud providers as a constant + const VALID_PROVIDERS = ["AWS", "AZURE", "GCP"] as const; + type CloudProvider = typeof VALID_PROVIDERS[number]; - const provider = providerParam && ["AWS", "AZURE", "GCP"].includes(providerParam) - ? providerParam as "AWS" | "AZURE" | "GCP" - : undefined; + const provider = providerParam && VALID_PROVIDERS.includes(providerParam as CloudProvider) + ? providerParam as CloudProvider + : undefined;apps/app/src/app/[locale]/(app)/(dashboard)/tests/[testId]/actions/types.ts (2)
11-14: Enhance testId validation with additional constraints.The current schema only validates that testId is a string without additional constraints. Consider adding further validation if needed.
// Define the input schema export const cloudTestDetailsInputSchema = z.object({ - testId: z.string() + testId: z.string().min(1, "TestId is required").trim() });
16-19: Add JSDoc comments to improve type documentation.Adding detailed JSDoc comments would help other developers understand how to use the ActionResponse type correctly.
+/** + * Represents a type-safe response from a server action. + * @template T The type of data returned on success + * @returns Either a success response with data or a failure response with an error + */ export type ActionResponse<T> = Promise< { success: true; data: T } | { success: false; error: AppError } >;apps/app/src/components/tables/tests/empty-states.tsx (1)
48-49: Improve positioning approach for NoTests component.Using absolute positioning with
top-0 left-0may cause layout issues if parent elements don't haveposition: relative. Consider a more robust layout approach.-<div className="mt-24 absolute w-full top-0 left-0 flex items-center justify-center z-20"> +<div className="mt-24 flex items-center justify-center w-full z-20">apps/app/src/app/[locale]/(app)/(dashboard)/tests/actions/get-tests.ts (3)
3-3: Update the import to match project branding.The import is referencing "@bubba/db" which appears to be from an old brand name. This should be updated to maintain consistent branding throughout the codebase.
-import { db } from "@bubba/db"; +import { db } from "@compai/db";
105-105: Avoid usinganytype in transformations.Using
anytype bypasses TypeScript's type checking benefits and can lead to runtime errors. Define proper types for the integration results to maintain type safety.- const transformedTests = integrationResults.map((result: any) => { + const transformedTests = integrationResults.map((result) => {For a more complete solution, you might want to define an interface for the result type:
interface IntegrationResult { id: string; label?: string; status: string; title?: string; completedAt?: Date; organizationIntegration: { id: string; name: string; integration_id: string; }; }
123-123: Enhance error logging to include error details.The current error logging only includes a generic message. Including error details would help with debugging issues.
- console.error("Error fetching integration results:", error); + console.error("Error fetching integration results:", error instanceof Error ? error.message : error); + if (error instanceof Error && error.stack) { + console.debug(error.stack); + }apps/app/src/components/tables/tests/data-table.tsx (2)
172-179: Simplify conditional class assignment for responsive table cells.The current implementation checks every column ID in a single conditional, which is redundant and harder to maintain. This can be simplified by checking if the current column ID is in a list of responsive columns.
-className={cn({ - "hidden md:table-cell": - cell.column.id === "severity" || - cell.column.id === "result" || - cell.column.id === "title" || - cell.column.id === "provider" || - cell.column.id === "createdAt" || - cell.column.id === "assignedUser", -})} +className={cn({ + "hidden md:table-cell": ["severity", "result", "title", "provider", "createdAt", "assignedUser"].includes(cell.column.id) +})}Alternatively, if some columns should remain visible on mobile, you could maintain a list of mobile-only columns:
+const mobileHiddenColumns = ["severity", "result", "provider", "createdAt", "assignedUser"]; + +className={cn({ + "hidden md:table-cell": mobileHiddenColumns.includes(cell.column.id) +})}
164-167: Add error handling for navigation to test details.The component navigates to a test detail page when a row is clicked, but it doesn't handle the case where the test ID might be missing or invalid.
onClick={() => { const test = row.original; - router.push(`/tests/${test.id}`); + if (test?.id) { + router.push(`/tests/${test.id}`); + } else { + console.warn('Cannot navigate: Test ID is missing'); + } }}apps/app/src/app/[locale]/(app)/(dashboard)/tests/components/TestsList.tsx (1)
67-67: Remove unnecessary type cast to TestType[].The type assertion
tests as TestType[]indicates a potential type mismatch. It would be better to ensure the types align properly between whatuseTestsreturns and whatDataTableexpects.<DataTable columnHeaders={columnHeaders} - data={tests as TestType[]} + data={tests} pageCount={Math.ceil(total / per_page)} currentPage={page} />To fix this properly, ensure that the
useTestshook returns the correct type that matches theTestTypeinterface.apps/app/src/app/[locale]/(app)/(dashboard)/tests/[testId]/components/Test.tsx (3)
60-66: Consider using an enum or constants for provider values.The provider mapping logic could be more maintainable with a lookup object rather than nested ternary operators.
- // Format the test provider for display - const providerLabel = cloudTest.provider === "aws" - ? "Amazon Web Services" - : cloudTest.provider === "AZURE" - ? "Microsoft Azure" - : "Google Cloud Platform"; + // Format the test provider for display + const providerLabels = { + aws: "Amazon Web Services", + AZURE: "Microsoft Azure", + GCP: "Google Cloud Platform" + }; + const providerLabel = providerLabels[cloudTest.provider as keyof typeof providerLabels] || cloudTest.provider;
82-99: Simplify the getRunStatusIcon function by removing unnecessary else clauses.The function contains several unnecessary else clauses that can be removed since previous branches already return.
// Helper function to get the appropriate icon for test run status const getRunStatusIcon = (status: string, result: string | null) => { if (status === "COMPLETED") { if (result === "PASS") { return <CheckCircle2 className="h-4 w-4 text-green-500" />; } else if (result === "FAIL") { return <XCircle className="h-4 w-4 text-red-500" />; - } else { - return <Info className="h-4 w-4 text-blue-500" />; } + return <Info className="h-4 w-4 text-blue-500" />; } else if (status === "IN_PROGRESS") { return <Clock className="h-4 w-4 text-blue-500 animate-pulse" />; } else if (status === "PENDING") { return <Clock className="h-4 w-4 text-yellow-500" />; - } else { - return <Info className="h-4 w-4" />; } + return <Info className="h-4 w-4" />; };🧰 Tools
🪛 Biome (1.9.4)
[error] 87-91: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 89-91: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
[error] 92-98: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 94-98: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
[error] 96-98: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
197-199: Consider adding a fallback message when no result details are available.If
cloudTest.resultDetailsis undefined, the pre block will only show "undefined". Adding a fallback message would be more user-friendly.<pre className="bg-muted p-4 rounded-md overflow-auto text-sm"> - {JSON.stringify(cloudTest.resultDetails, null, 2)} + {cloudTest.resultDetails + ? JSON.stringify(cloudTest.resultDetails, null, 2) + : "No detailed results available"} </pre>run-security-hub.ts (2)
26-28: Optimize performance by using for...of instead of forEach.For large arrays, replacing forEach with for...of can lead to better performance.
- Object.entries(groupedBySeverity).forEach(([severity, count]) => { - console.log(` ${severity}: ${count} findings`); - }); + for (const [severity, count] of Object.entries(groupedBySeverity)) { + console.log(` ${severity}: ${count} findings`); + }🧰 Tools
🪛 Biome (1.9.4)
[error] 26-28: Prefer for...of instead of forEach.
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
(lint/complexity/noForEach)
37-44: Consider adding command line options for filtering findings.The script could be more versatile by accepting command line arguments to filter findings by severity, age, or other criteria.
+#!/usr/bin/env bun + +import { fetchSecurityFindings } from './packages/integrations/src/aws/src/index'; +import { parseArgs } from 'node:util'; + +// Parse command line arguments +const { values } = parseArgs({ + options: { + severity: { + type: 'string', + short: 's', + default: 'all' + }, + limit: { + type: 'string', + short: 'l', + default: '10' + } + } +}); + +const severityFilter = values.severity; +const limit = parseInt(values.limit as string); // Then in your highSeverityFindings filter: const highSeverityFindings = findings .filter(f => { if (severityFilter === 'all') { return f.Severity?.Label === 'CRITICAL' || f.Severity?.Label === 'HIGH'; } return f.Severity?.Label === severityFilter.toUpperCase(); }) .slice(0, limit);Would you like me to create a pull request that enhances this script with command-line options for improved functionality?
apps/app/src/actions/tests/register-test-action.ts (2)
9-26: Consider extracting DEFAULT_TASKS to a shared constants file.These default tasks might be needed in other parts of the application. Consider moving them to a shared constants file for reusability.
-const DEFAULT_TASKS = [ - { - code: "POLICY-ACCEPT", - name: "Policy Acceptance", - description: "Review and accept company policies", - }, - { - code: "INSTALL-AGENT", - name: "Install Monitoring Agent", - description: - "Install and configure the security monitoring agent on your device", - }, - { - code: "DEVICE-SECURITY", - name: "Device Security", - description: "Complete device security checklist and configuration", - }, -] as const; +import { DEFAULT_TASKS } from "@/constants/employeeTasks";
62-74: Consider using Prisma's createMany for better performance.Using
Promise.allwith multiple database operations can be less efficient than using Prisma's bulk operations.- // Create or get the required task definitions first and store their IDs - const requiredTasks = await Promise.all( - DEFAULT_TASKS.map(async (task) => { - return db.employeeRequiredTask.upsert({ - where: { code: task.code }, - create: { - code: task.code, - name: task.name, - description: task.description, - }, - update: {}, - }); - }) - ); + // Create or get the required task definitions first and store their IDs + // Use transaction to ensure atomicity + const requiredTasks = await db.$transaction( + DEFAULT_TASKS.map(task => + db.employeeRequiredTask.upsert({ + where: { code: task.code }, + create: { + code: task.code, + name: task.name, + description: task.description, + }, + update: {}, + }) + ) + );apps/app/src/jobs/tasks/integration/integration-schedule.ts (1)
46-62: Consider partial-failure handling and retry strategies.While you log errors and return a failure status if any trigger fails, partial successes remain untracked if only some triggers fail. If partial success is acceptable, think about individually retrying only the failed triggers instead of failing the entire batch. For large sets of integrations, a more granular approach can prevent losing valid integration runs.
apps/app/src/components/tables/tests/filter-toolbar.tsx (2)
70-76: Unify refresh actions for responsive layouts.You have separate refresh button blocks for mobile (
md:hidden) and desktop (md:flex). While they serve responsive layouts, consider extracting them into a shared UI component or function to reduce future maintenance overhead.
79-99: Avoid duplicating the refresh button.Lines 72-75 and 96-99 both include essentially the same “Refresh” button. Extract a dedicated
<RefreshButton>to keep layout logic simpler and consistent.-<Button asChild variant="action"> - <Link href="/integrations"> - {t("tests.actions.refresh")} - </Link> -</Button> +function RefreshButton() { + return ( + <Button asChild variant="action"> + <Link href="/integrations"> + {t("tests.actions.refresh")} + </Link> + </Button> + ); +}apps/app/src/jobs/tasks/integration/integration-results.ts (4)
7-12: Useimport typefor type-only imports.The static analysis indicates some named imports appear to be used only as types (e.g.
GetFindingsCommandOutput). To reduce bundle size and improve clarity, import them withimport typerather than a runtime import.-import { +import type { SecurityHubClientConfig, GetFindingsCommandInput, GetFindingsCommandOutput } from "@aws-sdk/client-securityhub";🧰 Tools
🪛 Biome (1.9.4)
[error] 6-12: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
43-43: Useconstfor variables assigned only once.
commandis never reassigned after its creation. This is a minor style improvement to convey immutability and clarity.-let command = new GetFindingsCommand(params); +const command = new GetFindingsCommand(params);🧰 Tools
🪛 Biome (1.9.4)
[error] 43-43: This let declares a variable that is only assigned once.
'command' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
46-46: UseconstforallFindingswhenever possible.Similarly,
allFindingsremains unchanged after initialization, so usingconstis more appropriate.-let allFindings: any[] = response.Findings || []; +const allFindings: any[] = response.Findings || [];🧰 Tools
🪛 Biome (1.9.4)
[error] 46-46: This let declares a variable that is only assigned once.
'allFindings' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
135-137: Omit the else clause when the preceding block returns.Since you return at line 134, the else block at lines 135-137 is unreachable. You can remove it to simplify the flow.
} else { logger.warn(`Integration ${integrationId} does not have a fetchSecurityFindings method`); }🧰 Tools
🪛 Biome (1.9.4)
[error] 135-137: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
apps/app/src/app/[locale]/(app)/(dashboard)/tests/[testId]/actions/get-test.ts (4)
3-10: Consider usingimport typefor the Test importThe
Testimport is only used as a type and not as a value. Importing it withimport typeensures that it will be removed by the compiler and avoids loading unnecessary modules.import { db } from "@bubba/db"; import { auth } from "@/auth"; import { appErrors, - type ActionResponse + type ActionResponse } from "./types"; -import { Test } from "../../types"; +import type { Test } from "../../types";🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
23-31: Consider adding parameterized queries for better securityWhile Prisma's
$queryRawdoes provide some SQL injection protection, using template literals directly with user input is generally not the best practice. Consider using parameterized queries with Prisma's tagged template literal approach.// Using raw SQL query to get the result since the Prisma client property name is causing issues const results = await db.$queryRaw<any[]>` SELECT r.*, i.id as "integrationId", i.name as "integrationName", i.integration_id, i.settings, i.user_settings FROM "Organization_integration_results" r JOIN "OrganizationIntegrations" i ON r."organizationIntegrationId" = i.id - WHERE r.id = ${testId} AND r."organizationId" = ${organizationId} + WHERE r.id = ${db.raw(testId)} AND r."organizationId" = ${db.raw(organizationId)} LIMIT 1 `;
42-47: Document the reason for placeholder user objectThe comment mentions "Create a placeholder user object since the schema doesn't have user info anymore" but doesn't explain why this approach was chosen or if this is a temporary solution. Consider adding more detailed documentation about this design decision.
// Create a placeholder user object since the schema doesn't have user info anymore + // TODO: This is a temporary solution until we implement proper user assignment for tests + // The schema no longer contains user information directly, so we're using session data const placeholderUser = { id: session.user.id, name: session.user.name || null, email: session.user.email || null, };
50-62: Validate all required fields before mapping to the Test objectThe current implementation assumes that all required fields exist in the integration result. Consider adding validation to ensure all required fields are present before mapping to prevent runtime errors.
// Format the result to match the expected CloudTestResult structure + // Ensure all required fields exist + if (!integrationResult.id || !integrationResult.integration_id) { + throw new Error("Missing required fields in integration result"); + } + const result: Test = { id: integrationResult.id, title: integrationResult.title || integrationResult.integrationName, description: typeof integrationResult.resultDetails === 'object' && integrationResult.resultDetails ? (integrationResult.resultDetails as any).description || "" : "", provider: integrationResult.integration_id, status: integrationResult.status, resultDetails: integrationResult.resultDetails, label: integrationResult.label, assignedUserId: placeholderUser, completedAt: integrationResult.completedAt, };apps/app/src/components/tables/tests/data-table-pagination.tsx (2)
51-64: Consider using the per_page parameter from the URL with a default value fallbackThe component correctly uses the
per_pageparameter from the URL, but it would be better to also provide a default value if the parameter is missing or invalid.<Select - value={searchParams.get("per_page") || "10"} + value={searchParams.get("per_page") || "10"} + defaultValue="10" onValueChange={createPerPageQuery} >
47-87: Consider handling edge case when pageCount is 0If there are no items or pages (pageCount = 0), the component might display "1 of 0" which is confusing. Consider handling this edge case.
<div className="flex items-center justify-between mt-4"> <div className="flex items-center space-x-2"> <Select value={searchParams.get("per_page") || "10"} onValueChange={createPerPageQuery} > <SelectTrigger className="h-8 w-[70px]"> <SelectValue placeholder="10" /> </SelectTrigger> <SelectContent side="top"> {[10, 20, 30, 40, 50].map((pageSize) => ( <SelectItem key={pageSize} value={pageSize.toString()}> {pageSize} </SelectItem> ))} </SelectContent> </Select> </div> + {pageCount > 0 ? ( <div className="flex items-center space-x-2"> <Button variant="outline" className="h-8 w-8 p-0" onClick={() => createPageQuery(currentPage - 1)} disabled={currentPage <= 1} > <ChevronLeft className="h-4 w-4" /> </Button> <div className="flex w-[60px] items-center justify-center text-sm font-medium"> {currentPage} of {pageCount} </div> <Button variant="outline" className="h-8 w-8 p-0" onClick={() => createPageQuery(currentPage + 1)} disabled={currentPage >= pageCount} > <ChevronRight className="h-4 w-4" /> </Button> </div> + ) : ( + <div className="flex w-[60px] items-center justify-center text-sm font-medium"> + No pages + </div> + )} </div>apps/app/src/components/tables/tests/data-table-header.tsx (2)
52-57: Consider refactoring isVisible to be more robustThe current implementation of
isVisiblehas an optional chaining that might not work as expected iftableis undefined. A more robust implementation would check iftableexists before continuing.const isVisible = (id: string) => loading || - table - ?.getAllLeafColumns() - .find((col) => col.id === id) - ?.getIsVisible(); + (table && table.getAllLeafColumns().find((col) => col.id === id)?.getIsVisible());
25-25: Unused isEmpty propThe component accepts an
isEmptyprop in its type definition, but this prop is not used in the component implementation.- export function DataTableHeader({ table, loading }: Props) { + export function DataTableHeader({ table, loading, isEmpty }: Props) {Or remove it from the Props type if it's truly not needed:
type Props = { table?: { getIsAllPageRowsSelected: () => boolean; getIsSomePageRowsSelected: () => boolean; getAllLeafColumns: () => { id: string; getIsVisible: () => boolean; }[]; toggleAllPageRowsSelected: (value: boolean) => void; }; loading?: boolean; - isEmpty?: boolean; };apps/app/src/app/[locale]/(app)/(dashboard)/tests/types/index.ts (2)
19-25: Add validation rules to input schemaThe current
testsInputSchemalacks validation rules for its fields. Consider adding more specific validation for better type safety and user input handling.export const testsInputSchema = z.object({ - search: z.string().optional(), + search: z.string().trim().optional(), provider: z.enum(["AWS", "AZURE", "GCP"]).optional(), - status: z.string().optional(), + status: z.string().min(1).max(50).optional(), - page: z.number().optional(), + page: z.number().int().positive().optional(), - per_page: z.number().optional(), + per_page: z.number().int().min(1).max(100).optional(), });
48-57: Add more specific error codesThe current error codes are quite generic. Consider adding more specific error codes for better error handling and user feedback.
export const appErrors = { UNAUTHORIZED: { code: "UNAUTHORIZED" as const, message: "You are not authorized to view employees", }, UNEXPECTED_ERROR: { code: "UNEXPECTED_ERROR" as const, message: "An unexpected error occurred", }, + NOT_FOUND: { + code: "NOT_FOUND" as const, + message: "The requested test was not found", + }, + VALIDATION_ERROR: { + code: "VALIDATION_ERROR" as const, + message: "Invalid input parameters", + }, } as const;packages/db/prisma/schema.prisma (4)
162-163: Consider removing unnecessary empty line.There's an empty line at line 162 that doesn't serve any purpose and is inconsistent with the spacing pattern used throughout the rest of the schema file.
organizationId String organization Organization @relation(fields: [organizationId], references: [id], onDelete: Cascade) - results OrganizationIntegrationResults[]
1076-1095: Consider using an enum for the status field.The
statusfield is defined as a required string, but it would benefit from being constrained to a predefined set of values using an enum. This would ensure consistency and prevent invalid status values from being stored.+enum IntegrationResultStatus { + passed + failed + error + in_progress + pending +} model OrganizationIntegrationResults { id String @id @default(cuid()) title String? - status String + status IntegrationResultStatus label String? // PASS, FAIL, ERROR resultDetails Json? // Stores detailed test results completedAt DateTime? @default(now()) organizationIntegrationId String organizationId String assignedUserId String? assignedUser User? @relation(fields: [assignedUserId], references: [id], onDelete: Cascade) organizationIntegration OrganizationIntegrations @relation(fields: [organizationIntegrationId], references: [id], onDelete: Cascade) organization Organization @relation(fields: [organizationId], references: [id], onDelete: Cascade) @@index([assignedUserId]) @@index([organizationIntegrationId]) @@index([organizationId]) @@map("Organization_integration_results") }
1081-1082: Consider making label more type-safe.The
labelfield has a comment indicating it can be "PASS", "FAIL", or "ERROR", but it's defined as an optional string. Consider using an enum to enforce these values for better type safety and data integrity.+enum IntegrationResultLabel { + pass + fail + error +} model OrganizationIntegrationResults { id String @id @default(cuid()) title String? status String - label String? // PASS, FAIL, ERROR + label IntegrationResultLabel? resultDetails Json? // Stores detailed test results // rest of model... }
1087-1087: Verify onDelete cascade behavior implications.Using
onDelete: Cascadefor the User relationship means that if a user is deleted, all their assigned test results will also be deleted. Ensure this is the intended behavior, or consider usingSetNullinstead to preserve test history while removing the assignee reference.You may want to consider whether preserving test history is more important than maintaining referential integrity when a user is deleted. Using
onDelete: SetNullwould keep the test results but set theassignedUserIdto null, which might be preferable for audit purposes.packages/db/prisma/migrations/20250227185651_add_organization_integration_results/migration.sql (1)
7-7: Consider adding constraint for JSON validation.For the
resultDetailsJSONB field, consider adding a check constraint to validate the JSON structure if you have a specific expected format.While PostgreSQL doesn't provide built-in JSON schema validation, you could add a basic check constraint to ensure certain required fields are present:
CREATE TABLE "Organization_integration_results" ( // other fields... "resultDetails" JSONB, + CONSTRAINT "resultDetails_has_required_fields" CHECK ("resultDetails" ? 'testId' AND "resultDetails" ? 'findings'), // remaining fields... );Note: This is just an example. Adjust the fields based on your actual required structure.
apps/app/src/locales/en.ts (2)
614-615: Remove unnecessary blank line.There's an empty line after the
invalid_jsonproperty that doesn't serve any purpose and is inconsistent with the spacing pattern in the rest of the file.invalid_json: "Invalid JSON configuration provided", - title_field: {
633-634: Consider adding validation instructions for JSON fields.The translation for authentication configuration mentions JSON format but doesn't provide guidance on required structure or format. Consider adding a tooltip or help text to assist users in entering valid JSON.
auth_config: { label: "Authentication Configuration", - placeholder: "Enter JSON authentication configuration" + placeholder: "Enter JSON authentication configuration", + tooltip: "Enter valid JSON with required authentication details like API keys or credentials. Example: { \"apiKey\": \"your-key\", \"region\": \"us-east-1\" }" }apps/app/src/locales/fr.ts (1)
182-183: Ensure consistent formatting in sidebar entries.There's an unnecessary comma after the
peopleentry. While this doesn't affect functionality, it's inconsistent with the formatting in the rest of the file.- people: "Personnes", + people: "Personnes" tests: "Tests en nuage"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
apps/app/languine.lockis excluded by!**/*.lockbun.lockis excluded by!**/*.lockpackages/integrations/src/aws/assets/image.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (43)
apps/app/src/actions/schema.ts(1 hunks)apps/app/src/actions/tests/register-test-action.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/tests/[testId]/actions/get-test.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/tests/[testId]/actions/types.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/tests/[testId]/components/Test.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/tests/[testId]/page.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/tests/actions/get-tests.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/tests/components/TestsList.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/tests/components/TestsListSkeleton.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/tests/hooks/useTest.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/tests/hooks/useTests.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/tests/layout.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/tests/page.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/tests/types/index.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/tests/types/search-params.ts(1 hunks)apps/app/src/components/main-menu.tsx(2 hunks)apps/app/src/components/tables/tests/columns.tsx(1 hunks)apps/app/src/components/tables/tests/data-table-header.tsx(1 hunks)apps/app/src/components/tables/tests/data-table-pagination.tsx(1 hunks)apps/app/src/components/tables/tests/data-table.tsx(1 hunks)apps/app/src/components/tables/tests/empty-states.tsx(1 hunks)apps/app/src/components/tables/tests/filter-toolbar.tsx(1 hunks)apps/app/src/components/tables/tests/loading.tsx(1 hunks)apps/app/src/components/tables/tests/server-columns.tsx(1 hunks)apps/app/src/jobs/tasks/integration/integration-results.ts(1 hunks)apps/app/src/jobs/tasks/integration/integration-schedule.ts(1 hunks)apps/app/src/jobs/tasks/integration/utils/task-email-notification.tsx(1 hunks)apps/app/src/locales/en.ts(2 hunks)apps/app/src/locales/es.ts(2 hunks)apps/app/src/locales/fr.ts(2 hunks)apps/app/src/locales/no.ts(2 hunks)apps/app/src/locales/pt.ts(2 hunks)package.json(1 hunks)packages/db/prisma/migrations/20250227185651_add_organization_integration_results/migration.sql(1 hunks)packages/db/prisma/schema.prisma(4 hunks)packages/integrations/src/aws/assets/logo.tsx(1 hunks)packages/integrations/src/aws/config.ts(1 hunks)packages/integrations/src/aws/src/index.ts(1 hunks)packages/integrations/src/github/config.ts(1 hunks)packages/integrations/src/gusto/config.ts(1 hunks)packages/integrations/src/index.ts(1 hunks)packages/ui/src/components/icons.tsx(2 hunks)run-security-hub.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/integrations/src/github/config.ts
- packages/integrations/src/gusto/config.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/integrations/src/aws/src/index.ts
[error] 1-7: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
[error] 38-38: This let declares a variable that is only assigned once.
'command' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 41-41: This let declares a variable that is only assigned once.
'allFindings' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
run-security-hub.ts
[error] 26-28: Prefer for...of instead of forEach.
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
(lint/complexity/noForEach)
apps/app/src/app/[locale]/(app)/(dashboard)/tests/[testId]/actions/get-test.ts
[error] 10-10: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
apps/app/src/jobs/tasks/integration/integration-results.ts
[error] 135-137: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 6-12: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
[error] 43-43: This let declares a variable that is only assigned once.
'command' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 46-46: This let declares a variable that is only assigned once.
'allFindings' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 97-97: Unexpected constant condition.
(lint/correctness/noConstantCondition)
[error] 100-100: Unexpected constant condition.
(lint/correctness/noConstantCondition)
apps/app/src/app/[locale]/(app)/(dashboard)/tests/hooks/useTest.ts
[error] 12-14: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
apps/app/src/app/[locale]/(app)/(dashboard)/tests/[testId]/components/Test.tsx
[error] 87-91: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 89-91: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
[error] 92-98: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 94-98: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
[error] 96-98: This else clause can be omitted because previous branches break early.
(lint/style/noUselessElse)
🔇 Additional comments (55)
packages/ui/src/components/icons.tsx (2)
19-19: Clean icon import added correctly.The MdCloudSync icon is correctly imported from the react-icons/md package, following the established alphabetical ordering pattern of the existing imports.
735-735: Icon properly added to the exports.The CloudSync icon is correctly added to the Icons export object, maintaining consistency with the existing pattern in the file.
package.json (1)
28-28:Details
✅ Verification successful
Verify version compatibility of AWS Security Hub client.
The addition of this AWS SDK client is appropriate for integrating with AWS Security Hub functionality. Consider whether this dependency should be scoped to the integrations package rather than at the root level if it's only used there.
🏁 Script executed:
#!/bin/bash # Check where the AWS Security Hub client is being used echo "Searching for imports of @aws-sdk/client-securityhub..." rg --type ts "@aws-sdk/client-securityhub" -A 3 -B 3 echo "Checking if any workspace-specific package.json already includes this dependency..." fd "package.json" packages apps | xargs grep -l "@aws-sdk/client-securityhub" || echo "Not found in workspace package.json files"Length of output: 1586
Dependency Usage Confirmed: Continue with Current Setup
We've verified that the AWS Security Hub client (
@aws-sdk/client-securityhubat version^3.751.0) is imported in both:
apps/app/src/jobs/tasks/integration/integration-results.tspackages/integrations/src/aws/src/index.tsSince the dependency isn’t isolated solely to the integrations package, keeping it in the root appears justified. Please ensure that the SDK version remains compatible with your AWS integration workflows through your standard testing.
packages/integrations/src/index.ts (2)
3-3: AWS integration import added correctly.The Aws import is properly added from the correct path.
5-5: AWS integration properly added to exported integrations array.The AWS integration is correctly added to the integrations export array alongside the existing GitHub and Gusto integrations.
apps/app/src/app/[locale]/(app)/(dashboard)/tests/types/search-params.ts (1)
1-5: Appropriate imports from nuqs/server.The imports are correctly specified for the search parameters caching functionality.
apps/app/src/app/[locale]/(app)/(dashboard)/tests/components/TestsListSkeleton.tsx (2)
1-2: Looks good - imports are minimal and properly organized.The component imports only what it needs - the
FilterToolbarfor layout consistency and theSkeletoncomponent for loading state visualization.
4-7: Looks good - component structure follows best practices.The component is properly exported and has a clear, focused responsibility: displaying a loading state for the tests list.
apps/app/src/components/tables/tests/columns.tsx (2)
1-2: The "use client" directive suggests this component might be used in both client and server contexts.The "use client" directive indicates this component is intended for client-side usage, but the interface itself doesn't have any client-specific functionality.
12-18: Good explanatory comments about the file's purpose.The comments clearly explain that column definitions have been moved to another file and this file now serves a more focused purpose.
packages/integrations/src/aws/assets/logo.tsx (1)
10-14: Resolve mismatch between viewBox and enableBackground dimensions.There's a mismatch between the viewBox dimensions (500x182) and the enableBackground dimensions (304x182), which might cause scaling issues.
width={131.417} height={50} viewBox="0 0 500 182" - style={{ enableBackground: "new 0 0 304 182" }} + style={{ enableBackground: "new 0 0 500 182" }} xmlSpace="preserve">apps/app/src/components/tables/tests/server-columns.tsx (1)
1-5: Function follows the server component pattern correctly.The import and async function setup demonstrate proper usage of React Server Components with i18n.
apps/app/src/components/main-menu.tsx (2)
31-31: Good addition of the cloud sync icon for tests path.The new icon choice for the tests section is appropriate and consistent with the application's icon naming conventions.
187-191: Successfully added new menu item for tests section.The addition of the tests menu item follows the established pattern in the codebase, correctly using the translation function and setting appropriate defaults.
packages/integrations/src/aws/config.ts (1)
8-12:Details
❓ Verification inconclusive
Logo component should follow AWS brand guidelines.
Ensure the Logo component adheres to AWS's brand guidelines for partners and integrations. AWS typically has specific requirements for how their logo can be displayed in third-party applications.
🌐 Web query:
What are the AWS brand guidelines for logos in partner integrations?💡 Result:
AWS brand guidelines for logos in partner integrations are designed to maintain consistency, clarify partnerships, and ensure proper representation of AWS's role. Key requirements include:
Logo Usage Tiers & Permissions
- Select Tier+ Access: AWS Logo usage is restricted to Select tier partners and above for indicating solution support, cloud provider relationships, or technical integration[2][7].
- Badge Manager: Partners create custom badges (designation, certification, qualified devices/software) via AWS Partner Central to showcase achievements[1][7].
Messaging & Placement Guidelines
- Phrasing: Use "runs on AWS" or "integrates with AWS" instead of "in AWS"[2][3].
- Logo Placement:
- Partner-Led Scenarios: Partner logo primary (top-left), AWS branding secondary (e.g., badges in bottom-right)[3][7].
- AWS-Led Scenarios: AWS logo primary; partner logos appear under "In collaboration with" in secondary positions[3][9].
- Co-Branding: Requires written AWS approval and shared campaign control[3][9].
Design & Technical Specifications
- Clear Space: Maintain clear space around logos equal to the height of the logo's first character[2][3].
- Color Variations: Use Squid Ink/white logos on light/dark backgrounds respectively[3][8].
- Prohibited Uses:
- Ads, social media, or promotional items[2].
- Combining AWS logos with partner logos using connectors (e.g., "+", "&")[2].
- Standalone AWS logos in shapes/illustrations without clear space[2][3].
Regional Restrictions
- Mainland China: Use Amazon Cloud Technologies logos instead of AWS logos[3][8].
- Global Use: AWS logos are permitted worldwide except China[3][8].
Approved Brand Treatments
- Powered by AWS Logo: For solutions built on AWS infrastructure[3][7].
- AWS Marketplace Logo: For solutions listed on AWS Marketplace[1][7].
- Certification Badges: Display active AWS Certifications (e.g., "100 AWS Certifications")[3][7].
Partners can access the full AWS Partner Creative and Messaging Guide, logos, and templates via the Marketing Toolkit in AWS Partner Central[1][3][7].
Citations:
- 1: https://aws.amazon.com/blogs/apn/announcing-the-new-aws-partner-creative-and-messaging-guide/
- 2: https://awsglobalsponsorshipresources.s3-us-west-2.amazonaws.com/AWS_Content_Graphics_Collateral_Giveaway_Rules_Guidelines_AWS_Sponsorship_Events_2020.pdf
- 3: https://awsreinforce-2024.s3.amazonaws.com/AWS+Partner+Creative+and+Messaging+Guide+2023.pdf
- 4: https://docs.aws.amazon.com/securityhub/latest/partnerguide/guidelines-console-logo.html
- 5: https://aws.amazon.com/blogs/apn/tag/aws-partner-branding/
- 6: https://aws.amazon.com/blogs/apn/new-apn-marketing-toolkit-helps-partners-showcase-their-expertise-on-aws/
- 7: https://aws.amazon.com/partners/branding/
- 8: https://d1.awsstatic.com/awsmp/solutions/buy-with-aws/Buy-with-AWS-Supplemental-Branding-Guidelines-12-4-24.pdf
- 9: https://aws.amazon.com/trademark-guidelines/
- 10: https://www.aws.org/community-and-events/sections-and-student-chapters/digital-marketing-kit-for-sections/
Action Required: Update AWS Logo Compliance
Please ensure the
Logocomponent strictly adheres to AWS brand guidelines for partner integrations. In particular, verify that:
- The logo usage complies with AWS’ permitted tiers and approved brand treatments.
- Proper clear space is maintained (equal to the height of the logo’s first character).
- Appropriate color variations and placement guidelines are followed (e.g., correct messaging such as "runs on AWS" or "integrates with AWS" and logo positioning based on partnership context).
Refer to the AWS Partner Creative and Messaging Guide (as detailed in the provided web query results) for all specific requirements.
apps/app/src/locales/no.ts (1)
182-183: Ensure consistent comma formatting in the sidebar section.The comma after "Personer" on line 182 appears to be a formatting change. Make sure this follows the pattern used in other localization files for consistency.
apps/app/src/components/tables/tests/loading.tsx (1)
57-57: Empty table cell might need a skeleton or accessibility label.The last table cell is empty, which might be intentional for an action column, but consider adding a visual indicator or accessibility context.
Is this empty cell intended for an actions column? If so, consider adding a minimal skeleton representation for consistency:
- <TableCell className="w-[50px]" /> + <TableCell className="w-[50px]"> + {!isEmpty && ( + <Skeleton + className="h-[20px] w-[20px] rounded-full opacity-40" + aria-hidden="true" + /> + )} + </TableCell>apps/app/src/app/[locale]/(app)/(dashboard)/tests/hooks/useTests.ts (1)
37-37:Details
❓ Verification inconclusive
Verify the nested data structure and avoid double type assertion.
The line
return result.data.data as TestsResponsesuggests a confusing API design with nested data properties or unnecessary type assertions. Consider restructuring or adding comments to explain the data structure.
🏁 Script executed:
#!/bin/bash # Check the actual structure of the response from getTests ast-grep --pattern 'function getTests($_, $_) { $$$ return $_ }'Length of output: 70
Action Required: Verify and Clarify Nested Data Structure
The current implementation of
return result.data.data as TestsResponse;relies on a nested property (
data.data), which can be confusing and may indicate a redundant or unintended API response structure. Since our initial AST search for thegetTestsfunction’s return structure provided no clear output, please verify manually that:
- The API response is intentionally structured with nested
dataproperties.- A single type assertion wouldn’t suffice, or if additional documentation/comments could clarify why a double access (i.e.,
result.data.data) is necessary.- There isn’t an opportunity to restructure the response (or the type definitions) to avoid potential double assertions.
Once you confirm that the current design is intentional, consider adding an inline comment or refactoring the data structure to improve readability and maintainability.
apps/app/src/app/[locale]/(app)/(dashboard)/tests/actions/get-tests.ts (1)
110-114:Details
❓ Verification inconclusive
Handle potential null/undefined values in result transformation.
The code assumes that
result.organizationIntegrationalways exists and has the expected properties, which might cause runtime errors if the data structure changes.Consider adding safeguards:
return { id: result.id, severity: result.label, result: result.status, - title: result.title || result.organizationIntegration.name, - provider: result.organizationIntegration.integration_id, + title: result.title || (result.organizationIntegration?.name ?? 'Unnamed Test'), + provider: result.organizationIntegration?.integration_id ?? 'unknown', createdAt: result.completedAt || new Date(), // The executedBy information is no longer available in the new schema assignedUser: null, };
🏁 Script executed:
#!/bin/bash # Check if there are any integration results without valid organizationIntegration data # This will help us understand if this is a realistic concern cat << 'EOF' | node const { PrismaClient } = require('@prisma/client'); const prisma = new PrismaClient(); async function checkIntegrationResults() { try { const results = await prisma.organizationIntegrationResults.findMany({ where: { OR: [ { organizationIntegration: null }, { organizationIntegration: { name: null } }, { organizationIntegration: { integration_id: null } } ] }, select: { id: true } }); console.log(`Found ${results.length} integration results with missing organizationIntegration data`); if (results.length > 0) { console.log('Sample IDs:', results.slice(0, 5).map(r => r.id)); } } catch (error) { console.error('Error checking integration results:', error); } finally { await prisma.$disconnect(); } } checkIntegrationResults(); EOFLength of output: 789
Action Required: Verify Integration Data Handling in Production Environment
It appears that the verification script did not produce useful output due to a missing dependency (
@prisma/client). Because the script failed in this sandbox environment, it's important that you manually verify that missing or incompleteorganizationIntegrationdata is handled correctly in your production or development environment.A few points to note:
- Check Environment Dependencies: Ensure that the environment used for manual testing has the
@prisma/clientmodule installed.- Code Safety: Update the transformation logic to safely handle cases where
result.organizationIntegrationmight be null or missing expected properties.- Manual Verification: Re-run the integration data checks with a correctly configured environment to confirm that the database schema and corresponding data meet the new assumptions.
Below is the recommended diff snippet that applies safeguards by using optional chaining and default values:
return { id: result.id, severity: result.label, result: result.status, - title: result.title || result.organizationIntegration.name, - provider: result.organizationIntegration.integration_id, + title: result.title || (result.organizationIntegration?.name ?? 'Unnamed Test'), + provider: result.organizationIntegration?.integration_id ?? 'unknown', createdAt: result.completedAt || new Date(), // The executedBy information is no longer available in the new schema assignedUser: null, };Please verify these changes in an environment where the integration data can be properly checked.
apps/app/src/components/tables/tests/data-table.tsx (1)
138-142:Details
❓ Verification inconclusive
Add validation for column headers to prevent runtime errors.
The code assumes that all column IDs exist in the provided column headers object, which could lead to runtime errors if there's a mismatch between the expected headers and what's provided.
Consider adding validation or default values:
const columns = clientColumns.map((col) => ({ ...col, - header: columnHeaders[col.id as keyof typeof columnHeaders], + header: columnHeaders[col.id as keyof typeof columnHeaders] || col.id, accessorFn: (row: TestType) => row[col.id as keyof TestType], }));
🏁 Script executed:
#!/bin/bash # Check if all required column headers are consistently provided across the codebase # Find all instances where DataTable is imported echo "Looking for DataTable imports:" rg -A 3 "import.*DataTable" --type ts --type tsx # Find where columnHeaders are being passed to the DataTable echo "\nLooking for columnHeaders prop passed to DataTable:" rg "columnHeaders=\{.*\}" --type tsx -A 3Length of output: 338
Below is an updated request for manual verification with an improved shell script to search for DataTable imports and the use of the columnHeaders prop in both TypeScript (.ts) and TSX (.tsx) files.
Please verify the consistency of the columnHeaders usage across the codebase
The original verification command produced errors due to unrecognized file types for TSX files. To ensure that our validation for column headers is comprehensive and that no runtime errors will occur, please run the following updated shell script:#!/bin/bash echo "Searching for DataTable imports in .ts and .tsx files:" rg "import.*DataTable" -g "*.ts" -g "*.tsx" -A 3 echo -e "\nSearching for columnHeaders prop usage in .ts and .tsx files:" rg "columnHeaders=\{.*\}" -g "*.ts" -g "*.tsx" -A 3If the output indicates that there are cases where a column header might be missing, consider updating the mapping as suggested:
const columns = clientColumns.map((col) => ({ ...col, - header: columnHeaders[col.id as keyof typeof columnHeaders], + header: columnHeaders[col.id as keyof typeof columnHeaders] || col.id, accessorFn: (row: TestType) => row[col.id as keyof TestType], }));This fallback ensures that if a column header isn’t provided, the column ID is used instead, preventing potential runtime errors.
apps/app/src/app/[locale]/(app)/(dashboard)/tests/[testId]/components/Test.tsx (2)
22-34: Good error handling implementation.The error state is properly handled with an informative alert that displays the specific error message or falls back to a generic one.
36-58: Loading state implementation is well-designed.Good use of skeleton loaders to indicate content is being fetched, providing a better user experience while waiting for data.
run-security-hub.ts (3)
18-22: Well-implemented grouping of findings by severity.Good use of reduce to efficiently group the findings by severity, with proper type annotation for the accumulator.
54-62: Good use of Promise handling for the main function execution.The promise chain for executing the main function has proper success and error handling with appropriate exit codes.
3-3:Details
✅ Verification successful
Use absolute imports for better maintainability.
The relative import path is brittle and may break if the script location changes. Consider using absolute imports configured via tsconfig paths.
-import { fetchSecurityFindings } from './packages/integrations/src/aws/src/index'; +import { fetchSecurityFindings } from '@bubba/integrations/aws';
🏁 Script executed:
#!/bin/bash # Check if the project uses tsconfig paths for imports # Look for tsconfig.json files that might configure paths fd -t f "tsconfig.*json" -X grep -l "paths" {}Length of output: 154
Action: Update the import in
run-security-hub.tsfrom a brittle relative import to an absolute import using your configured tsconfig paths.
- The output from your search confirms that several tsconfig files (e.g., in
./apps/portal/,./apps/web/, etc.) contain path configurations. This supports the use of absolute imports.- Refactor the current import:
-import { fetchSecurityFindings } from './packages/integrations/src/aws/src/index'; +import { fetchSecurityFindings } from '@bubba/integrations/aws';- This update will improve maintainability by reducing risk if file locations change.
apps/app/src/actions/tests/register-test-action.ts (1)
50-59: Well-structured employee creation with proper data handling.The employee creation code properly structures the data and includes all necessary fields from the input and context.
apps/app/src/app/[locale]/(app)/(dashboard)/tests/page.tsx (3)
17-22: Good authorization check with appropriate redirect.The component correctly checks for the presence of an organization ID and redirects if not available, ensuring proper access control.
29-42: Well-implemented metadata generation.The
generateMetadatafunction correctly handles localization and sets the page title based on the translated string.
9-13:Details
❓ Verification inconclusive
Unusual Promise type for Next.js params.
The
paramsparameter is typed as a Promise, which is unusual for Next.js page components. Typically, Next.js passes these parameters directly as objects.
🏁 Script executed:
#!/bin/bash # Check for other instances of params as Promise in the codebase echo "Looking for other instances of 'params: Promise<{' in page components..." rg "params: Promise<\{" --type tsx echo "Checking Next.js documentation for typical params typing..." curl -s https://nextjs.org/docs/app/api-reference/file-conventions/page | grep -A 10 "export default function Page"Length of output: 472
Potential Issue with Asynchronous Typing for Next.js Page Params
The page component in
apps/app/src/app/[locale]/(app)/(dashboard)/tests/page.tsxdeclares itsparamsas a Promise (params: Promise<{ locale: string }>), which is unconventional since Next.js typically passes these parameters synchronously as plain objects.
- Action Required: Please verify whether this asynchronous typing was intentional. If it's by design (perhaps to accommodate an async data-fetching scenario), consider adding a comment to explain the rationale. Otherwise, updating
paramsto the expected type (e.g.,{ locale: string }) would better align with standard Next.js practices.apps/app/src/jobs/tasks/integration/integration-schedule.ts (1)
10-11: Consider utilizingupcomingThresholdin the DB query.Currently,
upcomingThresholdis computed but never used to filter or process integrations before triggering. If your intent is to only send integration runs for the upcoming 7 days, ensure the query or subsequent logic references this value.apps/app/src/components/tables/tests/filter-toolbar.tsx (1)
17-55: Validate the debounced routing logic.Your approach with
useTransitionanduseDebounceis a common pattern for improving performance. Confirm the transition remains responsive for users with slower connections or large data sets. You may also consider error handling or fallback UI for potential routing issues.apps/app/src/app/[locale]/(app)/(dashboard)/tests/[testId]/actions/get-test.ts (1)
15-20: LGTM: Authentication and organization ID validation is correctly implementedThe code properly authenticates the user and validates that an organization ID is available before proceeding.
apps/app/src/components/tables/tests/data-table-pagination.tsx (3)
15-18: LGTM: The component interface is well-definedThe interface clearly describes the required props for the DataTablePagination component.
28-35: LGTM: The page query creation is well-implementedThe
createPageQuerycallback correctly updates the URL parameters and preserves other query parameters.
37-45: LGTM: The per-page query creation resets to page 1 appropriatelyThe
createPerPageQuerycallback correctly resets to the first page when changing items per page, which is a good UX practice.apps/app/src/components/tables/tests/data-table-header.tsx (2)
11-23: LGTM: Good type definition with clear optional propertiesThe Props type is well-defined with appropriate optional properties and clear structure.
34-50: LGTM: Sort query creation logic is well-implementedThe sort query creation logic correctly toggles between ascending, descending, and no sorting states.
packages/db/prisma/schema.prisma (3)
105-105: Well-structured relationship field for integration results.The addition of the
integrationResultsfield to theUsermodel establishes a clear one-to-many relationship with the newOrganizationIntegrationResultsmodel, allowing users to be assigned to multiple integration test results.
148-148: Appropriate relationship field for integration results.The
OrganizationIntegrationResultsfield in theOrganizationmodel properly establishes the relationship between organizations and their integration test results. This follows the existing pattern in the schema for other organization-related models.
1082-1082: Add JSON validation for resultDetails.The
resultDetailsfield is using a JSON type to store flexible test result data. Consider adding validation at the application level to ensure the JSON structure meets your expected format for test results, especially if this data will be used for reporting or analytics.It appears that JSON validation would need to be handled in your application code since Prisma doesn't provide schema-level JSON validation. You may want to implement a validation layer in your API endpoints or services that handle test result creation and updates.
packages/db/prisma/migrations/20250227185651_add_organization_integration_results/migration.sql (3)
1-14: Well-structured table creation with appropriate fields.The table creation statement is well-structured and aligns properly with the Prisma schema. All necessary fields are defined with appropriate data types.
16-23: Appropriate indexes for query optimization.The creation of indexes on foreign key columns (
organizationIntegrationId,organizationId, andassignedUserId) is a good practice for optimizing query performance, especially for relationship lookups.
25-32: Cascade delete constraints properly defined.The foreign key constraints with
ON DELETE CASCADE ON UPDATE CASCADEare properly defined to maintain referential integrity. This ensures that when a related record is deleted or updated, the corresponding integration results are also appropriately handled.Ensure that the cascade delete behavior is the intended design, especially for the
assignedUserIdforeign key. If you want to preserve test results history when a user is deleted, you might want to consider usingON DELETE SET NULLinstead for this specific constraint.apps/app/src/locales/en.ts (2)
185-185: Sidebar entry added for new feature.The addition of "Cloud Tests" to the sidebar navigation is concise and follows the naming pattern of other entries.
586-647: Comprehensive localization for Cloud Tests feature.The localization section for the Cloud Tests feature is well-structured and complete. It includes all necessary strings for actions, empty states, filters, registration form, and table display.
apps/app/src/locales/es.ts (5)
182-183: LGTM: Sidebar menu entry for cloud tests added correctly.The addition of "Pruebas en la Nube" to the sidebar menu maintains consistency with the application's navigation structure.
718-736: LGTM: Cloud tests section properly structured with appropriate Spanish translations.The new section for cloud tests includes correctly translated strings for the main title, actions (create, clear, refresh), and empty states. This follows the same pattern as other feature sections in the localization file.
737-740: LGTM: Appropriate filter translations for cloud tests.The search and provider filter translations are clear and consistent with other filter translations in the application.
741-784: LGTM: Comprehensive form field translations for test registration.The registration section includes all necessary form fields with appropriate labels and placeholders. The section covers email, role, name, department fields as well as specialized cloud test fields like title, description, provider, and configuration options.
785-796: LGTM: Table column headers correctly translated.All necessary table columns for the cloud tests listing are appropriately translated, including title, provider, status, execution time, severity, results, creation date, and user assignment information.
apps/app/src/locales/pt.ts (5)
182-183: LGTM: Sidebar menu entry for cloud tests added correctly.The addition of "Testes na Nuvem" to the sidebar menu maintains consistency with the application's Portuguese navigation structure.
718-736: LGTM: Cloud tests section properly structured with appropriate Portuguese translations.The new section for cloud tests includes well-translated strings for the main title, actions (create, clear, refresh), and empty states, following the same pattern as other feature sections in the localization file.
737-740: LGTM: Appropriate filter translations for cloud tests.The search and provider filter translations are clear and consistent with other Portuguese filter translations in the application.
741-784: LGTM: Comprehensive form field translations for test registration.The registration section includes all necessary form fields with appropriate labels and placeholders translated to Portuguese. The section covers email, role, name, department fields as well as specialized cloud test fields like title, description, provider, and configuration options.
785-796: LGTM: Table column headers correctly translated to Portuguese.All necessary table columns for the cloud tests listing are appropriately translated, including title, provider, status, execution time, severity, results, creation date, and user assignment information.
| export const Logo = () => { | ||
| return ( | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| version="1.1" | ||
| id="Layer_1" | ||
| xmlnsXlink="http://www.w3.org/1999/xlink" | ||
| x="0px" | ||
| y="0px" | ||
| width={131.417} | ||
| height={50} | ||
| viewBox="0 0 500 182" | ||
| style={{ enableBackground: "new 0 0 304 182" }} | ||
| xmlSpace="preserve"> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility attributes to the SVG logo.
The SVG lacks accessibility attributes which are important for screen readers.
export const Logo = () => {
return (
<svg
xmlns="http://www.w3.org/2000/svg"
version="1.1"
id="Layer_1"
xmlnsXlink="http://www.w3.org/1999/xlink"
x="0px"
y="0px"
width={131.417}
height={50}
viewBox="0 0 500 182"
style={{ enableBackground: "new 0 0 304 182" }}
+ aria-label="AWS Logo"
+ role="img"
xmlSpace="preserve">📝 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 const Logo = () => { | |
| return ( | |
| <svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| version="1.1" | |
| id="Layer_1" | |
| xmlnsXlink="http://www.w3.org/1999/xlink" | |
| x="0px" | |
| y="0px" | |
| width={131.417} | |
| height={50} | |
| viewBox="0 0 500 182" | |
| style={{ enableBackground: "new 0 0 304 182" }} | |
| xmlSpace="preserve"> | |
| export const Logo = () => { | |
| return ( | |
| <svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| version="1.1" | |
| id="Layer_1" | |
| xmlnsXlink="http://www.w3.org/1999/xlink" | |
| x="0px" | |
| y="0px" | |
| width={131.417} | |
| height={50} | |
| viewBox="0 0 500 182" | |
| style={{ enableBackground: "new 0 0 304 182" }} | |
| aria-label="AWS Logo" | |
| role="img" | |
| xmlSpace="preserve"> | |
| {/* ...rest of the SVG content... */} | |
| </svg> | |
| ); | |
| }; |
| export async function getServerColumnHeaders() { | ||
| const t = await getI18n(); | ||
|
|
||
| return { | ||
| severity: t("tests.table.severity"), | ||
| result: t("tests.table.result"), | ||
| title: t("tests.table.title"), | ||
| provider: t("tests.table.provider"), | ||
| status: t("tests.table.status"), | ||
| createdAt: t("tests.table.createdAt"), | ||
| assignedUser: t("tests.table.assignedUser"), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for i18n function failures.
The function doesn't handle potential errors from the i18n system, which could lead to runtime failures.
export async function getServerColumnHeaders() {
- const t = await getI18n();
+ try {
+ const t = await getI18n();
+
+ return {
+ severity: t("tests.table.severity"),
+ result: t("tests.table.result"),
+ title: t("tests.table.title"),
+ provider: t("tests.table.provider"),
+ status: t("tests.table.status"),
+ createdAt: t("tests.table.createdAt"),
+ assignedUser: t("tests.table.assignedUser"),
+ };
+ } catch (error) {
+ console.error("Failed to load column headers:", error);
+ // Fallback to English or default strings
+ return {
+ severity: "Severity",
+ result: "Result",
+ title: "Title",
+ provider: "Provider",
+ status: "Status",
+ createdAt: "Created At",
+ assignedUser: "Assigned User",
+ };
+ }
-
- return {
- severity: t("tests.table.severity"),
- result: t("tests.table.result"),
- title: t("tests.table.title"),
- provider: t("tests.table.provider"),
- status: t("tests.table.status"),
- createdAt: t("tests.table.createdAt"),
- assignedUser: t("tests.table.assignedUser"),
- };
}📝 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 getServerColumnHeaders() { | |
| const t = await getI18n(); | |
| return { | |
| severity: t("tests.table.severity"), | |
| result: t("tests.table.result"), | |
| title: t("tests.table.title"), | |
| provider: t("tests.table.provider"), | |
| status: t("tests.table.status"), | |
| createdAt: t("tests.table.createdAt"), | |
| assignedUser: t("tests.table.assignedUser"), | |
| }; | |
| } | |
| export async function getServerColumnHeaders() { | |
| try { | |
| const t = await getI18n(); | |
| return { | |
| severity: t("tests.table.severity"), | |
| result: t("tests.table.result"), | |
| title: t("tests.table.title"), | |
| provider: t("tests.table.provider"), | |
| status: t("tests.table.status"), | |
| createdAt: t("tests.table.createdAt"), | |
| assignedUser: t("tests.table.assignedUser"), | |
| }; | |
| } catch (error) { | |
| console.error("Failed to load column headers:", error); | |
| // Fallback to English or default strings | |
| return { | |
| severity: "Severity", | |
| result: "Result", | |
| title: "Title", | |
| provider: "Provider", | |
| status: "Status", | |
| createdAt: "Created At", | |
| assignedUser: "Assigned User", | |
| }; | |
| } | |
| } |
| export const registerTestSchema = z.object({ | ||
| name: z.string().min(1, "Name is required"), | ||
| email: z.string().email("Invalid email address"), | ||
| department: z.nativeEnum(Departments, { | ||
| required_error: "Department is required", | ||
| }), | ||
| externalEmployeeId: z.string().optional(), | ||
| isActive: z.boolean().default(true), | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid schema duplication with createEmployeeSchema.
The registerTestSchema is identical to the existing createEmployeeSchema (lines 276-284). This duplication could lead to maintenance issues if one schema is updated while the other is forgotten.
Consider one of these approaches:
- Reuse the existing schema if the validation requirements are the same:
-export const registerTestSchema = z.object({
- name: z.string().min(1, "Name is required"),
- email: z.string().email("Invalid email address"),
- department: z.nativeEnum(Departments, {
- required_error: "Department is required",
- }),
- externalEmployeeId: z.string().optional(),
- isActive: z.boolean().default(true),
-});
+export const registerTestSchema = createEmployeeSchema;- Create a base schema for both to share:
+const personSchema = z.object({
+ name: z.string().min(1, "Name is required"),
+ email: z.string().email("Invalid email address"),
+ department: z.nativeEnum(Departments, {
+ required_error: "Department is required",
+ }),
+ externalEmployeeId: z.string().optional(),
+ isActive: z.boolean().default(true),
+});
+
-export const createEmployeeSchema = z.object({
- name: z.string().min(1, "Name is required"),
- email: z.string().email("Invalid email address"),
- department: z.nativeEnum(Departments, {
- required_error: "Department is required",
- }),
- externalEmployeeId: z.string().optional(),
- isActive: z.boolean().default(true),
-});
+export const createEmployeeSchema = personSchema;
+
-export const registerTestSchema = z.object({
- name: z.string().min(1, "Name is required"),
- email: z.string().email("Invalid email address"),
- department: z.nativeEnum(Departments, {
- required_error: "Department is required",
- }),
- externalEmployeeId: z.string().optional(),
- isActive: z.boolean().default(true),
-});
+export const registerTestSchema = personSchema;📝 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 const registerTestSchema = z.object({ | |
| name: z.string().min(1, "Name is required"), | |
| email: z.string().email("Invalid email address"), | |
| department: z.nativeEnum(Departments, { | |
| required_error: "Department is required", | |
| }), | |
| externalEmployeeId: z.string().optional(), | |
| isActive: z.boolean().default(true), | |
| }); | |
| export const registerTestSchema = createEmployeeSchema; |
| id: "session_token", | ||
| label: "AWS session token", | ||
| description: "The API session token AWS account", | ||
| type: "text", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve session token description.
The current description for the session token is incomplete and unclear.
{
id: "session_token",
label: "AWS session token",
- description: "The API session token AWS account",
+ description: "The temporary API session token for your AWS account (only required when using temporary credentials)",
type: "text",
required: true,
value: "",
},📝 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.
| id: "session_token", | |
| label: "AWS session token", | |
| description: "The API session token AWS account", | |
| type: "text", | |
| { | |
| id: "session_token", | |
| label: "AWS session token", | |
| description: "The temporary API session token for your AWS account (only required when using temporary credentials)", | |
| type: "text", | |
| required: true, | |
| value: "", | |
| }, |
| * Fetches security findings from AWS Security Hub | ||
| * @returns Promise containing an array of findings | ||
| */ | ||
| async function fetchSecurityFindings(AWS_REGION: string, AWS_ACCESS_KEY_ID: string, AWS_SECRET_ACCESS_KEY: string, AWS_SESSION_TOKEN: string): Promise<any[]> { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Define a specific return type instead of any[].
Using any[] as a return type loses type safety. It's better to define an interface for the findings structure.
-async function fetchSecurityFindings(AWS_REGION: string, AWS_ACCESS_KEY_ID: string, AWS_SECRET_ACCESS_KEY: string, AWS_SESSION_TOKEN: string): Promise<any[]> {
+interface SecurityFinding {
+ // Define the structure based on AWS Security Hub finding fields
+ Id: string;
+ ProductArn: string;
+ Title: string;
+ Description: string;
+ Severity: {
+ Label: string;
+ // other severity fields
+ };
+ // other relevant fields
+}
+
+async function fetchSecurityFindings(AWS_REGION: string, AWS_ACCESS_KEY_ID: string, AWS_SECRET_ACCESS_KEY: string, AWS_SESSION_TOKEN: string): Promise<SecurityFinding[]> {📝 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.
| async function fetchSecurityFindings(AWS_REGION: string, AWS_ACCESS_KEY_ID: string, AWS_SECRET_ACCESS_KEY: string, AWS_SESSION_TOKEN: string): Promise<any[]> { | |
| interface SecurityFinding { | |
| // Define the structure based on AWS Security Hub finding fields | |
| Id: string; | |
| ProductArn: string; | |
| Title: string; | |
| Description: string; | |
| Severity: { | |
| Label: string; | |
| // other severity fields | |
| }; | |
| // other relevant fields | |
| } | |
| async function fetchSecurityFindings(AWS_REGION: string, AWS_ACCESS_KEY_ID: string, AWS_SECRET_ACCESS_KEY: string, AWS_SESSION_TOKEN: string): Promise<SecurityFinding[]> { | |
| // function implementation remains here | |
| } |
| <TableHeader> | ||
| <TableRow className="h-[45px] hover:bg-transparent"> | ||
| {isVisible("severity") && ( | ||
| <TableHead className="min-w-[120px] px-3 md:px-4 py-2 hidden md:table-cell"> | ||
| <Button | ||
| className="p-0 hover:bg-transparent space-x-2" | ||
| variant="ghost" | ||
| onClick={() => createSortQuery("status")} | ||
| > | ||
| <span>{t("tests.table.severity")}</span> | ||
| {"severity" === column && value === "asc" && <ArrowDown size={16} />} | ||
| {"severity" === column && value === "desc" && <ArrowUp size={16} />} | ||
| </Button> | ||
| </TableHead> | ||
| )} | ||
| {isVisible("result") && ( | ||
| <TableHead className="min-w-[120px] px-3 md:px-4 py-2 hidden md:table-cell"> | ||
| <Button | ||
| className="p-0 hover:bg-transparent space-x-2" | ||
| variant="ghost" | ||
| onClick={() => createSortQuery("result")} | ||
| > | ||
| <span>{t("tests.table.result")}</span> | ||
| {"result" === column && value === "asc" && <ArrowDown size={16} />} | ||
| {"result" === column && value === "desc" && <ArrowUp size={16} />} | ||
| </Button> | ||
| </TableHead> | ||
| )} | ||
|
|
||
| {isVisible("title") && ( | ||
| <TableHead className="min-w-[120px] px-3 md:px-4 py-2 hidden md:table-cell"> | ||
| <Button | ||
| className="p-0 hover:bg-transparent space-x-2" | ||
| variant="ghost" | ||
| onClick={() => createSortQuery("title")} | ||
| > | ||
| <span>{t("tests.table.title")}</span> | ||
| {"title" === column && value === "asc" && ( | ||
| <ArrowDown size={16} /> | ||
| )} | ||
| {"title" === column && value === "desc" && ( | ||
| <ArrowUp size={16} /> | ||
| )} | ||
| </Button> | ||
| </TableHead> | ||
| )} | ||
| {isVisible("provider") && ( | ||
| <TableHead className="min-w-[120px] px-3 md:px-4 py-2 hidden md:table-cell"> | ||
| <Button | ||
| className="p-0 hover:bg-transparent space-x-2" | ||
| variant="ghost" | ||
| onClick={() => createSortQuery("provider")} | ||
| > | ||
| <span>{t("tests.table.provider")}</span> | ||
| {"provider" === column && value === "asc" && ( | ||
| <ArrowDown size={16} /> | ||
| )} | ||
| {"provider" === column && value === "desc" && ( | ||
| <ArrowUp size={16} /> | ||
| )} | ||
| </Button> | ||
| </TableHead> | ||
| )} | ||
| {isVisible("createdAt") && ( | ||
| <TableHead className="min-w-[120px] px-3 md:px-4 py-2 hidden md:table-cell"> | ||
| <Button | ||
| className="p-0 hover:bg-transparent space-x-2" | ||
| variant="ghost" | ||
| onClick={() => createSortQuery("createdAt")} | ||
| > | ||
| <span>{t("tests.table.createdAt")}</span> | ||
| {"createdAt" === column && value === "asc" && ( | ||
| <ArrowDown size={16} /> | ||
| )} | ||
| {"createdAt" === column && value === "desc" && ( | ||
| <ArrowUp size={16} /> | ||
| )} | ||
| </Button> | ||
| </TableHead> | ||
| )} | ||
| {isVisible("assignedUser") && ( | ||
| <TableHead className="min-w-[120px] px-3 md:px-4 py-2 hidden md:table-cell"> | ||
| <Button | ||
| className="p-0 hover:bg-transparent space-x-2" | ||
| variant="ghost" | ||
| onClick={() => createSortQuery("assignedUser")} | ||
| > | ||
| <span>{t("tests.table.assignedUser")}</span> | ||
| {"assignedUser" === column && value === "asc" && ( | ||
| <ArrowDown size={16} /> | ||
| )} | ||
| {"assignedUser" === column && value === "desc" && ( | ||
| <ArrowUp size={16} /> | ||
| )} | ||
| </Button> | ||
| </TableHead> | ||
| )} | ||
| </TableRow> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider refactoring to reduce code duplication
There's significant repetition in the table header rendering code. Consider creating a helper function to generate the column headers to reduce duplication.
Here's how you could refactor this:
+ const renderColumnHeader = (id: string, label: string) => {
+ if (!isVisible(id)) return null;
+
+ return (
+ <TableHead className="min-w-[120px] px-3 md:px-4 py-2 hidden md:table-cell">
+ <Button
+ className="p-0 hover:bg-transparent space-x-2"
+ variant="ghost"
+ onClick={() => createSortQuery(id)}
+ >
+ <span>{label}</span>
+ {id === column && value === "asc" && <ArrowDown size={16} />}
+ {id === column && value === "desc" && <ArrowUp size={16} />}
+ </Button>
+ </TableHead>
+ );
+ };
return (
<TableHeader>
<TableRow className="h-[45px] hover:bg-transparent">
- {isVisible("severity") && (
- <TableHead className="min-w-[120px] px-3 md:px-4 py-2 hidden md:table-cell">
- <Button
- className="p-0 hover:bg-transparent space-x-2"
- variant="ghost"
- onClick={() => createSortQuery("status")}
- >
- <span>{t("tests.table.severity")}</span>
- {"severity" === column && value === "asc" && <ArrowDown size={16} />}
- {"severity" === column && value === "desc" && <ArrowUp size={16} />}
- </Button>
- </TableHead>
- )}
+ {renderColumnHeader("severity", t("tests.table.severity"))}
+ {renderColumnHeader("result", t("tests.table.result"))}
+ {renderColumnHeader("title", t("tests.table.title"))}
+ {renderColumnHeader("provider", t("tests.table.provider"))}
+ {renderColumnHeader("createdAt", t("tests.table.createdAt"))}
+ {renderColumnHeader("assignedUser", t("tests.table.assignedUser"))}
- ... // Remove all the other repeated code blocks
</TableRow>
</TableHeader>
);📝 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.
| <TableHeader> | |
| <TableRow className="h-[45px] hover:bg-transparent"> | |
| {isVisible("severity") && ( | |
| <TableHead className="min-w-[120px] px-3 md:px-4 py-2 hidden md:table-cell"> | |
| <Button | |
| className="p-0 hover:bg-transparent space-x-2" | |
| variant="ghost" | |
| onClick={() => createSortQuery("status")} | |
| > | |
| <span>{t("tests.table.severity")}</span> | |
| {"severity" === column && value === "asc" && <ArrowDown size={16} />} | |
| {"severity" === column && value === "desc" && <ArrowUp size={16} />} | |
| </Button> | |
| </TableHead> | |
| )} | |
| {isVisible("result") && ( | |
| <TableHead className="min-w-[120px] px-3 md:px-4 py-2 hidden md:table-cell"> | |
| <Button | |
| className="p-0 hover:bg-transparent space-x-2" | |
| variant="ghost" | |
| onClick={() => createSortQuery("result")} | |
| > | |
| <span>{t("tests.table.result")}</span> | |
| {"result" === column && value === "asc" && <ArrowDown size={16} />} | |
| {"result" === column && value === "desc" && <ArrowUp size={16} />} | |
| </Button> | |
| </TableHead> | |
| )} | |
| {isVisible("title") && ( | |
| <TableHead className="min-w-[120px] px-3 md:px-4 py-2 hidden md:table-cell"> | |
| <Button | |
| className="p-0 hover:bg-transparent space-x-2" | |
| variant="ghost" | |
| onClick={() => createSortQuery("title")} | |
| > | |
| <span>{t("tests.table.title")}</span> | |
| {"title" === column && value === "asc" && ( | |
| <ArrowDown size={16} /> | |
| )} | |
| {"title" === column && value === "desc" && ( | |
| <ArrowUp size={16} /> | |
| )} | |
| </Button> | |
| </TableHead> | |
| )} | |
| {isVisible("provider") && ( | |
| <TableHead className="min-w-[120px] px-3 md:px-4 py-2 hidden md:table-cell"> | |
| <Button | |
| className="p-0 hover:bg-transparent space-x-2" | |
| variant="ghost" | |
| onClick={() => createSortQuery("provider")} | |
| > | |
| <span>{t("tests.table.provider")}</span> | |
| {"provider" === column && value === "asc" && ( | |
| <ArrowDown size={16} /> | |
| )} | |
| {"provider" === column && value === "desc" && ( | |
| <ArrowUp size={16} /> | |
| )} | |
| </Button> | |
| </TableHead> | |
| )} | |
| {isVisible("createdAt") && ( | |
| <TableHead className="min-w-[120px] px-3 md:px-4 py-2 hidden md:table-cell"> | |
| <Button | |
| className="p-0 hover:bg-transparent space-x-2" | |
| variant="ghost" | |
| onClick={() => createSortQuery("createdAt")} | |
| > | |
| <span>{t("tests.table.createdAt")}</span> | |
| {"createdAt" === column && value === "asc" && ( | |
| <ArrowDown size={16} /> | |
| )} | |
| {"createdAt" === column && value === "desc" && ( | |
| <ArrowUp size={16} /> | |
| )} | |
| </Button> | |
| </TableHead> | |
| )} | |
| {isVisible("assignedUser") && ( | |
| <TableHead className="min-w-[120px] px-3 md:px-4 py-2 hidden md:table-cell"> | |
| <Button | |
| className="p-0 hover:bg-transparent space-x-2" | |
| variant="ghost" | |
| onClick={() => createSortQuery("assignedUser")} | |
| > | |
| <span>{t("tests.table.assignedUser")}</span> | |
| {"assignedUser" === column && value === "asc" && ( | |
| <ArrowDown size={16} /> | |
| )} | |
| {"assignedUser" === column && value === "desc" && ( | |
| <ArrowUp size={16} /> | |
| )} | |
| </Button> | |
| </TableHead> | |
| )} | |
| </TableRow> | |
| // Ensure that this helper function is defined in the appropriate scope. | |
| const renderColumnHeader = (id: string, label: string) => { | |
| if (!isVisible(id)) return null; | |
| return ( | |
| <TableHead className="min-w-[120px] px-3 md:px-4 py-2 hidden md:table-cell"> | |
| <Button | |
| className="p-0 hover:bg-transparent space-x-2" | |
| variant="ghost" | |
| onClick={() => createSortQuery(id)} | |
| > | |
| <span>{label}</span> | |
| {id === column && value === "asc" && <ArrowDown size={16} />} | |
| {id === column && value === "desc" && <ArrowUp size={16} />} | |
| </Button> | |
| </TableHead> | |
| ); | |
| }; | |
| return ( | |
| <TableHeader> | |
| <TableRow className="h-[45px] hover:bg-transparent"> | |
| {renderColumnHeader("severity", t("tests.table.severity"))} | |
| {renderColumnHeader("result", t("tests.table.result"))} | |
| {renderColumnHeader("title", t("tests.table.title"))} | |
| {renderColumnHeader("provider", t("tests.table.provider"))} | |
| {renderColumnHeader("createdAt", t("tests.table.createdAt"))} | |
| {renderColumnHeader("assignedUser", t("tests.table.assignedUser"))} | |
| </TableRow> | |
| </TableHeader> | |
| ); |
| export const appErrors = { | ||
| UNAUTHORIZED: { | ||
| code: "UNAUTHORIZED" as const, | ||
| message: "You are not authorized to view employees", | ||
| }, | ||
| UNEXPECTED_ERROR: { | ||
| code: "UNEXPECTED_ERROR" as const, | ||
| message: "An unexpected error occurred", | ||
| }, | ||
| } as const; No newline at end of file |
There was a problem hiding this comment.
Fix incorrect error message in UNAUTHORIZED error
The error message for UNAUTHORIZED refers to "employees" but this file is about tests. Update the error message to reflect the correct context.
export const appErrors = {
UNAUTHORIZED: {
code: "UNAUTHORIZED" as const,
- message: "You are not authorized to view employees",
+ message: "You are not authorized to view tests",
},
UNEXPECTED_ERROR: {
code: "UNEXPECTED_ERROR" as const,
message: "An unexpected error occurred",
},
} as const;📝 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 const appErrors = { | |
| UNAUTHORIZED: { | |
| code: "UNAUTHORIZED" as const, | |
| message: "You are not authorized to view employees", | |
| }, | |
| UNEXPECTED_ERROR: { | |
| code: "UNEXPECTED_ERROR" as const, | |
| message: "An unexpected error occurred", | |
| }, | |
| } as const; | |
| export const appErrors = { | |
| UNAUTHORIZED: { | |
| code: "UNAUTHORIZED" as const, | |
| message: "You are not authorized to view tests", | |
| }, | |
| UNEXPECTED_ERROR: { | |
| code: "UNEXPECTED_ERROR" as const, | |
| message: "An unexpected error occurred", | |
| }, | |
| } as const; |
| export const testSchema = z.object({ | ||
| id: z.string(), | ||
| title: z.string(), | ||
| description: z.string().nullable(), | ||
| provider: z.string(), | ||
| status: z.string(), | ||
| resultDetails: z.any(), | ||
| label: z.string().nullable(), | ||
| completedAt: z.date(), | ||
| assignedUserId: z.object({ | ||
| id: z.string(), | ||
| name: z.string().nullable(), | ||
| email: z.string().nullable(), | ||
| }) | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety by avoiding z.any()
Using z.any() for resultDetails eliminates type safety benefits. Consider defining a more specific schema based on the expected structure of the result details.
export const testSchema = z.object({
id: z.string(),
title: z.string(),
description: z.string().nullable(),
provider: z.string(),
status: z.string(),
- resultDetails: z.any(),
+ resultDetails: z.record(z.unknown()).optional(),
label: z.string().nullable(),
completedAt: z.date(),
assignedUserId: z.object({
id: z.string(),
name: z.string().nullable(),
email: z.string().nullable(),
})
});📝 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 const testSchema = z.object({ | |
| id: z.string(), | |
| title: z.string(), | |
| description: z.string().nullable(), | |
| provider: z.string(), | |
| status: z.string(), | |
| resultDetails: z.any(), | |
| label: z.string().nullable(), | |
| completedAt: z.date(), | |
| assignedUserId: z.object({ | |
| id: z.string(), | |
| name: z.string().nullable(), | |
| email: z.string().nullable(), | |
| }) | |
| }); | |
| export const testSchema = z.object({ | |
| id: z.string(), | |
| title: z.string(), | |
| description: z.string().nullable(), | |
| provider: z.string(), | |
| status: z.string(), | |
| resultDetails: z.record(z.unknown()).optional(), | |
| label: z.string().nullable(), | |
| completedAt: z.date(), | |
| assignedUserId: z.object({ | |
| id: z.string(), | |
| name: z.string().nullable(), | |
| email: z.string().nullable(), | |
| }) | |
| }); |
| export interface TestsResponse { | ||
| tests: { | ||
| id: string; | ||
| severity: string | null; | ||
| result: string; | ||
| title: string; | ||
| provider: string; | ||
| createdAt: Date; | ||
| assignedUser: null; | ||
| }[]; | ||
| total: number; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure TestsResponse aligns with Test type
The TestsResponse interface doesn't align with the Test type derived from testSchema. This inconsistency could lead to confusion and bugs when mapping between these types.
Consider aligning the types by either:
- Making
TestsResponse.testsmatch theTesttype - Creating a separate, explicitly named type for the test list items
export interface TestsResponse {
tests: {
- id: string;
- severity: string | null;
- result: string;
- title: string;
- provider: string;
- createdAt: Date;
- assignedUser: null;
+ // Either align with Test type:
+ id: string;
+ title: string;
+ description: string | null;
+ provider: string;
+ status: string;
+ resultDetails: unknown;
+ label: string | null;
+ completedAt: Date;
+ assignedUserId: {
+ id: string;
+ name: string | null;
+ email: string | null;
+ };
+ // Or create an explicitly named separate type:
+ // create a new interface like TestListItem and use it here
}[];
total: number;
}🏁 Script executed:
#!/bin/bash
# Look for usages of TestsResponse to understand how it's being used
rg -A 3 -B 3 "TestsResponse" --type ts
# Look for conversions between Test and TestsResponse formats
rg -A 5 -B 5 "tests:" --type ts | grep -A 10 -B 10 "TestsResponse"Length of output: 4604
Action: Align the TestsResponse interface with the Test type defined by testSchema
It looks like the current definition of TestsResponse in
apps/app/src/app/[locale]/(app)/(dashboard)/tests/types/index.ts
(lines 30–41) does not match the structure of the Test type (i.e. the output of z.infer<typeof testSchema>). This discrepancy may lead to mapping issues in functions—such as in the fetch logic in hooks/useTests.ts—where the response is cast as TestsResponse.
Recommendations:
- Update the properties in
TestsResponse.testsso they mirror the fields defined in theTesttype (for example, including properties likeid,title,description,provider,status,resultDetails,label,completedAt, and a nestedassignedUserIdobject). - Alternatively, if the current structure is intentional, consider creating a new type (e.g.
TestListItem) to clearly differentiate between the raw response and the domain-specificTesttype.
export interface TestsResponse {
tests: {
- id: string;
- severity: string | null;
- result: string;
- title: string;
- provider: string;
- createdAt: Date;
- assignedUser: null;
+ // Option 1: Align these fields with the Test type
+ id: string;
+ title: string;
+ description: string | null;
+ provider: string;
+ status: string;
+ resultDetails: unknown;
+ label: string | null;
+ completedAt: Date;
+ assignedUserId: {
+ id: string;
+ name: string | null;
+ email: string | null;
+ };
+ // Option 2: Create a separately named type (e.g. TestListItem) and use it here
}[];
total: number;
}📝 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 interface TestsResponse { | |
| tests: { | |
| id: string; | |
| severity: string | null; | |
| result: string; | |
| title: string; | |
| provider: string; | |
| createdAt: Date; | |
| assignedUser: null; | |
| }[]; | |
| total: number; | |
| } | |
| export interface TestsResponse { | |
| tests: { | |
| // Option 1: Align these fields with the Test type | |
| id: string; | |
| title: string; | |
| description: string | null; | |
| provider: string; | |
| status: string; | |
| resultDetails: unknown; | |
| label: string | null; | |
| completedAt: Date; | |
| assignedUserId: { | |
| id: string; | |
| name: string | null; | |
| email: string | null; | |
| }; | |
| // Option 2: Create a separately named type (e.g. TestListItem) and use it here | |
| }[]; | |
| total: number; | |
| } |
| tests: { | ||
| name: "Tests en nuage", | ||
| title: "Tests en nuage", | ||
| actions: { | ||
| create: "Ajouter un test en nuage", | ||
| clear: "Effacer les filtres", | ||
| refresh: "Rafraîchir" | ||
| }, | ||
| empty: { | ||
| no_tests: { | ||
| title: "Aucun test en nuage pour l'instant", | ||
| description: "Commencez par créer votre premier test en nuage." | ||
| }, | ||
| no_results: { | ||
| title: "Aucun résultat trouvé", | ||
| description: "Aucun test ne correspond à votre recherche", | ||
| description_with_filters: "Essayez d'ajuster vos filtres" | ||
| } | ||
| }, | ||
| filters: { | ||
| search: "Rechercher des tests...", | ||
| role: "Filtrer par fournisseur" | ||
| }, | ||
| register: { | ||
| title: "Ajouter un test Cloud", | ||
| description: "Configurer un nouveau test de conformité cloud.", | ||
| email: { | ||
| label: "Adresse e-mail", | ||
| placeholder: "Entrez l'adresse e-mail" | ||
| }, | ||
| role: { | ||
| label: "Rôle", | ||
| placeholder: "Sélectionnez un rôle" | ||
| }, | ||
| name: { | ||
| label: "Nom", | ||
| placeholder: "Entrez le nom" | ||
| }, | ||
| department: { | ||
| label: "Département", | ||
| placeholder: "Sélectionnez un département" | ||
| }, | ||
| submit: "Créer un test", | ||
| success: "Test créé avec succès", | ||
| error: "Échec de l'ajout du test", | ||
| invalid_json: "Configuration JSON invalide fournie", | ||
| title_field: { | ||
| label: "Titre du test", | ||
| placeholder: "Entrez le titre du test" | ||
| }, | ||
| description_field: { | ||
| label: "Description", | ||
| placeholder: "Entrez la description du test" | ||
| }, | ||
| provider: { | ||
| label: "Fournisseur Cloud", | ||
| placeholder: "Sélectionnez le fournisseur cloud" | ||
| }, | ||
| config: { | ||
| label: "Configuration du test", | ||
| placeholder: "Entrez la configuration JSON pour le test" | ||
| }, | ||
| auth_config: { | ||
| label: "Configuration d'authentification", | ||
| placeholder: "Entrez la configuration d'authentification JSON" | ||
| } | ||
| }, | ||
| table: { | ||
| title: "Titre", | ||
| provider: "Fournisseur", | ||
| status: "Statut", | ||
| lastRun: "Dernière exécution", | ||
| no_results: "Aucun résultat trouvé", | ||
| severity: "Sévérité", | ||
| result: "Résultat", | ||
| createdAt: "Créé le", | ||
| assignedUser: "Utilisateur assigné", | ||
| assignedUserEmpty: "Non assigné" | ||
| } |
There was a problem hiding this comment.
French translation section contains inconsistencies with English version.
The French translation for the tests section includes fields like email, role, name, and department (lines 744-759) that don't appear in the English version. Additionally, the table structure includes a lastRun field (line 789) that isn't in the English version. These inconsistencies could lead to interface issues when switching languages.
Synchronize the French translations with the English structure to ensure consistency:
- Remove the email, role, name, and department sections that don't appear in the English version
- Add the missing translations for severity, result, createdAt fields
- Remove the lastRun field that isn't in the English version or add it to the English version if needed
Here's a partial example of the necessary changes:
register: {
title: "Ajouter un test Cloud",
description: "Configurer un nouveau test de conformité cloud.",
- email: {
- label: "Adresse e-mail",
- placeholder: "Entrez l'adresse e-mail"
- },
- role: {
- label: "Rôle",
- placeholder: "Sélectionnez un rôle"
- },
- name: {
- label: "Nom",
- placeholder: "Entrez le nom"
- },
- department: {
- label: "Département",
- placeholder: "Sélectionnez un département"
- },
submit: "Créer un test",
success: "Test créé avec succès",
error: "Échec de l'ajout du test",
// Keep the rest of the fields that match English...
},
table: {
title: "Titre",
provider: "Fournisseur",
status: "Statut",
- lastRun: "Dernière exécution",
no_results: "Aucun résultat trouvé",
// Keep the severity, result, etc. fields...
}📝 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.
| tests: { | |
| name: "Tests en nuage", | |
| title: "Tests en nuage", | |
| actions: { | |
| create: "Ajouter un test en nuage", | |
| clear: "Effacer les filtres", | |
| refresh: "Rafraîchir" | |
| }, | |
| empty: { | |
| no_tests: { | |
| title: "Aucun test en nuage pour l'instant", | |
| description: "Commencez par créer votre premier test en nuage." | |
| }, | |
| no_results: { | |
| title: "Aucun résultat trouvé", | |
| description: "Aucun test ne correspond à votre recherche", | |
| description_with_filters: "Essayez d'ajuster vos filtres" | |
| } | |
| }, | |
| filters: { | |
| search: "Rechercher des tests...", | |
| role: "Filtrer par fournisseur" | |
| }, | |
| register: { | |
| title: "Ajouter un test Cloud", | |
| description: "Configurer un nouveau test de conformité cloud.", | |
| email: { | |
| label: "Adresse e-mail", | |
| placeholder: "Entrez l'adresse e-mail" | |
| }, | |
| role: { | |
| label: "Rôle", | |
| placeholder: "Sélectionnez un rôle" | |
| }, | |
| name: { | |
| label: "Nom", | |
| placeholder: "Entrez le nom" | |
| }, | |
| department: { | |
| label: "Département", | |
| placeholder: "Sélectionnez un département" | |
| }, | |
| submit: "Créer un test", | |
| success: "Test créé avec succès", | |
| error: "Échec de l'ajout du test", | |
| invalid_json: "Configuration JSON invalide fournie", | |
| title_field: { | |
| label: "Titre du test", | |
| placeholder: "Entrez le titre du test" | |
| }, | |
| description_field: { | |
| label: "Description", | |
| placeholder: "Entrez la description du test" | |
| }, | |
| provider: { | |
| label: "Fournisseur Cloud", | |
| placeholder: "Sélectionnez le fournisseur cloud" | |
| }, | |
| config: { | |
| label: "Configuration du test", | |
| placeholder: "Entrez la configuration JSON pour le test" | |
| }, | |
| auth_config: { | |
| label: "Configuration d'authentification", | |
| placeholder: "Entrez la configuration d'authentification JSON" | |
| } | |
| }, | |
| table: { | |
| title: "Titre", | |
| provider: "Fournisseur", | |
| status: "Statut", | |
| lastRun: "Dernière exécution", | |
| no_results: "Aucun résultat trouvé", | |
| severity: "Sévérité", | |
| result: "Résultat", | |
| createdAt: "Créé le", | |
| assignedUser: "Utilisateur assigné", | |
| assignedUserEmpty: "Non assigné" | |
| } | |
| tests: { | |
| name: "Tests en nuage", | |
| title: "Tests en nuage", | |
| actions: { | |
| create: "Ajouter un test en nuage", | |
| clear: "Effacer les filtres", | |
| refresh: "Rafraîchir" | |
| }, | |
| empty: { | |
| no_tests: { | |
| title: "Aucun test en nuage pour l'instant", | |
| description: "Commencez par créer votre premier test en nuage." | |
| }, | |
| no_results: { | |
| title: "Aucun résultat trouvé", | |
| description: "Aucun test ne correspond à votre recherche", | |
| description_with_filters: "Essayez d'ajuster vos filtres" | |
| } | |
| }, | |
| filters: { | |
| search: "Rechercher des tests...", | |
| role: "Filtrer par fournisseur" | |
| }, | |
| register: { | |
| title: "Ajouter un test Cloud", | |
| description: "Configurer un nouveau test de conformité cloud.", | |
| submit: "Créer un test", | |
| success: "Test créé avec succès", | |
| error: "Échec de l'ajout du test", | |
| invalid_json: "Configuration JSON invalide fournie", | |
| title_field: { | |
| label: "Titre du test", | |
| placeholder: "Entrez le titre du test" | |
| }, | |
| description_field: { | |
| label: "Description", | |
| placeholder: "Entrez la description du test" | |
| }, | |
| provider: { | |
| label: "Fournisseur Cloud", | |
| placeholder: "Sélectionnez le fournisseur cloud" | |
| }, | |
| config: { | |
| label: "Configuration du test", | |
| placeholder: "Entrez la configuration JSON pour le test" | |
| }, | |
| auth_config: { | |
| label: "Configuration d'authentification", | |
| placeholder: "Entrez la configuration d'authentification JSON" | |
| } | |
| }, | |
| table: { | |
| title: "Titre", | |
| provider: "Fournisseur", | |
| status: "Statut", | |
| no_results: "Aucun résultat trouvé", | |
| severity: "Sévérité", | |
| result: "Résultat", | |
| createdAt: "Créé le", | |
| assignedUser: "Utilisateur assigné", | |
| assignedUserEmpty: "Non assigné" | |
| } | |
| } |
This branch had an error being deployed
4 participants
Summary by CodeRabbit
New Features
Documentation
Chores