Ecto Multi or Changeset - Business logic validation?

Questions / Help
, ,
1

Context: In my application, projects make offers to users.

I’ve just discovered Ecto.Multi and am really happy as it makes a lot of sense to me in certain circumstances. For example:

If user accepts an offer, then I need to update the offer AND attach user to the project

What I am still unclear on is where I should run the logic that validates other business logic which requires further database queries. For example:

  • whether a user has the correct user type to receive an offer
  • whether the user has any pending offers already
  • whether the newly assigned project role (from the offer) is already taken

Up until now I have kept this logic in the changeset pipeline, and as I have tried to keep changesets pure, it means passing in lots of arguments to the changeset function (eg, all project offers).

I wonder if I should rather be making use of Multi’s for this purpose:

offers.ex context:

def create_offer(params, project, current_user) do
  create_offer_multi =
    Multi.new()
    # run basic validation on the new offer form
    # if offer form is valid, go into calcs
    |> Multi.run(:offer, create_offer_changeset(params, project, current_user))
    |> Ecto.Multi.run(:valid_recipient, fn _repo, %{offer: offer} ->
      # run some validation that ensures recipient user:
      # - can receive an offer and
      # - they don't already have any other pending offers for this project
      Offers.validate_offer_recipient(project, offer)
    end)
    |> Multi.run(:add_project_role, fn _repo, %{offer: offer, valid_recipient: recipient} ->
      # if all successful, add user to project with the given role
      Projects.add_project_role(project, offer, recipient)
    end)

  case Repo.transaction(create_offer_multi) do
    {:ok, %{offer: offer}} -> {:ok, offer}
    error_res -> error_response(:create_offer, error_res)
  end
end

def create_offer_changeset(params, project, current_user) do
  recipient = Accounts.get_user_by_email(params["email"])

  params
  # run pure changeset casting & validation logic on offer
  |> Offer.create_offer_changeset(project)
  |> Ecto.Changeset.put_assoc(:project, project)
  |> Ecto.Changeset.put_assoc(:creator, current_user)
  |> Ecto.Changeset.put_assoc(:recipient, recipient)
  # if offer changeset is valid, then go and run a load of calculations
  # that need to access database and attach them to the changeset
  |> Offer.run_and_merge_calcs(project)
end

I have been working in Elixir/Phoenix for a while, but with no mentor and I am still quite confused:

How should I approach this sort of validation?
Are there any problems with this code? How could it be improved?

Any help greatly appreciated.

2 Likes
2

This is pretty much a great usecase for Ecto.Multi.

As you mentioned, it’s about choosing the best tool for the job, and don’t overuse changesets/multis.

If your multi step returns an error tuple, the entire transaction will rollback, so you could validate each step as you go along. This way you abstract each “step”.

Pros about multis:

  • Super easy to test, you can test each part to ensure it works
  • Super easy to integration test
3

We use Ecto.Multi a lot for long chains of constraint checks. Many checks disqualify themselves and return {:ok, :not_applicable}, and of course there are some that are performed and succeed or find a violation of constraints and return a useful error message.

The main downside is that stack traces become a lot harder to read, and for many cases with chains are preferred for that reason. The choice between the two usually comes down to two things: are we doing database updates along the way (e.g. is it a chain of constraints and database operations), and is the logic complicated enough to span several modules. Ecto.Multi pipelines scales pretty well across thousands of lines of code, you can always just add another constraint and know that the failure will be handled the right way.

We put validations in Changeset functions two, but our rule of thumb there is to only do that for pure functions that operate on data always available in the struct itself (which may include some associations, but its not good to make assumptions about what will always be preloaded, or you will be preloading more than is necessary).

1 Like
4

Thanks a lot for the answers @joshtaylor & @jeremyjh :+1: :pray:

So should I treat the the main insert_offer_changeset as a really simple validation that:

  • All required fields are there
  • Fields are of correct type
  • Fields are correct format

If so, it seems like until now I have been overusing changeset functions.

If I could extend the question slightly to understand how you would write good multi chains:

Scenario: User has an accepted offer (Offer A) for project, and they are receiving a new one (Offer B) for same project.
I need to add a check in that Offer A has an end_date set, and that Offer B has a start date after Offer As end_date.

Would that look like this:

In offers.ex

def create_offer(params, project, current_user) do
  create_offer_multi =
    Multi.new()
    |> Multi.insert(:offer, create_offer_changeset(params, project, current_user))
    |> OfferMultis.valid_start_date()
    |> OfferMultis.valid_recipient(project)
    |> Multi.insert(:add_project_role, fn _repo, %{offer: offer, valid_recipient: recipient} ->
      Projects.add_project_role(project, offer, recipient)
    end)

  case Repo.transaction(create_offer_multi) do
    {:ok, %{offer: offer}} -> {:ok, offer}
    error_res -> error_response(:create_offer, error_res)
  end
end

In offers/multis.ex:

# ensure new offer starts after previous offer ends
def valid_start_date(multi) do
  multi
  |> Ecto.Multi.run(:valid_start_date, fn _repo, %{offer: offer} ->
    %{user_id: user_id, project_id: project_id} = offer
    opts = [project_id: project_id, user_id: user_id, active_on: offer.start_date]

    case Offers.get_offer(opts) do
      nil -> {:ok, recipient}
      %Offer{} = offer -> {:error, {:user_has_active_offer_for_start, offer}}
    end
  end)
end

# check the user has correct user_type and they don't have pending offers
def valid_recipient(multi) do
  multi
  |> Ecto.Multi.run(:valid_recipient, fn _repo, %{offer: offer} ->
    recipient_opts =
      case Accounts.get_user(offer.user_id, user_type: "NORMAL") do
        nil -> [user_id: recipient.id]
        recipient -> [target_email: offer.target_email]
      end

    opts = recipient_opts ++ [project_id: offer.project_id, pending?: true]

    case Offers.get_offer(opts) do
      nil -> {:ok, :user_has_no_pending_offer}
      offer -> {:error, {:user_has_pending_offer, offer}}
    end
  end)
end

I am often unsure where to split these sorts of pieces of logic. Does the above splitting/organisation make sense? or would you rather split all this offer validation into 1, 2 or 3 Multis?

5

Yes this looks reasonable and is along the lines I was suggesting.

1 Like