Interfaces for polymorphic API resources (external accounts and payment sources)

[ob-stripe]

Done:

• IExternalAccount
• IPaymentSource

Todo (in followup PRs):

• IBalanceTransactionSource
• maybe IEventDataObject? (would replace (WIP) Consider improving StripeEvent polymorphic reference #1129)
• another generic converter for polymorphic nested hashes with a type attribute

[remi-stripe]

one minor confusion but otherwise this looks great and much cleaner

[remi-stripe]

Why do we implement IPaymentSource on Account? Forgot Account is a source now, damn that is confusing lol

[remi-stripe]

I don't understand why we have properties in this interface. I thought maybe you wanted to be smart and use this for the nested service but it does not seem to be the case. Can you clarify?

[ob-stripe]

Having the properties declared in the interface lets you access those properties without casting, e.g.:

var payout = payoutService.Get("po_123");
var account = payout.Destination.AccountId;

vs.

var payout = payoutService.Get("po_123");
var destination = payout.Destination;
var account = null;
if (destination is BankAccount)
{
    account = ((BankAccount)destination).AccountId;
}
else if (destination is Card) {
    account = ((Card)destination).AccountId;
}

[ob-stripe]

I also wanted to do something similar with IPaymentSource and Customer / CustomerId, but Account is an IPaymentSource that can never be attached to a customer :(

I could add an additional ICustomerPaymentSource interface to do this, but not sure if worth the trouble.

[remi-stripe]

Why would you access the Account id on an external account? You already know which account it's on since you retrieved the Payout on that account right?

[ob-stripe]

I don't think "why" matters. Conceptually, an external account is a payment destination that is attached to a (Stripe) account, so it makes sense that the interface reflects this. It also costs us literally nothing to add this since the two concrete classes that implement this interface already fulfill that contract.

[remi-stripe]

I disagree though. I think it matters to be consistent though and the fact that it does not happen on IPaymentSource feels confusing to me and not useful. If you were using it explicitly for the service I would say maybe but here I don't see the benefits. Like you could also say the same about Last4 which is on card and bank account and really useful to access without a cast right?

This is my personal opinion and I am fine to merge as is though as I agree it ultimately works and I don't want to block.

[remi-stripe]

Approving!