Add new constants, rename one and fix DueBy

[remi-stripe]

While working on fixing #580, I started improving TestAccountUnmarshal to ensure we tried to deserialize a complex account object to catch any issue.

Doing that, I caught a few mistakes that I wanted to fix at the same time:

• AccountRejectReason is now a type even though it's only used as a parameter. I should have fixed it when you asked me to, it's cleaner.
• verification[disabled_reason] now has its own type: IdentityVerificationDisabledReason and related constants
• verification[due_by] should not be a pointer so I fixed it back to a normal int64
• PayoutIntervalDay has been renamed to PayoutIntervalDaily.

I think we should merge this one as a major version as it makes enough breaking changes.

If we want to unblock the other issue without a major version though we could merge #582 first.

r? @brandur-stripe
cc @stripe/api-libraries

[remi-stripe]

You have no idea how long I spent on figuring out the write right combination of [] string map interface and {} to make that one work 😓

[brandur-stripe]

Oh god, haha.

[remi-stripe]

I made thorough tests of almost every property, not sure if too much or not, please let me know if I should cut those in half.

[brandur-stripe]

Probably not necessary, but I think it's okay now that you've already written it.

[remi-stripe]

Out of curiosity what do you think would have been a good test? I think this large structure is fairly easy to mess up as there are so many edge-cases. You could end up deserializing the wrong property in the wrong place like maiden name as first_name or vice versa, or identification documents like I did.

[brandur-stripe]

Probably one check per "level" (i.e., level of structures which you descend to) is probably a good idea. Agree that it's easy to misname a JSON field, but I think to some degree you need to rely on the fact that these are likely to trend towards correctness over time. Adding tests gives you a little more guarantee, but the tests themselves are likely to be butchered during any sizable refactor, and so not end up testing for regressions as well as you might hope anyway.

[remi-stripe]

Gotcha! I see where you're coming from. I think I had similar discussions with @ob-stripe on the stripe-java refactor so it is starting to make sense to me.

[brandur-stripe]

Yeah, it's definitely a little subjective :$

[nd2s]

@remi-stripe JFYI: There are more struct fields of type *string that should be a custom type instead. Like PayoutScheduleParams.Interval or .WeeklyAnchor.

Would be nice if these changes would find their way into the library soon.

[remi-stripe]

JFYI: There are more struct fields of type *string that should be a custom type instead. Like PayoutScheduleParams.Interval or .WeeklyAnchor.

@nd2s That is expected. You can see that all parameters are *string and not *CustomType. The reason is that if we had used the custom types, we would have needed two custom functions for every single type which would have been unmanageable so we stuck with *string for all of those.

Instead your code should be:

    account, err := account.New(&stripe.AccountParams{
      Type:      stripe.String(string(stripe.AccountTypeCustom)),
      Country: stripe.String("US"),
      PayoutSchedule: &stripe.PayoutScheduleParams {
        Interval: stripe.String(string(stripe.PayoutIntervalManual)),
      },
    })

[brandur-stripe]

If we want to unblock the other issue without a major version though we could merge #582 first.

+1.

This LGTM.

[brandur-stripe]

Released as 33.0.0.