-
Notifications
You must be signed in to change notification settings - Fork 199
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
feat: reworked getOrderedSteps & fixed tests #2868
Conversation
|
WalkthroughThe pull request includes updates to the changelogs of various packages within the Ballerine project, documenting new version entries and dependency updates. Key changes involve version bumps for several packages, including Changes
Possibly related PRs
Suggested reviewers
Warning Rate limit exceeded@chesterkmr has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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
🧹 Outside diff range and nitpick comments (3)
packages/common/src/utils/collection-flow/set-collection-flow-status.unit.test.ts (2)
22-31
: Consider adding spy cleanup for better test isolation.The test case correctly verifies the new warning behavior. However, it's a good practice to clean up spies after each test.
Consider adding:
it('will log a warning if the status is invalid', () => { const context = { collectionFlow: { state: {} }, }; const consoleSpy = vi.spyOn(console, 'warn'); + + try { const result = setCollectionFlowStatus(context as DefaultContextSchema, 'invalid' as any); expect(consoleSpy).toHaveBeenCalledWith('Invalid status: invalid'); expect(result).toBe(context); + } finally { + consoleSpy.mockRestore(); + } });
34-41
: Consider adding spy cleanup for better test isolation.Similar to the previous test case, consider adding spy cleanup.
Consider adding:
it('will log a warning if the collection flow state is not present', () => { const context = {}; const consoleSpy = vi.spyOn(console, 'warn'); + try { const result = setCollectionFlowStatus(context as DefaultContextSchema, 'completed'); expect(consoleSpy).toHaveBeenCalledWith('Collection flow state is not present.'); expect(result).toBe(context); + } finally { + consoleSpy.mockRestore(); + } });packages/common/CHANGELOG.md (1)
3-7
: Enhance the changelog entry with more details.While the entry follows the correct format, it would be more helpful to users if it included:
- Specific improvements made to
getOrderedSteps
- Nature of test fixes
- Any breaking changes or migration steps needed
Consider expanding the entry like this:
## 0.9.54 ### Patch Changes -Reworked getOrderedSteps & fixed tests +- Reworked getOrderedSteps: + - [Description of specific improvements] + - [Changes to error handling] + - [Changes to state transition logic] +- Fixed tests: + - [Description of test issues resolved] + +### Migration +- No breaking changes
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (22)
apps/backoffice-v2/CHANGELOG.md
(1 hunks)apps/backoffice-v2/package.json
(2 hunks)apps/kyb-app/CHANGELOG.md
(1 hunks)apps/kyb-app/package.json
(2 hunks)examples/headless-example/CHANGELOG.md
(1 hunks)examples/headless-example/package.json
(2 hunks)packages/common/CHANGELOG.md
(1 hunks)packages/common/package.json
(1 hunks)packages/common/src/utils/collection-flow/get-ordered-steps.ts
(1 hunks)packages/common/src/utils/collection-flow/set-collection-flow-status.unit.test.ts
(2 hunks)packages/workflow-core/CHANGELOG.md
(1 hunks)packages/workflow-core/package.json
(2 hunks)sdks/web-ui-sdk/CHANGELOG.md
(1 hunks)sdks/web-ui-sdk/package.json
(2 hunks)sdks/workflow-browser-sdk/CHANGELOG.md
(1 hunks)sdks/workflow-browser-sdk/package.json
(2 hunks)sdks/workflow-node-sdk/CHANGELOG.md
(1 hunks)sdks/workflow-node-sdk/package.json
(2 hunks)services/workflows-service/CHANGELOG.md
(1 hunks)services/workflows-service/package.json
(2 hunks)services/workflows-service/prisma/data-migrations
(1 hunks)websites/docs/package.json
(1 hunks)
✅ Files skipped from review due to trivial changes (18)
- apps/backoffice-v2/CHANGELOG.md
- apps/backoffice-v2/package.json
- apps/kyb-app/CHANGELOG.md
- apps/kyb-app/package.json
- examples/headless-example/CHANGELOG.md
- examples/headless-example/package.json
- packages/common/package.json
- packages/workflow-core/CHANGELOG.md
- packages/workflow-core/package.json
- sdks/web-ui-sdk/CHANGELOG.md
- sdks/web-ui-sdk/package.json
- sdks/workflow-browser-sdk/CHANGELOG.md
- sdks/workflow-browser-sdk/package.json
- sdks/workflow-node-sdk/CHANGELOG.md
- sdks/workflow-node-sdk/package.json
- services/workflows-service/CHANGELOG.md
- services/workflows-service/package.json
- services/workflows-service/prisma/data-migrations
🔇 Additional comments (5)
websites/docs/package.json (1)
20-20
: LGTM: Version bump maintains ecosystem consistency
The update of @ballerine/common
to ^0.9.54
aligns with the coordinated version updates across other Ballerine packages.
packages/common/src/utils/collection-flow/set-collection-flow-status.unit.test.ts (2)
2-2
: LGTM!
The addition of vi
import is necessary for the new test cases using console spies.
22-41
: Verify the intentional change from errors to warnings.
The test changes indicate a shift from throwing errors to logging warnings for invalid states. Please ensure this behavioral change is:
- Intentional and aligned with the project's error handling strategy
- Documented in the function's documentation
- Communicated to other developers who might rely on error handling
Let's check if this change is documented:
✅ Verification successful
Let me try a different search strategy to find the implementation file and any related changes.
Warning-based error handling is intentionally implemented
The implementation file shows this is a deliberate design choice:
- The function is documented as a context mutator that "Sets the status on the collection flow state"
- The warning-based approach is consistently implemented for both invalid states
- The function maintains its contract by always returning the context object, making it composable with other operations
The behavior is appropriate because:
- It follows a fail-soft approach for non-critical validation
- The warnings provide debugging information without breaking the flow
- The function's documentation accurately reflects its behavior
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for documentation or changelog entries about this change
rg -l "setCollectionFlowStatus" | xargs rg -l "error.*warning|throw.*warn"
# Check the implementation file for documentation
ast-grep --pattern 'export function setCollectionFlowStatus' -A 10
Length of output: 1548
Script:
#!/bin/bash
# Find the implementation file
fd "set-collection-flow-status" --type f
# Once found, let's look at its content and recent changes
fd "set-collection-flow-status" --type f --exec cat {}
Length of output: 2457
packages/common/src/utils/collection-flow/get-ordered-steps.ts (2)
15-17
: Good addition: Validate state machine definition
The added error handling ensures the provided definition contains valid states
and an initial
state, preventing potential runtime errors.
25-27
: Good addition: Validate current state
By checking if stateConfig
exists for the currentState
, you prevent errors due to invalid or undefined states.
Summary by CodeRabbit
New Features
Bug Fixes
getOrderedSteps
function to improve robustness.Chores