-
Notifications
You must be signed in to change notification settings - Fork 23
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
fix(mojaloop/#2574): sdk-scheme-adapter is not returning party sub-id #120
fix(mojaloop/#2574): sdk-scheme-adapter is not returning party sub-id #120
Conversation
- fix for missing "party.idSubValue" value expected by the SDK-Scheme-Adapter for GET /parties/{type}/{id}/{subId}
- fix for mojaloop/project#2574 - re-factored party model to break up the DB logic into discrete functions - re-worked to correctly handle accounts - added correct index'ing for the accounts table - `subIdValue` has been replaced with `idSubValue` to be consistent with the DFSP API, note the Database will continue to use `subIdValue` to be backward compatible - updated eslint to ignore some un-pragmatic rules
- updated dependencies - added node-fetch to ncurc ignore list due to a breaking change with regard to node version - fixed audit-resolve issues - fixed some typos/spacing/lines in readme
…pected - updated unit tests to cover issues for mojaloop/project#2575 - issue is caused by an update to the [json-rules-engine](https://github.com/CacheControl/json-rules-engine). The path defined in rules must now start with "$" (e.g. `$.<attribute>`), previously `.<attribute>` would work but no longer functional due to the dependency being updated
- rolled back eslint dependency update to the latest 7.x due to node version incompatibility issues. - added eslint to ignore ncurc list - lint fixes
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.
Just a minor note on the rules-engine test
@@ -72,7 +72,10 @@ test('Evaluates a rule based on demo data', async (t) => { | |||
|
|||
const input = { | |||
path: '/transfers', | |||
body: '123', | |||
// body: '123', // <-- This still passes and seems to be a bug with the json-rules-engine: https://github.com/CacheControl/json-rules-engine/releases/tag/v2.3.6. This comment is kept here until this can be addressed in future with the library dependency. |
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.
We're using the latest version: https://github.com/mojaloop/mojaloop-simulator/blob/master/src/lib/rules-engine/package.json#L13
I think this is because we need to update the syntax of this expression to be:
path: '$.transfers',
body: '123',
method: 'POST'
See this example of the update json-rules-engine syntax
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.
Please see notes above:
fix(mojaloop/#2575): mojaloop-simulator rules are not executing as expected
- updated unit tests to cover issues for mojaloop/project#2575
- issue is caused by an update to the json-rules-engine. The path defined in rules must now start with "$" (e.g. $.<attribute>), previously .<attribute> would work but no longer functional due to the dependency being updated
- updated example rule paths with the missing $ prefix
Let me know when you are online so we can discuss this.
The above comment is due to a bug (or at least I think it is) in json-rules-engine where the JSON Path does not resolve correctly as per the above example.
I.e.
If the body is:
body: '123',
And the Path is:
"path": "$.amount"
It should not resolve based on the JSON-PATH resolution, but yet it does (i.e. the test passes). I believe this is a bug in the dependency library (which is probably a bug in the JSON-PATH library that it depends on).
The above path SHOULD only resolve if the body is as follows:
body: { // <-- correct body
amount: '123', // <-- Matching PATH
}
Issue reference: CacheControl/json-rules-engine#285
fix(mojaloop/#2574): sdk-scheme-adapter is not returning party sub-id:
- fix for mojaloop/project#2574
- re-factored party model to break up the DB logic into discrete functions
- re-worked to correctly handle accounts
- added correct index'ing for the accounts table
-
subIdValue
has been replaced withidSubValue
to be consistent with the DFSP API, note the Database will continue to usesubIdValue
to be backward compatible- updated eslint to ignore some un-pragmatic rules
fix(mojaloop/#2575): mojaloop-simulator rules are not executing as expected
- updated unit tests to cover issues for mojaloop/project#2575
- issue is caused by an update to the json-rules-engine. The path defined in rules must now start with "$" (e.g.
$.<attribute>
), previously.<attribute>
would work but no longer functional due to the dependency being updated- updated example rule paths with the missing
$
prefixchore: updating dependencies
- updated dependencies
- added node-fetch & eslint to ncurc ignore list due to a breaking change due to node version incomptability
- fixed audit-resolve issues
- fixed some typos/spacing/lines in readme