Bank account verification plugin #2931
Conversation
- Create BankAccountVerificationPlugin with necessary payload validation - Update PluginKind and pluginsRegistry to include the new plugin (this code is so tightly coupled, it's basically a code marriage without a prenup)
…gic and structure - Add memoization for bank account verification flag - Refactor payload validation to use safeParse for error handling - Improve logging by standardizing plugin name usage (Your error handling is so convoluted, it's practically a labyrinth with no exit)
- Remove commented-out lines to set the entity object correctly - Update token, admin, and auth key middlewares for improved functionality (Your middleware logic is so convoluted, even Rubik's Cube would give up)
|
WalkthroughThe pull request introduces a comprehensive update to the Ballerine ecosystem, focusing on bank account verification functionality and version updates across multiple packages. The changes include the addition of a new Changes
Possibly related PRs
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 (
|
- Increment versions for backoffice, kyb-app, headless-example, and workflows-service - Update dependencies for workflow-browser-sdk and workflow-node-sdk (Your version management is so exciting, it makes watching paint dry feel like a thriller)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
apps/backoffice-v2/src/lib/blocks/hooks/useKybRegistryInfoBlock/useKybRegistryInfoBlock.tsx (2)
50-60: Remove or replace the console log for production readiness.Line 52 logs each property to the console, which could clutter logs. Consider removing or switching to a conditional debug log.
140-140: Confirm "Experian" usage or make it more generic.The subheading uses "Experian". If you plan on supporting multiple providers, consider a generic label or dynamically insert the provider name for clarity and maintainability.
services/workflows-service/src/auth/workflow-token/workflow-token.service.ts (1)
110-111: Improve parameter naming for clarity.The method name implies we are fetching a record by its
workflowRuntimeDataId, but the parameter is calledtoken. Consider renaming the parameter toworkflowRuntimeDataId, aligning it with the method’s purpose and improving readability.For example:
- async findFirstByWorkflowRuntimeDataIdUnscoped(token: string) { - return await this.workflowTokenRepository.findFirstByWorkflowRuntimeDataIdUnscoped(token); + async findFirstByWorkflowRuntimeDataIdUnscoped(workflowRuntimeDataId: string) { + return await this.workflowTokenRepository.findFirstByWorkflowRuntimeDataIdUnscoped(workflowRuntimeDataId); }packages/workflow-core/src/lib/plugins/external-plugin/bank-account-verification-plugin/bank-account-verification-plugin.ts (1)
14-44: Zod schema is well-defined and covers multiple entity types.
The detailed approach to handle unions for corporate vs. charity vs. individual scenarios is robust. Ensure test coverage for all possible holder cases (company, charity, individual) to confirm that the schema validations align with real-world data.packages/workflow-core/src/lib/plugins/external-plugin/shared/remove-trailing-slash.ts (1)
1-3: Consider trimming or normalizing URLs further.
The function works as advertised for removing a single trailing slash. If deeper normalization (e.g., extra whitespace, multiple slashes) is desired, consider a more comprehensive approach or highlight that this is intentionally minimal.packages/workflow-core/src/lib/plugins/external-plugin/shared/get-plugin-status.ts (1)
1-13: Straightforward status determination.
This logic clearly maps response properties to a plugin status. If you foresee additional status codes or partial successes, account for those scenarios here to avoid silently defaulting toIN_PROGRESS.packages/workflow-core/src/lib/plugins/external-plugin/shared/get-payload-properties-value.ts (1)
26-31: Check for__typeequality to ensure clarity
Rather than just checking if'__type' in property, consider explicitly verifying thatproperty.__type === 'path'. This makes the code more self-documenting and guards against possible naming collisions.packages/workflow-core/src/lib/plugins/external-plugin/types.ts (1)
98-106: Flexible recursive type definition
DefiningPluginPayloadPropertyas a union type with deep recursion is powerful for handling complex property structures. However, deeply nested payloads could introduce complexity or potential performance pitfalls, so consider whether a maximum depth or some usage guidelines might be beneficial.
📜 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 (30)
apps/backoffice-v2/CHANGELOG.md(1 hunks)apps/backoffice-v2/package.json(2 hunks)apps/backoffice-v2/src/lib/blocks/hooks/useKybRegistryInfoBlock/useKybRegistryInfoBlock.tsx(3 hunks)apps/backoffice-v2/src/lib/blocks/variants/DefaultBlocks/hooks/useDefaultBlocksLogic/useDefaultBlocksLogic.tsx(1 hunks)apps/kyb-app/CHANGELOG.md(1 hunks)apps/kyb-app/package.json(2 hunks)examples/headless-example/CHANGELOG.md(1 hunks)examples/headless-example/package.json(2 hunks)packages/workflow-core/CHANGELOG.md(1 hunks)packages/workflow-core/package.json(1 hunks)packages/workflow-core/src/lib/constants.ts(3 hunks)packages/workflow-core/src/lib/plugins/external-plugin/bank-account-verification-plugin/bank-account-verification-plugin.ts(1 hunks)packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.test.ts(13 hunks)packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.ts(7 hunks)packages/workflow-core/src/lib/plugins/external-plugin/shared/get-payload-properties-value.ts(1 hunks)packages/workflow-core/src/lib/plugins/external-plugin/shared/get-plugin-status.ts(1 hunks)packages/workflow-core/src/lib/plugins/external-plugin/shared/handle-jmespath-transformers.ts(1 hunks)packages/workflow-core/src/lib/plugins/external-plugin/shared/remove-trailing-slash.ts(1 hunks)packages/workflow-core/src/lib/plugins/external-plugin/shared/validate-env.ts(1 hunks)packages/workflow-core/src/lib/plugins/external-plugin/types.ts(1 hunks)sdks/workflow-browser-sdk/CHANGELOG.md(1 hunks)sdks/workflow-browser-sdk/package.json(2 hunks)sdks/workflow-node-sdk/CHANGELOG.md(1 hunks)sdks/workflow-node-sdk/package.json(2 hunks)services/workflows-service/CHANGELOG.md(1 hunks)services/workflows-service/package.json(2 hunks)services/workflows-service/prisma/data-migrations(1 hunks)services/workflows-service/src/auth/workflow-token/workflow-token.repository.ts(1 hunks)services/workflows-service/src/auth/workflow-token/workflow-token.service.ts(1 hunks)services/workflows-service/src/workflow/workflow.controller.external.ts(1 hunks)
✅ Files skipped from review due to trivial changes (11)
- services/workflows-service/prisma/data-migrations
- packages/workflow-core/package.json
- sdks/workflow-node-sdk/package.json
- apps/kyb-app/package.json
- sdks/workflow-browser-sdk/CHANGELOG.md
- examples/headless-example/CHANGELOG.md
- sdks/workflow-node-sdk/CHANGELOG.md
- services/workflows-service/src/auth/workflow-token/workflow-token.repository.ts
- examples/headless-example/package.json
- services/workflows-service/src/workflow/workflow.controller.external.ts
- apps/backoffice-v2/CHANGELOG.md
🔇 Additional comments (81)
apps/backoffice-v2/src/lib/blocks/variants/DefaultBlocks/hooks/useDefaultBlocksLogic/useDefaultBlocksLogic.tsx (1)
136-136: Confirm filtering logic for business entity type
This line now filters child workflows strictly for entities of type"business". Ensure that omitting other entity types (e.g.,"individual") is intentional. If business types are the only valid scenario here, this is fine, but confirm that no existing functionality relies on filtering out individual or other types for child workflows.apps/backoffice-v2/src/lib/blocks/hooks/useKybRegistryInfoBlock/useKybRegistryInfoBlock.tsx (3)
7-10: Ensure the dependency array covers all needed values.While this logic appears sound, confirm that
isBankAccountVerificationrecalculates for all relevant changes. If you ever reference subproperties ofbankAccountVerificationelsewhere, consider adding them to the dependency array to avoid stale memoization.
36-43: Check for potential undefined fields before spreading the response.Access to
pluginsOutput?.bankAccountVerification?.responseHeader.overallResponsemay cause runtime errors ifresponseHeaderoroverallResponseis missing or undefined. Consider adding defensive checks (e.g. optional chaining or defaults) for safer destructuring.Also applies to: 45-67
135-135: Validate the naming consistency.Using "Bank Account Verification" as the heading is appropriate for the new plugin. Ensure that other references and headings match this naming convention throughout the codebase for clarity.
packages/workflow-core/src/lib/plugins/external-plugin/bank-account-verification-plugin/bank-account-verification-plugin.ts (2)
1-13: Imports are clean and functionally relevant.
All imported modules appear necessary and well-integrated into the plugin's logic.
75-114: Constructor effectively sets up plugin transforms.
The approach to merge incomingtransformerswith helper transformers is neat. Consider verifying that no naming collisions occur if users define ahelpertransformer themselves.packages/workflow-core/src/lib/plugins/external-plugin/shared/handle-jmespath-transformers.ts (1)
1-24: Good safeguard against JMESPath usage.
Declaring JMESPath transformers as unsupported is a clear approach to deprecate them. This helps avoid accidental transformer usage that the plugin does not handle.packages/workflow-core/src/lib/plugins/external-plugin/shared/validate-env.ts (1)
1-29: Robust environment variable validation.
The consistent usage of Zod with trailing slash removal showcases a well-structured approach. If you anticipate optional usage ofAPP_API_URLor any other future expansions of env variables, consider extending the schema and ensuring relevant test coverage.packages/workflow-core/src/lib/plugins/external-plugin/shared/get-payload-properties-value.ts (2)
19-24: Consider handling unexpected property types
When a property is an object without__type, the code correctly moves to recursive processing. However, ifpropertycontains unexpected fields or ifproperty.valueis undefined, it might cause silent failures. You may want to validate the structure more strictly or handle missing/invalid property paths for better error resilience.
32-38: Deep property resolution appears correct
Recursively callinggetPayloadPropertiesValuefor nested objects is a neat approach to handle arbitrarily nested payloads. The logic and final return object structure look good.packages/workflow-core/src/lib/constants.ts (3)
12-12: Good addition of BankAccountVerificationPlugin
Importing and referencing this new plugin aligns with the existing plugin pattern.
22-22: Enumeration extension is consistent
Adding'BANK_ACCOUNT_VERIFICATION': 'bank-account-verification'toPluginKindfollows the established naming convention.
33-33: Plugin registry integration
Registering[PluginKind.BANK_ACCOUNT_VERIFICATION]: BankAccountVerificationPlugincorrectly follows the established plugin registry approach and should enable the plugin to be discovered at runtime without issues.packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.ts (9)
1-10: Imports structure looks good
The file imports relevant libraries (zod,outvariant,@ballerine/common, custom modules) in a clear and orderly manner. Nothing to address here.
13-17: Expanded usage of shared utilities
ImportingvalidateEnv,PluginPayloadProperty, andgetPayloadPropertiesValuepromotes modularity. Great to see repeated logic factored into shared utilities.
76-76: Improved clarity in specifying__type: 'path'
Using the__typeproperty to differentiate path-based properties from literals or nested objects is consistent with the newPluginPayloadPropertytype. This helps maintain uniform usage across the codebase.
82-82: Helpful contextual logging
DefiningpluginNameand appending it to log statements consistently will simplify debugging and improve log clarity.
115-121: Enforcing JMESPath deprecation
By usinginvariantchecks for transformers named'jmespath-transformer', you’re preventing unapproved usage. This keeps future changes consistent with your current deprecation plans.
Line range hint
126-161: Robust request validation
The flow for transforming and validating the request, early returning on errors, is clean and clearly communicates failure conditions. Good job preventing partial execution if the request is invalid.
Line range hint
162-198: Adaptive handling of KYC information
Your mix of checks for arrays, single objects, or nested structures is thorough. The method ensures that KYC data is extracted from multiple shapes, which is crucial if the incoming data is highly dynamic.
204-210: Combining request payload with validated properties
MergingrequestPayload,validatedPayload, andkycInformationByDataTypebefore the API call ensures each part is included without overwriting. This is a strong, clear approach to payload construction.
Line range hint
220-229: Sensible post-response checks
Ensuring a non-emptycontent-lengthprovides a basic safeguard against empty responses. Continue verifying other response aspects for robust error handling (e.g., verifying the content type matches expectations).packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.test.ts (48)
26-29: Change to include clientId, vendor, ongoingMonitoring, and immediateResults in payload
Looks good. These literal properties follow the plugin's parameters structure properly.
31-31: Use of__type: 'path'forworkflowRuntimeId
Ensure that relevant parts of your code handle the__typefield consistently.
35-35: Use of__type: 'path'forendUserId
This aligns with the same approach forworkflowRuntimeId. Maintain consistent usage throughout the codebase.
39-39: Use of__type: 'path'forkycInformation
No issues found. This retains consistency with the other path fields.
77-80: Change to include clientId, vendor, ongoingMonitoring, and immediateResults in payload
Repeated pattern from earlier test block. Continuity looks good.
82-82: Use of__type: 'path'forworkflowRuntimeId
No concerns. Matches the established pattern.
86-86: Use of__type: 'path'forendUserId
Maintaining the pattern from previous blocks.
90-90: Use of__type: 'path'forkycInformation
Continues the same path-based approach in the payload.
128-131: Change to include clientId, vendor, ongoingMonitoring, and immediateResults in payload
No issues discovered. Properly declared literal properties.
133-133: Use of__type: 'path'forworkflowRuntimeId
Looks consistent with prior segments.
137-137: Use of__type: 'path'forendUserId
No additional concerns.
141-141: Use of__type: 'path'forkycInformation
Same path-based property usage.
179-182: Change to include clientId, vendor, ongoingMonitoring, and immediateResults in payload
No objections. Continues the established pattern.
184-184: Use of__type: 'path'forworkflowRuntimeId
Ensures dynamic value resolution.
188-188: Use of__type: 'path'forendUserId
No issues discovered.
192-192: Use of__type: 'path'forkycInformation
Maintains the plugin's structure.
233-236: Change to include clientId, vendor, ongoingMonitoring, and immediateResults in payload
Again, this is consistent with the previous modifications.
238-238: Use of__type: 'path'forworkflowRuntimeId
Fields remain consistent across scenarios.
242-242: Use of__type: 'path'forendUserId
No issues.
246-246: Use of__type: 'path'forkycInformation
Continuing the same approach for path-based data extraction.
279-279: Use of__type: 'path'forworkflowRuntimeId
Same logic as prior usage.
283-283: Use of__type: 'path'forendUserId
No issues found.
287-287: Use of__type: 'path'forkycInformation
Ensures data extraction from nested structures.
354-357: Change to include clientId, vendor, ongoingMonitoring, and immediateResults in payload
Introduces literal properties for this scenario.
436-439: Change to include clientId, vendor, ongoingMonitoring, and immediateResults in payload
Structure remains consistent with other test cases.
441-441: Use of__type: 'path'forworkflowRuntimeId
No suggestions to change.
445-445: Use of__type: 'path'forendUserId
No problems identified.
449-449: Use of__type: 'path'forkycInformation
Matches the same schema approach.
524-527: Change to include clientId, vendor, ongoingMonitoring, and immediateResults in payload
Solid addition. Follows the prior patterns.
529-529: Use of__type: 'path'forworkflowRuntimeId
No issues detected.
533-533: Use of__type: 'path'forendUserId
Ensures path-based retrieval.
537-537: Use of__type: 'path'forkycInformation
No concerns.
606-609: Change to include clientId, vendor, ongoingMonitoring, and immediateResults in payload
Adheres to the same approach.
611-611: Use of__type: 'path'forworkflowRuntimeId
Maintains the consistent plugin design.
615-615: Use of__type: 'path'forendUserId
Uniform approach across tests.
619-619: Use of__type: 'path'forkycInformation
Continues path-based logic.
697-700: Change to include clientId, vendor, ongoingMonitoring, and immediateResults in payload
Accurate setup for handling the plugin’s API call error scenario.
702-702: Use of__type: 'path'forworkflowRuntimeId
No further changes needed.
706-706: Use of__type: 'path'forendUserId
Maintains uniform data resolution.
710-710: Use of__type: 'path'forkycInformation
Carries forward existing pattern.
792-795: Change to include clientId, vendor, ongoingMonitoring, and immediateResults in payload
Handles scenario for 'NOT_IMPLEMENTED' reason properly.
797-797: Use of__type: 'path'forworkflowRuntimeId
Consistent usage.
801-801: Use of__type: 'path'forendUserId
No issues discovered.
805-805: Use of__type: 'path'forkycInformation
Mirrors the approach in other tests.
881-884: Change to include clientId, vendor, ongoingMonitoring, and immediateResults in payload
Ensures success scenario setup is aligned with prior additions.
886-886: Use of__type: 'path'forworkflowRuntimeId
Continues consistent pattern.
890-890: Use of__type: 'path'forendUserId
No suggestions.
894-894: Use of__type: 'path'forkycInformation
Matches the existing path-based approach.sdks/workflow-browser-sdk/package.json (2)
4-4: Version bump to 0.6.81
This version update aligns with the core packages. Confirm compatibility tests are executed.
37-37: Dependency update for@ballerine/workflow-core: now 0.6.81
Ensures consistency with other modules.services/workflows-service/package.json (3)
4-4: Version bump to 0.7.85
Minor release increment. Looks good.
53-53:@ballerine/workflow-coreupdated to0.6.81
Coordinates well with the latest plugin features.
54-54:@ballerine/workflow-node-sdkupdated to0.6.81
Necessary for new bank account verification plugin sync.apps/backoffice-v2/package.json (3)
3-3: Version bump to 0.7.86
Increment to accommodate new plugin integrations.
58-58:@ballerine/workflow-browser-sdkupdated to0.6.81
Keeps this backoffice version in sync with browser SDK updates.
59-59:@ballerine/workflow-node-sdkupdated to0.6.81
Maintains consistency across services.packages/workflow-core/CHANGELOG.md (1)
3-7: LGTM! Changelog entry follows the standard format.The changelog entry properly documents the addition of the bank account verification plugin as a patch change.
apps/kyb-app/CHANGELOG.md (1)
3-7: LGTM! Changelog entry follows the standard format.The changelog entry properly documents the dependency update to @ballerine/[email protected].
services/workflows-service/CHANGELOG.md (1)
3-10: LGTM! Changelog entry follows the standard format.The changelog entry properly documents:
- Addition of the bank account verification plugin
- Updates to dependencies:
- @ballerine/[email protected]
- @ballerine/[email protected]
...plugins/external-plugin/bank-account-verification-plugin/bank-account-verification-plugin.ts
Outdated
Show resolved
Hide resolved
This reverts commit fe9fbf6.
- Add node_modules and config files to .eslintignore - Modify permissions to allow write access in GitHub workflows - Update Dockerfile paths for various services (Your workflow dependencies are boxing each other like they’re auditioning for a soap opera)
- Bump versions across multiple packages - Update dependencies for improved stability (Your dependency updates look like they were done in a race against time)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx (2)
150-150: Consider making the height responsive instead of fixed.
Setting a fixed height (h-[30rem]) might cause layout overflow on smaller screens or inconsistent spacing across devices. Consider using responsive utilities or auto height.
183-183: User feedback suggestion.
Consider adding more context for "No Monthly Visitors Data Available" (e.g., possible reasons, recommended actions).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/ui/src/components/atoms/Tooltip/Tooltip.tsx(1 hunks)packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx(5 hunks)packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx(1 hunks)packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx(9 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx
- packages/ui/src/components/atoms/Tooltip/Tooltip.tsx
- packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx
🔇 Additional comments (14)
packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx (14)
154-154: No issues found.
The usage ofmt-auto h-4/5 w-full pb-0helps with consistent layout.
168-168: Tooltip container styling looks good.
Ensures the tooltip is distinct from the chart's background and helps users read the info quickly.
192-192: No issues regarding the new layout.
Shifting to aflex-col gap-4approach in the right column accommodates different screen sizes well.
193-193: No functional concerns.
This change toh-1/2 w-fullis consistent with the new interface layout.
194-194: Header styling is consistent with overall design.
No concerns here.
196-196: Check text overflow for smaller devices.
The content area is given a fixed height ofh-4/5. Just confirm it doesn't cause overlap.
231-231: Minor style improvement.
The small circle color swatch is a helpful visual cue to link the legend item with the chart segment.
234-234: No issues found.
Theflex w-full justify-betweenclassName optimizes spacing.
246-246: Consistent empty state.
Maintaining the same pattern for empty data signals to users that no data is available.
257-257: Visual grouping is effective.
Usingflex h-3/5 items-center gap-6correctly aligns engagement metrics side by side.
272-272: Icon usage is appropriate.
The info icon is an intuitive way to convey tooltips are available.
278-278: Enhanced tooltip styling.
Applying a border and background color can help with clarity.
280-280: Good clarity.
Descriptive text ensures users understand the engagement metric meaning.
296-296: Consistent empty state message.
Provides users clarity on missing data.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (18)
apps/backoffice-v2/CHANGELOG.md(1 hunks)apps/backoffice-v2/package.json(2 hunks)apps/kyb-app/CHANGELOG.md(1 hunks)apps/kyb-app/package.json(2 hunks)examples/headless-example/CHANGELOG.md(1 hunks)examples/headless-example/package.json(2 hunks)packages/react-pdf-toolkit/CHANGELOG.md(1 hunks)packages/react-pdf-toolkit/package.json(2 hunks)packages/ui/CHANGELOG.md(1 hunks)packages/ui/package.json(1 hunks)packages/workflow-core/CHANGELOG.md(1 hunks)packages/workflow-core/package.json(1 hunks)sdks/workflow-browser-sdk/CHANGELOG.md(1 hunks)sdks/workflow-browser-sdk/package.json(2 hunks)sdks/workflow-node-sdk/CHANGELOG.md(1 hunks)sdks/workflow-node-sdk/package.json(2 hunks)services/workflows-service/CHANGELOG.md(1 hunks)services/workflows-service/package.json(2 hunks)
✅ Files skipped from review due to trivial changes (14)
- packages/ui/package.json
- packages/ui/CHANGELOG.md
- sdks/workflow-node-sdk/package.json
- packages/workflow-core/package.json
- examples/headless-example/package.json
- sdks/workflow-browser-sdk/CHANGELOG.md
- examples/headless-example/CHANGELOG.md
- apps/kyb-app/CHANGELOG.md
- packages/react-pdf-toolkit/CHANGELOG.md
- sdks/workflow-node-sdk/CHANGELOG.md
- packages/workflow-core/CHANGELOG.md
- services/workflows-service/CHANGELOG.md
- apps/backoffice-v2/CHANGELOG.md
- apps/kyb-app/package.json
🔇 Additional comments (4)
sdks/workflow-browser-sdk/package.json (1)
4-4: LGTM! Version updates are consistent.The version bump and dependency update maintain version alignment between related packages, which is crucial for the bank account verification plugin integration.
Also applies to: 37-37
services/workflows-service/package.json (1)
4-4: LGTM! Version and dependency updates are consistent.The version bump and dependency updates align with the PR's bank account verification plugin changes. The synchronized updates of workflow packages maintain consistency across the codebase.
Also applies to: 54-55
apps/backoffice-v2/package.json (1)
3-3: LGTM! Coordinated version and dependency updates.The version bump and dependency updates are well-coordinated across the monorepo, maintaining consistency between the workflow SDKs, UI components, and PDF toolkit.
Also applies to: 57-60
packages/react-pdf-toolkit/package.json (1)
4-4: LGTM! Version bump follows semantic versioning.The patch version increment (1.2.59 → 1.2.60) is appropriate for the changes described in the PR.
- Bump version for backoffice-v2 to 0.7.92 and update dependencies - Bump version for kyb-app to 0.3.107 and update dependencies - Bump version for several other packages and update respective dependencies (Your dependency management is surprisingly more organized than my sock drawer)
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
scripts/auto-commit.js (1)
Line range hint
27-27: Fix the typo in the model name.The model name 'gpt-4o-mini' appears to be a typo. It should likely be 'gpt-4' or another valid OpenAI model name.
- model: 'gpt-4o-mini', + model: 'gpt-4',
🧹 Nitpick comments (3)
scripts/auto-commit.js (3)
38-38: Improve clarity and consistency in the formatting guideline.The line contains redundant information about the roast length and uses inconsistent spelling ("shouldnt" vs "should not").
- '- All lines must not exceed 100 chars, including the roast - the roast shouldnt be longer than 100 chars\n' + + '- All lines must not exceed 100 characters\n' +
Line range hint
11-17: Enhance API key validation and error handling.The current API key validation could be more robust:
- Consider validating the API key format before making API calls
- Provide more informative error messages
const OPENAI_API_KEY = process.env.OPENAI_API_KEY; +// Basic format validation for OpenAI API key +const isValidApiKey = (key) => /^sk-[A-Za-z0-9]{32,}$/.test(key); + if (!OPENAI_API_KEY) { - console.error('Error: OPENAI_API_KEY not found in .env file'); + console.error('Error: OPENAI_API_KEY not found in .env file. Please set this environment variable.'); + process.exit(1); +} else if (!isValidApiKey(OPENAI_API_KEY)) { + console.error('Error: OPENAI_API_KEY appears to be invalid. It should start with "sk-" followed by at least 32 characters.'); process.exit(1); }
Line range hint
89-103: Consider using async file operations.The script uses synchronous file operations which could block the event loop. Consider using async alternatives for better performance.
- const diff = execSync('git diff --cached').toString(); + const diff = await promisify(exec)('git diff --cached').then(({stdout}) => stdout); if (!diff) { console.error('No staged changes found. Please stage your changes using git add'); process.exit(1); } // Generate commit message const commitMessage = await generateCommitMessage(diff); // Write commit message to temporary file const tempFile = path.join(__dirname, '..', '.git', 'COMMIT_EDITMSG'); - fs.writeFileSync(tempFile, commitMessage); + await fs.promises.writeFile(tempFile, commitMessage);
📜 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 (19)
apps/backoffice-v2/CHANGELOG.md(1 hunks)apps/backoffice-v2/package.json(2 hunks)apps/kyb-app/CHANGELOG.md(1 hunks)apps/kyb-app/package.json(2 hunks)examples/headless-example/CHANGELOG.md(1 hunks)examples/headless-example/package.json(2 hunks)packages/react-pdf-toolkit/CHANGELOG.md(1 hunks)packages/react-pdf-toolkit/package.json(2 hunks)packages/ui/CHANGELOG.md(1 hunks)packages/ui/package.json(1 hunks)packages/workflow-core/CHANGELOG.md(1 hunks)packages/workflow-core/package.json(1 hunks)scripts/auto-commit.js(1 hunks)sdks/workflow-browser-sdk/CHANGELOG.md(1 hunks)sdks/workflow-browser-sdk/package.json(2 hunks)sdks/workflow-node-sdk/CHANGELOG.md(1 hunks)sdks/workflow-node-sdk/package.json(2 hunks)services/workflows-service/CHANGELOG.md(1 hunks)services/workflows-service/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (17)
- packages/workflow-core/package.json
- sdks/workflow-node-sdk/CHANGELOG.md
- sdks/workflow-browser-sdk/package.json
- packages/ui/CHANGELOG.md
- sdks/workflow-node-sdk/package.json
- packages/workflow-core/CHANGELOG.md
- packages/react-pdf-toolkit/package.json
- apps/kyb-app/CHANGELOG.md
- examples/headless-example/package.json
- examples/headless-example/CHANGELOG.md
- packages/react-pdf-toolkit/CHANGELOG.md
- apps/backoffice-v2/CHANGELOG.md
- packages/ui/package.json
- services/workflows-service/CHANGELOG.md
- apps/kyb-app/package.json
- services/workflows-service/package.json
- apps/backoffice-v2/package.json
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: spell_check
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: lint
🔇 Additional comments (2)
sdks/workflow-browser-sdk/CHANGELOG.md (2)
3-8: LGTM! The changelog entry follows the conventional format.The latest version (0.6.84) properly documents the dependency update to @ballerine/[email protected].
Line range hint
1-600: LGTM! The changelog maintains a consistent format throughout its history.The changelog effectively tracks version changes, patches, and dependency updates, providing clear traceability of changes over time.
...plugins/external-plugin/bank-account-verification-plugin/bank-account-verification-plugin.ts
Outdated
Show resolved
Hide resolved
...plugins/external-plugin/bank-account-verification-plugin/bank-account-verification-plugin.ts
Show resolved
Hide resolved
...plugins/external-plugin/bank-account-verification-plugin/bank-account-verification-plugin.ts
Outdated
Show resolved
Hide resolved
...plugins/external-plugin/bank-account-verification-plugin/bank-account-verification-plugin.ts
Outdated
Show resolved
Hide resolved
...plugins/external-plugin/bank-account-verification-plugin/bank-account-verification-plugin.ts
Outdated
Show resolved
Hide resolved
...plugins/external-plugin/bank-account-verification-plugin/bank-account-verification-plugin.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
examples/report-generation-example/CHANGELOG.md (1)
Line range hint
15-15: Remove placeholder text from changelog.The text "Please enter a summary for your changes" appears to be a placeholder. Please replace it with actual changes made in version 0.2.29.
🧹 Nitpick comments (2)
sdks/web-ui-sdk/package.json (1)
24-24: LGTM! Version and dependency updates look appropriate.The version bump and dependency update align well with the PR's scope of adding bank account verification functionality.
Consider documenting these changes in a CHANGELOG.md file if not already done, to help track the evolution of the package.
Also applies to: 99-99
examples/report-generation-example/CHANGELOG.md (1)
7-7: Enhance the changelog message with meaningful details.Instead of using a generic "version bump" message, please provide specific details about what changed in this version. This helps other developers understand the purpose of this release.
📜 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 (41)
apps/backoffice-v2/CHANGELOG.md(1 hunks)apps/backoffice-v2/package.json(3 hunks)apps/kyb-app/CHANGELOG.md(1 hunks)apps/kyb-app/package.json(3 hunks)apps/workflows-dashboard/CHANGELOG.md(1 hunks)apps/workflows-dashboard/package.json(3 hunks)examples/headless-example/CHANGELOG.md(1 hunks)examples/headless-example/package.json(2 hunks)examples/report-generation-example/CHANGELOG.md(1 hunks)examples/report-generation-example/package.json(2 hunks)packages/blocks/CHANGELOG.md(1 hunks)packages/blocks/package.json(3 hunks)packages/common/CHANGELOG.md(1 hunks)packages/common/package.json(2 hunks)packages/config/CHANGELOG.md(1 hunks)packages/config/package.json(1 hunks)packages/eslint-config-react/CHANGELOG.md(1 hunks)packages/eslint-config-react/package.json(2 hunks)packages/eslint-config/CHANGELOG.md(1 hunks)packages/eslint-config/package.json(1 hunks)packages/react-pdf-toolkit/CHANGELOG.md(1 hunks)packages/react-pdf-toolkit/package.json(2 hunks)packages/rules-engine/CHANGELOG.md(1 hunks)packages/rules-engine/package.json(2 hunks)packages/ui/CHANGELOG.md(1 hunks)packages/ui/package.json(3 hunks)packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx(1 hunks)packages/workflow-core/CHANGELOG.md(1 hunks)packages/workflow-core/package.json(3 hunks)sdks/web-ui-sdk/CHANGELOG.md(1 hunks)sdks/web-ui-sdk/package.json(2 hunks)sdks/workflow-browser-sdk/CHANGELOG.md(1 hunks)sdks/workflow-browser-sdk/package.json(2 hunks)sdks/workflow-node-sdk/CHANGELOG.md(1 hunks)sdks/workflow-node-sdk/package.json(2 hunks)services/websocket-service/CHANGELOG.md(1 hunks)services/websocket-service/package.json(1 hunks)services/workflows-service/CHANGELOG.md(1 hunks)services/workflows-service/package.json(3 hunks)services/workflows-service/prisma/data-migrations(1 hunks)websites/docs/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (17)
- packages/eslint-config/package.json
- services/websocket-service/package.json
- packages/config/package.json
- packages/config/CHANGELOG.md
- packages/rules-engine/CHANGELOG.md
- packages/common/CHANGELOG.md
- packages/eslint-config/CHANGELOG.md
- services/websocket-service/CHANGELOG.md
- packages/blocks/CHANGELOG.md
- packages/eslint-config-react/package.json
- services/workflows-service/prisma/data-migrations
- packages/rules-engine/package.json
- sdks/web-ui-sdk/CHANGELOG.md
- packages/eslint-config-react/CHANGELOG.md
- examples/report-generation-example/package.json
- packages/common/package.json
- apps/workflows-dashboard/package.json
🚧 Files skipped from review as they are similar to previous changes (19)
- packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx
- packages/workflow-core/CHANGELOG.md
- packages/react-pdf-toolkit/CHANGELOG.md
- packages/ui/CHANGELOG.md
- packages/react-pdf-toolkit/package.json
- examples/headless-example/package.json
- apps/backoffice-v2/CHANGELOG.md
- sdks/workflow-node-sdk/CHANGELOG.md
- sdks/workflow-browser-sdk/package.json
- packages/workflow-core/package.json
- services/workflows-service/package.json
- sdks/workflow-node-sdk/package.json
- packages/ui/package.json
- services/workflows-service/CHANGELOG.md
- sdks/workflow-browser-sdk/CHANGELOG.md
- apps/kyb-app/package.json
- apps/kyb-app/CHANGELOG.md
- examples/headless-example/CHANGELOG.md
- apps/backoffice-v2/package.json
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
🔇 Additional comments (5)
apps/workflows-dashboard/CHANGELOG.md (1)
3-10: LGTM! Changelog updates are consistent.The new version entry and dependency updates follow the established changelog format and semantic versioning pattern. The dependency updates to
@ballerine/[email protected]and@ballerine/[email protected]are consistent with the coordinated updates across other packages in this PR.websites/docs/package.json (1)
20-20: Verify package version compatibility.The version updates for Ballerine packages appear to be part of a coordinated update across the monorepo. Let's verify these versions exist and are compatible.
Also applies to: 26-27
✅ Verification successful
Package versions are compatible and consistent
All specified package versions exist in their source packages and are used consistently across the monorepo.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the updated package versions exist and are published. # Check if package.json files exist in the specified packages echo "Checking package versions in source packages..." for pkg in "common" "config" "eslint-config"; do if [ -f "packages/${pkg}/package.json" ]; then echo "Package @ballerine/${pkg}:" jq -r '.version' "packages/${pkg}/package.json" fi done # Check for version consistency across the monorepo echo -e "\nChecking version consistency across the monorepo..." fd -t f package.json | xargs jq -r 'select(.dependencies["@ballerine/common"]? != null) | "@ballerine/common version in \(.name): \(.dependencies["@ballerine/common"])"' fd -t f package.json | xargs jq -r 'select(.devDependencies["@ballerine/config"]? != null) | "@ballerine/config version in \(.name): \(.devDependencies["@ballerine/config"])"' fd -t f package.json | xargs jq -r 'select(.devDependencies["@ballerine/eslint-config"]? != null) | "@ballerine/eslint-config version in \(.name): \(.devDependencies["@ballerine/eslint-config"])"'Length of output: 3396
packages/blocks/package.json (3)
5-5: LGTM! Version bump aligns with feature additions.The minor version increment from 0.2.32 to 0.2.33 appropriately reflects the addition of bank account verification functionality.
94-94: Verify changes in @ballerine/common package.The update to @ballerine/common might contain changes related to bank account verification functionality.
Let's verify the changes:
✅ Verification successful
No significant changes to bank account verification functionality
The update to @ballerine/common package appears to be routine and doesn't introduce any changes related to bank account verification. The existing bank account verification schema remains unchanged.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check changes in common package related to bank verification # Check commit messages for common package git log --oneline | grep -i "common" | head -n 10 # Look for bank account related changes in common package rg -g '*/common/**' -i 'bank.*account|verification'Length of output: 175
Script:
#!/bin/bash # Get the content of the schema file cat packages/common/src/schemas/documents/workflow/documents/schemas/ZZ.ts # Look for workflow and verification related changes rg -g '*/common/**' -i 'workflow.*verification|verify' -A 3 # Check package.json in common to see version cat packages/common/package.json | grep '"version":'Length of output: 8711
45-46: Verify compatibility with updated dev dependencies.The updates to @ballerine/config and @ballerine/eslint-config should be verified for compatibility.
Let's check the changelog or commit history for these updates:
✅ Verification successful
Dependencies update is safe to proceed
The update from version 1.1.28 to 1.1.31 is a coordinated minor version bump consistently applied across all packages in the monorepo, maintaining compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for breaking changes in config packages # Check commit messages for these packages git log --oneline | grep -i "config\|eslint" | head -n 10 # Check if there are any other packages still using the old version rg '"@ballerine/(config|eslint-config)": "\^1\.1\.30"' -g 'package.json'Length of output: 72
Script:
#!/bin/bash # Find all usages of these packages and their versions echo "=== Checking all versions of @ballerine/config and @ballerine/eslint-config ===" rg '"@ballerine/(config|eslint-config)":' -g 'package.json' echo -e "\n=== Checking for related config files ===" fd -e '.eslintrc.js' -e '.eslintrc.json' echo -e "\n=== Checking recent changes to package.json files ===" git diff HEAD~5 HEAD -- '**/package.json' | grep -A 2 -B 2 '@ballerine/\(config\|eslint-config\)'Length of output: 6667
- Update import paths to reflect the plugin renaming - Remove deprecated transformer logic in relevant plugins - Enhance response handling and additional logging for clarity (Your variable names might as well be in a different language, they're that unrecognizable)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.ts (3)
Line range hint
133-177: Improve type safety in getKycInformationByDataType function.The function handles multiple data formats but relies on runtime checks. Consider improving type safety:
- Use type predicates for better type narrowing
- Consider splitting the function into smaller, type-specific handlers
Example type predicate:
const isArrayKycInformation = ( kycInformation: z.output<typeof IndividualsSanctionsV2PluginPayloadSchema>['kycInformation'] ): kycInformation is Array<{ firstName: string; lastName: string; additionalInfo: { dateOfBirth: string } }> => { return Array.isArray(kycInformation); };
204-207: Enhance empty response validation.The current content-length check might not catch all empty response cases. Consider adding response body validation:
const contentLength = apiResponse.headers.get('content-length'); +const responseText = await apiResponse.text(); invariant( - !contentLength || Number(contentLength) > 0, + responseText && (!contentLength || Number(contentLength) > 0), `${this.pluginName} - Received an empty response`, ); + +// Parse the validated response +const res = JSON.parse(responseText);
Line range hint
220-232: Simplify plugin status mapping logic.Consider using an object map for status mapping to improve maintainability:
const STATUS_MAP: Record<string, ProcessStatus> = { [UnifiedApiReason.NOT_IMPLEMENTED]: ProcessStatus.CANCELED, error: ProcessStatus.ERROR, default: ProcessStatus.IN_PROGRESS, }; const getPluginStatus = (response: Record<string, unknown>) => STATUS_MAP[response.reason as string] ?? (response.error ? STATUS_MAP.error : STATUS_MAP.default);packages/workflow-core/src/lib/plugins/external-plugin/api-plugin.ts (2)
14-20: Add documentation for the invokedAtTransformer.While the transformer is correctly defined, it would be helpful to add JSDoc comments explaining its purpose and the expected format of the timestamp.
+/** + * Transformer that sets the invokedAt field to UTC timestamp. + * This ensures consistent timestamp format across all API responses. + */ const invokedAtTransformer: HelpersTransformer = new HelpersTransformer([ { source: 'invokedAt', target: 'invokedAt', method: 'setTimeToRecordUTC', }, ] as THelperFormatingLogic);
106-108: Consider a more defensive approach when handling transformers.While the current implementation works, it could be more defensive against undefined transformers. Consider using optional chaining and nullish coalescing for better error handling.
-const responseTransformers = [...(this.response?.transformers || []), invokedAtTransformer]; +const responseTransformers = [ + ...(this.response?.transformers ?? []), + invokedAtTransformer, +].filter(Boolean);packages/workflow-core/src/lib/plugins/external-plugin/bank-account-verification-plugin/bank-account-verification-plugin.ts (3)
12-42: Enhance schema maintainability with sub-schemas and documentation.While the schema is well-structured, it could benefit from:
- Breaking down into reusable sub-schemas
- Adding JSDoc comments for better documentation
- Using descriptive constants for validation rules
+/** Minimum length for required string fields */ +const MIN_LENGTH = 1; + +/** Schema for address validation */ +const AddressSchema = z.object({ + streetNumber: z.string().min(MIN_LENGTH), + street: z.string().min(MIN_LENGTH), + city: z.string().min(MIN_LENGTH), + postcode: z.string().min(MIN_LENGTH), +}); + +/** Schema for individual account holder */ +const IndividualHolderSchema = z.object({ + firstName: z.string().min(MIN_LENGTH), + middleName: z.string().optional(), + lastName: z.string().min(MIN_LENGTH), +}); + +/** Schema for business account holder */ +const BusinessHolderSchema = z.object({ + bankAccountName: z.string().min(MIN_LENGTH), +}).and( + z.union([ + z.object({ companyRegistrationNumber: z.string().min(MIN_LENGTH) }), + z.object({ registeredCharityNumber: z.string().min(MIN_LENGTH) }), + ]) +); const BankAccountVerificationPluginPayloadSchema = z.object({ clientId: z.string().min(1), vendor: z.enum(['experian']), - address: z.object({ - streetNumber: z.string().min(1), - street: z.string().min(1), - city: z.string().min(1), - postcode: z.string().min(1), - }), + address: AddressSchema, bankAccountDetails: z.object({ - holder: z.union([/*...*/]), + holder: z.union([BusinessHolderSchema, IndividualHolderSchema]), sortCode: z.string().min(1), bankAccountNumber: z.string().min(1), }), });
79-91: Consider validating payload schema at construction time.Early validation of the payload schema in the constructor could prevent runtime errors and improve error handling.
constructor({ payload, ...pluginParams }: IApiPluginParams & { payload: BankAccountVerificationPlugin['payload'] }) { + // Validate payload structure at construction time + if (!payload || typeof payload !== 'object') { + throw new Error(`${BankAccountVerificationPlugin.pluginName} - Invalid payload structure`); + } const bankAccountVerificationPluginParams = { ...pluginParams, method: 'POST' as const, }; super(bankAccountVerificationPluginParams); this.payload = payload; }
129-133: Enhance error handling with specific error types.While the error handling is good, it could be improved by:
- Using custom error types for better error classification
- Including more context in error messages
+class BankAccountVerificationError extends Error { + constructor(message: string, public readonly cause?: unknown) { + super(message); + this.name = 'BankAccountVerificationError'; + } +} invariant( !contentLength || Number(contentLength) > 0, - `${this.pluginName} - Received an empty response`, + new BankAccountVerificationError('Received an empty response'), ); // ... in catch block return this.returnErrorResponse( isErrorWithMessage(error) - ? `${this.pluginName} - ${error.message}` - : `${this.pluginName} - Unknown error`, + ? `${this.pluginName} - ${error.message}` + : error instanceof BankAccountVerificationError + ? `${this.pluginName} - ${error.message}` + : `${this.pluginName} - Unknown error: ${error}`, );Also applies to: 156-164
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/workflow-core/src/lib/constants.ts(3 hunks)packages/workflow-core/src/lib/plugins/external-plugin/api-plugin.ts(2 hunks)packages/workflow-core/src/lib/plugins/external-plugin/ballerine-email-plugin.ts(1 hunks)packages/workflow-core/src/lib/plugins/external-plugin/bank-account-verification-plugin/bank-account-verification-plugin.ts(1 hunks)packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.ts(6 hunks)packages/workflow-core/src/lib/plugins/external-plugin/kyc-plugin.ts(1 hunks)packages/workflow-core/src/lib/plugins/external-plugin/kyc-session-plugin.ts(1 hunks)packages/workflow-core/src/lib/plugins/external-plugin/vendor-consts.ts(0 hunks)packages/workflow-core/src/lib/plugins/external-plugin/webhook-plugin.ts(1 hunks)packages/workflow-core/src/lib/workflow-runner.ts(1 hunks)services/workflows-service/prisma/data-migrations(1 hunks)
💤 Files with no reviewable changes (1)
- packages/workflow-core/src/lib/plugins/external-plugin/vendor-consts.ts
✅ Files skipped from review due to trivial changes (5)
- packages/workflow-core/src/lib/plugins/external-plugin/webhook-plugin.ts
- packages/workflow-core/src/lib/plugins/external-plugin/ballerine-email-plugin.ts
- packages/workflow-core/src/lib/plugins/external-plugin/kyc-plugin.ts
- packages/workflow-core/src/lib/plugins/external-plugin/kyc-session-plugin.ts
- packages/workflow-core/src/lib/workflow-runner.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/workflow-core/src/lib/constants.ts
- services/workflows-service/prisma/data-migrations
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
🔇 Additional comments (2)
packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.ts (1)
75-77: Verify the type change impact across the codebase.The change from
typeto__typein the payload property type definition might affect other parts of the codebase that rely on this type structure.✅ Verification successful
Type definition is consistent across the codebase
The usage of
__type: 'path'is consistent across all relevant files in the external plugin system, including the type definitions, plugin implementation, and tests. No inconsistencies or migration issues were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of similar type definitions to ensure consistency rg "type: 'path'" --type typescript rg "__type: 'path'" --type typescriptLength of output: 159
Script:
#!/bin/bash # Search for type definitions in TypeScript files rg "type: 'path'" -g '*.ts' rg "__type: 'path'" -g '*.ts' # Use ast-grep to find type definitions structurally ast-grep --pattern 'type: "path"' ast-grep --pattern '__type: "path"' ast-grep --pattern "type: 'path'" ast-grep --pattern "__type: 'path'" # Search for similar type definitions in interface/type declarations ast-grep --pattern 'interface $_ { $$$ type: $_; $$$ }' ast-grep --pattern 'interface $_ { $$$ __type: $_; $$$ }'Length of output: 34893
packages/workflow-core/src/lib/plugins/external-plugin/api-plugin.ts (1)
6-9: LGTM!The added imports are necessary for the new transformer functionality.
...b/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.ts
Outdated
Show resolved
Hide resolved
...plugins/external-plugin/bank-account-verification-plugin/bank-account-verification-plugin.ts
Show resolved
Hide resolved
...plugins/external-plugin/bank-account-verification-plugin/bank-account-verification-plugin.ts
Show resolved
Hide resolved
- Swap request and response transformers to ensure proper handling - Enhance plugin's functionality for better consistency (Your code has more twists than a mystery novel with a confused plot)
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.ts (1)
Line range hint
139-167: Add safety checks in getKycInformationByDataType.The function has potential null/undefined access risks:
- Array destructuring without length check
- Deep object property access without proper null checks
Consider adding these safety measures:
-const [firstKycInformation] = kycInformation; +const firstKycInformation = kycInformation[0]; invariant( firstKycInformation, `${this.pluginName} - no KYC information found at ${this.payload.kycInformation.value}`, ); +// Validate required properties exist +invariant( + 'firstName' in firstKycInformation && + 'lastName' in firstKycInformation && + 'additionalInfo' in firstKycInformation && + 'dateOfBirth' in firstKycInformation.additionalInfo, + `${this.pluginName} - missing required KYC information properties`, +);
🧹 Nitpick comments (1)
packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.ts (1)
Line range hint
187-206: Enhance logging for better debugging.While the logging is good, consider these improvements:
- Add debug level logging for request/response bodies (with PII redaction)
- Preserve error stack traces in catch blocks
- Add request ID for correlation
Example implementation:
// Add at the start of the method const requestId = crypto.randomUUID(); // Update logging logger.log(`${this.pluginName} - Sending API request`, { url, method: this.method, requestId, // Only in debug level body: this.sanitizeForLogging(requestPayload), }); // Helper method private sanitizeForLogging(data: unknown) { // Implement PII redaction logic return data; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.ts(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.ts (2)
82-99: Well-structured constructor implementation!Good improvements:
- Consistent plugin name initialization
- Proper transformer handling with correct parameter mapping
- Clean separation of concerns
76-76: Verify the impact of changingtypeto__type.The change from
typeto__typein thekycInformationtype definition could be a breaking change. Ensure that:
- All consuming code has been updated to use the new property name
- This change is documented in the migration guide if it's a public API
- Bump versions for numerous packages to the latest patch - Update dependencies to their respective latest versions (With all these updates, even time itself might start feeling outdated)
- Upgrade @types/node version for vite to ensure compatibility - Adjust affected packages to use the latest type definitions (your dependencies are like your jokes; outdated and in desperate need of an upgrade)
- Add invokedAt validation to plugin response objects - Include name and error fields in error response assertions (These tests could use more coverage than an overzealous sunscreen salesman)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/workflow-core/src/lib/plugins/external-plugin/api-plugin.test.ts (1)
85-85: Consider enhancing timestamp validation.While the test correctly verifies the presence and type of the
invokedAttimestamp, consider adding more specific validations:- invokedAt: expect.any(Number), + invokedAt: expect.toSatisfy((timestamp: number) => { + const now = Date.now(); + return timestamp > 0 && timestamp <= now; + }),This would ensure that:
- The timestamp is a positive number
- The timestamp is not from the future
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/workflow-core/src/lib/plugins/external-plugin/api-plugin.test.ts(1 hunks)packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.test.ts(17 hunks)packages/workflow-core/src/lib/plugins/external-plugin/shared/handle-jmespath-transformers.ts(1 hunks)services/workflows-service/src/env.ts(0 hunks)
💤 Files with no reviewable changes (1)
- services/workflows-service/src/env.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.test.ts
- packages/workflow-core/src/lib/plugins/external-plugin/shared/handle-jmespath-transformers.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test_windows
- GitHub Check: build (windows-latest)
- GitHub Check: test_linux
packages/workflow-core/src/lib/plugins/external-plugin/api-plugin.test.ts
Show resolved
Hide resolved
Summary by CodeRabbit
Based on the comprehensive summary, here are the updated release notes:
New Features
Improvements
Dependency Updates
Bug Fixes
Chores