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

Failed transaction side effects #434

Closed
mjallday opened this issue Dec 5, 2013 · 15 comments
Closed

Failed transaction side effects #434

mjallday opened this issue Dec 5, 2013 · 15 comments

Comments

@mjallday
Copy link
Contributor

mjallday commented Dec 5, 2013

To ease debugging card declines we at Balanced propose creating transactions in a failed state if a user attempts to debit a card.

This is the default behavior in revision 1.1 but we believe there is enough benefit in doing this for users of revision 1.0 of the API as well.

Doing so would change the behavior of the current API in one of two ways:

  1. Indexing debits (GET /v1/marketplaces/MP123123/debits) would begin returning debits with a failed state whenever a 402 response is received when trying to create a debit.
  2. Indexing debits (GET /v1/marketplaces/MP123123/debits) would only return debits in a failed state when explicitly queried for them. e.g. GET /v1/marketplaces/MP123123/debits?status=failed.

Implementing the first behaviour would mean that users would begin to see debits returned when there previously were none.

Implementing the second behaviour would mean that bank account debits that failed would begin to not be returned by default unless explicitly queried for.

Potentially we could opt for a 3rd behaviour, returning only failed bank account debits and hiding failed card debits but this seems (to me at least) to avoid the principle of least surprise.

@mjallday
Copy link
Contributor Author

mjallday commented Dec 5, 2013

ping @steveklabnik @matin @mahmoudimus

@steveklabnik
Copy link
Contributor

I personally vote #2, we shouldn't break compatibility.

@msherry
Copy link
Contributor

msherry commented Dec 6, 2013

What would happen on POST requests to /v1/customers/CU123/debits that generate a 402? Will we now return a 201 and a debit in state failed, or continue with 402, and require a GET to see the failure reason? What uri do I GET to see the debit that just failed?

@matin
Copy link
Member

matin commented Dec 6, 2013

+1 for the third approach since it's exactly the same as the current behavior but requires a special flag to see transactions that would normally not be returned.

@steveklabnik the second approach is actually a change in current behavior.

@mjallday
Copy link
Contributor Author

mjallday commented Dec 6, 2013

@msherry a failed POST will still generate a 402 for backwards compatibility. in the extras field of the exception we return a URI to the debit that was created.

@mahmoudimus
Copy link
Contributor

@matin while I agree, who actually queries for bank account debits that have failed by an index call? our statistics show that's only a dashboard call.

@matin
Copy link
Member

matin commented Dec 6, 2013

@mahmoudimus

who actually queries for bank account debits that have failed by an index call? our statistics show that's only a dashboard call.

I'm not sure how you're tracking this. If I query for all transactions, I could be filtering for failed bank account transactions on my side instead of through the API. Balanced would have no information on this.

You still make a fair point. The second approach is clean in terms of what makes sense if I was integrating today, but I've become cautious of making any changes that is different from existing behavior unless there's some benefit to the marketplace as a result. In this case, we can implement the new behavior without any changes to existing integrations.

@mjallday
Copy link
Contributor Author

mjallday commented Dec 6, 2013

I change my stance. Option 3 is the only way to avoid changing how the API currently behaves for consumers today. It leaves a slight inconsistency but the tradeoff is no existing customer will experience a change.

[EDIT]

I would also be OK with filtering failed card debits from rev0 completely no matter what filter is specified. The dashboard runs rev1.1 so it will still see the failed card debits. This would also prevent any changes to existing behaviour for current consumers of rev 1.0.

@mjallday
Copy link
Contributor Author

mjallday commented Dec 9, 2013

@mahmoudimus @steveklabnik @matin - I have a PR ready to roll if we're happy with option 3.

https://github.com/PoundPay/balanced/pull/271

@mjallday
Copy link
Contributor Author

Ye Olde Deployment Notificatione Announcement

Ye, verily and with much pompt, Balanced presents a new featurette, The Failed Transaction Recordereum:

Step 1. Production of a failed credit using a test card number that doth often transpire in a unsuccessful debitation.

curl https://api.balancedpayments.com/v1/customers/CU76Gzqs6TEsoBov1LIlERlw/debits -u ak-test-PE0kAimMqYDGzebPIHUzYwuLUM86yCxP: -d "amount=5000" -d source_uri=/v1/customers/CU76Gzqs6TEsoBov1LIlERlw/cards/CC741HFtwai6n3f3U0wWZ2N8

{
  "status": "Payment Required",
  "category_code": "card-declined",
  "additional": "Account Frozen",
  "status_code": 402,
  "description": "R758: Account Frozen. Your request id is OHMfb7e371661e211e3a155026ba7c1aba6.",
  "category_type": "banking",
  "_uris": {},
  "request_id": "OHMfb7e371661e211e3a155026ba7c1aba6",
  "extras": {
    "uri": "/v1/marketplaces/TEST-MP5KRaRwei0JslKuwqAfgP0P/debits/WD7Ccw9l7FOTAe7O589Ab89g"
  }
}

Step 2. Querying for yonder list of failed money movements

curl https://api.balancedpayments.com/v1/customers/CU76Gzqs6TEsoBov1LIlERlw/debits?status=failed -u ak-test-PE0kAimMqYDGzebPIHUzYwuLUM86yCxP:

{
  "first_uri": "/v1/customers/CU76Gzqs6TEsoBov1LIlERlw/debits?status=failed&limit=10&offset=0",
  "_type": "page",
  "items": [
    {
      "status": "failed",
      "customer": {
...
      },
      "_type": "debit",
      "fee": null,
      "description": null,
      "_uris": {
        "refunds_uri": {
          "_type": "page",
          "key": "refunds"
        },
        "events_uri": {
          "_type": "page",
          "key": "events"
        }
      },
      "source": {
...
      },
      "created_at": "2013-12-10T21:35:02.362906Z",
      "transaction_number": "W289-905-1047",
      "events_uri": "/v1/debits/WD7dJ1YJ5fmiyaKDJJpeqnLU/events",
      "uri": "/v1/marketplaces/TEST-MP5KRaRwei0JslKuwqAfgP0P/debits/WD7dJ1YJ5fmiyaKDJJpeqnLU",
      "refunds_uri": "/v1/marketplaces/TEST-MP5KRaRwei0JslKuwqAfgP0P/debits/WD7dJ1YJ5fmiyaKDJJpeqnLU/refunds",
      "amount": 5000,
      "meta": {},
      "on_behalf_of": null,
      "appears_on_statement_as": "example.com",
      "hold": null,
      "id": "WD7dJ1YJ5fmiyaKDJJpeqnLU",
      "available_at": "2013-12-10T21:35:04.082004Z"
    }
  ],
  "previous_uri": null,
  "uri": "/v1/customers/CU76Gzqs6TEsoBov1LIlERlw/debits?status=failed&limit=10&offset=0",
  "_uris": {
...
  },
  "limit": 10,
  "offset": 0,
  "total": 2,
  "next_uri": null,
  "last_uri": "/v1/customers/CU76Gzqs6TEsoBov1LIlERlw/debits?status=failed&limit=10&offset=0"
}

Step 3. Ensurification that failed, money movements are not scribed whence not prompted.

curl https://api.balancedpayments.com/v1/customers/CU76Gzqs6TEsoBov1LIlERlw/debits -u ak-test-PE0kAimMqYDGzebPIHUzYwuLUM86yCxP: | grep status

curl https://api.balancedpayments.com/v1/customers/CU76Gzqs6TEsoBov1LIlERlw/debits      -u ak-test-PE0kAimMqYDGzebPIHUzYwuLUM86yCxP:    | grep status
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0      "status": "succeeded",
      "status": "succeeded",
      "status": "succeeded",
      "status": "succeeded",
100 48071  100 48071    0     0  38060      0  0:00:01  0:00:01 --:--:-- 38091
      "status": "succeeded",
      "status": "succeeded",
      "status": "succeeded",
      "status": "succeeded",
      "status": "succeeded",

exitus @mjallday

@mahmoudimus
Copy link
Contributor

Bravo ol' chap! Bravo!

bravo!

@matin
Copy link
Member

matin commented Dec 11, 2013

@mjallday what about information on why the transaction failed in the debit that was created?

@mahmoudimus
Copy link
Contributor

@matin second iteration is to bubble up the reasons - this is cehduled for this sprint

@mjallday
Copy link
Contributor Author

@matin
Copy link
Member

matin commented Dec 11, 2013

😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants