From 41275eecf7563d12ef02e68d1e00dd0785d8f4ff Mon Sep 17 00:00:00 2001 From: Nikola Begedin Date: Mon, 5 Dec 2016 12:01:26 +0100 Subject: [PATCH] Move user updates into a transaction, which also manages associated platform and connect customer updates --- lib/code_corps/services/user_service.ex | 99 +++++++++++++++++++ .../stripe_service/stripe_connect_customer.ex | 13 ++- .../stripe_platform_customer.ex | 19 +++- lib/code_corps/stripe_testing/customer.ex | 21 ++++ .../code_corps/services/user_service_test.exs | 83 ++++++++++++++++ .../stripe_platform_customer_service_test.exs | 21 ++++ test/support/factories.ex | 3 +- web/controllers/user_controller.ex | 35 ++++--- web/models/stripe_connect_customer.ex | 5 + web/models/stripe_platform_customer.ex | 8 ++ 10 files changed, 286 insertions(+), 21 deletions(-) create mode 100644 lib/code_corps/services/user_service.ex create mode 100644 test/lib/code_corps/services/user_service_test.exs create mode 100644 test/lib/code_corps/stripe_service/stripe_platform_customer_service_test.exs diff --git a/lib/code_corps/services/user_service.ex b/lib/code_corps/services/user_service.ex new file mode 100644 index 000000000..4b8c98e70 --- /dev/null +++ b/lib/code_corps/services/user_service.ex @@ -0,0 +1,99 @@ +defmodule CodeCorps.Services.UserService do + @moduledoc """ + Handles CRUD operations for users. + + When operations happen on `CodeCorps.User`, we need to make sure changes + are propagated to related records, ex., `CodeCorps.StripePlatformCustomer` or + `CodeCorps.StripeConnectCustomer` + + """ + + alias CodeCorps.{Repo, StripeConnectCustomer, StripePlatformCustomer, User} + alias CodeCorps.StripeService.{StripeConnectCustomerService,StripePlatformCustomerService} + alias Ecto.Changeset + alias Ecto.Multi + + @doc """ + Updates a `CodeCorps.User` record and, if necessary, associated + `CodeCorps.StripePlatformCustomer` and `CodeCorps.StripeConnectCustomer` records. + + These related records inherit the email field from the user, + so they need to be kept in sync, both locally, and on the Stripe platform. + + Returns one of + * `{:ok, %CodeCorps.User{}, :nothing_to_update, :nothing_to_update}` + * `{:ok, %CodeCorps.User{}, %CodeCorps.StripePlatformCustomer{}, :nothing_to_update}` + * `{:ok, %CodeCorps.User{}, %CodeCorps.StripePlatformCustomer{}, %CodeCorps.StripeConnectCustomer{}}` + * `{:error, %Ecto.Changeset{}}` + * `{:error, :unhandled}` + + """ + def update(%User{} = user, attributes) do + changeset = user |> User.update_changeset(attributes) + do_update(changeset) + end + + defp do_update(%Changeset{changes: %{email: _email}} = changeset) do + multi = + Multi.new + |> Multi.update(:update_user, changeset) + |> Multi.run(:update_platform_customer, &update_platform_customer/1) + |> Multi.run(:update_connect_customers, &update_connect_customers/1) + + case Repo.transaction(multi) do + {:ok, %{ + update_user: user, + update_platform_customer: update_platform_customer_result, + update_connect_customers: update_connect_customers_results + }} -> + {:ok, user, update_platform_customer_result, update_connect_customers_results} + {:error, :update_user, %Ecto.Changeset{} = changeset, %{}} -> + {:error, changeset} + {:error, _failed_operation, _failed_value, _changes_so_far} -> + {:error, :unhandled} + other -> + IO.inspect(other, pretty: true) + end + end + + defp do_update(%Changeset{} = changeset) do + with {:ok, user} <- Repo.update(changeset) do + {:ok, user, :nothing_to_update, :nothing_to_update} + else + {:error, changeset} -> {:error, changeset} + _ -> {:error, :unhandled} + end + end + + defp update_platform_customer(%{update_user: %User{id: user_id, email: email}}) do + StripePlatformCustomer + |> Repo.get_by(user_id: user_id) + |> do_update_platform_customer(%{email: email}) + end + + defp do_update_platform_customer(nil, _), do: {:ok, :nothing_to_update} + defp do_update_platform_customer(%StripePlatformCustomer{} = stripe_platform_customer, attributes) do + StripePlatformCustomerService.update(stripe_platform_customer, attributes) + end + + defp update_connect_customers(%{update_platform_customer: :nothing_to_update}), do: {:ok, :nothing_to_update} + + defp update_connect_customers(%{update_platform_customer: %StripePlatformCustomer{email: email} = stripe_platform_customer}) do + case do_update_connect_customers(stripe_platform_customer, %{email: email}) do + [_h | _t] = results -> {:ok, results} + [] -> {:ok, :nothing_to_update} + end + end + + defp do_update_connect_customers(stripe_platform_customer, attributes) do + stripe_platform_customer + |> Repo.preload([stripe_connect_customers: :stripe_connect_account]) + |> Map.get(:stripe_connect_customers) + |> Enum.map(&do_update_connect_customer(&1, attributes)) + end + + defp do_update_connect_customer(%StripeConnectCustomer{} = stripe_connect_customer, attributes) do + stripe_connect_customer + |> StripeConnectCustomerService.update(attributes) + end +end diff --git a/lib/code_corps/stripe_service/stripe_connect_customer.ex b/lib/code_corps/stripe_service/stripe_connect_customer.ex index 5f10ff9b0..e35e0cc9e 100644 --- a/lib/code_corps/stripe_service/stripe_connect_customer.ex +++ b/lib/code_corps/stripe_service/stripe_connect_customer.ex @@ -19,11 +19,16 @@ defmodule CodeCorps.StripeService.StripeConnectCustomerService do end end + def update(%StripeConnectCustomer{stripe_connect_account: connect_account} = connect_customer, attributes) do + @api.Customer.update(connect_customer.id_from_stripe, attributes, connect_account: connect_account.id_from_stripe) + end + defp create(%StripePlatformCustomer{} = platform_customer, %StripeConnectAccount{} = connect_account) do attributes = platform_customer |> create_non_stripe_attributes(connect_account) + stripe_attributes = create_stripe_attributes(platform_customer) with {:ok, customer} <- - @api.Customer.create(%{}, connect_account: connect_account.id_from_stripe), + @api.Customer.create(stripe_attributes, connect_account: connect_account.id_from_stripe), {:ok, params} <- StripeConnectCustomerAdapter.to_params(customer, attributes) do @@ -48,4 +53,10 @@ defmodule CodeCorps.StripeService.StripeConnectCustomerService do |> Map.put(:stripe_connect_account_id, connect_account.id) |> keys_to_string end + + defp create_stripe_attributes(platform_customer) do + platform_customer + |> Map.from_struct + |> Map.take([:email]) + end end diff --git a/lib/code_corps/stripe_service/stripe_platform_customer.ex b/lib/code_corps/stripe_service/stripe_platform_customer.ex index 57dd3522d..3a9c7d4b3 100644 --- a/lib/code_corps/stripe_service/stripe_platform_customer.ex +++ b/lib/code_corps/stripe_service/stripe_platform_customer.ex @@ -2,11 +2,13 @@ defmodule CodeCorps.StripeService.StripePlatformCustomerService do alias CodeCorps.Repo alias CodeCorps.StripeService.Adapters.StripePlatformCustomerAdapter alias CodeCorps.StripePlatformCustomer + alias CodeCorps.User @api Application.get_env(:code_corps, :stripe) def create(attributes) do - with {:ok, customer} <- @api.Customer.create(attributes), + with stripe_attributes <- build_stripe_attributes(attributes), + {:ok, customer} <- @api.Customer.create(stripe_attributes), {:ok, params} <- StripePlatformCustomerAdapter.to_params(customer, attributes) do %StripePlatformCustomer{} @@ -14,4 +16,19 @@ defmodule CodeCorps.StripeService.StripePlatformCustomerService do |> Repo.insert end end + + def update(%StripePlatformCustomer{id_from_stripe: id_from_stripe} = customer, attributes) do + with {:ok, stripe_customer} <- @api.Customer.update(id_from_stripe, attributes), + {:ok, params} <- StripePlatformCustomerAdapter.to_params(stripe_customer, attributes) + do + customer + |> StripePlatformCustomer.update_changeset(params) + |> Repo.update + end + end + + defp build_stripe_attributes(%{"user_id" => user_id}) do + %User{email: email} = Repo.get(User, user_id) + %{email: email} + end end diff --git a/lib/code_corps/stripe_testing/customer.ex b/lib/code_corps/stripe_testing/customer.ex index 25b1416f7..890792581 100644 --- a/lib/code_corps/stripe_testing/customer.ex +++ b/lib/code_corps/stripe_testing/customer.ex @@ -3,6 +3,10 @@ defmodule CodeCorps.StripeTesting.Customer do {:ok, do_create(map)} end + def update(id, map, _opts \\ []) do + {:ok, do_update(id, map)} + end + defp do_create(_) do {:ok, created} = DateTime.from_unix(1479472835) @@ -19,4 +23,21 @@ defmodule CodeCorps.StripeTesting.Customer do metadata: %{} } end + + defp do_update(id, map) do + {:ok, created} = DateTime.from_unix(1479472835) + + %Stripe.Customer{ + id: id, + account_balance: 0, + created: created, + currency: "usd", + default_source: nil, + delinquent: false, + description: nil, + email: map.email, + livemode: false, + metadata: %{} + } + end end diff --git a/test/lib/code_corps/services/user_service_test.exs b/test/lib/code_corps/services/user_service_test.exs new file mode 100644 index 000000000..66059c115 --- /dev/null +++ b/test/lib/code_corps/services/user_service_test.exs @@ -0,0 +1,83 @@ +defmodule CodeCorps.Services.UserServiceTest do + use ExUnit.Case, async: true + + use CodeCorps.ModelCase + + alias CodeCorps.StripePlatformCustomer + alias CodeCorps.Services.UserService + + describe "update/1" do + test "it just updates the user if there is nothing associated to update" do + user = insert(:user, email: "mail@mail.com", first_name: "Joe") + + {:ok, user, :nothing_to_update, :nothing_to_update} + = UserService.update(user, %{email: "changed@mail.com"}) + + assert user.email == "changed@mail.com" + assert user.first_name == "Joe" + end + + test "it returns an {:error, changeset} if there are validation errors with the user" do + user = insert(:user, email: "mail@mail.com") + {:error, changeset} = UserService.update(user, %{email: ""}) + + refute changeset.valid? + end + + test "it just updates the user if the changeset does not contain an email" do + user = insert(:user, email: "mail@mail.com") + stripe_platform_customer = insert(:stripe_platform_customer, email: "mail@mail.com", user: user) + + {:ok, user, :nothing_to_update, :nothing_to_update} + = UserService.update(user, %{first_name: "Mark"}) + + assert user.first_name == "Mark" + assert user.email == "mail@mail.com" + + stripe_platform_customer = Repo.get(StripePlatformCustomer, stripe_platform_customer.id) + + assert stripe_platform_customer.email == "mail@mail.com" + end + + test "it also updates the associated platform customer if there is one" do + user = insert(:user, email: "mail@mail.com") + platform_customer = insert(:stripe_platform_customer, user: user) + + {:ok, user, %StripePlatformCustomer{}, :nothing_to_update} + = UserService.update(user, %{email: "changed@mail.com"}) + + assert user.email == "changed@mail.com" + + platform_customer = Repo.get(StripePlatformCustomer, platform_customer.id) + + assert platform_customer.email == "changed@mail.com" + end + + test "it also updates the associated connect customers if there are any" do + user = insert(:user, email: "mail@mail.com") + + platform_customer = %{id_from_stripe: platform_customer_id} + = insert(:stripe_platform_customer, user: user) + + [connect_customer_1, connect_customer_2] = + insert_pair(:stripe_connect_customer, stripe_platform_customer: platform_customer) + + {:ok, user, %StripePlatformCustomer{}, connect_updates} = UserService.update(user, %{email: "changed@mail.com"}) + + assert user.email == "changed@mail.com" + + platform_customer = Repo.get_by(StripePlatformCustomer, id_from_stripe: platform_customer_id) + assert platform_customer.email == "changed@mail.com" + + [ + {:ok, %Stripe.Customer{} = stripe_record_1}, + {:ok, %Stripe.Customer{} = stripe_record_2} + ] = connect_updates + + assert stripe_record_1.id == connect_customer_1.id_from_stripe + assert stripe_record_1.email == "changed@mail.com" + assert stripe_record_2.id == connect_customer_2.id_from_stripe + assert stripe_record_2.email == "changed@mail.com" + end + end +end diff --git a/test/lib/code_corps/stripe_service/stripe_platform_customer_service_test.exs b/test/lib/code_corps/stripe_service/stripe_platform_customer_service_test.exs new file mode 100644 index 000000000..249e44ea0 --- /dev/null +++ b/test/lib/code_corps/stripe_service/stripe_platform_customer_service_test.exs @@ -0,0 +1,21 @@ +defmodule CodeCorps.StripeService.StripePlatformCustomerServiceTest do + use CodeCorps.ModelCase + + alias CodeCorps.StripeService.StripePlatformCustomerService + + describe "update/2" do + test "performs update" do + customer = insert(:stripe_platform_customer) + {:ok, customer} = StripePlatformCustomerService.update(customer, %{email: "mail@mail.com"}) + assert customer.email == "mail@mail.com" + + # TODO: Figure out testing if stripe API request was made + end + + test "returns changeset with validation errors if there is an issue" do + customer = insert(:stripe_platform_customer) + {:error, changeset} = StripePlatformCustomerService.update(customer, %{email: nil}) + refute changeset.valid? + end + end +end diff --git a/test/support/factories.ex b/test/support/factories.ex index 39b465c5e..61ffb3a6c 100644 --- a/test/support/factories.ex +++ b/test/support/factories.ex @@ -113,7 +113,8 @@ defmodule CodeCorps.Factories do def stripe_connect_customer_factory do %CodeCorps.StripeConnectCustomer{ - id_from_stripe: sequence(:id_from_stripe, &"stripe_id_#{&1}") + id_from_stripe: sequence(:id_from_stripe, &"stripe_id_#{&1}"), + stripe_connect_account: build(:stripe_connect_account) } end diff --git a/web/controllers/user_controller.ex b/web/controllers/user_controller.ex index fcd27755c..7e921e7f3 100644 --- a/web/controllers/user_controller.ex +++ b/web/controllers/user_controller.ex @@ -5,6 +5,7 @@ defmodule CodeCorps.UserController do import CodeCorps.Helpers.Query, only: [id_filter: 2] alias CodeCorps.User + alias CodeCorps.Services.UserService plug :load_and_authorize_resource, model: User, only: [:update] plug JaResource @@ -14,27 +15,25 @@ defmodule CodeCorps.UserController do end def handle_create(conn, attributes) do - %User{} - |> User.registration_changeset(attributes) - |> Repo.insert - |> login(conn) - |> track_signup + with {:ok, user} <- %User{} |> User.registration_changeset(attributes) |> Repo.insert, + conn <- login(user, conn) + do + CodeCorps.Analytics.Segment.track({:ok, user}, :signed_up, conn) + else + {:error, changeset} -> changeset + end end - defp login({:error, changeset}, conn), do: {:error, changeset, conn} - defp login({:ok, model}, conn) do - {:ok, model, conn |> Plug.Conn.assign(:current_user, model)} - end - - defp track_signup({status, model_or_changeset, conn}) do - CodeCorps.Analytics.Segment.track({status, model_or_changeset}, :signed_up, conn) - end + defp login(user, conn), do: Plug.Conn.assign(conn, :current_user, user) - def handle_update(conn, model, attributes) do - model - |> User.update_changeset(attributes) - |> Repo.update - |> CodeCorps.Analytics.Segment.track(:updated_profile, conn) + def handle_update(conn, record, attributes) do + with {:ok, user, _platform_customer_updates, _connect_customer_updates} <- UserService.update(record, attributes) + do + {:ok, user} |> CodeCorps.Analytics.Segment.track(:updated_profile, conn) + else + {:error, changeset} -> changeset + {:error, :unhandled} -> conn + end end def email_available(conn, %{"email" => email}) do diff --git a/web/models/stripe_connect_customer.ex b/web/models/stripe_connect_customer.ex index a0d49baec..f4db64547 100644 --- a/web/models/stripe_connect_customer.ex +++ b/web/models/stripe_connect_customer.ex @@ -19,4 +19,9 @@ defmodule CodeCorps.StripeConnectCustomer do |> unique_constraint(:id_from_stripe) |> unique_constraint(:stripe_connect_account_id, name: :index_projects_on_user_id_role_id) end + + def update_changeset(struct, params) do + struct + |> cast(params, [:email]) + end end diff --git a/web/models/stripe_platform_customer.ex b/web/models/stripe_platform_customer.ex index 7c1cafdb6..50252670e 100644 --- a/web/models/stripe_platform_customer.ex +++ b/web/models/stripe_platform_customer.ex @@ -10,6 +10,8 @@ defmodule CodeCorps.StripePlatformCustomer do belongs_to :user, CodeCorps.User + has_many :stripe_connect_customers, CodeCorps.StripeConnectCustomer + timestamps() end @@ -19,4 +21,10 @@ defmodule CodeCorps.StripePlatformCustomer do |> validate_required([:id_from_stripe, :user_id]) |> assoc_constraint(:user) end + + def update_changeset(struct, params) do + struct + |> cast(params, [:email]) + |> validate_required([:email]) + end end