Dow Jones ongoing changes #2836
Conversation
|
WalkthroughThe pull request introduces several changes across multiple files, primarily enhancing the configuration and schema definitions. A new plugin for Tailwind CSS is added to the PostCSS configuration, enabling nested styles. In the schema files, a new Changes
Possibly related PRs
Suggested reviewers
Poem
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: 3
🧹 Outside diff range and nitpick comments (6)
apps/backoffice-v2/postcss.config.cjs (1)
4-4: LGTM! Consider adding a comment for clarity.The plugin addition is correct and properly ordered before the main Tailwind plugin.
Consider adding a brief comment explaining the purpose of the nesting plugin:
module.exports = { modules: true, plugins: { + // Enable nested CSS syntax support for better style organization 'tailwindcss/nesting': {}, tailwindcss: {}, autoprefixer: {}, }, };services/workflows-service/scripts/run-validation.ts (1)
1-17: Consider architectural improvements for validation frameworkThis validation script appears to be standalone, but could benefit from better integration with the workflows service.
Suggestions:
- Create a reusable validation framework that can be used across different parts of the workflow service
- Add documentation explaining:
- Purpose and usage of the validation script
- Expected input/output formats
- Integration points with CI/CD
- Consider exposing validation as an API endpoint for runtime validation
- Add configuration file support for different validation rules per environment
Would you like me to help create a more comprehensive validation framework design?
packages/common/src/schemas/workflow/end-user.schema.ts (1)
21-21: Document the reason for makingmatchedNamenullableTo improve maintainability, consider adding a comment explaining the scenarios where
matchedNamecan be null.Example:
+ // matchedName can be null when [describe the scenario] matchedName: z.string().nullable(),packages/common/src/schemas/documents/schemas/aml-schema.ts (1)
32-32: Consider adding documentation and centralizing vendor constantsWhile the vendor property addition is structurally sound, consider these improvements:
- Add JSDoc comments explaining the purpose of each vendor option
- Consider moving vendor string literals to a central constants file for better maintainability
Example improvement:
/** * Vendor configuration for AML checks * - veriff: Used for Veriff's AML verification service * - dow-jones: Used for Dow Jones risk and compliance data */ vendor: TypeStringEnum(['veriff', 'dow-jones']),services/workflows-service/src/webhooks/webhooks.service.ts (1)
77-82: Consider documenting the vendor mapping logicThe mapping of AML hits with vendor information is a critical part of the integration. Consider adding a comment explaining the vendor assignment strategy and its significance.
const hits = data?.hits ?? []; const amlHits = hits.map(hit => ({ + // Assign vendor (veriff/dow-jones) from webhook data + // This supports multiple AML providers ...hit, vendor: data?.vendor, }));services/workflows-service/src/workflow/workflow.service.ts (1)
1565-1575: Simplify type assertions for better readability and type safetyThe current code uses multiple nested type assertions which can reduce readability and may lead to runtime errors if types are not as expected. Consider defining proper TypeScript interfaces or types for
uiDefinition.definitionto avoid excessive casting.Here's a refactored version:
steps: uiDefinition?.definition ? getOrderedSteps( - (uiDefinition?.definition as Prisma.JsonObject)?.definition as Record< - string, - Record<string, unknown> - >, + uiDefinition.definition as Record<string, Record<string, unknown>>, { finalStates: [...WORKFLOW_FINAL_STATES] }, ).map(stepName => ({ stateName: stepName, })) : [],Alternatively, define a specific interface for
uiDefinition.definition:interface UiDefinition { definition: Record<string, Record<string, unknown>>; } // Then use it when retrieving uiDefinition const uiDefinition = await this.uiDefinitionService.getByWorkflowDefinitionId( workflowDefinitionId, UiDefinitionContext.collection_flow, projectIds, ) as UiDefinition; ... steps: uiDefinition?.definition ? getOrderedSteps( uiDefinition.definition, { finalStates: [...WORKFLOW_FINAL_STATES] }, ).map(stepName => ({ stateName: stepName, })) : [],This approach improves type safety and makes the code more maintainable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
apps/backoffice-v2/postcss.config.cjs(1 hunks)packages/common/src/schemas/documents/schemas/aml-schema.ts(2 hunks)packages/common/src/schemas/workflow/end-user.schema.ts(1 hunks)services/workflows-service/package.json(1 hunks)services/workflows-service/scripts/run-validation.ts(1 hunks)services/workflows-service/src/webhooks/webhooks.service.ts(1 hunks)services/workflows-service/src/workflow/workflow.service.ts(1 hunks)
🔇 Additional comments (7)
packages/common/src/schemas/workflow/end-user.schema.ts (1)
21-21:
matchedName is now nullable
This change modifies the type from string to string | null, which could impact existing code that assumes matchedName is always a string.
Let's verify the impact of this change:
Consider:
- Adding runtime checks for null values where
matchedNameis used - Updating TypeScript interfaces that depend on this schema
- Updating tests to cover the null case scenario
✅ Verification successful
Let me try a different search strategy to find direct usages of matchedName:
The previous search didn't provide conclusive results about the usage of matchedName. Let's try searching with more context and also look for the type/interface usage to understand the impact better.
Let me try one more search with corrected file type flags:
This should help us find any TypeScript files that use the matchedName field or import the schema.
Making matchedName nullable is safe in this context
Based on the codebase analysis, this change is safe because:
- The UI components in
backoffice-v2already handle nullablematchedName(HitSchema defines it asz.string().optional().nullable()) - The ecosystem table components properly handle null values with
TextWithNAFallback - All usages in test data and mock data already treat it as a string value
- The AML service integration code in
webhooksandend-userservices use the schema for validation, ensuring type safety - No direct string operations (like
.length) are performed on thematchedNamefield
The change aligns with existing implementations and doesn't introduce any breaking changes in practice.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for direct usage of matchedName to identify potential breaking changes
rg -l 'matchedName' --type typescript
# Search for specific patterns that might break
ast-grep --pattern 'matchedName.' # Find property access without null checks
ast-grep --pattern 'matchedName.length' # Find direct string operations
Length of output: 19912
Script:
#!/bin/bash
# Search for direct usage of matchedName with context
rg 'matchedName' -A 2 -B 2 --type ts --type tsx
# Search for AmlHit type/interface usage
rg 'AmlHit' -A 2 -B 2 --type ts --type tsx
Length of output: 146
Script:
#!/bin/bash
# Search for direct usage of matchedName and AmlHit in TypeScript files
rg 'matchedName|AmlHit' -A 2 -B 2 --type-add 'ts:*.{ts,tsx}' --type ts
# Find all TypeScript files that might use this schema
fd -e ts -e tsx | xargs rg -l 'end-user\.schema'
Length of output: 17877
packages/common/src/schemas/documents/schemas/aml-schema.ts (2)
2-2: LGTM: Clean import addition
The TypeStringEnum import is appropriately placed and follows the project's import conventions.
32-32: Verify vendor integration completeness
Since this is part of the Dow Jones integration, let's verify that all necessary files are updated to handle the new vendor.
✅ Verification successful
Let me analyze the webhook handling implementation to ensure proper vendor integration. I'll check for any potential gaps.
Let me check one more aspect to ensure complete vendor integration by looking at the webhook handling and configuration patterns.
Based on the results from our verification scripts, I can now provide a final assessment:
Dow Jones vendor integration appears complete
The vendor integration for 'dow-jones' is properly implemented across the necessary components:
- Schema definitions in both workflow and document schemas
- Vendor configuration constants and mapping logic
- Webhook handling through the WebhooksService that processes AML hits vendor-agnostically
The implementation follows the same pattern as the existing 'veriff' vendor, with proper type definitions and handling in:
- packages/common/src/schemas/documents/schemas/aml-schema.ts
- packages/common/src/schemas/workflow/end-user.schema.ts
- packages/workflow-core/src/lib/plugins/external-plugin/vendor-consts.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for vendor handling across the codebase
# Check for vendor-related logic in service files
echo "Checking for vendor handling in services..."
rg -l "vendor.*['\"]dow-jones['\"]" --type ts
# Check for potential missing vendor handling
echo "Checking for potential places that might need vendor handling..."
rg -l "aml.*check|aml.*verification" --type ts
# Check for WebhooksService implementation
echo "Checking WebhooksService implementation..."
ast-grep --pattern 'class WebhooksService {
$$$
handleIndividualAmlHit($$$) {
$$$
}
$$$
}'
Length of output: 10241
Script:
#!/bin/bash
# Check for AML vendor handling in workflow implementations
echo "Checking AML vendor handling in workflow implementations..."
rg -A 5 "vendor.*['\"]dow-jones['\"]" --type ts
# Check for AML webhook input type definition
echo "Checking AML webhook input type..."
ast-grep --pattern 'interface IndividualAmlWebhookInput {
$$$
}'
# Check for vendor-specific configurations
echo "Checking vendor configurations..."
rg -A 5 "aml.*config.*vendor" --type ts
Length of output: 3237
Script:
#!/bin/bash
# Check for AML-related webhook handlers and configurations
echo "Checking webhook handlers..."
rg -A 10 "handleAml|processAml|amlWebhook" --type ts
# Check for vendor-specific configuration patterns
echo "Checking vendor configuration patterns..."
ast-grep --pattern 'interface $_ {
$$$
vendor: $_
$$$
}'
# Check for AML-related service implementations
echo "Checking AML service implementations..."
rg -l "class.*AmlService" --type ts
Length of output: 406
services/workflows-service/src/webhooks/webhooks.service.ts (2)
Line range hint 102-121: Verify workflow context structure with vendor information
The workflow context should properly handle the vendor information being passed through. Ensure that downstream processors can handle different vendor data structures.
Let's check for workflow context handling in the codebase:
#!/bin/bash
# Description: Check how workflow context handles vendor-specific data
# Expected: Find workflow processors or handlers that consume vendor information
# Search for workflow context handling
rg -A 10 "context.*aml" --glob "*.ts"
# Search for vendor-specific processing
ast-grep --pattern 'vendor.*=.*data.vendor'80-80: 🛠️ Refactor suggestion
Verify vendor validation and add null safety
While dynamically setting the vendor from data?.vendor aligns with supporting multiple vendors (Veriff, Dow Jones), consider adding validation to ensure the vendor value matches the allowed options.
Let's verify the vendor validation in related schema files:
Consider adding explicit validation:
- vendor: data?.vendor,
+ vendor: data?.vendor ?? 'veriff', // Provide default or
+ // Alternatively, validate against allowed values
+ vendor: ['veriff', 'dow-jones'].includes(data?.vendor)
+ ? data?.vendor
+ : 'veriff',services/workflows-service/package.json (2)
44-44: Verify the new validation script implementation
The new validation script is properly configured with tsx runtime and follows the project's script path conventions.
#!/bin/bash
# Description: Verify the validation script exists and uses zod
# Check if the validation script exists and uses zod
rg -l "import.*from\s+'zod'" ./scripts/run-validation.ts
# Verify the script follows project's validation patterns
ast-grep --pattern 'import { z } from "zod"
$$$
schema = z.$_'43-43: Verify the data migration import script's functionality
The script appears to be using the correct runtime and follows the project's path conventions.
✅ Verification successful
The data migration import script is properly implemented and functional
The script is correctly implemented with:
- A TypeScript wrapper (
import.ts) that executes a shell script - A shell script (
data-migration-import.sh) that:- Downloads workflow data migrations from a GitHub repository
- Correctly handles different environments (staging, sandbox, prod, dev)
- Places migrations in the appropriate
prisma/data-migrationsdirectory
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the data migration import script exists and follows conventions
# Check if the import script exists and contains actual implementation
rg -l "import\s+.*\s+from" ./src/data-migration/scripts/import
Length of output: 201
Script:
#!/bin/bash
# Let's verify if the import script exists and its location
fd "import" src/data-migration/scripts
# If the file exists, let's check its content
fd "import" src/data-migration/scripts -x cat {}
# Let's also check for any TypeScript/JavaScript files in that directory
fd "\.(ts|js)$" src/data-migration/scripts
Length of output: 476
Script:
#!/bin/bash
# Let's first check if the data-migration directory exists and its location
fd -t d "data-migration"
# Let's look for any TypeScript files containing "import" in their name
fd -e ts "import"
# Let's also check the package.json to understand the project structure
cat services/workflows-service/package.json
Length of output: 6478
Script:
#!/bin/bash
# Let's check the content of the import script
cat services/workflows-service/src/data-migration/scripts/import.ts
# Let's also check if there are any imports or references to this script
rg -l "data-migration.*import" services/workflows-service/src
Length of output: 358
Script:
#!/bin/bash
# Let's check if the shell script that's being executed exists
ls -l services/workflows-service/src/data-migration/scripts/data-migration-import.sh
# And check its content if it exists
cat services/workflows-service/src/data-migration/scripts/data-migration-import.sh
Length of output: 1533
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
services/workflows-service/CHANGELOG.md (1)
3-10: Consider adding more context to the changelog entry.While the dependency updates are clearly documented, it would be helpful to include:
- The reason for these dependency updates
- Any notable changes or fixes included in the updated dependencies
- Potential impact on existing functionality
services/workflows-service/src/workflow/workflow.service.ts (1)
Line range hint
1-2836: Consider splitting the WorkflowService into smaller, focused servicesThe current service class is handling too many responsibilities, which makes it harder to maintain and test. Consider breaking it down into smaller, more focused services:
- WorkflowDefinitionService - Handle workflow definition operations
- WorkflowRuntimeService - Manage workflow runtime state
- WorkflowDocumentService - Handle document processing and OCR
- WorkflowEventService - Manage workflow events
This separation would:
- Improve code organization and maintainability
- Make the codebase easier to test
- Reduce cognitive load when working with the code
- Allow for better dependency injection and mocking in tests
Additionally, consider:
- Extracting common business logic into separate utilities
- Creating dedicated DTOs for data transformation
- Implementing a proper error handling strategy with custom error types
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
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/common/CHANGELOG.md(1 hunks)packages/common/package.json(1 hunks)packages/workflow-core/CHANGELOG.md(1 hunks)packages/workflow-core/package.json(2 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/workflows-service/CHANGELOG.md(1 hunks)services/workflows-service/package.json(2 hunks)services/workflows-service/src/workflow/workflow.service.ts(1 hunks)websites/docs/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (15)
- apps/backoffice-v2/CHANGELOG.md
- apps/backoffice-v2/package.json
- apps/kyb-app/CHANGELOG.md
- apps/kyb-app/package.json
- examples/headless-example/CHANGELOG.md
- examples/headless-example/package.json
- packages/common/CHANGELOG.md
- packages/common/package.json
- packages/workflow-core/CHANGELOG.md
- packages/workflow-core/package.json
- sdks/web-ui-sdk/CHANGELOG.md
- sdks/workflow-browser-sdk/CHANGELOG.md
- sdks/workflow-browser-sdk/package.json
- sdks/workflow-node-sdk/CHANGELOG.md
- sdks/workflow-node-sdk/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- services/workflows-service/package.json
🔇 Additional comments (1)
sdks/web-ui-sdk/package.json (1)
Line range hint 1-106: Please clarify the connection to Dow Jones changes.
The PR is titled "Dow Jones ongoing changes" but the changes in this file appear to be routine version bumps. Could you please update the PR description to explain how these version updates relate to the Dow Jones changes?
Let's check for any Dow Jones related changes in the codebase:
| @@ -17,7 +17,7 @@ | |||
| "dependencies": { | |||
| "@astrojs/starlight": "0.11.1", | |||
| "@astrojs/tailwind": "^4.0.0", | |||
| "@ballerine/common": "^0.9.50", | |||
| "@ballerine/common": "^0.9.51", | |||
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent versions of @ballerine/common detected across the monorepo
The version check reveals inconsistencies in @ballerine/common dependency versions:
-
Using version ^0.9.50:
- packages/ui/package.json
- packages/blocks/package.json
- apps/workflows-dashboard/package.json
-
Using version ^0.9.51 or 0.9.51:
- websites/docs/package.json
- sdks/workflow-browser-sdk/package.json
- services/workflows-service/package.json
- examples/headless-example/package.json
- packages/workflow-core/package.json
- apps/kyb-app/package.json
- apps/backoffice-v2/package.json
- sdks/web-ui-sdk/package.json
These inconsistencies should be addressed to maintain version alignment across the monorepo.
🔗 Analysis chain
Verify version consistency across the monorepo
The version bump of @ballerine/common to ^0.9.51 looks good. This aligns with the coordinated update across other services mentioned in the summary.
Let's verify the version consistency across the monorepo:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for version consistency of @ballerine/common across the monorepo
# Expected: All references should be using ^0.9.51
# Find all package.json files and check @ballerine/common version
fd package.json --type f --exec sh -c 'echo "=== $1 ==="; cat "$1" | jq -r ".dependencies[\"@ballerine/common\"] // empty"' sh {}
Length of output: 1346
| @@ -21,7 +21,7 @@ | |||
| "types": "dist/index.d.ts", | |||
| "name": "@ballerine/web-ui-sdk", | |||
| "private": false, | |||
| "version": "1.5.51", | |||
| "version": "1.5.52", | |||
There was a problem hiding this comment.
💡 Codebase verification
Version inconsistency detected across packages
The version of @ballerine/common is not consistent across all packages:
-
Most packages use version
0.9.51:- services/workflows-service
- sdks/web-ui-sdk
- sdks/workflow-browser-sdk
- packages/workflow-core
- examples/headless-example
- apps/backoffice-v2
-
Some packages still use version
0.9.50:- packages/ui
- packages/blocks
- apps/workflows-dashboard
This inconsistency should be addressed to maintain version alignment across the monorepo.
🔗 Analysis chain
LGTM! Version updates look consistent.
The version bumps follow semantic versioning and align with the coordinated updates across multiple packages in the ecosystem:
- Package version: 1.5.51 → 1.5.52
- @ballerine/common: 0.9.50 → 0.9.51
Let's verify the version consistency across related packages:
Also applies to: 99-99
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check @ballerine/common version across related packages
# Expected: Version 0.9.51 should be used consistently
echo "Checking @ballerine/common version across packages..."
fd package.json | xargs rg "@ballerine/common.*0\.9\."
Length of output: 922
| steps: uiDefinition?.definition | ||
| ? getOrderedSteps( | ||
| (uiDefinition?.definition as Prisma.JsonObject)?.definition as Record< | ||
| string, | ||
| Record<string, unknown> | ||
| >, | ||
| { finalStates: [...WORKFLOW_FINAL_STATES] }, | ||
| ).map(stepName => ({ | ||
| stateName: stepName, | ||
| })) | ||
| : [], |
There was a problem hiding this comment.
Improve type safety and null checks in steps handling
The current implementation has several type safety and potential null reference issues:
- Multiple type assertions could lead to runtime errors
- Missing null check for
uiDefinitionbefore accessingdefinition - Complex nested property access without proper type guards
Consider refactoring using this safer approach:
- steps: uiDefinition?.definition
- ? getOrderedSteps(
- (uiDefinition?.definition as Prisma.JsonObject)?.definition as Record<
- string,
- Record<string, unknown>
- >,
- { finalStates: [...WORKFLOW_FINAL_STATES] },
- ).map(stepName => ({
- stateName: stepName,
- }))
- : [],
+ steps: (() => {
+ const definition = uiDefinition?.definition;
+ if (!definition || typeof definition !== 'object') return [];
+
+ const stateDefinition = (definition as Prisma.JsonObject)?.definition;
+ if (!stateDefinition || typeof stateDefinition !== 'object') return [];
+
+ return getOrderedSteps(
+ stateDefinition as Record<string, Record<string, unknown>>,
+ { finalStates: [...WORKFLOW_FINAL_STATES] }
+ ).map(stepName => ({
+ stateName: stepName,
+ }));
+ })(),Additionally:
- Consider extracting the steps transformation logic into a separate method for better maintainability
- Add documentation for the WORKFLOW_FINAL_STATES array to clarify its purpose
- Consider adding validation for the step name format
📝 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.
| steps: uiDefinition?.definition | |
| ? getOrderedSteps( | |
| (uiDefinition?.definition as Prisma.JsonObject)?.definition as Record< | |
| string, | |
| Record<string, unknown> | |
| >, | |
| { finalStates: [...WORKFLOW_FINAL_STATES] }, | |
| ).map(stepName => ({ | |
| stateName: stepName, | |
| })) | |
| : [], | |
| steps: (() => { | |
| const definition = uiDefinition?.definition; | |
| if (!definition || typeof definition !== 'object') return []; | |
| const stateDefinition = (definition as Prisma.JsonObject)?.definition; | |
| if (!stateDefinition || typeof stateDefinition !== 'object') return []; | |
| return getOrderedSteps( | |
| stateDefinition as Record<string, Record<string, unknown>>, | |
| { finalStates: [...WORKFLOW_FINAL_STATES] } | |
| ).map(stepName => ({ | |
| stateName: stepName, | |
| })); | |
| })(), |
Summary by CodeRabbit
Release Notes
New Features
vendorproperty in the AML schema, allowing specific vendor values.Improvements
matchedNamefield in the AML hit schema to accept null values.