Revisions and UBOs documents fixes #2989
Conversation
|
WalkthroughThe pull request introduces a new optional Changes
Suggested Reviewers
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 (
|
services/workflows-service/src/workflow/hook-callback-handler.service.ts
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/backoffice-v2/src/lib/blocks/components/DirectorBlock/hooks/useDirectorBlock/helpers.ts (1)
7-7: Consider type safety with spread operator.While using spread operator simplifies the code, it might include unwanted properties. Consider explicitly typing the expected document properties or implementing a sanitization function.
Example type definition:
type DirectorDocument = { id: string; category: string; type: string; // ... other expected properties }; // Then use type assertion or sanitization const sanitizeDocument = (doc: unknown): DirectorDocument => { // Implementation to ensure only expected properties are included };Also applies to: 11-14
apps/backoffice-v2/src/lib/blocks/components/EditableDetails/EditableDetails.tsx (1)
103-103: LGTM! Consider adding TypeScript type annotations.The addition of
directorIdprop and its integration with the mutation hook looks good. However, consider adding explicit type annotations:- directorId, + directorId?: string,Also applies to: 137-137
services/workflows-service/src/workflow/hook-callback-handler.service.ts (1)
366-370: LGTM! Consider adding type safety.The spread operator usage correctly preserves existing documents while adding new ones, effectively preventing KYC documents from being overridden. However, consider adding type safety:
- context.documents = [ - // @ts-expect-error - we don't validate `context` is an object - ...context.documents, - ...persistedDocuments, - ]; + context.documents = [ + ...(Array.isArray(context.documents) ? context.documents : []), + ...persistedDocuments, + ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/backoffice-v2/src/domains/workflows/hooks/mutations/useUpdateDocumentByIdMutation/useUpdateDocumentByIdMutation.tsx(2 hunks)apps/backoffice-v2/src/lib/blocks/components/Details/Details.tsx(2 hunks)apps/backoffice-v2/src/lib/blocks/components/DirectorBlock/hooks/useDirectorBlock/helpers.ts(1 hunks)apps/backoffice-v2/src/lib/blocks/components/DirectorBlock/hooks/useDirectorBlock/useDirectorBlock.tsx(5 hunks)apps/backoffice-v2/src/lib/blocks/components/EditableDetails/EditableDetails.tsx(2 hunks)apps/backoffice-v2/src/lib/blocks/components/EditableDetails/interfaces.ts(1 hunks)apps/backoffice-v2/src/lib/blocks/create-blocks-typed/types.ts(1 hunks)services/workflows-service/prisma/data-migrations(1 hunks)services/workflows-service/src/workflow/dtos/document-update-update-input.ts(2 hunks)services/workflows-service/src/workflow/hook-callback-handler.service.ts(1 hunks)services/workflows-service/src/workflow/workflow.controller.internal.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- services/workflows-service/prisma/data-migrations
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Analyze (javascript)
- GitHub Check: lint
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (8)
services/workflows-service/src/workflow/dtos/document-update-update-input.ts (1)
12-18: LGTM! Properly validated optional directorId.The addition of
directorIdwith proper validation and API documentation aligns with the PR objectives.Let's verify consistent usage of this new parameter across the codebase:
✅ Verification successful
Verified: directorId is consistently implemented across the codebase
The optional directorId parameter is properly integrated throughout the application:
- Validated in backend DTOs
- Correctly handled in workflow service and controller
- Consistently used in frontend components and mutations
- Proper typing and error handling in place
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for directorId usage in TypeScript files rg -t typescript "directorId.*?:" --multilineLength of output: 82
Script:
#!/bin/bash # Search for directorId in all files rg "directorId" -A 2 -B 2 # Find property definitions containing directorId ast-grep --pattern 'directorId: $_'Length of output: 39017
apps/backoffice-v2/src/lib/blocks/components/EditableDetails/interfaces.ts (1)
25-25: LGTM! Interface updated consistently.The addition of optional
directorIdmatches the DTO changes and maintains type consistency.apps/backoffice-v2/src/lib/blocks/components/Details/Details.tsx (1)
13-13: LGTM! Proper prop handling.The
directorIdis correctly passed through to EditableDetails component, maintaining consistency with the interface changes.Also applies to: 38-38
apps/backoffice-v2/src/domains/workflows/hooks/mutations/useUpdateDocumentByIdMutation/useUpdateDocumentByIdMutation.tsx (1)
10-10: LGTM! Clean implementation of the director ID parameter.The changes correctly introduce the optional
directorIdparameter and properly integrate it into the mutation payload, aligning with the PR objectives to fix issues with updating director properties.Also applies to: 14-14, 34-34
apps/backoffice-v2/src/lib/blocks/create-blocks-typed/types.ts (1)
116-116: LGTM! Type definition aligns with the feature requirements.The optional
directorIdproperty is correctly added to theTDetailsCelltype, maintaining consistency with the changes across other files.services/workflows-service/src/workflow/workflow.controller.internal.ts (1)
248-248: LGTM! Backend integration of the director ID parameter.The
directorIdis correctly extracted from the input data and properly passed to the service method, completing the end-to-end integration of the feature.apps/backoffice-v2/src/lib/blocks/components/DirectorBlock/hooks/useDirectorBlock/useDirectorBlock.tsx (2)
99-102: Good optimization: Removing unnecessary imageUrl from document data.The transformation efficiently removes the
imageUrlproperty while preserving all other document data, preventing unnecessary data from being saved.
119-119: LGTM! Consistent integration of director context and document optimization.The changes properly:
- Add the director context through
directorId- Use the optimized document data without
imageUrl- Include clear comments explaining the optimization
Also applies to: 132-133, 335-335, 376-377
Summary by CodeRabbit
New Features
directorIdparameter to several components and servicesImprovements
Technical Updates