Push to card specs and scenarios

[matin]

These are the specifications and scenarios for push to card.

Scenarios still required:

• [x] Credit to a card with can_credit of false should fail

Open issues required to merge:

• [x] Card.type and Card.category defaults #607
• [x] Use bank_name instead of bank #608
• [x] Exceeding transaction limit should product a 409 instead of 400 #611

[steveklabnik]

One thing I've been thinking about this: the Right Way (tm) to do this via hypermedia is to expose the link depending on the actions available, rather than booleans.

Semi-realistic mockup:

This way:

{
    "cards" [{
        "number": "444444444444",
        "can_debit": true,
        "can_credit": true
    }],
    "links": [{
        "cards.debit": "...",
        "cards.credit": "..."
    }]
}

vs

{
    "cards" [{
        "number": "444444444444"
    }],
    "links": [{
        "cards.debit": "..."
    }]
}

and

{
    "cards" [{
        "number": "444444444444"
    }],
    "links": [{
        "cards.debit": "...",
        "cards.credit": "..."
    }]
}

What do you think?

[matin]

@steveklabnik we talked about this before. You're right that this is technically the Right Way to represent this information from a hypermedia API standpoint, but it's not the best experience.

can_debit is also consistent with how we do it for Bank Accounts. Think about the code in each case:

if card.can_credit: ...

vs.

if hasattr(card, credit): ...

or

try card.credit(amount):
    ...
except AttributeError:
    ...

The first way makes more sense—at least to me.

[msherry]

Isn't that just an issue for the clients?

@property
def can_credit(self):
    return hasattr(self.credit)

[matin]

@msherry You make a good point.

Would you do something similar for balanced.js?

Can you create a similar type of abstraction in Java?

[steveklabnik]

You should be able to do in any language.

[matin]

@steveklabnik Shouldn't it be cards.credits (plural) instead of cards.credits? That link, from my understanding, would be used both for a fetch and create.

What would you do if a Bank Account has prior debits but can_debit changes to false?

Also ... should ≠ can

[steveklabnik]

We currently use the singular everywhere already, I think.

Do bank accounts change their ability to debit and be debited? Isn't that a feature of the kind of account?

By "should" I meant "I can't think of a counterexample." Ruby, Python, C++, Java, PHP, C#.... it would be a pretty serious deficiency.

[msherry]

@steveklabnik They do. You can't debit a bank account that hasn't been verified, for instance.

[remear]

Java uses ResourceField to simulate object properties like other languages. I'd really have to do some thinking on how one could be set based on the existence of a link. So, I'll have to go with maybe, but I'm not fond of the idea. Of course, there could be a class method, but then it would be card.can_debit() instead of card.can_debit, which might confuse users.

[matin]

@steveklabnik Do we now ...

from https://docs.balancedpayments.com/1.1/api/cards/#create-a-card-direct

{
    "cards": [
        {
            "address": {
                "city": null, 
                "country_code": null, 
                "line1": null, 
                "line2": null, 
                "postal_code": null, 
                "state": null
            }, 
            "avs_postal_match": null, 
            "avs_result": null, 
            "avs_street_match": null, 
            "brand": "MasterCard", 
            "created_at": "2014-04-29T00:46:42.022131Z", 
            "cvv": "xxx", 
            "cvv_match": "yes", 
            "cvv_result": "Match", 
            "expiration_month": 12, 
            "expiration_year": 2020, 
            "fingerprint": "fc4ccd5de54f42a5e75f76fbfde60948440c7a382ee7d21b2bc509ab9cfed788", 
            "href": "/cards/CC5H1ZZk43sPUu0f9lIxbhgF", 
            "id": "CC5H1ZZk43sPUu0f9lIxbhgF", 
            "is_verified": true, 
            "links": {
                "customer": null
            }, 
            "meta": {}, 
            "name": null, 
            "number": "xxxxxxxxxxxx5100", 
            "updated_at": "2014-04-29T00:46:42.022134Z"
        }
    ], 
    "links": {
        "cards.card_holds": "/cards/{cards.id}/card_holds", 
        "cards.customer": "/customers/{cards.customer}", 
        "cards.debits": "/cards/{cards.id}/debits"
    }
}

[steveklabnik]

@matin Cool! I just mis-remembered. I don't care at all, I was just trying to be consistent.

@remear you construct the object based on the JSON that comes back, right? So why wouldn't you be able to?

[steveklabnik]

@remear

there could be a class method, but then it would be card.can_debit() instead of card.can_debit, which might confuse users.

Ahh, I think this is the difference. Are parenthesis confusing? I don't understand this, but maybe that's just me.

[steveklabnik]

One question about this interface is that can_credit implies that a credit will succeed. It's a statement of possibility, not a guarantee about debit-ability. :/

@msherry @mjallday @mahmoudimus are there scenarios in which can_{debit,credit} transitions from true to false? @msherry , your earlier example was from false to true.

[msherry]

@steveklabnik Sure, if we receive a NOC (Notice of Change) from the RDFI, we can no longer credit/debit that bank account. If a card is lost or stolen, this would likely apply there, as well.

[steveklabnik]

Seems legit.

[steveklabnik]

After exploring all these options, seems like can_debit and can_credit should be there.

[matin]

The scenarios still don't pass, but I think it's largely related to the responses not validating against the schema, which would be fixed with #607.

[matin]

There's still some failing scenarios.

@steveklabnik could you help me debug the following?

undefined method `each_pair' for true:TrueClass (NoMethodError)
./features/step_definitions/http_steps.rb:161:in `checker'
./features/step_definitions/http_steps.rb:167:in `block in checker'
./features/step_definitions/http_steps.rb:161:in `each_pair'
./features/step_definitions/http_steps.rb:161:in `checker'
./features/step_definitions/http_steps.rb:173:in `/^the fields on this (.*) match:$/'
features/rest/push_to_card.feature:16:in `And the fields on this card match:'

undefined method `[]' for nil:NilClass (NoMethodError)
./features/step_definitions/credits.rb:17:in `/^the credit was successfully created$/'
features/rest/push_to_card.feature:99:in `But the credit was successfully created'

and

You can implement step definitions for undefined steps with these snippets:

Then(/^When I GET to \{"(.*?)"=>\[\{"(.*?)"=>"(.*?)", "(.*?)"=>nil, "(.*?)"=>\{"(.*?)"=>nil, "(.*?)"=>"(.*?)", "(.*?)"=>nil\}, "(.*?)"=>"(.*?)", "(.*?)"=>"(.*?)", "(.*?)"=>"(.*?)", "(.*?)"=>nil, "(.*?)"=>"(.*?)", "(.*?)"=>(\d+), "(.*?)"=>nil, "(.*?)"=>\{\}, "(.*?)"=>"(.*?)", "(.*?)"=>"(.*?)", "(.*?)"=>"(.*?)"\}\], "(.*?)"=>\{"(.*?)"=>"(.*?)", "(.*?)"=>"(.*?)", "(.*?)"=>"(.*?)", "(.*?)"=>"(.*?)", "(.*?)"=>"(.*?)"\}\}$/) do |arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15, arg16, arg17, arg18, arg19, arg20, arg21, arg22, arg23, arg24, arg25, arg26, arg27, arg28, arg29, arg30, arg31, arg32, arg33, arg34, arg35, arg36, arg37, arg38, arg39|
  pending # express the regexp above with the code you wish you had
end

[steveklabnik]

The second one means you have some of your language wrong, it's not matching any of the steps it knows about. It looks like you're passing in a hash somewhere where it should be a URL.

First means you think you have an array, but you have true.

I'll pull down your branch soon.

[steveklabnik]

Second one fixed, PR sent.

[matin]

@steveklabnik Still getting the following:

undefined method `[]' for nil:NilClass (NoMethodError)
./features/step_definitions/credits.rb:17:in `/^the credit was successfully created$/'
features/rest/push_to_card.feature:99:in `But the credit was successfully created'

[steveklabnik]

@matin that failure is from https://github.com/balanced/balanced-api/issues/591

[matin]

Why does it say push_to_card.feature:99 then in that message?

[steveklabnik]

Because it's the same underlying root cause. If you don't have an empty array in the response, it blows up.

[mjallday]

@steveklabnik please note that according to #616 it will still fail once a fix is deployed for #591

[matin]

Looks like @mjallday just merged a fix for #591. Rerunning the scenarios now.

[mjallday]

@matin still has to be built and go through acceptance. hopefully in an hour or so it will be ready. note my message about amending the specs to allow empty collections.

[matin]

@mjallday Rebased master.

[matin]

Going to merge :smile:. The only failing scenarios are unrelated to push to card.