Individuals sanctions plugin fixes + workflow observability (2 commits) #3143
Conversation
WalkthroughThe changes span multiple areas across the codebase. They include enhancements to error handling and conditional logic in utilities and services, minor UI style adjustments, and added safety checks via optional chaining. New functionality was introduced for filling placeholders in collection flows and for comprehensive workflow logging, involving new UI components, API endpoints, repository classes, service methods, controller endpoints, and corresponding schema and environment configuration updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WorkflowService
participant WorkflowRunner
participant LogService as WorkflowLogService
participant DB as WorkflowLogRepository
Client->>WorkflowService: Send workflow event
WorkflowService->>WorkflowRunner: Process event
WorkflowRunner-->>WorkflowService: Return log data
WorkflowService->>LogService: Process and persist logs
LogService->>DB: Save logs
DB-->>LogService: Confirmation
LogService-->>WorkflowService: Logs processed
sequenceDiagram
participant User
participant CollectionFlow
participant ContextAPI
User->>CollectionFlow: Click "Fill Placeholders" button
CollectionFlow->>ContextAPI: Retrieve current context
CollectionFlow->>CollectionFlow: Recursively scan for placeholders
CollectionFlow->>ContextAPI: Update context with found placeholders
ContextAPI-->>CollectionFlow: Acknowledge update
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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: 0
♻️ Duplicate comments (1)
apps/kyb-app/src/pages/CollectionFlow/versions/v2/CollectionFlowV2.tsx (1)
250-398: 🛠️ Refactor suggestionDuplicate implementation of placeholder filling functionality
This implementation of the "Fill Placeholders" functionality is nearly identical to the one in CollectionFlowV1, with only minor differences in handling birth date fields.
As mentioned in the review of CollectionFlowV1, this functionality should be extracted into a shared utility to avoid code duplication.
The current implementation has a slightly improved date handling logic compared to V1, as it also checks for "birth" in field names. Consider using this enhanced version when refactoring into a shared utility:
// In the shared utility function let value = placeholder; if ( lastKey.toLowerCase().includes('date') || lastKey.toLowerCase().includes('birth') || valueDestination.toLowerCase().includes('date') || valueDestination.toLowerCase().includes('birth') ) { value = '11/11/1990'; } current[lastKey] = value;This will ensure consistent behavior across both component versions.
🧹 Nitpick comments (14)
apps/kyb-app/src/pages/CollectionFlow/versions/v1/CollectionFlowV1.tsx (1)
306-448: New debug utility for filling placeholdersAdding a "Fill Placeholders" button in debug mode provides a useful development tool to quickly populate form fields with placeholder values, which can save time during testing.
The implementation correctly:
- Only appears in debug mode
- Finds visible elements with placeholders
- Handles nested element structures
- Provides special handling for date fields
- Includes error handling
However, there are some areas for improvement:
Consider extracting this placeholder filling functionality into a shared utility function instead of duplicating it in both V1 and V2 components. This would make maintenance easier and ensure consistent behavior.
// Create a new utility file, e.g., src/utils/formUtils.ts + export const fillPlaceholders = (stateApi, currentPage) => { + try { + console.log('Filling placeholders...'); + + const filledPayload = { ...stateApi.getContext() }; + console.log('Current context:', filledPayload); + + const allElements = []; + + const findElementsWithPlaceholders = (elements) => { + // existing implementation + }; + + if (currentPage?.elements) { + findElementsWithPlaceholders(currentPage.elements); + } + + // rest of the implementation + + stateApi.setContext(filledPayload); + console.log('Context updated successfully'); + } catch (error) { + console.error('Error filling placeholders:', error); + } + }; // Then in both components, replace the onClick handler with: + onClick={() => fillPlaceholders(stateApi, currentPage)}The code has a few small issues that could be addressed:
- The
visibilityRulesvariable on line 364 is declared but never used- There's heavy use of console logs which should be conditionally enabled in development mode
- Consider extracting the date placeholder '11/11/1990' into a constant or configuration
services/workflows-service/prisma/schema.prisma (1)
962-991: NewWorkflowLogTypeenum andWorkflowLogmodel appear well-defined.
The schema precisely covers required properties and index fields. Should logs grow large, consider an archiving or partitioning strategy to maintain performance.apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogGraph.tsx (2)
21-45: Consider consolidating repeated node component logic.
The three custom nodes only differ by styling. Factoring them into a single component with style variants can reduce duplication.-const CustomStateNode = ... -const CustomEventNode = ... -const CustomPluginNode = ... +function CustomNode({ label, themeColor }) { + return ( + <div + className={`rounded-md border border-${themeColor}-300 bg-${themeColor}-50 px-3 py-1 shadow-sm`} + > + <div className="text-xs">{label}</div> + <Handle type="target" position={Position.Top} style={{ background: themeColor }} /> + <Handle type="source" position={Position.Bottom} style={{ background: themeColor }} /> + </div> + ); +}
70-388: MainWorkflowLogGraphcomponent is comprehensive but quite large.
The logic for sorting logs, grouping events/plugins, and building state transitions works properly. Consider extracting certain chunks (e.g., node/edge creation) into smaller helper functions or custom hooks to improve readability and maintainability.apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogsModal.tsx (1)
106-133: Use optional chaining for improved code safety.The filter function properly handles search queries, but the code could benefit from using optional chaining when checking optional properties.
return ( log.type.toLowerCase().includes(searchLower) || log.message.toLowerCase().includes(searchLower) || - (log.eventName && log.eventName.toLowerCase().includes(searchLower)) || - (log.pluginName && log.pluginName.toLowerCase().includes(searchLower)) || - (log.fromState && log.fromState.toLowerCase().includes(searchLower)) || - (log.toState && log.toState.toLowerCase().includes(searchLower)) || + log.eventName?.toLowerCase().includes(searchLower) || + log.pluginName?.toLowerCase().includes(searchLower) || + log.fromState?.toLowerCase().includes(searchLower) || + log.toState?.toLowerCase().includes(searchLower) || String(log.id).includes(searchLower) );🧰 Tools
🪛 Biome (1.9.4)
[error] 114-114: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 115-115: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 116-116: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 117-117: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
services/workflows-service/src/workflow/workflow-log.service.ts (1)
61-95: Consider batch insertion for improved performance.The current implementation processes logs one by one with individual database calls. For better performance, especially with many logs, consider using batch insertion.
try { const formattedLogs = logs.map(log => ({ workflowRuntimeDataId, type: log.category as WorkflowLogType, fromState: log.previousState, toState: log.newState, message: log.message, metadata: log.metadata, eventName: log.eventName, pluginName: log.pluginName, projectId, })); const prismaClient = transaction || this.prismaService; - for (const log of formattedLogs) { - await prismaClient.workflowLog.create({ - data: { - workflowRuntimeDataId: log.workflowRuntimeDataId, - type: log.type, - fromState: log.fromState, - toState: log.toState, - message: log.message, - metadata: log.metadata || {}, - projectId: log.projectId, - eventName: log.eventName, - pluginName: log.pluginName, - }, - }); - } + // Use createMany for better performance with multiple logs + await prismaClient.workflowLog.createMany({ + data: formattedLogs.map(log => ({ + workflowRuntimeDataId: log.workflowRuntimeDataId, + type: log.type, + fromState: log.fromState, + toState: log.toState, + message: log.message, + metadata: log.metadata || {}, + projectId: log.projectId, + eventName: log.eventName, + pluginName: log.pluginName, + })), + });packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.ts (1)
73-77: Good addition of configurable result destinationAdding the
resultDestinationproperty with a default value maintains backward compatibility while providing flexibility for result storage locations.There's a typo in the TODO comment: "proabably" should be "probably".
- // TODO: proabably can be kept undefined and let the parent class handle it, for now keeping our old path as default + // TODO: probably can be kept undefined and let the parent class handle it, for now keeping our old path as defaultservices/workflows-service/src/workflow/workflow-log.controller.ts (1)
11-42: Well-structured DTO with proper validationThe
GetWorkflowLogsQueryDtois well designed with:
- Appropriate validations for each field
- Type transformations using
class-transformer- Sensible defaults for pagination parameters
Consider adding a description to each field using the
@ApiPropertydecorator to enhance API documentation.import { ApiOperation, ApiResponse } from '@nestjs/swagger'; -import { ApiTags } from '@nestjs/swagger'; +import { ApiTags, ApiProperty } from '@nestjs/swagger'; class GetWorkflowLogsQueryDto { @IsOptional() @IsEnum(WorkflowLogType, { each: true }) + @ApiProperty({ + description: 'Filter logs by specific types', + enum: WorkflowLogType, + isArray: true, + required: false + }) types?: WorkflowLogType[]; @IsOptional() @IsDate() @Type(() => Date) + @ApiProperty({ + description: 'Filter logs from this date', + required: false + }) fromDate?: Date; // Add ApiProperty for other fields as wellservices/workflows-service/prisma/migrations/20250308172521_workflow_logs/migration.sql (2)
4-19: Comprehensive log table structureThe
WorkflowLogtable is well designed with:
- Fields for various log aspects (states, messages, events, plugins)
- JSONB metadata for flexible additional data storage
- Timestamp for chronological tracking
- Proper relationships to workflows and projects
Consider adding an optional
userIdcolumn to track which user's action triggered the log entry, especially for manual actions.
33-37: Proper foreign key constraintsThe foreign key constraints maintain referential integrity, ensuring logs always reference valid workflows and projects.
Consider adding
ON DELETE CASCADEforworkflowRuntimeDataIdif you want logs to be automatically deleted when a workflow is removed.apps/workflows-dashboard/src/domains/workflows/api/workflow-logs/workflow-logs.api.ts (2)
26-29: Simple and focused parameters interfaceThe
GetWorkflowLogsParamsinterface focuses on pagination parameters, making the API easy to use for common use cases.Consider adding type filters to match the backend capabilities:
export interface GetWorkflowLogsParams { page?: number; pageSize?: number; + types?: string[]; + fromDate?: Date | string; + toDate?: Date | string; + orderBy?: 'asc' | 'desc'; }
31-51: Well-documented and implemented API functionThe
fetchWorkflowLogsfunction:
- Has clear JSDoc comments explaining parameters and return values
- Provides sensible defaults for pagination
- Properly handles request parameters
- Returns the expected response type
Consider adding support for additional query parameters like types, dates, and sort order.
export const fetchWorkflowLogs = async ( workflowId: string, params: GetWorkflowLogsParams = {}, ): Promise<GetWorkflowLogsResponse> => { - const { page = 1, pageSize = 100 } = params; + const { page = 1, pageSize = 100, types, fromDate, toDate, orderBy } = params; const result = await request.get<GetWorkflowLogsResponse>(`/workflow-logs/${workflowId}`, { params: { page, pageSize, + types, + fromDate, + toDate, + orderBy, }, }); return result.data; };services/workflows-service/src/workflow/workflow-log.repository.ts (2)
37-37: Consider using a more specific type instead ofanyUsing
anytype reduces type safety. Consider using a more specific type that matches Prisma's filter structure, such as:- const where: any = {}; + const where: Prisma.WorkflowLogWhereInput = {};You'll need to import the Prisma namespace from @prisma/client:
import { Prisma, WorkflowLog, WorkflowLogType } from '@prisma/client';
116-122: Optimize reducer to avoid spread operator performance issuesUsing spread syntax in reducers can cause O(n²) time complexity as noted by the static analysis hint. While the enum size may be small enough that it's not a practical issue, it's good practice to avoid this pattern.
- const summary = Object.values(WorkflowLogType).reduce( - (acc, type) => ({ - ...acc, - [type]: 0, - }), - {} as Record<WorkflowLogType, number>, - ); + const summary = {} as Record<WorkflowLogType, number>; + Object.values(WorkflowLogType).forEach((type) => { + summary[type] = 0; + });🧰 Tools
🪛 Biome (1.9.4)
[error] 118-118: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
📜 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 (32)
apps/backoffice-v2/src/common/utils/fetcher/fetcher.ts(2 hunks)apps/backoffice-v2/src/domains/auth/components/AuthenticatedLayout/components/NavIntroduction.tsx(1 hunks)apps/backoffice-v2/src/domains/auth/components/AuthenticatedLayout/components/NavItem.tsx(1 hunks)apps/backoffice-v2/src/lib/blocks/hooks/useBankAccountVerificationBlock/useBankAccountVerificationBlock.tsx(2 hunks)apps/kyb-app/src/common/components/atoms/StepperProgress/StepperProgress.tsx(1 hunks)apps/kyb-app/src/pages/CollectionFlow/versions/v1/CollectionFlowV1.tsx(1 hunks)apps/kyb-app/src/pages/CollectionFlow/versions/v2/CollectionFlowV2.tsx(1 hunks)apps/workflows-dashboard/package.json(1 hunks)apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogGraph.tsx(1 hunks)apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogsModal.tsx(1 hunks)apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/index.ts(1 hunks)apps/workflows-dashboard/src/components/molecules/WorkflowsTable/columns.tsx(2 hunks)apps/workflows-dashboard/src/components/molecules/WorkflowsTable/types.ts(1 hunks)apps/workflows-dashboard/src/domains/workflows/api/workflow-logs/index.ts(1 hunks)apps/workflows-dashboard/src/domains/workflows/api/workflow-logs/workflow-logs.api.ts(1 hunks)packages/common/src/schemas/documents/default-context-schema.ts(2 hunks)packages/workflow-core/src/lib/plugins/external-plugin/api-plugin/api-plugin.ts(1 hunks)packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.ts(5 hunks)packages/workflow-core/src/lib/types.ts(3 hunks)packages/workflow-core/src/lib/workflow-runner.ts(13 hunks)services/workflows-service/.env.example(1 hunks)services/workflows-service/prisma/migrations/20250308172521_workflow_logs/migration.sql(1 hunks)services/workflows-service/prisma/schema.prisma(3 hunks)services/workflows-service/src/business/business.service.ts(2 hunks)services/workflows-service/src/customer/customer.service.ts(3 hunks)services/workflows-service/src/env.ts(1 hunks)services/workflows-service/src/workflow/workflow-log.controller.ts(1 hunks)services/workflows-service/src/workflow/workflow-log.repository.ts(1 hunks)services/workflows-service/src/workflow/workflow-log.service.ts(1 hunks)services/workflows-service/src/workflow/workflow.controller.external.ts(1 hunks)services/workflows-service/src/workflow/workflow.module.ts(3 hunks)services/workflows-service/src/workflow/workflow.service.ts(7 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogsModal.tsx
[error] 114-114: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 115-115: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 116-116: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 117-117: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
services/workflows-service/src/workflow/workflow-log.repository.ts
[error] 118-118: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Analyze (javascript)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
🔇 Additional comments (81)
services/workflows-service/.env.example (1)
39-39: Configuration Update: SYNC_UNIFIED_API DisabledThe environment configuration now sets
SYNC_UNIFIED_APItofalse, which disables unified API synchronization as described in the PR objectives. Please ensure that all services relying on this variable (such asBusinessServiceandCustomerService) have been updated accordingly and that the behavior changes are well documented and tested in their respective workflows.apps/backoffice-v2/src/common/utils/fetcher/fetcher.ts (2)
25-32: Good improvement to prevent development timeouts!Adding conditional timeout handling based on environment is a thoughtful enhancement. This will prevent requests from timing out during development debugging sessions while maintaining the safeguard in production.
43-43: Nice defensive programming!Adding the conditional check before calling
clearTimeouthandles the case wheretimeoutRefis null in development mode. This prevents potential errors and aligns with the changes made above.apps/backoffice-v2/src/lib/blocks/hooks/useBankAccountVerificationBlock/useBankAccountVerificationBlock.tsx (2)
56-62: Improved null safety with optional chainingAdding optional chaining (
?.) operators for accessingpluginsOutputproperties helps prevent potential runtime errors ifpluginsOutputis undefined or null. This defensive programming approach aligns with existing patterns in the codebase.
114-114: Good dependency array updateUpdating the dependency array with optional chaining ensures the hook correctly reacts to changes in
pluginsOutput.bankAccountVerificationwhile maintaining null safety.apps/kyb-app/src/common/components/atoms/StepperProgress/StepperProgress.tsx (1)
16-16: Uniform Step Indicator Styling.
The class name for the current step has been updated to usetext-blue-600, which provides a slightly bolder blue tone that aligns well with the updated design language. Please verify that this shade is consistent with the design guidelines applied across similar components.apps/backoffice-v2/src/domains/auth/components/AuthenticatedLayout/components/NavIntroduction.tsx (1)
67-67: Standardized Skeleton Component Class Ordering.
The class name order for the<Skeleton>component has been updated to"absolute inset-0 size-full". Although the reordering of class names is functionally equivalent, it improves consistency in styling across components.apps/backoffice-v2/src/domains/auth/components/AuthenticatedLayout/components/NavItem.tsx (1)
41-41: Consistent Styling for Skeleton in Premium Hover Card.
The modified<Skeleton>component now uses the class name"absolute inset-0 size-full", matching the changes in other components for a unified style approach. This update is purely cosmetic and maintains the functionality as intended.apps/workflows-dashboard/package.json (1)
54-54: Dependencies added for workflow logs visualizationThe addition of
react-json-treeandreactflowlibraries appears to support the new workflow logs visualization functionality. These are appropriate choices for displaying JSON data and creating interactive flow diagrams respectively.Also applies to: 57-57
apps/workflows-dashboard/src/domains/workflows/api/workflow-logs/index.ts (1)
1-1: Clean API export patternThis barrel file exports all entities from the workflow-logs API module, following the standard pattern for creating a clean module interface.
apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/index.ts (1)
1-2: Standard component export patternThis barrel file correctly exports both named exports and the default export from the WorkflowLogsModal component, enabling flexible import patterns.
apps/workflows-dashboard/src/components/molecules/WorkflowsTable/types.ts (1)
9-21: Type definition extended for workflow logs columnThe addition of
'workflow-logs'to theWorkflowTableColumnKeystype union is appropriate and enables the addition of a new column in the workflows table for displaying workflow logs.services/workflows-service/src/business/business.service.ts (2)
70-72: Simplified environment variable check for better flexibility.Changed from strict equality comparison with 'true' string to a truthiness check, making the code more flexible in accepting different truthy values.
145-147: Simplified environment variable check for better flexibility.Similar to the earlier change, this simplifies the condition to check truthiness rather than strict equality with 'true'.
services/workflows-service/src/customer/customer.service.ts (3)
71-73: Simplified environment variable check for better flexibility.Changed from strict equality comparison with 'true' string to a truthiness check, consistent with changes in business.service.ts.
103-105: Simplified environment variable check for better flexibility.Consistent with previous changes, this simplifies the condition to check truthiness rather than strict equality.
115-117: Simplified environment variable check for better flexibility.Consistent with other changes in this service and business.service.ts, this simplifies the condition for better flexibility.
packages/common/src/schemas/documents/default-context-schema.ts (3)
24-30: LGTM: Well-structured plugin schema definition.The new defaultPluginSchema provides a solid base structure for plugin schemas with essential fields.
32-37: LGTM: Good use of schema composition for individual sanctions.Using Type.Composite to combine the default plugin schema with specific data requirements for individual sanctions is a clean approach.
49-49: LGTM: Successfully integrated the individual sanctions plugin.The individualSanctions plugin has been properly integrated into the pluginsOutput object schema.
services/workflows-service/src/workflow/workflow.module.ts (4)
49-51: LGTM: Added new workflow logging imports.Properly imported the necessary components for workflow logging functionality.
54-54: LGTM: Registered workflow log controller.The WorkflowLogController has been correctly added to the module's controllers array.
101-102: LGTM: Registered workflow logging providers.The WorkflowLogService and WorkflowLogRepository have been properly added to the providers array.
117-118: LGTM: Exported workflow logging services.Correctly exported the workflow logging services for use in other modules.
services/workflows-service/src/workflow/workflow.controller.external.ts (1)
556-566: Added safety check to prevent event invocation with undefined valuesThis change improves error handling by checking if
params.eventexists and isn't 'undefined' before proceeding with the event call. This prevents unnecessary processing when no valid event is provided.packages/workflow-core/src/lib/plugins/external-plugin/api-plugin/api-plugin.ts (1)
149-152: Enhanced error logging with payload informationThis addition significantly improves observability by capturing both the error and the request payload in the logs, making it easier to diagnose API plugin issues in production.
services/workflows-service/src/env.ts (2)
96-99: Improved environment variable type handlingConverting
SYNC_UNIFIED_APIfrom a string to a boolean with proper preprocessing improves type safety and makes configuration more intuitive. The preprocessing function correctly handles both string 'true' and boolean true values.
103-106: Added workflow logging configuration flagThe new
WORKFLOW_LOGGING_ENABLEDenvironment variable with proper preprocessing supports the workflow observability features being added. It defaults to false which is a safe choice for backward compatibility.services/workflows-service/src/workflow/workflow.service.ts (5)
47-47: Added workflow logging importsAdded necessary imports for the new workflow logging functionality.
164-164: Injected workflow log service dependencyAdded the WorkflowLogService as a dependency to enable logging of workflow execution events.
1642-1642: Added endUserId to metadata for collection flowThe addition of endUserId to the metadata ensures that user context is properly maintained in the collection flow, improving traceability.
1982-1985: Enhanced error reporting with context valuesImproved the error logging in workflow validation by mapping error paths to their corresponding values in the context. This makes debugging workflow definition context validation failures much easier.
2252-2265:Details
✅ Verification successful
Added workflow logs processing
This block implements workflow execution logging functionality, retrieving logs from the workflow runner, processing them through the workflow log service, and clearing them afterward. The error handling ensures that failures in logging won't affect the core workflow execution.
However, verify that the necessary feature flag check is implemented at the service level to respect the
WORKFLOW_LOGGING_ENABLEDenvironment variable.
🏁 Script executed:
#!/bin/bash # Check if the workflow-log.service.ts respects the WORKFLOW_LOGGING_ENABLED flag rg -A 3 "WORKFLOW_LOGGING_ENABLED" --type ts services/workflows-service/src/Length of output: 714
Final Verification: Feature Flag Check Confirmed
The workflow logs processing block in
services/workflows-service/src/workflow/workflow.service.tshas been implemented as intended. Additionally, the logging feature toggle (WORKFLOW_LOGGING_ENABLED) is correctly checked within the logging service (services/workflows-service/src/workflow/workflow-log.service.ts). No further changes are required.apps/workflows-dashboard/src/components/molecules/WorkflowsTable/columns.tsx (3)
10-14: Imports look well-structured and purposeful.
These additions cleanly introduce the needed icon, state hook, button, and modal components. No concerns here.
16-39: Modal toggling inWorkflowLogsButtonis straightforward and correct.
This component cleanly manages its local modal state. The approach of passingworkflowIdas a prop toWorkflowLogsModalis well-organized, enabling clear data flow.
138-143: New'workflow-logs'column integrates well.
Defining a dedicated accessor key and renderingWorkflowLogsButtonprovides a user-friendly entry point for logs while staying consistent with the existing table structure.services/workflows-service/prisma/schema.prisma (2)
285-285: Addedlogsrelation inWorkflowRuntimeDatais consistent.
Linking multiple workflow log entries to runtime data aligns well with typical logging patterns.
431-431:workflowLogsfield inProject
Allowing projects to reference all associated workflow logs is a logical design decision for advanced audit and reporting needs.apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogGraph.tsx (5)
1-19: Imports and interface definition look solid.
Using a dedicatedWorkflowLogtype from your domain models clarifies contract boundaries.
46-48:getNodeIdhelper is straightforward.
This neatly avoids accidental collisions between different node types.
50-55: Layout constants nicely centralize graph spacing settings.
Naming them clearly as widths/spacings ensures maintainability. No issues found.
56-69: Event lookup logic infindTriggeringEventis correct.
Filtering for the last event ID below the current transition reliably identifies the relevant trigger.
390-390: Exporting both named and default is a valid approach.
Though the dual export is consistent, review whether the default export is necessary if named exports suffice.packages/workflow-core/src/lib/types.ts (4)
29-30: LGTM: New methods added to the Workflow interface.The addition of
getLogsandclearLogsmethods to the Workflow interface provides a clean API for accessing and managing workflow log entries.
102-102: LGTM: Optional logging flag added to WorkflowRunnerArgs.The optional
enableLoggingproperty allows for controlling the logging behavior when creating a workflow runner.
148-155: LGTM: Well-structured enum for log categories.The
WorkflowLogCategoryenum provides a comprehensive set of categories for different types of workflow events, which will help in organizing and filtering logs.
157-166: LGTM: Comprehensive log entry structure.The
WorkflowLogEntryinterface defines a well-structured format for log entries with all necessary fields to track workflow events, state transitions, and plugin invocations.apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogsModal.tsx (7)
1-42: LGTM: Well-structured component setup and search highlighting function.Good organization of imports and component props interface. The
highlightTextfunction provides a nice visual enhancement for search results.
43-103: LGTM: Comprehensive state management and sorting logic.The component properly manages various states including logs, loading states, pagination, search queries, and sorting. The sorting logic is well-implemented with toggle functionality.
135-162: LGTM: Well-defined theme and sort indicator.The theme definition for the JSON tree and sort indicator render function are well implemented.
164-193: LGTM: Clean modal structure with loading states.Good implementation of the modal structure with appropriate loading states, error handling, and empty state messaging.
194-231: LGTM: Well-designed search and sort UI.The search and sort UI is well-structured with clear visual indicators for the current sorting field.
232-342: LGTM: Comprehensive log table with detailed information.The log table is well-structured with proper formatting for different log types and detailed information display.
343-366: LGTM: Good pagination implementation.The "Load More" button is properly conditionally rendered based on the
hasMorestate and search query.services/workflows-service/src/workflow/workflow-log.service.ts (4)
6-15: LGTM: Clear log type definition.Good implementation of the log type constants and TypeScript type definition.
17-37: LGTM: Comprehensive interface definitions.The interfaces
WorkflowLogEntryandWorkflowRunnerLogEntryprovide clear structure for log entries.
39-60: LGTM: Well-structured service with proper early return.Good implementation of the service with logger initialization and early return if logging is disabled or no logs are provided.
96-100: LGTM: Proper error handling.Good error handling with descriptive error messages and stack trace logging.
packages/workflow-core/src/lib/workflow-runner.ts (9)
65-67: LGTM: Added imports for new log types.Correctly added imports for the WorkflowLogEntry and WorkflowLogCategory types.
84-85: LGTM: Private fields for logging control and storage.Good implementation of private class fields for enabling/disabling logging and storing log entries.
107-107: LGTM: Proper initialization of logging flag.Good default value for enableLogging parameter and proper initialization of the private field.
Also applies to: 117-117
587-596: LGTM: Event received logging.Good implementation of logging for received events with appropriate categorization and metadata.
608-620: LGTM: State transition logging.Good implementation of logging for state transitions with appropriate categorization and metadata.
649-655: LGTM: Context change logging.Good implementation of logging for context changes.
667-677: LGTM: Error logging for invalid events.Good implementation of logging for error conditions when an invalid event is received.
693-703: LGTM: Comprehensive plugin invocation logging.Thorough implementation of logging for all types of plugin invocations with appropriate categorization and metadata.
Also applies to: 721-725, 748-758, 765-775, 782-792, 799-809, 828-838
1114-1154: LGTM: Well-implemented log management methods.Good implementation of methods to get logs, clear logs, and create log entries with proper checks for logging being enabled.
packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.ts (4)
23-31: Great addition of date validation and normalization!The new
dateSchemaensures consistent date formatting while providing proper validation. This preprocessor handles both string and Date inputs, normalizing them to the "YYYY-MM-DD" format when valid.
36-36: Good implementation of the new date schemaUpdating the
dateOfBirthfield to use the newdateSchemaensures consistent date validation and normalization throughout the application.
144-147: Properly implemented configurable result destinationThe destructuring now includes
resultDestinationand correctly uses it in thecallbackUrlconstruction, making the plugin more flexible for different workflow configurations.
190-191: Improved code readabilityThe refactoring to assign the result to a variable before returning it improves code readability without changing functionality.
services/workflows-service/src/workflow/workflow-log.controller.ts (4)
44-48: Well-structured controller with dependency injectionThe controller follows NestJS best practices with proper class decoration and dependency injection.
49-74: Well-implemented endpoint with proper paginationThe
getWorkflowLogsendpoint is well designed:
- Uses ValidationPipe for query parameter transformation and validation
- Returns paginated data with metadata
- Properly typed with Swagger documentation
Good job on implementing pagination and documenting the API endpoint with Swagger.
76-86: Simple and effective endpoint for log typesThe
getLogTypesendpoint provides a clean way to retrieve all available log types.
88-103: Well-implemented summary endpoint with UUID validationThe
getWorkflowLogSummaryendpoint properly validates the workflowId parameter using ParseUUIDPipe, which is a good security practice.services/workflows-service/prisma/migrations/20250308172521_workflow_logs/migration.sql (2)
1-3: Well-defined enum type for log categorizationThe
WorkflowLogTypeenum covers various log types needed for comprehensive workflow monitoring.
21-31: Good index optimization for query performanceThe indexes on
workflowRuntimeDataId,type,createdAt, andprojectIdwill help optimize the most common query patterns. This is excellent database design practice.apps/workflows-dashboard/src/domains/workflows/api/workflow-logs/workflow-logs.api.ts (3)
3-15: Comprehensive log interface definitionThe
WorkflowLoginterface properly defines all fields needed for log display and analysis. The typing is appropriate for each field, including nullability where needed.
17-24: Well-structured response interface with pagination supportThe
GetWorkflowLogsResponseinterface properly includes both data and metadata for pagination, following API best practices.
53-67: Clear summary interface and fetch functionThe summary interface and fetch function are well designed and documented. The interface properly defines the expected shape of summary data, and the function clearly communicates its purpose and parameters.
services/workflows-service/src/workflow/workflow-log.repository.ts (1)
1-130: Overall well-structured repository with clear separation of concernsThe repository implementation is clean with well-organized methods that have clear responsibilities. The pagination, filtering, and ordering logic is handled well, and the convenience methods make the API more user-friendly. The use of Promise.all for concurrent database operations is a good practice for performance.
🧰 Tools
🪛 Biome (1.9.4)
[error] 118-118: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
apps/kyb-app/src/common/components/atoms/StepperProgress/StepperProgress.tsx
Show resolved
Hide resolved
| ); | ||
|
|
||
| stateApi.setContext(filledPayload); | ||
| console.log('Context updated successfully'); |
There was a problem hiding this comment.
remove all console.logs
| console.log('Filling placeholders...'); | ||
|
|
||
| const filledPayload = { ...stateApi.getContext() }; | ||
| console.log('Current context:', filledPayload); |
There was a problem hiding this comment.
remove all console.logs
- Introduce new WorkflowLog model and associated types - Implement WorkflowLog repository, service, and controller - Modify environment configuration to include workflow logging settings
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/kyb-app/src/pages/CollectionFlow/versions/v1/CollectionFlowV1.tsx (1)
307-449: 🛠️ Refactor suggestionRemove console logs and consider code reuse with V2 implementation.
The newly added "Fill Placeholders" button and its implementation looks good functionally, but has a few issues:
- There are multiple
console.logstatements that should be removed (lines 310, 313, 396, 435, 441).- This code is duplicated almost identically in the V2 implementation.
Consider:
- Removing all console logs
- Extracting this functionality into a shared utility function used by both V1 and V2
- onClick={() => { - try { - console.log('Filling placeholders...'); - - const filledPayload = { ...stateApi.getContext() }; - console.log('Current context:', filledPayload); - - const allElements: Array<{ - valueDestination?: string; - placeholder?: string; - }> = []; - - const findElementsWithPlaceholders = ( - elements: Array<any>, - ) => { - if (!elements || !Array.isArray(elements)) return; - - elements.forEach((element: any) => { - const isHidden = - element?.hidden === true || - element?.options?.hidden === true || - element?.visibleOn === false; - - let isVisible = true; - if ( - element?.visibleOn && - Array.isArray(element.visibleOn) - ) { - isVisible = false; - } - - if ( - !isHidden && - isVisible && - element?.valueDestination - ) { - const placeholder = - element?.options?.uiSchema?.[ - 'ui:placeholder' - ] || element?.options?.hint; - - if (placeholder) { - allElements.push({ - valueDestination: element.valueDestination, - placeholder, - }); - } - } - - const hasVisibilityConditions = - element?.visibleOn && - Array.isArray(element.visibleOn); - - if ( - element?.type === 'json-form' && - hasVisibilityConditions - ) { - const visibilityRules = element.visibleOn; - return; - } - - if ( - element?.elements && - Array.isArray(element.elements) - ) { - findElementsWithPlaceholders(element.elements); - } - - if ( - element?.schema && - Array.isArray(element.schema) - ) { - findElementsWithPlaceholders(element.schema); - } - - if ( - element?.children && - Array.isArray(element.children) - ) { - findElementsWithPlaceholders(element.children); - } - }); - }; - - if (currentPage?.elements) { - findElementsWithPlaceholders(currentPage.elements); - } - - console.log( - 'Found visible elements with placeholders:', - allElements, - ); - - allElements.forEach( - ({ valueDestination, placeholder }) => { - if (!valueDestination || !placeholder) return; - - const path = valueDestination.split('.'); - - let current: any = filledPayload; - - for (let i = 0; i < path.length - 1; i++) { - const key = path[i]; - if (!key) continue; - - if (!current[key]) { - current[key] = {}; - } - current = current[key]; - } - - const lastKey = path[path.length - 1]; - if (lastKey) { - if ( - lastKey.toLowerCase().includes('date') || - valueDestination - .toLowerCase() - .includes('date') - ) { - current[lastKey] = '11/11/1990'; - } else { - current[lastKey] = placeholder; - } - } - }, - ); - - console.log( - 'Updated payload with visible field placeholders:', - filledPayload, - ); - - stateApi.setContext(filledPayload); - console.log('Context updated successfully'); - } catch (error) { - console.error('Error filling placeholders:', error); - } - }} + onClick={() => fillPlaceholders(currentPage, stateApi)}Then create a shared utility function:
// In a shared utility file export const fillPlaceholders = (currentPage: any, stateApi: any) => { try { const filledPayload = { ...stateApi.getContext() }; const allElements: Array<{ valueDestination?: string; placeholder?: string; }> = []; const findElementsWithPlaceholders = (elements: Array<any>) => { // Implementation remains the same but without console.logs // ... }; if (currentPage?.elements) { findElementsWithPlaceholders(currentPage.elements); } allElements.forEach(({ valueDestination, placeholder }) => { // Implementation remains the same but without console.logs // ... }); stateApi.setContext(filledPayload); } catch (error) { // Consider better error handling } };apps/kyb-app/src/pages/CollectionFlow/versions/v2/CollectionFlowV2.tsx (1)
255-403: 🛠️ Refactor suggestionRemove console logs and extract to shared utility
This implementation is almost identical to the one in CollectionFlowV1, with just a minor difference in date handling. The same issues apply:
- Multiple console.log statements should be removed
- The code is duplicated from V1
Extract this functionality to a shared utility function as suggested in the V1 file review. The shared function could accept a parameter to determine if birth-related fields should be handled specially (which is the only difference in this implementation).
🧹 Nitpick comments (9)
apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogGraph.tsx (2)
22-44: Consider improving type safety for custom node components.The data prop is typed as
anywhich reduces type safety. Consider creating specific interfaces for each node type's data.-const CustomStateNode = ({ data }: { data: any }) => ( +interface StateNodeData { + label: string; +} +const CustomStateNode = ({ data }: { data: StateNodeData }) => ( -const CustomEventNode = ({ data }: { data: any }) => ( +interface EventNodeData { + label: string; +} +const CustomEventNode = ({ data }: { data: EventNodeData }) => ( -const CustomPluginNode = ({ data }: { data: any }) => ( +interface PluginNodeData { + label: string; +} +const CustomPluginNode = ({ data }: { data: PluginNodeData }) => (
75-348: Consider breaking down the useEffect hook to improve maintainability.The useEffect hook contains a lot of complex logic for creating nodes and edges, positioning them, and connecting them. Consider extracting this logic into smaller, focused helper functions to improve readability and maintainability.
For example, you could create separate functions for:
- Creating state nodes
- Creating event nodes
- Creating plugin nodes
- Establishing connections between nodes
- Positioning nodes in the layout
This would make the component more maintainable and easier to test.
apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogsModal.tsx (3)
112-119: Use optional chaining for null checks.The current code uses logical AND checks for potentially null or undefined values. Using optional chaining would improve readability and safety.
- (log.eventName && log.eventName.toLowerCase().includes(searchLower)) || - (log.pluginName && log.pluginName.toLowerCase().includes(searchLower)) || - (log.fromState && log.fromState.toLowerCase().includes(searchLower)) || - (log.toState && log.toState.toLowerCase().includes(searchLower)) || + log.eventName?.toLowerCase().includes(searchLower) || + log.pluginName?.toLowerCase().includes(searchLower) || + log.fromState?.toLowerCase().includes(searchLower) || + log.toState?.toLowerCase().includes(searchLower) ||🧰 Tools
🪛 Biome (1.9.4)
[error] 114-114: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 115-115: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 116-116: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 117-117: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
106-133: Consider using useMemo for filtered and sorted logs.The component filters and sorts logs on every render, which could impact performance with large datasets. Consider memoizing the filtered and sorted logs using useMemo to prevent unnecessary recalculations.
- const filteredAndSortedLogs = logs + const filteredAndSortedLogs = useMemo(() => logs .filter(log => { // Filtering logic... }) .sort((a, b) => { // Sorting logic... }); + , [logs, searchQuery, sortField, sortDirection]);🧰 Tools
🪛 Biome (1.9.4)
[error] 114-114: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 115-115: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 116-116: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 117-117: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
312-329: Consider improving metadata popup UX.The metadata popup appears on hover, which can be difficult for users to interact with, especially on touch devices. Consider implementing a click-to-open and click-to-close pattern for better usability.
- <div className="group relative"> - <Info className="h-4 w-4 cursor-pointer text-gray-400 group-hover:text-blue-500" /> - <div className="absolute right-0 z-30 mt-1 hidden w-80 rounded-md border bg-white p-2 shadow-lg group-hover:block"> + <div className="relative"> + <Info + className="h-4 w-4 cursor-pointer text-gray-400 hover:text-blue-500" + onClick={() => setActiveMetadata(log.id === activeMetadata ? null : log.id)} + /> + {activeMetadata === log.id && ( + <div className="absolute right-0 z-30 mt-1 w-80 rounded-md border bg-white p-2 shadow-lg">This would require adding a new state variable:
const [activeMetadata, setActiveMetadata] = useState<number | null>(null);apps/kyb-app/src/pages/CollectionFlow/versions/v1/CollectionFlowV1.tsx (2)
367-367: Unused variablevisibilityRulesThe variable
visibilityRulesis assigned but never used.- const visibilityRules = element.visibleOn; - return; + return;
427-428: Consider using a configurable default dateThe hardcoded date
'11/11/1990'should be defined as a constant at the top of the file or in a configuration file for easier maintenance.+// At the top of the file +const DEFAULT_DATE_VALUE = '11/11/1990'; // Then in the code - current[lastKey] = '11/11/1990'; + current[lastKey] = DEFAULT_DATE_VALUE;apps/kyb-app/src/pages/CollectionFlow/versions/v2/CollectionFlowV2.tsx (2)
314-314: Unused variablevisibilityRulesThe variable
visibilityRulesis assigned but never used.- const visibilityRules = element.visibleOn; - return; + return;
371-382: Consider more robust date detectionThe date detection logic is more comprehensive here than in V1, checking for both 'date' and 'birth' in keys. This inconsistency should be resolved by using a shared implementation. Additionally, consider using a more robust way to identify date fields rather than string matching.
// In a shared utility const isDateField = (key: string): boolean => { const dateKeywords = ['date', 'birth', 'dob']; return dateKeywords.some(keyword => key.toLowerCase().includes(keyword)); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/backoffice-v2/src/common/utils/fetcher/fetcher.ts(2 hunks)apps/backoffice-v2/src/domains/auth/components/AuthenticatedLayout/components/NavIntroduction.tsx(1 hunks)apps/backoffice-v2/src/domains/auth/components/AuthenticatedLayout/components/NavItem.tsx(1 hunks)apps/backoffice-v2/src/lib/blocks/hooks/useBankAccountVerificationBlock/useBankAccountVerificationBlock.tsx(2 hunks)apps/kyb-app/src/common/components/atoms/StepperProgress/StepperProgress.tsx(1 hunks)apps/kyb-app/src/pages/CollectionFlow/versions/v1/CollectionFlowV1.tsx(1 hunks)apps/kyb-app/src/pages/CollectionFlow/versions/v2/CollectionFlowV2.tsx(1 hunks)apps/workflows-dashboard/package.json(1 hunks)apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogGraph.tsx(1 hunks)apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogsModal.tsx(1 hunks)apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/index.ts(1 hunks)apps/workflows-dashboard/src/components/molecules/WorkflowsTable/columns.tsx(2 hunks)apps/workflows-dashboard/src/components/molecules/WorkflowsTable/types.ts(1 hunks)apps/workflows-dashboard/src/domains/workflows/api/workflow-logs/index.ts(1 hunks)apps/workflows-dashboard/src/domains/workflows/api/workflow-logs/workflow-logs.api.ts(1 hunks)packages/common/src/schemas/documents/default-context-schema.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/workflows-dashboard/src/domains/workflows/api/workflow-logs/index.ts
- apps/kyb-app/src/common/components/atoms/StepperProgress/StepperProgress.tsx
- apps/workflows-dashboard/src/components/molecules/WorkflowsTable/types.ts
- packages/common/src/schemas/documents/default-context-schema.ts
- apps/backoffice-v2/src/domains/auth/components/AuthenticatedLayout/components/NavItem.tsx
- apps/workflows-dashboard/package.json
- apps/backoffice-v2/src/domains/auth/components/AuthenticatedLayout/components/NavIntroduction.tsx
- apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/index.ts
- apps/backoffice-v2/src/common/utils/fetcher/fetcher.ts
- apps/workflows-dashboard/src/domains/workflows/api/workflow-logs/workflow-logs.api.ts
- apps/workflows-dashboard/src/components/molecules/WorkflowsTable/columns.tsx
- apps/backoffice-v2/src/lib/blocks/hooks/useBankAccountVerificationBlock/useBankAccountVerificationBlock.tsx
🧰 Additional context used
🧬 Code Definitions (1)
apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogsModal.tsx (2)
apps/workflows-dashboard/src/domains/workflows/api/workflow-logs/workflow-logs.api.ts (2)
WorkflowLog(3-15)fetchWorkflowLogs(37-51)apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogGraph.tsx (1)
WorkflowLogGraph(70-388)
🪛 Biome (1.9.4)
apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogsModal.tsx
[error] 114-114: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 115-115: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 116-116: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 117-117: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
- GitHub Check: Analyze (javascript)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: spell_check
- GitHub Check: format
- GitHub Check: lint
🔇 Additional comments (15)
apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogGraph.tsx (4)
1-19: Well-structured imports and interface definition.The component properly imports all necessary dependencies from reactflow and React, and clearly defines the WorkflowLogGraphProps interface.
57-68: Well-implemented helper function for finding triggering events.The
findTriggeringEventfunction effectively searches for the most recent event before a state transition, which is crucial for establishing correct connections in the workflow graph.
361-363: Good error handling for empty logs.The component properly handles the case when no logs are provided by displaying a user-friendly message.
365-387: Well-configured ReactFlow component.The ReactFlow component is well-configured with appropriate options for zooming, panning, and fitting the view. The addition of controls and a background enhances the user experience.
apps/workflows-dashboard/src/components/molecules/WorkflowLogsModal/WorkflowLogsModal.tsx (6)
17-21: Well-defined props interface.The component clearly defines its required props with appropriate types.
28-41: Effective text highlighting implementation.The
highlightTextfunction provides a clean way to highlight search matches, enhancing the user experience when searching through logs.
54-69: Good error handling in fetchLogs function.The fetchLogs function properly handles errors and loading states, providing clear feedback to users when logs cannot be fetched.
233-335: Well-structured logs table with comprehensive information.The logs table effectively presents all relevant information with clear visual cues for different log types. The inclusion of state transitions, event details, and plugin information provides a complete picture of the workflow.
180-190: Good implementation of Visualization tab using WorkflowLogGraph.The visualization tab properly integrates the WorkflowLogGraph component, providing a useful graphical representation of workflow logs.
344-357: Effective pagination implementation.The load more button is well-implemented, showing a loading indicator during data fetching and being conditionally displayed based on whether more data is available.
apps/kyb-app/src/pages/CollectionFlow/versions/v1/CollectionFlowV1.tsx (5)
310-310: Remove console.logRemove this console log statement.
313-313: Remove console.logRemove this console log statement.
396-399: Remove console.logRemove this console log statement.
435-438: Remove console.logRemove this console log statement.
441-441: Remove console.logRemove this console log statement.
- Import WorkflowLogService in integration test files for consistency - Include WorkflowLogService in the providers for dependency injection in tests - Ensure WorkflowLogService is referenced in all relevant test setups
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
39-40: CI Build Matrix Simplification: Ubuntu OnlyThe build job matrix now includes only
ubuntu-latest, which streamlines the build environment. Please ensure that any platform-specific behavior or dependencies that previously required testing on Windows are adequately covered by the dedicatedtest_windowsjob.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/workflows/ci.yml(1 hunks)services/workflows-service/src/alert/alert.service.intg.test.ts(2 hunks)services/workflows-service/src/collection-flow/collection-flow.service.intg.test.ts(2 hunks)services/workflows-service/src/collection-flow/controllers/collection-flow.no-user.controller.intg.test.ts(2 hunks)services/workflows-service/src/customer/customer.controller.external.intg.test.ts(2 hunks)services/workflows-service/src/end-user/end-user.controller.external.intg.test.ts(2 hunks)services/workflows-service/src/workflow/workflow-runtime-data.repository.intg.test.ts(2 hunks)services/workflows-service/src/workflow/workflow.controller.external.intg.test.ts(2 hunks)services/workflows-service/src/workflow/workflow.controller.internal.intg.test.ts(2 hunks)services/workflows-service/src/workflow/workflow.service.intg.test.ts(2 hunks)services/workflows-service/src/workflow/workflow.service.unit.test.ts(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (23)
services/workflows-service/src/workflow/workflow.controller.internal.intg.test.ts (2)
44-44: Good addition of WorkflowLogService import.Adding the WorkflowLogService import is necessary for the workflow observability enhancement mentioned in the PR title.
95-95: Well-implemented service provider addition.Adding WorkflowLogService to the servicesProviders array ensures proper dependency injection for tests, maintaining consistency with the production code that now depends on workflow logging capabilities.
services/workflows-service/src/workflow/workflow.controller.external.intg.test.ts (2)
51-51: Good addition of WorkflowLogService import.Adding the WorkflowLogService import is necessary for the workflow observability enhancement mentioned in the PR title.
107-107: Well-implemented service provider addition.Adding WorkflowLogService to the servicesProviders array ensures proper dependency injection for tests, maintaining consistency with the production code that now depends on workflow logging capabilities.
services/workflows-service/src/workflow/workflow-runtime-data.repository.intg.test.ts (2)
41-41: Good addition of WorkflowLogService import.Adding the WorkflowLogService import is necessary for the workflow observability enhancement mentioned in the PR title.
85-85: Well-implemented service provider addition.Adding WorkflowLogService to the servicesProviders array ensures proper dependency injection for tests, maintaining consistency with the production code that now depends on workflow logging capabilities.
services/workflows-service/src/collection-flow/collection-flow.service.intg.test.ts (2)
42-42: Good addition of WorkflowLogService import.Adding the WorkflowLogService import is necessary for the workflow observability enhancement mentioned in the PR title.
156-156: Well-implemented service provider addition.Adding WorkflowLogService to the providers array ensures proper dependency injection for tests, maintaining consistency with the production code that now depends on workflow logging capabilities.
services/workflows-service/src/customer/customer.controller.external.intg.test.ts (2)
40-40: Integration of WorkflowLogService into test setupThe WorkflowLogService import has been added to support workflow logging capabilities in the test environment.
84-84: Properly registered WorkflowLogService in the test providersThe WorkflowLogService is correctly added to the servicesProviders array, ensuring it's available for dependency injection in the test suite.
services/workflows-service/src/collection-flow/controllers/collection-flow.no-user.controller.intg.test.ts (2)
44-44: WorkflowLogService import addedThe import of WorkflowLogService has been added to support enhanced workflow logging capabilities in this test suite.
96-96: WorkflowLogService registered in testing module providersThe WorkflowLogService has been correctly included in the providers array for the TestingModule, making it available for dependency injection in the test context.
services/workflows-service/src/end-user/end-user.controller.external.intg.test.ts (2)
45-45: Added WorkflowLogService importThe WorkflowLogService import has been added to support workflow logging capabilities in the test environment.
94-94: Registered WorkflowLogService in test providersThe WorkflowLogService has been correctly added to the servicesProviders array, ensuring it's available for dependency injection throughout the test suite.
services/workflows-service/src/workflow/workflow.service.intg.test.ts (2)
42-42: Added WorkflowLogService import for workflow loggingThe WorkflowLogService import has been added to support enhanced observability in workflow tests.
88-88: Registered WorkflowLogService in test providersThe WorkflowLogService is correctly added to the servicesProviders array, making it available for dependency injection in the workflow service tests.
services/workflows-service/src/alert/alert.service.intg.test.ts (2)
41-41: Import of WorkflowLogService is properly added.The import statement for the WorkflowLogService is correctly added, following the project's import style conventions.
109-109: WorkflowLogService added to test providers.The WorkflowLogService has been appropriately added to the providers array in the test module setup, ensuring it's available for dependency injection during testing.
services/workflows-service/src/workflow/workflow.service.unit.test.ts (5)
8-8: Import of WorkflowLogService is properly added.The import statement for the WorkflowLogService is correctly added, following the project's import style conventions.
60-64: Mock implementation of WorkflowLogService follows existing patterns.The implementation of FakeWorkflowLogService correctly follows the established pattern in this file for creating mock services by extending BaseFakeRepository.
124-124: Variable declaration for workflowLogService follows existing patterns.The declaration of the workflowLogService variable is consistent with other service variables in this file.
157-157: Initialization of workflowLogService follows existing patterns.The workflowLogService is properly initialized with a new instance of FakeWorkflowLogService in the beforeEach block, consistent with how other service mocks are created.
221-221: WorkflowLogService injected into WorkflowService constructor.The workflowLogService is correctly passed as a parameter to the WorkflowService constructor, ensuring proper dependency injection for testing.
- Add waitFor to ensure state updates are applied before assertion - Improve readability and reliability of the test by using waitFor
- Remove debug console logs related to filling placeholders - Enhance readability by eliminating commented-out code
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/kyb-app/src/pages/CollectionFlow/versions/v1/CollectionFlowV1.tsx (1)
307-435:⚠️ Potential issueRemove console.log and address unused variables.
While the "Fill Placeholders" feature is useful for development, there are a few issues to address:
- Remove the
console.errorstatement on line 429- The
visibilityRulesvariable on line 362 is declared but never usedApply these changes:
- console.error('Error filling placeholders:', error); + // Handle error silently in development modeAlso, either use or remove the unused variable:
- const visibilityRules = element.visibleOn; - return; + // Skip elements with visibility conditions + return;apps/kyb-app/src/pages/CollectionFlow/versions/v2/CollectionFlowV2.tsx (1)
255-389:⚠️ Potential issueRemove console.log and address unused variables.
Similar to the V1 component, there are issues to address:
- Remove the
console.errorstatement on line 383- The
visibilityRulesvariable on line 310 is declared but never usedApply these changes:
- console.error('Error filling placeholders:', error); + // Handle error silently in development modeAlso, either use or remove the unused variable:
- const visibilityRules = element.visibleOn; - return; + // Skip elements with visibility conditions + return;
🧹 Nitpick comments (4)
apps/kyb-app/src/pages/CollectionFlow/versions/v1/CollectionFlowV1.tsx (2)
307-435: Consider extracting the placeholder filling logic into a reusable utility.The placeholder filling functionality is duplicated between
CollectionFlowV1andCollectionFlowV2.Consider extracting this logic into a shared utility function to improve maintainability and reduce duplication:
+ // In a shared utils file (e.g., apps/kyb-app/src/common/utils/fillPlaceholders.ts) + export const fillElementPlaceholders = (currentPage: any, stateApi: any) => { + try { + const filledPayload = { ...stateApi.getContext() }; + // ... rest of the placeholder filling logic + stateApi.setContext(filledPayload); + } catch (error) { + // Handle error appropriately + } + };Then in both components:
- onClick={() => { - try { - // ... all the placeholder filling logic - } catch (error) { - console.error('Error filling placeholders:', error); - } - }} + onClick={() => fillElementPlaceholders(currentPage, stateApi)}
317-387: Optimize the recursive element search function.The recursive
findElementsWithPlaceholdersfunction has some potential optimizations and clarity improvements.Consider these improvements:
- Early return for visibility conditions instead of nested if statements
- More efficient element traversal
- Better variable naming for clarity
const findElementsWithPlaceholders = (elements: Array<any>) => { if (!elements || !Array.isArray(elements)) return; elements.forEach((element: any) => { // Early return for hidden elements or visibility conditions const isHidden = element?.hidden === true || element?.options?.hidden === true || element?.visibleOn === false; const hasVisibilityConditions = element?.visibleOn && Array.isArray(element.visibleOn); if (isHidden || (hasVisibilityConditions && element?.type === 'json-form')) { return; } + // Check for placeholder and add to collection if (element?.valueDestination) { const placeholder = element?.options?.uiSchema?.['ui:placeholder'] || element?.options?.hint; if (placeholder) { allElements.push({ valueDestination: element.valueDestination, placeholder, }); } } + // Recursively process nested elements ['elements', 'schema', 'children'].forEach(childKey => { if (element?.[childKey] && Array.isArray(element[childKey])) { findElementsWithPlaceholders(element[childKey]); } }); }); };apps/kyb-app/src/pages/CollectionFlow/versions/v2/CollectionFlowV2.tsx (2)
362-377: Improve date/birth field detection logic.The current implementation has repetitive string checking logic for determining date and birth fields.
Consider refactoring this into a cleaner implementation:
- if ( - lastKey.toLowerCase().includes('date') || - lastKey.toLowerCase().includes('birth') || - valueDestination.toLowerCase().includes('date') || - valueDestination.toLowerCase().includes('birth') - ) { - value = '11/11/1990'; - } + const isDateOrBirthField = (fieldName: string): boolean => { + const lowerField = fieldName.toLowerCase(); + return lowerField.includes('date') || lowerField.includes('birth'); + }; + + if (isDateOrBirthField(lastKey) || isDateOrBirthField(valueDestination)) { + value = '11/11/1990'; + }This makes the code more maintainable and easier to extend with additional date field patterns if needed.
255-389: Consider adding a notification for successful placeholder filling.The current implementation silently fills placeholders without providing feedback to the developer.
Since this is a developer tool, consider adding a visual notification to confirm that placeholders were successfully filled:
try { // ... existing code for filling placeholders stateApi.setContext(filledPayload); + // Add a temporary success notification if in dev mode + const debugContainer = document.querySelector('.bg-amber-50'); + if (debugContainer) { + const notification = document.createElement('div'); + notification.className = 'mt-3 text-sm text-green-700 bg-green-50 p-2 rounded'; + notification.textContent = `Placeholders filled for ${allElements.length} fields`; + debugContainer.appendChild(notification); + setTimeout(() => notification.remove(), 3000); + } } catch (error) { // ... error handling }This provides valuable feedback to developers using this feature.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/kyb-app/src/pages/CollectionFlow/versions/v1/CollectionFlowV1.tsx(1 hunks)apps/kyb-app/src/pages/CollectionFlow/versions/v2/CollectionFlowV2.tsx(1 hunks)packages/ui/src/components/organisms/Form/hooks/useRuleEngine/useRuleEngine.unit.test.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test_windows
- GitHub Check: Analyze (javascript)
- GitHub Check: test_linux
- GitHub Check: build (ubuntu-latest)
- GitHub Check: spell_check
- GitHub Check: format
- GitHub Check: lint
🔇 Additional comments (2)
packages/ui/src/components/organisms/Form/hooks/useRuleEngine/useRuleEngine.unit.test.ts (2)
2-2: Added waitFor for more reliable asynchronous testing.Good addition of the
waitForutility from@testing-library/react, which will help ensure that state updates are properly applied before assertions are made.
109-114: Great improvement to test reliability.The changes make the test more accurate and reliable by:
- Using the
customDelayvariable instead of a hardcoded delay value- Adding
waitForto ensure state updates are fully applied before making assertionsThis approach better handles the asynchronous nature of the component's state updates.
Summary by CodeRabbit
New Features
WorkflowLogControllerto manage workflow log operations, including fetching logs and log types.Style