-
Notifications
You must be signed in to change notification settings - Fork 86
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
Hook up stripe external account with connect account, expose bank_account info #604
Hook up stripe external account with connect account, expose bank_account info #604
Conversation
@paulsullivanjr In the interest of wrapping this up and launching the feature as soon as possible, I hope you don't mind my taking this PR over. Good work up to this point! @begedin makes sense. Definitely didn't want to hold you guys up from finishing! |
daf283d
to
c50aef8
Compare
do | ||
%StripeExternalAccount{} | ||
|> StripeExternalAccount.changeset(params) | ||
|> Repo.insert | ||
end | ||
end | ||
|
||
defp get_connect_account(account_id_from_stripe) 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.
The reason we do this at the service instead of the adapter level is because we want the service to return a useful error response in case the connect account was not found.
Specifically, the service will return {:ok, created_external_account}
if everything is ok, and {:error, :not_found}
if the associated connect account was not found locally.
Due to this behavior, the event handling system will mark the event as errored.
We could technically push it further into the adapter, but the code would be more complicated.
c50aef8
to
cce5203
Compare
…s the two on create
cce5203
to
d5ab57f
Compare
@joshsmith Should be good to go. The relationship is stored on the external account record. My reasoning is that we may end up at one point having a
With that in mind,
makes more sense at the moment There is no chance of us having
so
makes much less sense. |
I think we should also add a |
What's in this PR?
This is in progress and not complete. Initial changes to associated the external account with StripeConnectAccount to provide access to the last 4 digits of the account and routing numbers.
References
Fixes #597