fix: replace isPacs002Transaction gate with isStructuredTransaction#405
fix: replace isPacs002Transaction gate with isStructuredTransaction#405Justus-at-Tazama merged 2 commits intodevfrom
Conversation
…add unsupported-type guards
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. 📝 WalkthroughWalkthroughThis PR implements structured transaction type classification in ChangesStructured Transaction Type Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/logic.service.ts (1)
191-197: ⚡ Quick winInclude transaction type in the error message for better debugging.
The error log at line 195 uses a static message without including the actual transaction type. Adding the transaction type (e.g.,
parsedTrans.TxTp) would help operators identify which structured types are being received but not supported.♻️ Suggested improvement
if (isPacs002Transaction(transaction)) { transactionId = transaction.FIToFIPmtSts.GrpHdr.MsgId; } else { - loggerService.error('Unsupported structured transaction type', new Error('Unsupported structured transaction type'), context); + loggerService.error(`Unsupported structured transaction type: ${parsedTrans.TxTp}`, new Error(`Unsupported structured transaction type: ${parsedTrans.TxTp}`), context); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/logic.service.ts` around lines 191 - 197, The error log in the isStructuredTransaction branch uses a static message; update the loggerService.error call in the block guarded by isStructuredTransaction / isPacs002Transaction so it includes the actual structured transaction type (e.g., parsedTrans.TxTp or transaction.TxTp) in the message and/or Error object; locate the conditional using isStructuredTransaction and isPacs002Transaction and change the loggerService.error('Unsupported structured transaction type', new Error(...), context) to include the transaction type value and a clear message referencing transactionId/transaction or parsedTrans so operators can see which TxTp was unsupported.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 39-40: The package.json lists non-published release-candidate
versions for dependencies `@tazama-lf/frms-coe-lib` and
`@tazama-lf/frms-coe-startup-lib` which fail install; update the version strings
for these identifiers to valid published versions (or remove the -rc suffix and
pin to the latest stable) in package.json, then run npm info or npm view for
`@tazama-lf/frms-coe-lib` and `@tazama-lf/frms-coe-startup-lib` to confirm the
chosen versions exist and run npm install to verify resolution before merging.
---
Nitpick comments:
In `@src/logic.service.ts`:
- Around line 191-197: The error log in the isStructuredTransaction branch uses
a static message; update the loggerService.error call in the block guarded by
isStructuredTransaction / isPacs002Transaction so it includes the actual
structured transaction type (e.g., parsedTrans.TxTp or transaction.TxTp) in the
message and/or Error object; locate the conditional using
isStructuredTransaction and isPacs002Transaction and change the
loggerService.error('Unsupported structured transaction type', new Error(...),
context) to include the transaction type value and a clear message referencing
transactionId/transaction or parsedTrans so operators can see which TxTp was
unsupported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 66921bea-a32d-4e10-982d-5a9b3e049858
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
__tests__/unit/logic.service.test.tseslint.config.mjspackage.jsonsrc/logic.service.ts
Summary
Replaces the
isPacs002Transactionouter gate withisStructuredTransactionas the umbrella guard for all ISO 20022 structured transaction types, withisPacs002Transactionretained as an inner narrowing guard for Pacs002-specific field access.Also disables the
@typescript-eslint/unified-signatureslint rule which crashes when it encounters aSupportedTransactionMessageunion type parameter (bug in@typescript-eslint/eslint-plugin<= 8.46.2).Changes
src/logic.service.ts- outer gate replaced withisStructuredTransaction, innerisPacs002Transactionguard retained for field access__tests__/unit/logic.service.test.ts- new test: unsupported structured type (Pacs008) returns earlyeslint.config.mjs-@typescript-eslint/unified-signaturesdisabled to work around linter crashpackage.json- lib deps bumped tofrms-coe-lib@8.0.0-rc.3andfrms-coe-startup-lib@3.0.2-rc.5Related
Closes #404
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests
Chores