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.

Sign up for GitHub

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

Add new constants, rename one and fix DueBy #583

Merged
merged 2 commits into from
Jun 11, 2018
Merged

Add new constants, rename one and fix DueBy #583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08a8d2a
Fix legal_entity[additional_owners][][verification]
remi-stripe Jun 9, 2018
d399d0c
Add new constants and change DueBy to not be a pointer
remi-stripe Jun 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 31 additions & 14 deletions account.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,20 @@ const (
IdentityVerificationDetailsCodeScanNotUploaded IdentityVerificationDetailsCode = "scan_not_uploaded"
)

// IdentityVerificationDisabledReason describes the valid reason to disable account
type IdentityVerificationDisabledReason string

const (
IdentityVerificationDisabledReasonFieldsNeeded IdentityVerificationDisabledReason = "fields_needed"
IdentityVerificationDisabledReasonListed IdentityVerificationDisabledReason = "listed"
IdentityVerificationDisabledReasonOther IdentityVerificationDisabledReason = "other"
IdentityVerificationDisabledReasonRejectedFraud IdentityVerificationDisabledReason = "rejected.fraud"
IdentityVerificationDisabledReasonRejectedListed IdentityVerificationDisabledReason = "rejected.listed"
IdentityVerificationDisabledReasonRejectedOther IdentityVerificationDisabledReason = "rejected.other"
IdentityVerificationDisabledReasonRejectedTermsOfService IdentityVerificationDisabledReason = "rejected.terms_of_service"
IdentityVerificationDisabledReasonUnderReview IdentityVerificationDisabledReason = "under_review"
)

// IdentityVerificationStatus describes the different statuses for identity verification.
type IdentityVerificationStatus string

Expand All @@ -61,16 +75,19 @@ const (
type PayoutInterval string

const (
PayoutIntervalDay PayoutInterval = "daily"
PayoutIntervalDaily PayoutInterval = "daily"
PayoutIntervalManual PayoutInterval = "manual"
PayoutIntervalMonthly PayoutInterval = "monthly"
PayoutIntervalWeekly PayoutInterval = "weekly"
)

// AccountRejectReason describes the valid reason to reject an account
type AccountRejectReason string

const (
AccountRejectReasonFraud string = "fraud"
AccountRejectReasonOther string = "other"
AccountRejectReasonTermsOfService string = "terms_of_service"
AccountRejectReasonFraud AccountRejectReason = "fraud"
AccountRejectReasonOther AccountRejectReason = "other"
AccountRejectReasonTermsOfService AccountRejectReason = "terms_of_service"
)

// AccountParams are the parameters allowed during account creation/updates.
Expand Down Expand Up @@ -268,9 +285,9 @@ type Account struct {
Type AccountType `json:"type"`

Verification *struct {
DisabledReason string `json:"disabled_reason"`
DueBy *int64 `json:"due_by"`
FieldsNeeded []string `json:"fields_needed"`
DisabledReason IdentityVerificationDisabledReason `json:"disabled_reason"`
DueBy int64 `json:"due_by"`
FieldsNeeded []string `json:"fields_needed"`
} `json:"verification"`
}

Expand Down Expand Up @@ -396,13 +413,13 @@ type DOB struct {

// AdditionalOwner is the structure for an account owner.
type AdditionalOwner struct {
Address *AccountAddress `json:"address"`
DOB DOB `json:"dob"`
FirstName string `json:"first_name"`
LastName string `json:"last_name"`
MaidenName string `json:"maiden_name"`
PersonalIDNumberProvided bool `json:"personal_id_number_provided"`
Verification string `json:"verification"`
Address *AccountAddress `json:"address"`
DOB DOB `json:"dob"`
FirstName string `json:"first_name"`
LastName string `json:"last_name"`
MaidenName string `json:"maiden_name"`
PersonalIDNumberProvided bool `json:"personal_id_number_provided"`
Verification IdentityVerification `json:"verification"`
}

// IdentityVerification is the structure for an account's verification.
Expand Down
2 changes: 1 addition & 1 deletion account/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func TestAccountNew(t *testing.T) {

func TestAccountReject(t *testing.T) {
account, err := Reject("acct_123", &stripe.AccountRejectParams{
Reason: stripe.String(stripe.AccountRejectReasonFraud),
Reason: stripe.String(string(stripe.AccountRejectReasonFraud)),
})
assert.Nil(t, err)
assert.NotNil(t, account)
Expand Down
183 changes: 182 additions & 1 deletion account_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,104 @@ func TestAccountUnmarshal(t *testing.T) {
},
},
},
"legal_entity": map[string]interface{}{
"additional_owners": []map[string]interface{}{
{
"address": map[string]interface{}{
"city": "city1",
"country": "US",
"line1": "line1",
"line2": "line2",
"postal_code": "90210",
"state": "CA",
},
"dob": map[string]interface{}{
"day": 1,
"month": 1,
"year": 1970,
},
"first_name": "First",
"last_name": "Last",
"maiden_name": "Maiden",
"personal_id_number_provided": true,
"verification": map[string]interface{}{
"details": "details",
"details_code": "failed_other",
"document": "file_123",
"document_back": "file_234",
"status": "pending",
},
},
{
"address": map[string]interface{}{
"city": "city2",
},
"dob": map[string]interface{}{
"day": 1,
"month": 1,
"year": 1960,
},
"first_name": "First2",
},
},
"address": map[string]interface{}{
"city": "city-3",
"country": "US",
"line1": "line1-3",
"line2": "line2-3",
"postal_code": "91111",
"state": "CA",
},
"business_tax_id_provided": true,
"dob": map[string]interface{}{
"day": 1,
"month": 1,
"year": 1980,
},
"first_name": "John",
"last_name": "Doe",
"maiden_name": "Maiden",
"personal_address": map[string]interface{}{
"city": "personal_city1",
"country": "US",
"line1": "personal_line1",
"line2": "personal_line2",
"postal_code": "90210",
"state": "CA",
},
"personal_id_number_provided": true,
"ssn_last_4_provided": true,
"type": "company",
"verification": map[string]interface{}{
"details": "details",
"details_code": "failed_other",
"document": "file_345",
"document_back": "file_456",
"status": "unverified",
},
},
"metadata": map[string]interface{}{
"key1": "value1",
"key2": "value2",
},
"payout_schedule": map[string]interface{}{
"delay_days": 2,
"interval": "weekly",
},
"tos_acceptance": map[string]interface{}{
"date": 1528573382,
"ip": "127.0.0.1",
"user_agent": "user agent",
},
"type": "custom",
"verification": map[string]interface{}{
"disabled_reason": "fields_needed",
"due_by": 1528573382,
"fields_needed": []interface{}{
Copy link
Contributor Author

@remi-stripe remi-stripe Jun 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😓

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh god, haha.

"legal_entity.verification.document",
"legal_entity.business_name",
},
},
}

bytes, err := json.Marshal(&accountData)
Expand All @@ -57,13 +154,97 @@ func TestAccountUnmarshal(t *testing.T) {
err = json.Unmarshal(bytes, &account)
assert.NoError(t, err)

assert.Equal(t, AccountTypeCustom, account.Type)
assert.Equal(t, "acct_123", account.ID)

assert.Equal(t, "value1", account.Metadata["key1"])
assert.Equal(t, "value2", account.Metadata["key2"])

assert.Equal(t, int64(2), account.PayoutSchedule.DelayDays)
assert.Equal(t, PayoutIntervalWeekly, account.PayoutSchedule.Interval)

assert.Equal(t, AccountTypeCustom, account.Type)

assert.Equal(t, int64(1528573382), account.TOSAcceptance.Date)
assert.Equal(t, "127.0.0.1", account.TOSAcceptance.IP)
assert.Equal(t, "user agent", account.TOSAcceptance.UserAgent)

assert.Equal(t, IdentityVerificationDisabledReasonFieldsNeeded, account.Verification.DisabledReason)
assert.Equal(t, int64(1528573382), account.Verification.DueBy)
assert.Equal(t, 2, len(account.Verification.FieldsNeeded))
assert.Equal(t, "legal_entity.verification.document", account.Verification.FieldsNeeded[0])
assert.Equal(t, "legal_entity.business_name", account.Verification.FieldsNeeded[1])

// Assert ExternalAccounts are fully deserialized
assert.Equal(t, true, account.ExternalAccounts.HasMore)

assert.Equal(t, 2, len(account.ExternalAccounts.Data))
assert.Equal(t, "ba_123", account.ExternalAccounts.Data[0].ID)
assert.Equal(t, "card_123", account.ExternalAccounts.Data[1].ID)

// Ensure LegalEntity is fully deserialized
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's definitely a little subjective :$

assert.NotNil(t, account.LegalEntity)

assert.Equal(t, 2, len(account.LegalEntity.AdditionalOwners))
assert.Equal(t, "city1", account.LegalEntity.AdditionalOwners[0].Address.City)
assert.Equal(t, "US", account.LegalEntity.AdditionalOwners[0].Address.Country)
assert.Equal(t, "line1", account.LegalEntity.AdditionalOwners[0].Address.Line1)
assert.Equal(t, "line2", account.LegalEntity.AdditionalOwners[0].Address.Line2)
assert.Equal(t, "90210", account.LegalEntity.AdditionalOwners[0].Address.PostalCode)
assert.Equal(t, "CA", account.LegalEntity.AdditionalOwners[0].Address.State)
assert.Equal(t, int64(1), account.LegalEntity.AdditionalOwners[0].DOB.Day)
assert.Equal(t, int64(1), account.LegalEntity.AdditionalOwners[0].DOB.Month)
assert.Equal(t, int64(1970), account.LegalEntity.AdditionalOwners[0].DOB.Year)
assert.Equal(t, "First", account.LegalEntity.AdditionalOwners[0].FirstName)
assert.Equal(t, "Last", account.LegalEntity.AdditionalOwners[0].LastName)
assert.Equal(t, "Maiden", account.LegalEntity.AdditionalOwners[0].MaidenName)
assert.True(t, account.LegalEntity.AdditionalOwners[0].PersonalIDNumberProvided)
assert.Equal(t, "details", account.LegalEntity.AdditionalOwners[0].Verification.Details)
assert.Equal(t, IdentityVerificationDetailsCodeFailedOther, account.LegalEntity.AdditionalOwners[0].Verification.DetailsCode)
assert.Equal(t, "file_123", account.LegalEntity.AdditionalOwners[0].Verification.Document.ID)
assert.Equal(t, "file_234", account.LegalEntity.AdditionalOwners[0].Verification.DocumentBack.ID)
assert.Equal(t, IdentityVerificationStatusPending, account.LegalEntity.AdditionalOwners[0].Verification.Status)

assert.Equal(t, "city2", account.LegalEntity.AdditionalOwners[1].Address.City)
assert.Equal(t, "First2", account.LegalEntity.AdditionalOwners[1].FirstName)

assert.Equal(t, "city1", account.LegalEntity.AdditionalOwners[0].Address.City)
assert.Equal(t, "US", account.LegalEntity.AdditionalOwners[0].Address.Country)
assert.Equal(t, "line1", account.LegalEntity.AdditionalOwners[0].Address.Line1)
assert.Equal(t, "line2", account.LegalEntity.AdditionalOwners[0].Address.Line2)
assert.Equal(t, "90210", account.LegalEntity.AdditionalOwners[0].Address.PostalCode)
assert.Equal(t, "CA", account.LegalEntity.AdditionalOwners[0].Address.State)

assert.Equal(t, "city-3", account.LegalEntity.Address.City)
assert.Equal(t, "US", account.LegalEntity.Address.Country)
assert.Equal(t, "line1-3", account.LegalEntity.Address.Line1)
assert.Equal(t, "line2-3", account.LegalEntity.Address.Line2)
assert.Equal(t, "91111", account.LegalEntity.Address.PostalCode)
assert.Equal(t, "CA", account.LegalEntity.Address.State)

assert.True(t, account.LegalEntity.BusinessTaxIDProvided)
assert.Equal(t, int64(1), account.LegalEntity.DOB.Day)
assert.Equal(t, int64(1), account.LegalEntity.DOB.Month)
assert.Equal(t, int64(1980), account.LegalEntity.DOB.Year)
assert.True(t, account.LegalEntity.PersonalIDNumberProvided)
assert.Equal(t, "John", account.LegalEntity.FirstName)
assert.Equal(t, "Doe", account.LegalEntity.LastName)
assert.Equal(t, "Maiden", account.LegalEntity.MaidenName)

assert.Equal(t, "personal_city1", account.LegalEntity.PersonalAddress.City)
assert.Equal(t, "US", account.LegalEntity.PersonalAddress.Country)
assert.Equal(t, "personal_line1", account.LegalEntity.PersonalAddress.Line1)
assert.Equal(t, "personal_line2", account.LegalEntity.PersonalAddress.Line2)
assert.Equal(t, "90210", account.LegalEntity.PersonalAddress.PostalCode)
assert.Equal(t, "CA", account.LegalEntity.PersonalAddress.State)

assert.True(t, account.LegalEntity.PersonalIDNumberProvided)
assert.True(t, account.LegalEntity.SSNLast4Provided)
assert.Equal(t, LegalEntityTypeCompany, account.LegalEntity.Type)

assert.Equal(t, IdentityVerificationDetailsCodeFailedOther, account.LegalEntity.Verification.DetailsCode)
assert.Equal(t, "file_345", account.LegalEntity.Verification.Document.ID)
assert.Equal(t, "file_456", account.LegalEntity.Verification.DocumentBack.ID)
assert.Equal(t, IdentityVerificationStatusUnverified, account.LegalEntity.Verification.Status)
}

func TestPayoutScheduleParams_AppendTo(t *testing.T) {
Expand Down