feat: reworked documents revision & added soft deletion for documents & files #3454
Conversation
|
|
Warning Rate limit exceeded@chesterkmr has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 7 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis update introduces a soft deletion mechanism for documents and document files by adding a Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend
participant API
participant CaseManagementService
participant DocumentFileService
participant WorkflowService
participant DB
Frontend->>API: POST /workflows/:workflowId/revision (documentIds)
API->>CaseManagementService: caseRevision(workflowId, documentIds, projectIds)
CaseManagementService->>DocumentFileService: deleteManyByDocumentIds(documentIds, projectIds)
DocumentFileService->>DB: Update DocumentFile (set deletedAt)
CaseManagementService->>WorkflowService: triggerEvent('revision', workflowId, projectIds)
API-->>Frontend: Response (deleted files info)
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
🔭 Outside diff range comments (1)
services/workflows-service/src/document-file/document-file.repository.ts (1)
127-146: Add missing args parameter for consistency.The new method lacks an
argsparameter that other repository methods have, breaking the established pattern and limiting flexibility.async deleteManyByDocumentIds( documentIds: string[], projectIds: TProjectId[], + args?: Prisma.DocumentFileUpdateManyArgs, transaction: PrismaTransactionClient = this.prismaService, ) { return transaction.documentFile.updateMany( this.projectScopeService.scopeUpdateMany( { + ...args, where: { + ...args?.where, deletedAt: null, documentId: { in: documentIds }, }, data: { deletedAt: new Date(), }, }, projectIds, ), ); }
🧹 Nitpick comments (4)
services/workflows-service/src/case-management/controllers/case-management.controller.ts (1)
284-291: LGTM! Well-structured controller endpoint.The
caseRevisionendpoint correctly:
- Uses appropriate POST method for the revision operation
- Follows existing controller patterns with proper decorators
- Includes multi-tenant support via
@ProjectIds()decorator- Maintains clean separation of concerns by delegating to service layer
- Uses descriptive endpoint path
/workflows/:workflowId/revisionConsider adding API documentation decorators (e.g.,
@ApiOperation,@ApiResponse) and explicit error handling for consistency with other endpoints in the controller.apps/backoffice-v2/src/pages/Entity/components/Case/actions-variants/DefaultActions/hooks/useDefaultActionsLogic/useDefaultActionsLogic.tsx (2)
53-61: Consider memoization optimization and naming consistency.The document filtering and ID extraction logic is correct, but there are some optimization and naming opportunities.
Consider these improvements:
- const documentsUnderRevision = useMemo( + const documentsForRevision = useMemo( () => documents?.filter(document => document?.decision?.status === 'revision'), [documents], ); - const documentsUnderRevisionIds = useMemo( - () => documentsUnderRevision?.map(document => document?.id!) || [], - [documentsUnderRevision], + const documentIds = useMemo( + () => documentsForRevision?.map(document => document?.id!) || [], + [documentsForRevision], );This improves naming consistency and clarity.
80-80: Update variable reference for consistency.The variable reference should be updated to match the renamed variable from the previous suggestion.
- documentsToReviseCount: documentsUnderRevision?.length ?? 0, + documentsToReviseCount: documentsForRevision?.length ?? 0,services/workflows-service/src/case-management/case-management.service.ts (1)
293-293: Consider using a more descriptive event name.The workflow event name
'revision'is generic. Consider using a more specific name like'document_revision'or'case_revision'to better describe the specific type of revision being performed.- name: 'revision', + name: 'case_revision',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
apps/backoffice-v2/src/domains/workflows/fetchers.ts(1 hunks)apps/backoffice-v2/src/domains/workflows/hooks/mutations/useRevisionCaseMutation/useRevisionCaseMutation.tsx(1 hunks)apps/backoffice-v2/src/pages/Entity/components/Case/actions-variants/DefaultActions/hooks/useDefaultActionsLogic/useDefaultActionsLogic.tsx(3 hunks)apps/backoffice-v2/src/pages/Entity/components/Case/hooks/usePendingRevisionEvents/usePendingRevisionEvents.tsx(2 hunks)packages/ui/src/components/organisms/Form/DynamicForm/fields/EntityFieldGroup/EntityFieldGroup.tsx(1 hunks)services/workflows-service/package.json(1 hunks)services/workflows-service/prisma/data-migrations(1 hunks)services/workflows-service/prisma/migrations/20250625091910_add_deleted_at_to_document_and_document_files/migration.sql(1 hunks)services/workflows-service/prisma/schema.prisma(2 hunks)services/workflows-service/src/case-management/case-management.module.ts(2 hunks)services/workflows-service/src/case-management/case-management.service.ts(4 hunks)services/workflows-service/src/case-management/controllers/case-management.controller.ts(2 hunks)services/workflows-service/src/case-management/dtos/case-revision.dto.ts(1 hunks)services/workflows-service/src/document-file/document-file.repository.ts(3 hunks)services/workflows-service/src/document-file/document-file.service.ts(1 hunks)services/workflows-service/src/document/document.repository.ts(10 hunks)services/workflows-service/src/document/document.service.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`services/workflows-service/**/*.service.ts`: Service implementation files must be placed in feature modules and named with the .service.ts suffix.
services/workflows-service/**/*.service.ts: Service implementation files must be placed in feature modules and named with the .service.ts suffix.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/workflows-service.mdc)
List of files the instruction was applied to:
services/workflows-service/src/document/document.service.tsservices/workflows-service/src/document-file/document-file.service.tsservices/workflows-service/src/case-management/case-management.service.ts
`services/workflows-service/**/*.controller.ts`: Controller implementation files must be placed in feature modules and named with the .controller.ts suffix.
services/workflows-service/**/*.controller.ts: Controller implementation files must be placed in feature modules and named with the .controller.ts suffix.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/workflows-service.mdc)
List of files the instruction was applied to:
services/workflows-service/src/case-management/controllers/case-management.controller.ts
`apps/backoffice-v2/**/*.{ts,tsx}`: Use functional components with TypeScript. I...
apps/backoffice-v2/**/*.{ts,tsx}: Use functional components with TypeScript.
Implement smart/dumb component pattern.
Place components in feature-based directories.
Use compound components for complex UIs.
Follow atomic design principles.
Use React Query for server state and API calls.
Use Context for shared state.
Implement state machines for complex flows.
Use local state for UI-only state.
Follow unidirectional data flow.
Use strict TypeScript configuration.
Define interfaces for all props.
Use discriminated unions for state.
Leverage type inference.
Use Radix UI for accessible components.
Implement proper ARIA attributes.
Follow consistent styling patterns.
Use composition over inheritance.
Keep components small and focused.
Use React Hook Form for forms.
Implement Zod for validation.
Handle form submission states.
Show validation feedback.
Use controlled inputs when needed.
Implement proper loading states for data fetching.
Handle error states gracefully.
Cache responses appropriately.
Type API responses.
Use error boundaries.
Implement fallback UI.
Handle async errors.
Show user-friendly error messages.
Log errors appropriately.
Use React.memo wisely.
Implement proper code splitting.
Use lazy loading for routes.
Optimize re-renders.
Profile performance regularly.
Write unit tests for components.
Use React Testing Library.
Mock external dependencies in tests.
Maintain good test coverage.
Follow feature-based organization.
Use index files for exports.
Keep related files together.
Use consistent naming.
Implement barrel exports.
Use Tailwind CSS for styling.
Follow utility-first approach for styling.
Use CSS variables for theming.
Keep styles maintainable.
Use CSS modules when needed.
Document complex logic.
Write clear component documentation.
Keep documentation up to date.
Use JSDoc when helpful.
Follow ESLint rules.
Use consistent formatting.
Write clear variable names.
Keep functions pure.
Use meaningful types.
Validate user input.
Implement proper authentication.
Handle sensitive data carefully.
Follow security best practices.
Use HTTPS for API calls.
Follow WCAG guidelines for accessibility.
Use semantic HTML.
Test with screen readers.
Ensure keyboard navigation.
Provide proper focus management.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/backoffice-v2.mdc)
List of files the instruction was applied to:
apps/backoffice-v2/src/domains/workflows/fetchers.tsapps/backoffice-v2/src/domains/workflows/hooks/mutations/useRevisionCaseMutation/useRevisionCaseMutation.tsxapps/backoffice-v2/src/pages/Entity/components/Case/hooks/usePendingRevisionEvents/usePendingRevisionEvents.tsxapps/backoffice-v2/src/pages/Entity/components/Case/actions-variants/DefaultActions/hooks/useDefaultActionsLogic/useDefaultActionsLogic.tsx
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Analyze (javascript)
- GitHub Check: test_linux
- GitHub Check: build (ubuntu-latest)
- GitHub Check: lint
🔇 Additional comments (32)
services/workflows-service/prisma/data-migrations (1)
1-1: Submodule bump LGTM – validate migration is applied in all envsThe update merely advances the
data-migrationssubmodule SHA. No code issues here, but please double-check that:
- The new migration introducing
deletedAthas been executed locally and in CI.- Rollback scripts exist (or are not needed) in case the soft-delete column causes unforeseen issues in staging/production.
If both checks are green, this change is good to merge.
packages/ui/src/components/organisms/Form/DynamicForm/fields/EntityFieldGroup/EntityFieldGroup.tsx (1)
222-225: LGTM! Proper cleanup implementation.The addition of a cleanup function to remove tasks when the component unmounts or before the effect re-runs is a good practice that prevents memory leaks and ensures proper resource management.
services/workflows-service/src/case-management/case-management.module.ts (2)
10-10: LGTM! Required import for new functionality.Adding the
DocumentFileModuleimport is necessary to support the new case revision functionality that involves document file operations.
20-20: LGTM! Module integration for document file services.Adding
DocumentFileModuleto the imports enables dependency injection ofDocumentFileServicein the case management service, supporting the new soft deletion and revision features.services/workflows-service/package.json (1)
22-22: LGTM! Developer experience improvement.Adding the
--watchflag to the debug script enables automatic restart on file changes, improving the development workflow when debugging the workflows-service.services/workflows-service/prisma/migrations/20250625091910_add_deleted_at_to_document_and_document_files/migration.sql (1)
1-5: LGTM! Well-structured soft deletion migration.The migration correctly adds nullable
deletedAtcolumns with millisecond precision to bothDocumentandDocumentFiletables. This enables proper soft deletion functionality while preserving existing data.services/workflows-service/src/document/document.service.ts (2)
47-50: LGTM! Cleaner import organization.Consolidating the import statements for
defaultPrismaTransactionOptionsandbeginTransactionIfNotExistCurryimproves code organization and readability.
96-108: ```shell
#!/bin/bashLocate the createDocumentFile method to inspect its implementation
rg -n "async createDocumentFile" -A30 -B5 services/workflows-service/src/document/document.service.ts
</details> <details> <summary>services/workflows-service/src/case-management/dtos/case-revision.dto.ts (1)</summary> `1-8`: **LGTM! Well-structured DTO with proper validation.** The DTO correctly implements validation for the case revision request: - Proper array validation with `@IsArray()` and `@ArrayNotEmpty()` - Element-level string validation with `@IsString({ each: true })` - Clean TypeScript typing and follows naming conventions </details> <details> <summary>services/workflows-service/prisma/schema.prisma (1)</summary> `1019-1021`: **LGTM! Proper soft deletion implementation.** The addition of nullable `deletedAt` fields to both `Document` and `DocumentFile` models correctly implements soft deletion: - Consistent with existing soft deletion patterns in the schema (e.g., `WorkflowRuntimeData`) - Proper nullable `DateTime?` type allows distinguishing between active and soft-deleted records - Field placement is logical after timestamp fields Also applies to: 1066-1068 </details> <details> <summary>services/workflows-service/src/document-file/document-file.service.ts (1)</summary> `63-69`: **LGTM! Clean service method implementation.** The new `deleteManyByDocumentIds` method properly: - Follows existing service patterns with consistent parameter structure - Includes transaction support for atomic operations - Uses appropriate typing with `TProjectId[]` and optional `PrismaTransactionClient` - Delegates to repository layer maintaining separation of concerns </details> <details> <summary>apps/backoffice-v2/src/domains/workflows/fetchers.ts (1)</summary> `402-421`: **LGTM! Well-implemented fetcher function.** The `fetchCaseRevisionForDocuments` function correctly: - Uses proper TypeScript typing and parameter structure - Makes appropriate POST request to the revision endpoint - Includes proper error handling with `handleZodError` - Body structure aligns with backend `CaseRevisionDto` expectations The `z.any()` schema is acceptable for now, but consider adding a more specific response schema in the future for better type safety. </details> <details> <summary>services/workflows-service/src/case-management/controllers/case-management.controller.ts (1)</summary> `34-34`: **LGTM! Clean import addition.** Proper import of the new `CaseRevisionDto` following existing import patterns. </details> <details> <summary>apps/backoffice-v2/src/pages/Entity/components/Case/hooks/usePendingRevisionEvents/usePendingRevisionEvents.tsx (3)</summary> `7-15`: **LGTM: Clean API refactoring with proper TypeScript definitions.** The hook signature refactoring to use a single object parameter is a good improvement that makes the API more maintainable and follows React best practices. The TypeScript interface properly defines all required parameters. --- `21-21`: **Verify the mutation contract aligns with backend expectations.** The mutation call now includes `documentIds` which should align with the new backend API. Ensure that the backend endpoint expects this parameter structure. --- `35-35`: **Dependency array correctly updated.** The useCallback dependency array properly includes the new `documentIds` parameter, ensuring the callback is recreated when document IDs change. </details> <details> <summary>apps/backoffice-v2/src/pages/Entity/components/Case/actions-variants/DefaultActions/hooks/useDefaultActionsLogic/useDefaultActionsLogic.tsx (2)</summary> `38-39`: **LGTM: Simplified mutation hook usage.** Removing the `onSelectNextCase` parameter from the mutation hook simplifies the API and follows the separation of concerns principle. --- `63-67`: **LGTM: Proper object parameter structure.** The updated call to `usePendingRevisionEvents` correctly uses the new object parameter structure with all required fields. </details> <details> <summary>apps/backoffice-v2/src/domains/workflows/hooks/mutations/useRevisionCaseMutation/useRevisionCaseMutation.tsx (3)</summary> `9-9`: **LGTM: Simplified hook signature.** Removing external parameters from the hook constructor makes it more reusable and focused on its core responsibility. --- `13-17`: **LGTM: Proper TypeScript interface for mutation parameters.** The mutation function correctly defines the required parameters with proper TypeScript typing. The parameter structure aligns with the backend API expectations. --- `21-21`: **Good addition: Document query invalidation.** Adding `documentsQueryKeys._def` to the invalidation list ensures that document-related queries are refreshed after the revision operation, maintaining cache consistency. </details> <details> <summary>services/workflows-service/src/document/document.repository.ts (5)</summary> `42-52`: **LGTM: Consistent soft deletion filtering in findMany.** The soft deletion filter is properly implemented with `deletedAt: null` to exclude soft-deleted records. --- `68-68`: **LGTM: Soft deletion filtering applied consistently.** All query methods consistently apply the `deletedAt: null` filter to exclude soft-deleted records. --- `163-165`: **LGTM: Proper soft deletion filtering for related files.** The files relation correctly filters out soft-deleted document files with `deletedAt: null`. --- `302-333`: **LGTM: Proper soft deletion implementation with transaction safety.** The `deleteByIds` method correctly implements soft deletion by: 1. First soft deleting related documentFile records 2. Then soft deleting the document records 3. Using proper scoping with project IDs 4. Maintaining transaction safety The order of operations ensures referential integrity is maintained. --- `345-345`: **LGTM: Consistent soft deletion filtering in document files query.** The document files query properly filters out soft-deleted records. </details> <details> <summary>services/workflows-service/src/case-management/case-management.service.ts (3)</summary> `29-29`: **LGTM: Proper service dependency injection.** The DocumentFileService is correctly injected into the constructor to support the new revision functionality. --- `269-302`: **LGTM: Well-structured caseRevision method with proper transaction handling.** The method implementation follows best practices: - Proper input validation with `assertIsValidProjectIds` - Transaction management using utility functions - Clear separation of concerns by delegating file deletion to helper method - Proper workflow event triggering - Good return value for operation tracking --- `304-318`: **LGTM: Clean helper method with proper delegation.** The `removeFilesFromDocuments` helper method: - Validates project IDs consistently - Properly delegates to the DocumentFileService - Maintains clear responsibility boundaries - Returns meaningful result for tracking </details> <details> <summary>services/workflows-service/src/document-file/document-file.repository.ts (3)</summary> `48-48`: **LGTM - Proper soft deletion filtering implemented.** The addition of `deletedAt: null` correctly excludes soft-deleted records from query results, which is essential for maintaining data integrity in a soft deletion system. --- `109-125`: **Soft deletion implementation is consistent.** The implementation follows the same pattern as `deleteById` and correctly handles soft deletion by document ID. --- `85-100`: **Soft deletion implementation is correct but verify method contract.** The conversion from hard delete to soft delete is properly implemented with the `deletedAt` timestamp. However, ensure that callers expecting a delete operation are compatible with the `updateMany` return type. ```shell #!/bin/bash # Verify usage of deleteById method to ensure compatibility with updateMany return type ast-grep --pattern 'deleteById($$$)' | head -20
services/workflows-service/src/document-file/document-file.repository.ts
Outdated
Show resolved
Hide resolved
services/workflows-service/src/document-file/document-file.repository.ts
Outdated
Show resolved
Hide resolved
| ); | ||
|
|
||
| const documentsUnderRevisionIds = useMemo( | ||
| () => documentsUnderRevision?.map(document => document?.id!) || [], |
There was a problem hiding this comment.
Do we need to filter out undefined items?
There was a problem hiding this comment.
Good point. Added filtering.
But its something that should be removed after complete v1 documents deprecation.
| } | ||
|
|
||
| mutateRevisionCase({ workflowId: workflow?.id }); | ||
| mutateRevisionCase({ workflowId: workflow?.id!, documentIds }); |
There was a problem hiding this comment.
Let's add an early return with an error toast / log
There was a problem hiding this comment.
Added few toasts here.
| name: 'revision', | ||
| }, | ||
| projectIds, | ||
| projectIds[0]!, |
There was a problem hiding this comment.
Validate over null assertion.
There was a problem hiding this comment.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/backoffice-v2/public/locales/en/toast.json(1 hunks)apps/backoffice-v2/src/pages/Entity/components/Case/hooks/usePendingRevisionEvents/usePendingRevisionEvents.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
apps/backoffice-v2/src/pages/Entity/components/Case/hooks/usePendingRevisionEvents/usePendingRevisionEvents.tsx (4)
Learnt from: CR
PR: ballerine-io/ballerine#0
File: .cursor/rules/workflows-dashboard.mdc:0-0
Timestamp: 2025-06-24T09:37:03.204Z
Learning: Custom hooks in React should be single-purpose, follow the 'use' prefix naming convention, and be placed within feature directories for better organization and reusability.
Learnt from: CR
PR: ballerine-io/ballerine#0
File: .cursor/rules/backoffice-v2.mdc:0-0
Timestamp: 2025-06-24T09:35:48.303Z
Learning: Extract business logic into custom React hooks, placing them in dedicated hooks directories and using the 'use' prefix for naming, to promote reusability and separation of concerns.
Learnt from: CR
PR: ballerine-io/ballerine#0
File: .cursor/rules/backoffice-v2.mdc:0-0
Timestamp: 2025-06-24T09:35:48.303Z
Learning: Document complex logic, component usage, and custom hooks clearly, keeping documentation up to date and using JSDoc where helpful.
Learnt from: CR
PR: ballerine-io/ballerine#0
File: .cursor/rules/data-migrations.mdc:0-0
Timestamp: 2025-06-24T09:35:54.963Z
Learning: In Ballerine's workflow migration scripts (TypeScript), always establish the relationship between workflow definitions and UI definitions solely through the 'workflowDefinitionId' field in the UiDefinition model; do not create a separate junction table or relation.
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Analyze (javascript)
- GitHub Check: test_linux
- GitHub Check: spell_check
- GitHub Check: build (ubuntu-latest)
- GitHub Check: lint
- GitHub Check: format
🔇 Additional comments (3)
apps/backoffice-v2/src/pages/Entity/components/Case/hooks/usePendingRevisionEvents/usePendingRevisionEvents.tsx (2)
9-17: LGTM: Clean refactoring to object parameter pattern.The change from separate parameters to an object parameter improves readability and maintainability, especially with the addition of the
documentIdsarray.
47-47: LGTM: Correct dependency array management.The useCallback dependency array correctly includes all referenced variables, ensuring proper memoization behavior.
apps/backoffice-v2/public/locales/en/toast.json (1)
164-166: LGTM: Appropriate generic error message for fallback handling.The new common error message provides a good user-facing fallback for unexpected errors and aligns well with the error handling pattern implemented in the hook.
...src/pages/Entity/components/Case/hooks/usePendingRevisionEvents/usePendingRevisionEvents.tsx
Show resolved
Hide resolved
chesterkmr
force-pushed
the
bal-4355-when-a-document-is-in-revision-users-can-submit-the-form
branch
from
0086d5f to
d659fe2
Compare
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores
Database