-
Notifications
You must be signed in to change notification settings - Fork 86
Code spike for switch to managed accounts #552
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.
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
Conversation
Hadn't had a chance to comment on this, but may want to consider reducing scope by pulling off some of the individual tasks in the milestone now that they're around. |
@@ -3,15 +3,15 @@ defmodule CodeCorps.StripeConnectAccountController do | |||
use JaResource | |||
|
|||
alias CodeCorps.StripeConnectAccount | |||
alias CodeCorps.StripeService.StripeConnectAccountService | |||
alias CodeCorps.StripeService.StripeManagedConnectAccountService |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would just keep this StripeConnectAccountService
personally. managed
is just a param.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was just a temporary thing so I can work on a new one while still having the old one for reference.
last_name: last_name, | ||
ssn_last_4: ssn_last4, | ||
type: recipient_type, | ||
# TODO: Decide on setting legal_entity.address or legal_entity.personal_address |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is mentioned in that Ember issue.
|> CodeCorps.Repo.insert | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rewrote some of this with pseudocode in #575. I would consider taking the pattern matching of that function. Would also just use create_changeset/2
rather than a special one for managed
, per comment above.
:managed, :id_from_stripe, :charges_enabled, :transfers_enabled | ||
] | ||
|
||
def managed_create_changeset(struct, params) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think we want distinction between create
and managed_create
. Also, create
fields will be just country
and managed
, where the latter is always true
.
f04a125
to
1af50be
Compare
I'm going to close this since I think it's pretty out of date. |
What's in this PR?
This is a code spike to make the switch to managed stripe accounts
What I got
StripeService.StripeManagedConnectAccountService.create
NOTES:
Haven't added/changed any database fields yet. Not sure which ones will be needed. I use those we have, others are virtual for now.
References