Created transfer method endpoint #9
Conversation
peter-joseph
left a comment
peter-joseph
left a comment
There was a problem hiding this comment.
Done with review. Please take a look on my code review comments.
|
|
||
| HashMap<String, String> headers = new HashMap<String, String>(); | ||
| headers.put("Json-Cache-Token", jsonCacheToken); | ||
|
|
There was a problem hiding this comment.
give space before and after +
There was a problem hiding this comment.
I think to all + sign to make it balance.
| throw new HyperwalletException("JSON token is required"); | ||
| } | ||
|
|
||
| HyperwalletTransferMethod transferMethod = new HyperwalletTransferMethod(); |
There was a problem hiding this comment.
I think we cannot create TransferMethod without Type? i.e. PREPAID_CARD, BANK_ACCOUNT, WIRE_ACCOUNT or PAPER_CHECK
There was a problem hiding this comment.
if that is the case we should either not provide this method, or have a default value.
There was a problem hiding this comment.
Yep this method for me should be removed since even if we set a default but in this context we cannot know for sure what are the users available transfer methods or the concept of default transfer method to a specific user given only the user token, please also confirm with @fkrauthan-hyperwallet thanks.
|
|
||
| HashMap<String, String> headers = new HashMap<String, String>(); | ||
| headers.put("Json-Cache-Token", jsonCacheToken); | ||
|
|
There was a problem hiding this comment.
give space before and after +
There was a problem hiding this comment.
I think to all + sign to make it balance.
| } | ||
| if (StringUtils.isEmpty(jsonCacheToken)) { | ||
| throw new HyperwalletException("JSON token is required"); | ||
| } |
There was a problem hiding this comment.
I think we need to check if transfer method type is set or we rely on service validators?
There was a problem hiding this comment.
according to the documentation i was following you are correct.
https://confluence.site1.hyperwallet.local/pages/viewpage.action?pageId=22299508
the other SDKs seem to handle it at the service level.
|
Looks good... |
We need to add the transfer methods endpoint to support the widget use case (it is important to support the json cache token and overriding of any transfer method field). We just need to support creation!