Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bal 3035 #2986

Merged
merged 9 commits into from
Jan 23, 2025
Merged

Bal 3035 #2986

merged 9 commits into from
Jan 23, 2025

Conversation

chesterkmr
Copy link
Collaborator

@chesterkmr chesterkmr commented Jan 22, 2025

  • WorkflowRunner now adds pluginsInput to context
  • ApiPlugin & MerchantScreening plugin updated to return requestPayload
  • invokedAt transformer now can be disabled via includeInvokedAt parameter.
  • Added Checked Properties button to Mathc Results which renders request payload of Merchant Screening plugin.
  • Renamed Full Json Data to Results according to design

Back Office - Match Resutls
image

Back Office Checked Properties Dialog content
PR image

Summary by CodeRabbit

Based on the comprehensive summary, here are the updated release notes:

  • Dependency Updates

    • Updated @ballerine/common to version 0.9.70
    • Updated @ballerine/workflow-browser-sdk to version 0.6.89
    • Updated @ballerine/workflow-node-sdk to version 0.6.89
    • Updated @ballerine/workflow-core to version 0.6.89
  • New Features

    • Added pluginsInput support for plugins
    • Introduced option to disable invokedAt functionality
    • Enhanced handling of merchantScreeningInput
    • Added directorId support in relevant components and interfaces
  • Improvements

    • More robust error handling in API plugins
    • Better context tracking for plugin invocations
    • Improved request payload management
    • Enhanced test coverage for core functionalities

Copy link

changeset-bot bot commented Jan 22, 2025

⚠️ No Changeset found

Latest commit: 76d0201

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

coderabbitai bot commented Jan 22, 2025

Walkthrough

This pull request introduces multiple updates across various packages in the Ballerine ecosystem, focusing on version increments and dependency updates. Key changes include the addition of new version entries in the changelogs for @ballerine/workflow-browser-sdk, @ballerine/workflow-node-sdk, and other related packages, reflecting updates to their dependencies. The @ballerine/workflow-core package has been updated to version 0.6.89, and new properties and methods have been introduced in several plugins to enhance functionality.

Changes

File Change Summary
apps/backoffice-v2/CHANGELOG.md Added version entries for 0.7.96 and 0.7.97, updated dependencies to 0.6.89
apps/backoffice-v2/package.json Updated version to 0.7.97, dependencies to 0.6.89
apps/kyb-app/CHANGELOG.md Added version entry 0.3.116, updated dependency to 0.6.89
apps/kyb-app/package.json Updated version to 0.3.116, dependency to 0.6.89
examples/headless-example/CHANGELOG.md Added version entry 0.3.88, updated dependency to 0.6.89
examples/headless-example/package.json Updated version to 0.3.88, dependency to 0.6.89
packages/workflow-core/CHANGELOG.md Added version entries for 0.6.88 and 0.6.89, updated dependencies
packages/workflow-core/package.json Updated version to 0.6.89
packages/workflow-core/src/lib/plugins/external-plugin/api-plugin/api-plugin.ts Added properties and methods for request payload handling
packages/workflow-core/src/lib/plugins/external-plugin/types.ts Added new optional properties to IApiPluginParams interface
sdks/workflow-browser-sdk/CHANGELOG.md Added version 0.6.89, updated @ballerine/workflow-core dependency
sdks/workflow-browser-sdk/package.json Updated version to 0.6.89, dependency to 0.6.89
sdks/workflow-node-sdk/CHANGELOG.md Added version 0.6.89, updated @ballerine/workflow-core dependency
sdks/workflow-node-sdk/package.json Updated version to 0.6.89, dependency to 0.6.89
services/workflows-service/CHANGELOG.md Added version 0.7.93, updated dependencies to 0.6.89
services/workflows-service/package.json Updated version to 0.7.93, dependencies to 0.6.89
services/workflows-service/prisma/data-migrations Updated subproject commit hash
apps/backoffice-v2/src/lib/blocks/components/CallToActionLegacy/CallToActionLegacy.tsx Added directorId property to component's props
apps/backoffice-v2/src/lib/blocks/components/CallToActionLegacy/interfaces.ts Updated interface to include directorId
apps/backoffice-v2/src/lib/blocks/components/DirectorBlock/hooks/useDirectorBlock/useDirectorBlock.tsx Updated getReUploadedNeededAction function to include directorId

Possibly related PRs

  • Common version release #2447: This PR updates the dependencies @ballerine/common, @ballerine/workflow-browser-sdk, and @ballerine/workflow-node-sdk, which are also updated in the main PR.
  • wf service version bump #2710: This PR updates the dependencies @ballerine/common, @ballerine/workflow-core, and @ballerine/workflow-node-sdk, which are also updated in the main PR.
  • chore: version bump #2690: This PR updates the dependencies @ballerine/common, @ballerine/workflow-browser-sdk, and @ballerine/workflow-node-sdk, which are similarly updated in the main PR.

Suggested Reviewers

  • alonp99

Poem

🐰 Hoppity hop, the versions are new!
Dependencies dancing, all shiny and true.
From core to browser, we leap and we bound,
With plugins and features, new joy to be found!
Ballerine's magic, in code we delight,
Updating our world, everything feels right! 🚀

✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (5)
packages/workflow-core/src/lib/plugins/external-plugin/types.ts (1)

30-31: LGTM! Consider adding JSDoc comments.

The new optional properties are well-typed and align with the PR objectives. Consider adding JSDoc comments to document their purpose and usage.

 export interface IApiPluginParams {
+  /** List of property names that are allowed in the plugin input */
   whitelistedInputProperties?: string[];
+  /** When true, includes the invocation timestamp in the plugin response */
   includeInvokedAt?: boolean;
apps/backoffice-v2/src/lib/blocks/hooks/useMerchantScreeningBlock/columns.tsx (1)

126-145: Consider extracting common JsonDialog rendering logic.

There's duplicate code for rendering JsonDialog in both columns. Consider extracting this into a reusable component.

+const JsonDataCell = ({ data }: { data: unknown }) => (
+  <div className={`flex items-end justify-start`}>
+    <JsonDialog
+      buttonProps={{
+        variant: 'link',
+        className: 'p-0 text-blue-500',
+        disabled: !isObject(data) && !Array.isArray(data),
+      }}
+      dialogButtonText={`View`}
+      json={JSON.stringify(data)}
+    />
+  </div>
+);

 summaryColumnHelper.accessor('merchantScreeningInput', {
   header: 'Checked Properties',
   cell: info => {
-    const fullJsonData = info.getValue();
-    return (
-      <div className={`flex items-end justify-start`}>
-        <JsonDialog
-          buttonProps={{
-            variant: 'link',
-            className: 'p-0 text-blue-500',
-            disabled: !isObject(fullJsonData) && !Array.isArray(fullJsonData),
-          }}
-          dialogButtonText={`View`}
-          json={JSON.stringify(fullJsonData)}
-        />
-      </div>
-    );
+    return <JsonDataCell data={info.getValue()} />;
   },
 }),

Also applies to: 147-163

packages/workflow-core/src/lib/plugins/external-plugin/api-plugin/api-plugin.ts (1)

437-454: Consider optimizing the whitelist payload generation.

The current implementation has a potential performance optimization opportunity:

  1. The recursive check for nested objects could be more efficient
  2. The empty value check at line 446 might skip valid falsy values

Consider this optimization:

  generateRequestPayloadFromWhitelist(payload: AnyRecord = {}) {
    if (!this.whitelistedInputProperties) return payload;

    const whitelistedPayload: AnyRecord = {};

    for (const key of this.whitelistedInputProperties) {
      const value = payload[key];
-     whitelistedPayload[key] = value;
-
-     if (value) continue;
-
      if (typeof value === 'object' && !Array.isArray(value)) {
        whitelistedPayload[key] = this.generateRequestPayloadFromWhitelist(value as AnyRecord);
+     } else {
+       whitelistedPayload[key] = value;
      }
    }

    return whitelistedPayload;
  }
packages/workflow-core/src/lib/workflow-runner.ts (1)

808-812: Consider consolidating duplicate context update logic.

The same pattern for updating the context with plugin input is repeated in three places. This could be refactored into a helper method to improve maintainability and reduce code duplication.

Consider creating a helper method:

private updatePluginInputContext(
  pluginName: string,
  requestPayload: unknown,
  status: ProcessStatus,
  error?: unknown,
) {
  return this.mergeToContext(
    this.context,
    { requestPayload, ...(error ? { error } : {}), status },
    `pluginsInput.${pluginName}`,
  );
}

Then use it in place of the repeated blocks:

this.context = this.updatePluginInputContext(
  apiPlugin.name,
  requestPayload,
  error ? ProcessStatus.ERROR : ProcessStatus.SUCCESS,
  error,
);

Also applies to: 822-826, 836-840

packages/workflow-core/CHANGELOG.md (1)

7-7: Minor grammar improvement in changelog.

When using "per", the following noun is typically singular.

-Added pluginsInput with requestPayload per plugins & invokedAt now disablable
+Added pluginsInput with requestPayload per plugin & invokedAt now disablable
🧰 Tools
🪛 LanguageTool

[uncategorized] ~7-~7: Did you mean “plugin”? If following ‘per’, nouns are often singular.
Context: ...ed pluginsInput with requestPayload per plugins & invokedAt now disablable ## 0.6.87 ...

(CONFUSION_OF_NNS_NN_UN)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b220184 and 9cf9d7c.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (29)
  • apps/backoffice-v2/CHANGELOG.md (1 hunks)
  • apps/backoffice-v2/package.json (2 hunks)
  • apps/backoffice-v2/src/domains/workflows/fetchers.ts (1 hunks)
  • apps/backoffice-v2/src/lib/blocks/hooks/useMerchantScreeningBlock/columns.tsx (3 hunks)
  • apps/backoffice-v2/src/lib/blocks/hooks/useMerchantScreeningBlock/useMerchantScreeningBlock.tsx (2 hunks)
  • apps/backoffice-v2/src/lib/blocks/variants/DefaultBlocks/hooks/useDefaultBlocksLogic/useDefaultBlocksLogic.tsx (4 hunks)
  • apps/kyb-app/CHANGELOG.md (1 hunks)
  • apps/kyb-app/package.json (2 hunks)
  • examples/headless-example/CHANGELOG.md (1 hunks)
  • examples/headless-example/package.json (2 hunks)
  • packages/workflow-core/CHANGELOG.md (1 hunks)
  • packages/workflow-core/package.json (1 hunks)
  • packages/workflow-core/src/lib/plugins/external-plugin/api-plugin/api-plugin-workflow-runner.test.ts (1 hunks)
  • packages/workflow-core/src/lib/plugins/external-plugin/api-plugin/api-plugin.test.ts (1 hunks)
  • packages/workflow-core/src/lib/plugins/external-plugin/api-plugin/api-plugin.ts (9 hunks)
  • packages/workflow-core/src/lib/plugins/external-plugin/api-plugin/index.ts (1 hunks)
  • packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.test.ts (1 hunks)
  • packages/workflow-core/src/lib/plugins/external-plugin/mastercard-merchant-screening-plugin.test.ts (1 hunks)
  • packages/workflow-core/src/lib/plugins/external-plugin/mastercard-merchant-screening-plugin.ts (5 hunks)
  • packages/workflow-core/src/lib/plugins/external-plugin/types.ts (2 hunks)
  • packages/workflow-core/src/lib/plugins/types.ts (1 hunks)
  • packages/workflow-core/src/lib/workflow-runner.ts (5 hunks)
  • sdks/workflow-browser-sdk/CHANGELOG.md (1 hunks)
  • sdks/workflow-browser-sdk/package.json (2 hunks)
  • sdks/workflow-node-sdk/CHANGELOG.md (1 hunks)
  • sdks/workflow-node-sdk/package.json (2 hunks)
  • services/workflows-service/CHANGELOG.md (1 hunks)
  • services/workflows-service/package.json (2 hunks)
  • services/workflows-service/prisma/data-migrations (1 hunks)
✅ Files skipped from review due to trivial changes (15)
  • packages/workflow-core/src/lib/plugins/external-plugin/api-plugin/index.ts
  • packages/workflow-core/package.json
  • services/workflows-service/prisma/data-migrations
  • packages/workflow-core/src/lib/plugins/types.ts
  • examples/headless-example/package.json
  • apps/kyb-app/package.json
  • sdks/workflow-node-sdk/package.json
  • sdks/workflow-browser-sdk/CHANGELOG.md
  • sdks/workflow-node-sdk/CHANGELOG.md
  • services/workflows-service/CHANGELOG.md
  • apps/backoffice-v2/package.json
  • services/workflows-service/package.json
  • examples/headless-example/CHANGELOG.md
  • packages/workflow-core/src/lib/plugins/external-plugin/api-plugin/api-plugin-workflow-runner.test.ts
  • apps/backoffice-v2/CHANGELOG.md
🧰 Additional context used
🪛 LanguageTool
packages/workflow-core/CHANGELOG.md

[uncategorized] ~7-~7: Did you mean “plugin”? If following ‘per’, nouns are often singular.
Context: ...ed pluginsInput with requestPayload per plugins & invokedAt now disablable ## 0.6.87 ...

(CONFUSION_OF_NNS_NN_UN)

🪛 Biome (1.9.4)
packages/workflow-core/src/lib/workflow-runner.ts

[error] 775-775: Unsafe usage of optional chaining.

If it short-circuits with 'undefined' the evaluation will throw TypeError here:

(lint/correctness/noUnsafeOptionalChaining)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (21)
apps/kyb-app/CHANGELOG.md (1)

3-7: LGTM! Changelog entry follows the conventional format.

The new version entry properly documents the dependency update for @ballerine/workflow-browser-sdk.

sdks/workflow-browser-sdk/package.json (2)

4-4: LGTM! Version bump consistency maintained.

The version bump from 0.6.87 to 0.6.88 is consistent between the package version and its @ballerine/workflow-core dependency. This maintains version parity and aligns with the feature additions mentioned in the PR objectives.

Also applies to: 37-37


4-4: Verify changelog entries for version 0.6.88.

Please ensure that the changelog has been updated to document the new features:

  • Addition of pluginsInput in WorkflowRunner context
  • Updates to ApiPlugin and MerchantScreening plugin
  • New includeInvokedAt parameter
packages/workflow-core/src/lib/plugins/external-plugin/mastercard-merchant-screening-plugin.ts (2)

16-16: LGTM! Appropriate whitelisted properties.

The whitelisted properties are correctly defined for merchant screening functionality.


32-35: Consistent error handling with improved debugging context.

The error handling has been enhanced to consistently include the request payload across all error paths, which will help with debugging issues.

Also applies to: 109-112, 116-122, 131-131, 137-140

packages/workflow-core/src/lib/plugins/external-plugin/mastercard-merchant-screening-plugin.test.ts (1)

5-157: LGTM! Comprehensive test coverage.

The test suite thoroughly covers:

  • Plugin initialization with default properties
  • Request payload inclusion in successful responses
  • Request payload inclusion in failed responses
  • Whitelisted property filtering
apps/backoffice-v2/src/lib/blocks/hooks/useMerchantScreeningBlock/columns.tsx (1)

126-145: LGTM! New column added for checked properties.

The new column correctly implements the "Checked Properties" functionality as per PR objectives.

packages/workflow-core/src/lib/plugins/external-plugin/api-plugin/api-plugin.test.ts (5)

1-318: Great test coverage and organization!

The test suite is well-structured with:

  • Comprehensive coverage of core functionalities
  • Clear test organization using describe blocks
  • Proper handling of edge cases
  • Effective use of mocks and specific assertions

11-48: LGTM! Well-structured basic invocation test.

The test properly validates that generateRequestPayloadFromWhitelist is called with the correct context.


50-147: LGTM! Comprehensive request payload testing.

The test suite thoroughly covers various scenarios:

  • Success case with status ok
  • Failed requests
  • Invalid responses
  • Invalid requests

149-216: LGTM! Thorough invokedAt transformer testing.

Tests properly verify the conditional inclusion of the invokedAt transformer based on the includeInvokedAt flag.


218-316: LGTM! Well-structured whitelist testing.

The test suite thoroughly covers the generateRequestPayloadFromWhitelist functionality:

  • Handling of whitelisted properties
  • Nested object handling
  • Array handling
  • Default behavior when no whitelist is provided
packages/workflow-core/src/lib/plugins/external-plugin/api-plugin/api-plugin.ts (4)

40-41: LGTM! Well-typed property declarations.

The new properties are properly typed and align with the plugin's functionality.


13-17: LGTM! Clear transformer definition.

The invokedAtTransformerDefinition is well-defined with clear property mappings.


40-41: LGTM! Well-structured property additions.

The new properties are properly typed and initialized with sensible defaults:

  • whitelistedInputProperties is optional
  • includeInvokedAt defaults to true

Also applies to: 61-62


437-454: LGTM! Well-implemented whitelist generation.

The generateRequestPayloadFromWhitelist method:

  • Handles the case when no whitelist is provided
  • Properly processes nested objects
  • Correctly handles undefined values
apps/backoffice-v2/src/lib/blocks/variants/DefaultBlocks/hooks/useDefaultBlocksLogic/useDefaultBlocksLogic.tsx (2)

472-473: LGTM! Clean integration of merchantScreeningInput.

The integration of pluginsInput data into the merchant screening block is clean and follows the component's data flow pattern.


472-473: LGTM! Clean integration of merchant screening input.

The addition of merchantScreeningInput properly utilizes the new pluginsInput data structure with appropriate null coalescing.

apps/backoffice-v2/src/lib/blocks/hooks/useMerchantScreeningBlock/useMerchantScreeningBlock.tsx (1)

21-21: LGTM! Clean integration of merchant screening input.

The changes properly handle the new merchant screening input data with appropriate type safety and null checks.

Also applies to: 28-28, 78-78

packages/workflow-core/src/lib/plugins/external-plugin/individuals-sanctions-v2-plugin/individuals-sanctions-v2-plugin.test.ts (1)

11-11: LGTM! Improved test isolation.

Adding vi.clearAllMocks() ensures proper test isolation by clearing all mocks before each test run.

apps/backoffice-v2/src/domains/workflows/fetchers.ts (1)

136-146: LGTM! Well-structured schema extension.

The addition of pluginsInput with nested merchantScreening follows the existing schema patterns and properly handles optional fields.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
packages/workflow-core/CHANGELOG.md (2)

3-8: Enhance the changelog entry with more details.

The current description "Updated workflow-core" is too vague. Consider providing specific details about what was updated to help users understand the changes better.

 ## 0.6.89
 
 ### Patch Changes
 
-Updated workflow-core
+- Enhanced WorkflowRunner to include pluginsInput in context
+- Updated ApiPlugin and MerchantScreening plugin to return requestPayload
+- Added support for disabling invokedAt transformer

9-13: Improve changelog formatting and grammar.

The current entry combines multiple changes in a single line and has some grammatical issues.

 ## 0.6.88
 
 ### Patch Changes
 
-Added pluginsInput with requestPayload per plugins & invokedAt now disablable
+- Added pluginsInput with requestPayload per plugin
+- Made invokedAt transformer optionally disabled
🧰 Tools
🪛 LanguageTool

[uncategorized] ~13-~13: Did you mean “plugin”? If following ‘per’, nouns are often singular.
Context: ...ed pluginsInput with requestPayload per plugins & invokedAt now disablable ## 0.6.87 ...

(CONFUSION_OF_NNS_NN_UN)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9cf9d7c and 1ba4689.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (15)
  • 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/workflow-core/CHANGELOG.md (1 hunks)
  • packages/workflow-core/package.json (1 hunks)
  • sdks/workflow-browser-sdk/CHANGELOG.md (1 hunks)
  • sdks/workflow-browser-sdk/package.json (2 hunks)
  • sdks/workflow-node-sdk/CHANGELOG.md (1 hunks)
  • sdks/workflow-node-sdk/package.json (2 hunks)
  • services/workflows-service/CHANGELOG.md (1 hunks)
  • services/workflows-service/package.json (2 hunks)
  • services/workflows-service/prisma/data-migrations (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
  • packages/workflow-core/package.json
  • apps/kyb-app/CHANGELOG.md
  • sdks/workflow-browser-sdk/CHANGELOG.md
  • examples/headless-example/CHANGELOG.md
  • sdks/workflow-node-sdk/package.json
  • apps/kyb-app/package.json
  • services/workflows-service/prisma/data-migrations
  • sdks/workflow-node-sdk/CHANGELOG.md
  • sdks/workflow-browser-sdk/package.json
  • apps/backoffice-v2/package.json
  • services/workflows-service/package.json
  • examples/headless-example/package.json
🧰 Additional context used
🪛 LanguageTool
packages/workflow-core/CHANGELOG.md

[uncategorized] ~13-~13: Did you mean “plugin”? If following ‘per’, nouns are often singular.
Context: ...ed pluginsInput with requestPayload per plugins & invokedAt now disablable ## 0.6.87 ...

(CONFUSION_OF_NNS_NN_UN)

⏰ Context from checks skipped due to timeout of 90000ms (7)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: test_windows
  • GitHub Check: test_linux
  • GitHub Check: build (windows-latest)
  • GitHub Check: build (ubuntu-latest)
  • GitHub Check: lint
  • GitHub Check: spell_check
🔇 Additional comments (2)
services/workflows-service/CHANGELOG.md (1)

3-18: LGTM! Consistent dependency updates.

The changelog entries properly document the dependency updates to:

apps/backoffice-v2/CHANGELOG.md (1)

3-16: LGTM! Consistent dependency updates.

The changelog entries properly document the dependency updates to:

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
apps/backoffice-v2/src/lib/blocks/variants/DefaultBlocks/hooks/useDefaultBlocksLogic/useDefaultBlocksLogic.tsx (2)

Line range hint 63-592: Consider breaking down the large hook into smaller, focused hooks.

The useDefaultBlocksLogic hook is quite extensive and handles multiple responsibilities. Consider splitting it into smaller, more focused custom hooks for better maintainability and testability. For example:

  • useWorkflowData for workflow-related state
  • useBlocksConfiguration for blocks setup
  • useUserInteractions for user action handlers

506-507: Enhance error handling and type safety for merchantScreeningInput.

Consider these improvements:

  1. Add runtime validation for the requestPayload structure to catch potential issues early.
  2. Define and use TypeScript interfaces for the payload shape.
  3. Consider logging when falling back to the empty object to help debug issues in production.
+interface MerchantScreeningPayload {
+  // Define expected payload structure
+}
+
 merchantScreeningInput:
-  workflow?.context?.pluginsInput?.merchantScreening?.requestPayload || {},
+  (workflow?.context?.pluginsInput?.merchantScreening?.requestPayload as MerchantScreeningPayload) || (() => {
+    if (workflow?.context?.pluginsInput?.merchantScreening) {
+      console.warn('merchantScreening.requestPayload is missing or invalid');
+    }
+    return {};
+  })(),
packages/workflow-core/src/lib/workflow-runner.ts (1)

815-819: Consider consolidating duplicate context merge logic.

The code repeats the same context merge pattern for pluginsInput in three different places. This could be refactored into a helper method to improve maintainability and reduce duplication.

Consider applying this refactor:

+ private mergePluginInput(context: any, apiPlugin: HttpPlugin, requestPayload: any, status: ProcessStatus, error?: any) {
+   return this.mergeToContext(
+     context,
+     { requestPayload, status, ...(error ? { error } : {}) },
+     `pluginsInput.${apiPlugin.name}`,
+   );
+ }

  private async __invokeApiPlugin(apiPlugin: HttpPlugin, additionalContext?: AnyRecord) {
    // ... existing code ...

    if (apiPlugin.persistResponseDestination && responseBody) {
      this.context = this.mergeToContext(
        this.context,
        responseBody,
        apiPlugin.persistResponseDestination,
      );
-      this.context = this.mergeToContext(
-        this.context,
-        { requestPayload, status: ProcessStatus.SUCCESS },
-        `pluginsInput.${apiPlugin.name}`,
-      );
+      this.context = this.mergePluginInput(this.context, apiPlugin, requestPayload, ProcessStatus.SUCCESS);
    }

    if (!apiPlugin.persistResponseDestination && responseBody) {
      this.context = this.mergeToContext(
        this.context,
        responseBody,
        `pluginsOutput.${apiPlugin.name}`,
      );
-      this.context = this.mergeToContext(
-        this.context,
-        { requestPayload, status: ProcessStatus.SUCCESS },
-        `pluginsInput.${apiPlugin.name}`,
-      );
+      this.context = this.mergePluginInput(this.context, apiPlugin, requestPayload, ProcessStatus.SUCCESS);
    }

    if (error) {
      this.context = this.mergeToContext(
        this.context,
        { name: apiPlugin.name, error, status: ProcessStatus.ERROR },
        `pluginsOutput.${apiPlugin.name}`,
      );
-      this.context = this.mergeToContext(
-        this.context,
-        { requestPayload, error, status: ProcessStatus.ERROR },
-        `pluginsInput.${apiPlugin.name}`,
-      );
+      this.context = this.mergePluginInput(this.context, apiPlugin, requestPayload, ProcessStatus.ERROR, error);
    }

Also applies to: 829-833, 843-847

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ba4689 and 76d0201.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (23)
  • apps/backoffice-v2/CHANGELOG.md (1 hunks)
  • apps/backoffice-v2/package.json (2 hunks)
  • apps/backoffice-v2/src/domains/workflows/fetchers.ts (1 hunks)
  • apps/backoffice-v2/src/lib/blocks/components/CallToActionLegacy/CallToActionLegacy.tsx (2 hunks)
  • apps/backoffice-v2/src/lib/blocks/components/CallToActionLegacy/interfaces.ts (2 hunks)
  • apps/backoffice-v2/src/lib/blocks/components/DirectorBlock/hooks/useDirectorBlock/useDirectorBlock.tsx (1 hunks)
  • apps/backoffice-v2/src/lib/blocks/variants/DefaultBlocks/hooks/useDefaultBlocksLogic/useDefaultBlocksLogic.tsx (4 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)
  • packages/workflow-core/src/lib/workflow-runner.ts (5 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)
  • services/workflows-service/CHANGELOG.md (1 hunks)
  • services/workflows-service/package.json (2 hunks)
  • websites/docs/package.json (1 hunks)
✅ Files skipped from review due to trivial changes (4)
  • packages/common/package.json
  • sdks/web-ui-sdk/CHANGELOG.md
  • sdks/web-ui-sdk/package.json
  • packages/common/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (11)
  • apps/kyb-app/CHANGELOG.md
  • sdks/workflow-browser-sdk/CHANGELOG.md
  • packages/workflow-core/package.json
  • apps/backoffice-v2/src/domains/workflows/fetchers.ts
  • examples/headless-example/package.json
  • apps/kyb-app/package.json
  • examples/headless-example/CHANGELOG.md
  • apps/backoffice-v2/package.json
  • sdks/workflow-browser-sdk/package.json
  • services/workflows-service/package.json
  • services/workflows-service/CHANGELOG.md
🧰 Additional context used
🪛 Biome (1.9.4)
packages/workflow-core/src/lib/workflow-runner.ts

[error] 782-782: Unsafe usage of optional chaining.

If it short-circuits with 'undefined' the evaluation will throw TypeError here:

(lint/correctness/noUnsafeOptionalChaining)

⏰ Context from checks skipped due to timeout of 90000ms (6)
  • GitHub Check: test_windows
  • GitHub Check: test_linux
  • GitHub Check: build (windows-latest)
  • GitHub Check: build (ubuntu-latest)
  • GitHub Check: lint
  • GitHub Check: spell_check
🔇 Additional comments (7)
packages/workflow-core/CHANGELOG.md (1)

3-10: LGTM! Changelog entry is well-formatted.

The new version entry follows the conventional format and correctly documents the dependency updates.

apps/backoffice-v2/CHANGELOG.md (1)

3-11: LGTM! Changelog entry is well-formatted and consistent.

The new version entry correctly documents all dependency updates with consistent version numbers across packages.

apps/backoffice-v2/src/lib/blocks/components/CallToActionLegacy/interfaces.ts (1)

9-9: LGTM! Interface changes maintain backward compatibility.

The addition of optional directorId properties is well-structured and maintains backward compatibility through the use of optional chaining (?).

Also applies to: 24-24

apps/backoffice-v2/src/lib/blocks/components/CallToActionLegacy/CallToActionLegacy.tsx (1)

36-36: LGTM! Component changes align with interface updates.

The component correctly implements the interface changes by:

  1. Destructuring the optional directorId from props
  2. Passing it through to the onReuploadNeeded callback

Also applies to: 267-267

apps/backoffice-v2/src/lib/blocks/components/DirectorBlock/hooks/useDirectorBlock/useDirectorBlock.tsx (1)

192-192: LGTM! Hook changes complete the director ID propagation.

The hook correctly propagates the director's ID to the reUploadNeededBlock, completing the chain of director ID handling from the interface through to the actual usage.

packages/workflow-core/src/lib/workflow-runner.ts (2)

782-782: ⚠️ Potential issue

Fix unsafe optional chaining usage.

The optional chaining operator could lead to a TypeError if the method returns undefined and we try to destructure from it.

Apply this diff to handle the case when the method returns undefined:

-    const { callbackAction, responseBody, requestPayload, error } = await apiPlugin.invoke?.(
+    const { callbackAction, responseBody, requestPayload, error } = (await apiPlugin.invoke?.(
      {
        ...this.context,
        workflowRuntimeConfig: this.#__config,
        workflowRuntimeId: this.#__runtimeId,
      },
      additionalContext,
-    );
+    )) ?? {};

Likely invalid or redundant comment.

🧰 Tools
🪛 Biome (1.9.4)

[error] 782-782: Unsafe usage of optional chaining.

If it short-circuits with 'undefined' the evaluation will throw TypeError here:

(lint/correctness/noUnsafeOptionalChaining)


815-819: Verify the impact of storing request payloads in context.

The addition of request payloads to the context could potentially increase memory usage if the payloads are large or numerous. Consider:

  1. Adding size limits for request payloads
  2. Implementing cleanup strategies for old plugin inputs
  3. Documenting the memory implications in comments

Run the following script to analyze the typical size of request payloads:

Also applies to: 829-833, 843-847

✅ Verification successful

Request payload storage is properly controlled and safe

The codebase already implements multiple safeguards for request payload storage:

  • Schema validation ensures payload structure and size constraints
  • Whitelist filtering prevents storing unnecessary data
  • Transformation pipeline allows fine-grained control over stored data
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for request payload definitions and usages to understand their typical structure and size

# Test: Search for request payload definitions
echo "Request payload definitions:"
ast-grep --pattern 'interface $_ {
  requestPayload: $_;
}'

# Test: Search for request payload assignments
echo "Request payload assignments:"
rg -A 5 'requestPayload.*='

Length of output: 14505

websites/docs/package.json Show resolved Hide resolved
@Omri-Levy Omri-Levy merged commit d02335d into dev Jan 23, 2025
17 of 18 checks passed
@Omri-Levy Omri-Levy deleted the bal-3035 branch January 23, 2025 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants