Skip to content
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

V3 java-SDK - ppplmer-96596 update query parameters - Part C #136

Merged
merged 12 commits into from
Jul 30, 2021

Conversation

Rrathinasabapath
Copy link
Contributor

Made query parameter changes for the following objects and their list methods. Please review and approve.

  1. Transfers
  2. Spendback
  3. Balances
    • List User Balances
    • List Prepaid Card Balances
    • List Account Balances
  4. Receipts
    • List User Receipts
    • List Prepaid Card Receipts
    • List Account Receipts
  5. Webhook Notifications
  6. Status Transitions
    • Bank Account
    • Bank Card
    • Paper Check
    • Payment
    • Paypal Acount
    • Prepaid Card
    • User
    • Venmo Account

@@ -21,6 +21,7 @@

private Type type;
private Status status;
private Status transition;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bank Account transition has only two possible states:

  1. DE_ACTIVATED
  2. ACTIVATED
    Please create a separate enum to fix this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the documentation possible status' are as given below.

ACTIVATED
VERIFIED
INVALID
DE_ACTIVATED

and for transitions they are:

ACTIVATED
VERIFIED
INVALID
DE_ACTIVATED

They are declared in the following lines as enumerations already. Status is used for transition too.
public static enum Status {ACTIVATED, VERIFIED, INVALID, DE_ACTIVATED}

Let me know if anymore change is still required.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm referring to this link: https://docs.hyperwallet.com/content/api/v3/resources/status-transitions/bank-accounts-create for Bank Account Status transition. Please let me know if I have to refer to another link.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an inconsistency. A new ticket has to be created for this to update the document. However, addition of Verified and Invalid in the query will not break the functionality. Please approve the request now as I am going by the document https://docs.hyperwallet.com/content/api/v4/resources/status-transitions/bank-accounts-retrieve for now.

@@ -4,6 +4,7 @@

private HyperwalletBankAccount.Type type;
private HyperwalletBankAccount.Status status;
private HyperwalletBankAccount.Status transition;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as given above.

@@ -21,6 +21,7 @@

private Type type;
private Status status;
private Status transition;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as given above.

@@ -19,6 +19,8 @@
private HyperwalletTransferMethod.Type type;
private String token;
private HyperwalletTransferMethod.Status status;
private HyperwalletTransferMethod.Status transition;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment with respect to transition is generic. Pls fix in all the places.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were not refactored to keep it in accordance with V4 and the existing approach taken to refer to HyperwalletTransferMethod class for other fields like type and status. Changing the approach will call for changes in V4 and also other methods where HyperwalletPaperCheck is called.

@@ -10,14 +10,13 @@ protected HyperwalletPayment createBaseModel() {
HyperwalletPayment payment = new HyperwalletPayment();
payment
.status("COMPLETED")
.transition("COMPLETED")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CANCELLED is the only available transition for HyperwalletPayment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following are the status transitions allowed for HyperwalletPayment. 'COMPLETED' is one of them and 'CANCELLED' is not one of them. Details are per https://docs.hyperwallet.com/content/api/v3/resources/status-transitions/list. Please let me know if there is any error in my statement.

CREATED
SCHEDULED
PENDING_ACCOUNT_ACTIVATION
PENDING_ID_VERIFICATION
PENDING_TAX_VERIFICATION
PENDING_TRANSFER_METHOD_ACTION
PENDING_TRANSACTION_VERIFICATION
IN_PROGRESS
UNCLAIMED
COMPLETED
FAILED
RECALLED
RETURNED
EXPIRED

Copy link
Collaborator

@ramahalingam ramahalingam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the comments.

@arao6 arao6 merged commit 8852e83 into support/SDK-V3 Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants