-
Notifications
You must be signed in to change notification settings - Fork 23
Monnify #1456
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
base: main
Are you sure you want to change the base?
Monnify #1456
Conversation
| }, | ||
| "type": "object", | ||
| "additionalProperties": true, | ||
| "required": ["baseUrl","password", "username"] |
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.
I think this needs to be secretKey and apiKey
packages/monnify/src/Adaptor.js
Outdated
| * @property {object} query - An object of query parameters to be encoded into the URL. | ||
| * @property {object} headers - An object of headers to append to the request. | ||
| * @property {string} parseAs - Parse the response body as json, text or stream. By default will use the response headers. | ||
| * @property {number} timeout - Request timeout in ms. Default: 300 seconds. |
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.
Clean this up and remove the items that are not needed for this adaptor
packages/monnify/src/Utils.js
Outdated
| const { baseUrl } = configuration; | ||
|
|
||
| if(!access_token) | ||
| access_token = await getAccessToken(configuration, options.headers) |
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 can write access_token back to state, and in the config, have it as optional.
Has there been an instance where people just give their access_token? if yest then we check if there is no token, then secret and apikey, else vice versa
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.
-
RE: Writing to state:
Yes I think that is a better way of doing it.
-
RE: Giving your access token while configuring an adaptor
The issue is the token is short lived, 25 minutes . It doesn't make sense having it in config for this reason since you would have to edit it every 25 minutes and monnify doesn't provide a refresh token.
| it('makes a successful GET request', async () => { | ||
| testServer | ||
| .intercept({ | ||
| path: '/api/v2/disbursements/search-transactions?pageNo=0&pageSize=10&sourceAccountNumber=4864192954', |
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.
Can we use the query object here instead of placing them in the url as a string?
| "unsorted": false, | ||
| "empty": false | ||
| }, | ||
| "pageNumber": 0, |
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 have these pagination options.
"pageNumber": 0,
"pageSize": 10,
"offset": 0,
```
We could implement pagination. Say we fetch all disbursments in batches, but if we have `offset` or `pageSize`, we respect them
|
…nses Implemented requestWithPagination function to automatically fetch all pages when no pagination params are provided, while respecting explicit pageSize/pageNo parameters when specified. Includes validation for max pageSize of 1000 and comprehensive test coverage.
|
@mtuchi you can review this at your own time |
Summary
Create an adaptor for the Monnify API
Fixes #1450
Details
Add technical details of what you've changed (and why).
AI Usage
Please disclose how you've used AI in this work (it's cool, we just want to
know!):
You can read more details in our
Responsible AI Policy
Review Checklist
Before merging, the reviewer should check the following items:
production? Is it safe to release?
dev only changes don't need a changeset.