fix evidence statuses across app #80
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request modifies the ordering and handling of organization evidence and control requirements. In the business logic, evidence is now created before control requirements, and the related functions are updated to fetch and link the evidence appropriately. The data queries for controls and categories have been enhanced to include nested relations for evidence and policy, and UI components have been refactored to use new types and data sources. Additionally, the database schema and migrations are adjusted to add new columns and relationships for organization evidence and policies, and the seeding function now explicitly links evidence records to control requirements. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant SF as selectFrameworksAction
participant OE as createOrganizationEvidence
participant CCR as createOrganizationControlRequirements
participant DB as Database
U->>SF: Initiate framework selection
SF->>OE: Create organization evidence
OE->>DB: Insert new evidence record
DB-->>OE: Return evidence data
SF->>CCR: Create control requirements
CCR->>DB: Query with evidence & policy inclusions
DB-->>CCR: Return control requirements
SF-->>U: Return updated control data
sequenceDiagram
participant S as seedEvidence
participant DB as Prisma
participant CR as ControlRequirement
S->>DB: Upsert evidence record
DB-->>S: New evidence created
S->>DB: Update control requirement with evidenceId
DB-->>S: Control requirement linked
S->>Console: Log success message
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Components/data-table/data-table.tsx (1)
38-55: Improved navigation logic with switch statement.The refactored
onRowClickfunction with a switch statement is cleaner and more maintainable than nested conditions. It provides a clear and scalable way to handle different requirement types.Consider removing the console.log statements in lines 39-41 before deploying to production:
- console.log({ - requirement, - });apps/app/src/actions/framework/select-frameworks-action.ts (1)
246-260: Remove debug logging statements.The console.log statements should be removed before merging to production as they may impact performance and pollute the logs.
- console.log({ - evidenceId, - }); - console.log({ - organizationEvidence, - });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/app/src/actions/framework/select-frameworks-action.ts(4 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Actions/getOrganizationControl.ts(2 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Actions/getOrganizationControlProgress.ts(2 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Actions/getOrganizationControlRequirements.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Components/SingleControl.tsx(3 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Components/data-table/columns.tsx(2 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Components/data-table/data-table-header.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Components/data-table/data-table.tsx(2 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Components/data-table/types.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/hooks/useOrganizationControl.ts(2 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/frameworks/[frameworkId]/Actions/getOrganizationCategories.ts(1 hunks)apps/app/src/components/tables/frameworks/columns.tsx(3 hunks)packages/db/prisma/migrations/20250218202303_add_organization_policy_to_org_control_req/migration.sql(1 hunks)packages/db/prisma/migrations/20250225000803_add_org_ev_relation_to_org_ctrl_req/migration.sql(1 hunks)packages/db/prisma/schema.prisma(3 hunks)packages/db/prisma/seed.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/db/prisma/migrations/20250218202303_add_organization_policy_to_org_control_req/migration.sql
🔇 Additional comments (26)
apps/app/src/app/[locale]/(app)/(dashboard)/frameworks/[frameworkId]/Actions/getOrganizationCategories.ts (1)
65-69: Appropriate inclusion of organizationEvidenceThe addition of
organizationEvidencein the query's include clause is well-structured and consistent with the PR objective of fixing evidence statuses across the app. This change allows the published status of evidence to be retrieved alongside policy status, which will help in determining the overall status of control requirements.apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Actions/getOrganizationControl.ts (1)
34-39: Improved data retrieval with related entitiesThe inclusion of
OrganizationControlRequirementwith its relatedorganizationPolicyandorganizationEvidenceentities enhances the data retrieval process. This change aligns well with the PR objective of fixing evidence statuses across the app by ensuring all required data is fetched in a single query.apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Components/SingleControl.tsx (1)
65-67:✅ Verification successful
Verify that the DataTable component accepts the correct data format
The conditional check and data passing might need adjustment depending on the actual structure of
control.OrganizationControlRequirement. If it's an array (as suggested by database queries in other files), consider checking the array length instead.-{control.OrganizationControlRequirement && ( +{control.OrganizationControlRequirement?.length > 0 && ( <DataTable data={control.OrganizationControlRequirement} /> )}Additionally, ensure that the
DataTablecomponent is designed to accept the data in this format, as it was previously receiving data from the now-removeduseOrganizationControlRequirementshook.
🏁 Script executed:
#!/bin/bash # Check the DataTable component implementation to verify expected props echo "Checking DataTable component implementation:" cat "apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Components/data-table/data-table.tsx" | grep -A 10 "interface" || echo "No interface definition found" echo "\nChecking DataTable usage elsewhere:" rg "DataTable\s+data=" --type=tsx -A 2Length of output: 620
Action: Confirm the proper array check for DataTable usage
- The
DataTablecomponent’s interface clearly defines itsdataprop as an array (RequirementTableData[]), so verifying that the data is non-empty by performing a length check is appropriate.- Changing the conditional from checking a truthy value to checking that
control.OrganizationControlRequirement?.length > 0ensures that the component is only rendered when an array with at least one element is available.- Diff Suggestion:
- {control.OrganizationControlRequirement && ( + {control.OrganizationControlRequirement?.length > 0 && ( <DataTable data={control.OrganizationControlRequirement} /> -)} +)}- Please ensure that
control.OrganizationControlRequirementis always returned as an array (even if empty) to maintain consistency across the codebase.apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Actions/getOrganizationControlRequirements.ts (2)
38-47: Enhanced query with nested includes for comprehensive data retrievalThe addition of nested includes for
organizationControlandcontrolRequirementimproves the data fetching strategy by retrieving related entities in a single query. This change ensures you have access to the control and evidence data when processing control requirements.
53-53: Included organizationEvidence in the queryIncluding
organizationEvidencein the query is essential for the PR objective of fixing evidence statuses across the app. This change works in conjunction with the database schema modifications and UI status checks.packages/db/prisma/seed.ts (3)
391-407: Improved evidence record tracking with variable assignmentStoring the result of the
upsertoperation in theevidencevariable enables the link-back operation in the next step. This is good practice as it avoids duplicate database queries.
409-417: Added explicit linking between evidence and control requirementsThis change ensures a bidirectional relationship between evidence records and control requirements, which is crucial for maintaining data integrity and the proper functioning of evidence status tracking across the application.
419-419: Added helpful logging for tracking seeding progressThe log message clearly indicates both processing and linking, providing better visibility into the seeding process. This is especially helpful when debugging data relationship issues.
packages/db/prisma/migrations/20250225000803_add_org_ev_relation_to_org_ctrl_req/migration.sql (2)
1-2: Added organizationEvidenceId column to support evidence relationshipThis migration adds the necessary database column to store the association between organization control requirements and organization evidence. This is a foundational change for the PR objective of fixing evidence statuses.
4-5: Added cascade foreign key constraint for data integrityThe foreign key constraint ensures referential integrity between the
OrganizationControlRequirementandOrganizationEvidencetables. The cascade behavior on delete and update ensures that related records stay in sync, preventing orphaned references.apps/app/src/components/tables/frameworks/columns.tsx (3)
35-37: Added organizationEvidence type with published flagThis type definition enhancement properly models the relationship between organization control requirements and organization evidence, with the
publishedflag being crucial for status tracking.
54-54: Updated evidence status check to use published flagThis change fixes the status logic for evidence requirements by checking the
publishedstate of the related organization evidence rather than content existence. This aligns with the PR objective of fixing evidence statuses.
115-115: Consistent evidence status implementation in table columnsThe same status check logic is correctly applied in the table columns, ensuring consistent status determination throughout the UI. This parallel change maintains code consistency and prevents status discrepancies.
apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Components/data-table/data-table-header.tsx (1)
4-5: Good type safety improvements.The changes correctly implement proper type safety by:
- Importing the specific
Tabletype from@tanstack/react-table- Using the
RequirementTableDatatype for table data- Making the
tableprop required rather than optional- Removing optional chaining where it's no longer needed
These changes will help prevent type-related bugs and improve code maintainability.
Also applies to: 8-8, 16-16
apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Actions/getOrganizationControlProgress.ts (2)
45-45: Good addition to include organizationEvidence in the query.Including
organizationEvidencein the query ensures that the evidence data is available for status checking, which aligns with the PR objective of fixing evidence statuses.
78-78: Correct implementation of evidence status check.The logic update properly checks for published status on organization evidence, consistent with how other requirement types are handled (e.g., policy type).
apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/hooks/useOrganizationControl.ts (1)
4-4:✅ Verification successful
Type simplification looks good but ensure type inference works correctly.
The removal of explicit type annotations simplifies the code but relies on TypeScript's type inference. This should be fine as long as the inferred types match the expected types throughout the application.
Also applies to: 6-6, 22-29
🏁 Script executed:
#!/bin/bash # Check the return type of getOrganizationControl to ensure type inference works correctly rg -A 5 "export const getOrganizationControl" --type tsLength of output: 3438
Subject: Type Inference Validated for Organization Control Hook
The explicit type annotations have been removed, and our check confirms that TypeScript correctly infers the return types from the associated zod schema in
getOrganizationControl(as defined inActions/getOrganizationControl.ts). The inferred types appear to match the expected structure, ensuring that the type simplification does not introduce discrepancies.
- In
apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/hooks/useOrganizationControl.ts(line 4), the import and subsequent usage rely on inferred types fromgetOrganizationControl.- Similar patterns at lines 6-6 and 22-29 follow the same inference mechanism, which is consistent with our investigation.
Given these findings, the refactoring to depend on type inference is sound.
apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Components/data-table/columns.tsx (1)
5-5: Good update to use RequirementTableData type.The column definitions now properly use the
RequirementTableDatatype, which aligns with the changes in the rest of the application.Also applies to: 7-7
apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Components/data-table/data-table.tsx (1)
19-23: Good addition of RequirementTableData type that extends OrganizationControlRequirement.This type definition improves type safety by explicitly defining the shape of data expected from your data source, including the relationships to OrganizationPolicy and OrganizationEvidence.
packages/db/prisma/schema.prisma (3)
293-298: Improved formatting for consistency.The realignment of the OrganizationControl model properties improves readability and maintains consistent formatting across the schema file.
975-976: Proper relation between OrganizationControlRequirement and OrganizationEvidence.The new bidirectional relation allows for efficient queries in both directions and ensures referential integrity with the Cascade delete behavior, which is appropriate for this parent-child relationship.
1064-1066: Established proper reverse relation for OrganizationEvidence.Completing the bidirectional relation from the OrganizationEvidence side enables efficient querying of related requirements, enhancing the overall data model integrity.
apps/app/src/actions/framework/select-frameworks-action.ts (4)
46-50: Correctly reordered operations to create evidence before requirements.This change ensures that organization evidence records exist before control requirements are created, fixing a potential issue where requirements might reference non-existent evidence.
214-215: Good inclusion of evidence in control requirements query.Including the evidence relationship in the query ensures you have all necessary data to establish the proper connections between requirements and evidence.
225-231: Added comprehensive fetch of organization evidences.Retrieving all organization evidences for the user's organization is an efficient approach to have the complete dataset available for linking with requirements.
267-268: Proper linking of evidence to control requirements.Including the organizationEvidenceId in the createMany operation ensures that control requirements are correctly linked to their respective organization evidence, which is essential for the status tracking functionality.
| // Define a type for the data returned by the useOrganizationControlRequirements hook | ||
| export type RequirementWithRelations = OrganizationControl & { | ||
| OrganizationControlRequirement: OrganizationControlRequirement & { | ||
| organizationPolicy: OrganizationPolicy; | ||
| organizationEvidence: OrganizationEvidence; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Type definition may need an array notation
The type definition suggests that an OrganizationControl has a single OrganizationControlRequirement, but based on the query structure in other files (like getOrganizationCategories.ts), it appears that an OrganizationControl can have multiple requirements.
Consider updating the type definition to use an array:
export type RequirementWithRelations = OrganizationControl & {
- OrganizationControlRequirement: OrganizationControlRequirement & {
+ OrganizationControlRequirement: (OrganizationControlRequirement & {
organizationPolicy: OrganizationPolicy;
organizationEvidence: OrganizationEvidence;
- };
+ })[];
};🏁 Script executed:
#!/bin/bash
# Check if OrganizationControlRequirement is defined as an array in database schema or other type definitions
# Look for OrganizationControl relation definitions
echo "Checking OrganizationControl relation definitions:"
rg -A 5 "OrganizationControl\s*{" --type=ts
# Check how OrganizationControlRequirement is used in queries
echo "\nChecking how OrganizationControlRequirement is queried:"
rg "OrganizationControlRequirement\W+\{" --type=ts -A 2Length of output: 2924
Action Required: Update type definition for OrganizationControlRequirement to an array
Based on our review of the code and how OrganizationControlRequirement is used across the codebase (e.g., in the getOrganizationCategories.ts file), it appears that an OrganizationControl can have multiple requirements. To maintain consistency with other parts of the code (where the property is declared with array notation), please update the type definition in:
apps/app/src/app/[locale]/(app)/(dashboard)/controls/[id]/Components/data-table/types.ts
Specifically, change the current definition:
export type RequirementWithRelations = OrganizationControl & {
- OrganizationControlRequirement: OrganizationControlRequirement & {
- organizationPolicy: OrganizationPolicy;
- organizationEvidence: OrganizationEvidence;
- };
+ OrganizationControlRequirement: (OrganizationControlRequirement & {
+ organizationPolicy: OrganizationPolicy;
+ organizationEvidence: OrganizationEvidence;
+ })[];
};This change will align the type with how it's handled in other query files (such as getOrganizationCategories.ts) and avoid potential runtime issues where multiple requirements are expected.
📝 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.
| // Define a type for the data returned by the useOrganizationControlRequirements hook | |
| export type RequirementWithRelations = OrganizationControl & { | |
| OrganizationControlRequirement: OrganizationControlRequirement & { | |
| organizationPolicy: OrganizationPolicy; | |
| organizationEvidence: OrganizationEvidence; | |
| }; | |
| }; | |
| // Define a type for the data returned by the useOrganizationControlRequirements hook | |
| export type RequirementWithRelations = OrganizationControl & { | |
| OrganizationControlRequirement: (OrganizationControlRequirement & { | |
| organizationPolicy: OrganizationPolicy; | |
| organizationEvidence: OrganizationEvidence; | |
| })[]; | |
| }; |
| : requirement.type === "evidence" | ||
| ? requirement.organizationEvidence?.published | ||
| : false; |
There was a problem hiding this comment.
Inconsistent handling of different requirement types.
While the evidence type logic is correctly implemented, the current implementation only handles "policy" and "evidence" types, defaulting all other types to false. This doesn't match the more comprehensive switch statement in getOrganizationControlProgress.ts which also handles "file" type and has a different default case.
Consider updating the logic to handle all requirement types consistently:
const isCompleted =
requirement.type === "policy"
? requirement.organizationPolicy?.status === "published"
- : requirement.type === "evidence"
- ? requirement.organizationEvidence?.published
- : false;
+ : requirement.type === "evidence"
+ ? requirement.organizationEvidence?.published === true
+ : requirement.type === "file"
+ ? !!requirement.fileUrl
+ : requirement.published;📝 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.
| : requirement.type === "evidence" | |
| ? requirement.organizationEvidence?.published | |
| : false; | |
| const isCompleted = | |
| requirement.type === "policy" | |
| ? requirement.organizationPolicy?.status === "published" | |
| : requirement.type === "evidence" | |
| ? requirement.organizationEvidence?.published === true | |
| : requirement.type === "file" | |
| ? !!requirement.fileUrl | |
| : requirement.published; |
Summary by CodeRabbit
New Features
Refactor
Chores