balanced / balanced-api

Balanced API specification.
220 stars 72 forks source link

Old customers don't have the account resource #728

Closed rserna2010 closed 9 years ago

sophistry commented 9 years ago

:+1:

sophistry commented 9 years ago

also, in the meantime... can we make a one-time request to have the account object provisioned to the marketplace account? - assuming it has not been added to the marketplace yet.

mjallday commented 9 years ago

@rserna2010 can you expand on what you mean?

mahmoudimus commented 9 years ago

@sophistry @rserna2010 what does "old customers" mean? what account resource are we referring to? what version of the API is this talking about?

mjallday commented 9 years ago

Closing, will reopen if more information is provided.

sophistry commented 9 years ago

Hi @mahmoudimus and @mjallday - thanks for the kind attention. I am dan_UFN on IRC by the way.

We want to be able to use the new v1.1 Settlements feature to do single deposits each day instead of the multiple per-order deposits.

However, it seems that existing customer objects do not automatically have accounts objects attached to them - only newly-created customers (post 12/29/2014 on our system) seem to have them.

So, code from the API documentation that attempts to access Account objects on "old" pre-existing customers fails to find the Account object attached to the customer.

Here it is in a pithy oneliner: "v1.1 API Accounts objects do not exist on pre-existing customer objects. They are only being created on new customers."

For example: Here is the result from the API call to an old (seller/merchant) customer - accounts is [].

curl https://api.balancedpayments.com/customers/CU63DKZeJQRHcUDMRPi7yYgl/accounts      -H "Accept: application/vnd.api+json;revision=1.1"      -u key:
{
  "meta": {
    "last": "/customers/CU63DKZeJQRHcUDMRPi7yYgl/accounts?limit=10&offset=0",
    "next": null,
    "href": "/customers/CU63DKZeJQRHcUDMRPi7yYgl/accounts?limit=10&offset=0",
    "limit": 10,
    "offset": 0,
    "previous": null,
    "total": 0,
    "first": "/customers/CU63DKZeJQRHcUDMRPi7yYgl/accounts?limit=10&offset=0"
  },
  "accounts": [],
  "links": {}
}

Here is what I expected (returned only from a newly-created -post 12/29/2014 customer) - accounts is not []:

curl https://api.balancedpayments.com/customers/CU6ofoOvopLQ7QSUcdtYUEKs/accounts      -H "Accept: application/vnd.api+json;revision=1.1"      -u key:
{
  "meta": {
    "last": "/customers/CU6ofoOvopLQ7QSUcdtYUEKs/accounts?limit=10&offset=0",
    "next": null,
    "href": "/customers/CU6ofoOvopLQ7QSUcdtYUEKs/accounts?limit=10&offset=0",
    "limit": 10,
    "offset": 0,
    "previous": null,
    "total": 1,
    "first": "/customers/CU6ofoOvopLQ7QSUcdtYUEKs/accounts?limit=10&offset=0"
  },
  "accounts": [
    {
      "links": {
        "customer": "CU6ofoOvopLQ7QSUcdtYUEKs"
      },
      "can_credit": true,
      "type": "payable",
      "created_at": "2015-01-02T15:58:10.984467Z",
      "updated_at": "2015-01-02T15:58:10.984468Z",
      "currency": "USD",
      "meta": {},
      "href": "/accounts/AT6w82myDEtAn11azjyIGUOk",
      "balance": 0,
      "can_debit": true,
      "id": "AT6w82myDEtAn11azjyIGUOk"
    }
  ],
  "links": {
    "accounts.credits": "/accounts/{accounts.id}/credits",
    "accounts.reversals": "/accounts/{accounts.id}/reversals",
    "accounts.customer": "/customers/{accounts.customer}",
    "accounts.debits": "/accounts/{accounts.id}/debits",
    "accounts.settlements": "/accounts/{accounts.id}/settlements",
    "accounts.refunds": "/accounts/{accounts.id}/refunds"
  }
}
mjallday commented 9 years ago

thanks @sophistry, i see what you mean now. we should have backfilled accounts for existing customers so we will investigate and resolve this for you.

sophistry commented 9 years ago

So, is this issue re-opened? I would assume it affects multiple other customers...

mahmoudimus commented 9 years ago

@sophistry yup, will open for now. thx for reporting. investigating this and getting back to you.

sophistry commented 9 years ago

This data is from a test account BTW.

kpowerinfinity commented 9 years ago

+1 - tried on my test account as well and still see the issue. On Jan 5, 2015 1:12 PM, "sophistry" notifications@github.com wrote:

This data is from a test account BTW.

— Reply to this email directly or view it on GitHub https://github.com/balanced/balanced-api/issues/728#issuecomment-68782218 .

mjallday commented 9 years ago

@sophistry that explains it. we did not backfill test marketplaces. going forward we recommend you create new test marketplaces rather than using old ones.

sophistry commented 9 years ago

Hi @mjallday - that doesn't sound quite right - and it makes me lose a certain amount of confidence in the API. I think this needs to be addressed by backfilling test accounts too.

sophistry commented 9 years ago

By the way, we've created new test accounts for local development, but you are saying that we have to throwaway all of our legacy data?

Actually, that won't work for us for one very big reason: we recently upgraded to the v1.1 API and one of the issues we've had to deal with is to make sure old API transactions (of which we have many on our staging and live servers) are handled correctly. There are multiple code paths required when handling refunds for example - the old API didn't require use of Events or polling, but the new API (with the Orders construct) does.

For instance, a refund coming from an old API debit has to be paid out from a main escrow credit rather than a per-order credit.

If you force us to update away from our existing cache of old API transactions, we will have no way to test that these code paths still work until a live refund hits.

Please re-open and backfill +1

sophistry commented 9 years ago

Additionally, there should be a note in the API documentation - this took a few hours to arrive at and discuss over IRC, support@balancedpayments.com and this ticket - it should not be something devs are expected to stumble on and divine.

mjallday commented 9 years ago

@sophistry yes, i have backfilled this particular test marketplace, i was stating our general policy, apologies that that was not clear.

i appreciate what you're saying regarding migrating between revisions of the API. for that particular use case you can still create transactions that do not use orders and are therefore credited into the general balance for your marketplace rather than a balance associated to an order.

the issue if you continue using an old marketplace and relying on transactions created before you migrated is that eventually you will run out of these old transactions to test your refund logic against.

if you'd like some help setting up the flow i've suggested here then please jump into our IRC room and we can show you how to setup the marketplace with this information.

i agree that this should be announced and we'll strive to do that going forward, thanks for your feedback :)

sophistry commented 9 years ago

Ok - Thank you for the reply. I see the accounts objects now on the test account I sent with this issue.

This marketplace is not our staging marketplace though...

This is our staging marketplace - it would be nice to have the ability to test through our whole release process: TEST-MP4fky1d481RiIUHX2tgLjLW

Thanks!