Add verification_document_id handling to StripeAccountController by joshsmith · Pull Request #581 · code-corps/code-corps-api

joshsmith · 2016-12-17T07:20:17Z

What's in this PR?

Fixes #570 when done.

joshsmith · 2016-12-17T07:22:01Z

web/controllers/stripe_file_upload_controller.ex

+
+  defp handle_create_result({:error, %Ecto.Changeset{} = changeset}, _conn), do: changeset
+  defp handle_create_result({:ok, %StripeFileUpload{}} = result, conn) do
+    result |> CodeCorps.Analytics.Segment.track(:created, conn)


Tracking this is a good idea, I think!

Oh, you were asking about the error. I don't think we'll track that.

joshsmith · 2016-12-17T07:24:36Z

lib/code_corps/stripe_service/adapters/stripe_file_upload.ex

+  Converts a map received from the Stripe API into a map that can be used
+  to create a `CodeCorps.StripeFileUpload` record
+  """
+  @stripe_attributes [:file, :purpose, :id]


I think we want to pull in all of Stripe's attributes for the file upload here:

{ "id": "file_19RAITKUlS7mA39fpztNbjpd", "object": "file_upload", "created": 1481850845, "purpose": "identity_document", "size": 238253, "type": "jpg" }

So this would mean id, created, purpose, size, and type (not object).

joshsmith · 2016-12-17T07:26:14Z

To fix the error on Circle, you'll need to create an app/lib/code_corps/stripe_testing/file_upload.ex file similar to the other files there. That's just a mock, and you can use the example JSON I provided above.

joshsmith · 2016-12-17T07:28:09Z

lib/code_corps/stripe_service/adapters/stripe_file_upload.ex

+    {:ok, result}
+  end
+
+  @non_stripe_attributes ["project_id"]


I think this would be stripe_connect_account_id.

joshsmith · 2016-12-17T23:59:27Z

Unfortunately, Stripe's documentation was really unclear on the fact that you can upload files via the browser.

This PR is going to change to the following:

`StripeAccountController`:

PUT with a verification_document_id
Pattern match against the verification_document_id in handle_update
handle_update will call StripeAccountService.add_verification_document()

`StripeAccountService.add_verification_document()`

`with`

Pattern match against the verification_document_id and the Stripe Account information
Use verification_document_id to fetch the file with Stripe.FileUpload.retrieve (to ensure proper secure data coming from the client)
Call Stripe.Account.update with verification[document] set to the id from %Stripe.FileUpload{}

`do`

Inside a transaction:

Use the returned %Stripe.FileUpload{} to insert a %CodeCorps.FileUpload{}
Use the returned %Stripe.Account{} to update our %CodeCorps.StripeAccount{}

Additional changes

We're going to need to:

Remove the StripeFileUploadController and its test, along with the line from the router

Sorry about these changes @snewcomer. Wish I had caught this earlier since it would have saved lots of hours for many people involved.

joshsmith · 2016-12-18T00:23:23Z

test/controllers/stripe_file_upload_controller_test.exs

@@ -0,0 +1,30 @@
+defmodule CodeCorps.StripeFileUploadControllerTest do


This file needs deleted.

joshsmith · 2016-12-18T00:23:28Z

web/controllers/stripe_file_upload_controller.ex

@@ -0,0 +1,21 @@
+defmodule CodeCorps.StripeFileUploadController do


This file needs deleted.

joshsmith · 2016-12-18T00:23:36Z

web/router.ex

    resources "/stripe-connect-accounts", StripeConnectAccountController, only: [:show, :create]
    resources "/stripe-connect-plans", StripeConnectPlanController, only: [:show, :create]
    resources "/stripe-connect-subscriptions", StripeConnectSubscriptionController, only: [:show, :create]
+    resources "/stripe-file-uploads", StripeFileUploadController, only: [:create]


This line needs deleted.

joshsmith · 2016-12-18T00:24:00Z

lib/code_corps/stripe_service/stripe_file_upload.ex

+
+  @api Application.get_env(:code_corps, :stripe)
+
+  def create(%{"file" => file, "purpose" => purpose, "stripe_connect_account_id" => stripe_connect_account_id} = attributes) do


We'll remove create and replace with retrieve.

joshsmith · 2016-12-18T00:32:21Z

This is partly blocked by #572 now. Can be resolved by creating a mock for the update in StripeTesting.

joshsmith · 2016-12-19T03:30:58Z

lib/code_corps/stripe_service/stripe_connect_account.ex

+
+  def add_vertification_document(%{"verification_document_id" => verification_document_id, "stripe_connect_account" => connect_account} = attributes) do
+    with {:ok, %Stripe.FileUpload{} = file} <- Stripe.FileUpload.retrieve(verification_document_id),
+         {:ok, %Stripe.Account{} = account} <- Stripe.Account.update(%{verification_document_id: file.id})


Stripe.FileUpload => @api.FileUpload

Stripe.Account => @api.Account

joshsmith · 2016-12-19T03:32:02Z

lib/code_corps/stripe_service/stripe_connect_account.ex

+      |> CodeCorps.StripeAccount.update
+
+      file_upload
+      |> CodeCorps.FileUpload.create_identity_document_changeset


I assume you're looping back but both these should be wrapped in a transaction.

joshsmith · 2016-12-20T08:03:10Z

No longer blocked.

begedin · 2016-12-20T16:10:23Z

@joshsmith This PR seems to mostly be dealing with the file upload controller. I'll implement adding a verification_document_id to a StripeConnectAccount in a separate PR. I think it will be simpler that way.

begedin · 2016-12-20T16:20:22Z

Was a bit too quick with the previous comment. Quick glance had me thinking the file upload controller is still what's the focus of the PR.

I see that this part has been cleaned up somewhat. I'll just take over from this pr @snewcomer. Hope you don't mind. Good work so far.

begedin · 2016-12-21T15:24:48Z

@joshsmith I renamed the document id field to legal_entity_verification_document to accomodate changes made in #588.

#588 should be merged first, then conflicts created by that resolved here. Until then, we should consider it blocked.

joshsmith commented Dec 17, 2016

View reviewed changes

joshsmith added this to the Launch Donations milestone Dec 17, 2016

joshsmith assigned snewcomer Dec 17, 2016

joshsmith added the needs submitter response label Dec 17, 2016

joshsmith changed the title ~~[FILE UPLOAD CONTROLLER] WIP~~ Add verification_document_id handling to StripeAccountController Dec 18, 2016

joshsmith added the needs work label Dec 18, 2016

joshsmith commented Dec 18, 2016

View reviewed changes

joshsmith added the blocked label Dec 18, 2016

joshsmith commented Dec 19, 2016

View reviewed changes

joshsmith removed the blocked label Dec 20, 2016

begedin self-assigned this Dec 20, 2016

begedin force-pushed the file_upload_controller branch 2 times, most recently from 16f7be5 to c1395c5 Compare December 20, 2016 17:30

begedin added blocked needs code review and removed needs submitter response needs work labels Dec 21, 2016

begedin removed this from the Launch Donations milestone Dec 21, 2016

begedin modified the milestone: Launch Donations Dec 21, 2016

joshsmith force-pushed the file_upload_controller branch from 5d37d90 to 0868928 Compare December 22, 2016 00:09

Add verification_document_id handling to StripeAccountController

4c57465

joshsmith force-pushed the file_upload_controller branch from 0868928 to 4c57465 Compare December 22, 2016 00:11

joshsmith added ready to merge and removed blocked needs code review labels Dec 22, 2016

joshsmith merged commit 65488b4 into develop Dec 22, 2016

joshsmith deleted the file_upload_controller branch December 22, 2016 00:15

		@@ -0,0 +1,30 @@
		defmodule CodeCorps.StripeFileUploadControllerTest do

		@@ -0,0 +1,21 @@
		defmodule CodeCorps.StripeFileUploadController do


		@api Application.get_env(:code_corps, :stripe)

		def create(%{"file" => file, "purpose" => purpose, "stripe_connect_account_id" => stripe_connect_account_id} = attributes) do

Conversation

joshsmith commented Dec 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's in this PR?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshsmith commented Dec 17, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshsmith commented Dec 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

StripeAccountController:

StripeAccountService.add_verification_document()

with

do

Additional changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshsmith commented Dec 18, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshsmith commented Dec 20, 2016

Uh oh!

begedin commented Dec 20, 2016

Uh oh!

begedin commented Dec 20, 2016

Uh oh!

begedin commented Dec 21, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

joshsmith commented Dec 17, 2016 •

edited

Loading

joshsmith commented Dec 17, 2016 •

edited

Loading

`StripeAccountController`:

`StripeAccountService.add_verification_document()`

`with`

`do`