-
Notifications
You must be signed in to change notification settings - Fork 31
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(csi-1266): reset party mappings in interscheme transfer scenario #533
base: minor/csi-1233
Are you sure you want to change the base?
Conversation
|
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.
Pull Request Overview
This pull request introduces changes to support resetting party mappings in interscheme transfer scenarios. Key modifications include new unit tests for step state management, the consolidation and renaming of parties utilities (from “utils” to “partiesUtils”), and refactored error handling and logging across PUT and GET party flows.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
test/unit/lib/util.test.js | Added tests for the step state functionality in Util. |
src/domain/parties/index.js | Refactored module exports to use individual function imports from partiesUtils. |
src/domain/parties/partiesUtils.js | Updated parties utilities with license header and consolidated functions. |
src/lib/util.js | Added license header and updated dependencies; introduced a TODO comment regarding naming for initStepState. |
src/domain/parties/putParties.js | Adjusted logger usage and updated error handling to use partiesUtils functions. |
src/domain/parties/getPartiesByTypeAndID.js | Major refactor including step state management, clearer logging, and separation of responsibilities (e.g. validateRequester, forward request, proxy flows). |
test/unit/domain/parties/parties.test.js & utils.test.js | Updated import paths to reflect the renaming of parties utilities. |
src/domain/timeout/dto.js | Updated import path for parties utilities. |
src/lib/headers.js | Minor formatting revisions in the callback header creation. |
Comments suppressed due to low confidence (2)
src/lib/util.js:101
- [nitpick] Consider updating the function name 'initStepState' for improved clarity, as the TODO indicates potential ambiguity.
// todo: think better name
src/domain/parties/getPartiesByTypeAndID.js:402
- The variable 'proxyCacheTtlSec' is referenced but not defined within this scope; please ensure it is declared or passed as a parameter to avoid runtime errors.
log.info('starting setSendToProxiesList flow: ', { proxyNames, alsReq, proxyCacheTtlSec })
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.
It is hard to validate the logic by just code review, but looks like the logic is improving. A lot of commented code in src/domain/parties/getPartiesByTypeAndID.js
gives impression that the functionality is not complete.
Hopefully the manual and automated tests can help, so I approve.
No description provided.