Is this the correct way of creating multiple items from an array inside a multi?

Hey, i have this code:

Multi.new()
    |> Multi.insert(:poll, poll_changeset(attrs))
    |> Multi.run(:poll_with_hash, fn _repo, %{poll: poll_changeset} ->
      Utils.generate_hash_id_for_item(poll_changeset, "poll")
    end)
    |> Multi.run(:poll_options, fn repo, %{poll: poll} ->
      Enum.each(options, fn option ->
        repo.insert(poll_option_changeset(option))
        |> Utils.generate_hash_id_from_changeset("poll_option")
      end)
      |> case do
        :ok -> {:ok, nil}
        :error -> {:error, "Couldn't insert options!"}
      end
    end)
    |> Repo.transaction()

It is working as I expect it to work, I am just wondering about the part after inserting the options, that I am checking for the result, isn’t there a way that returns what is expected {:ok, value}?

The better way is using Ecto.Multi.merge and build up a to be merged multi within the callback. This allows each db action to be it‘s own step in the multi. Keep in mind that steps can have any term as name, so names like {:insert, index} are perfectly valid.

About your solution. Use at least something different to Enum.each. It simply swallows any return values of the callback and always returns :ok.

4 Likes

Thanks, I ended up with this:

multi =
      Multi.new()
      |> Multi.insert(:poll, poll_changeset(attrs))
      |> Multi.run(:poll_with_hash, fn _repo, %{poll: poll_changeset} ->
        Utils.generate_hash_id_for_item(poll_changeset, "poll")
      end)

    Enum.reduce(Enum.with_index(options), multi, fn option_with_index, multi ->
      {option, index} = option_with_index
      poll_option = String.to_atom("poll_option_#{index}")
      poll_option_with_hash = String.to_atom("poll_option_with_hash_#{index}")

      Multi.merge(multi, fn %{poll: poll} ->
        Multi.new()
        |> Multi.insert(poll_option, poll_option_changeset(poll, option))
        |> Multi.run(poll_option_with_hash, fn _repo, %{^poll_option => poll_option_changeset} ->
          Utils.generate_hash_id_for_item(poll_option_changeset, "poll_option")
        end)
      end)
    end)
    |> Repo.transaction()

If you have the list of options before executing the multi you don’t even need Ecto.Multi.merge. Just append to multi directly.

1 Like

Does that have any noticeable upsides?

Probably not, but I like to preserve Ecto.Multi.merge and ….run for the cases where I need results from prev. multi steps.

2 Likes

I see, thanks!

Wouldn’t that depend on the internal representation of the Ecto.Multi struct (which is not part of its public API)?

I would’ve switched out

      Multi.merge(multi, fn %{poll: poll} ->
        Multi.new()
        |> Multi.insert(poll_option, poll_option_changeset(poll, option))
        |> Multi.run(poll_option_with_hash, fn _repo, %{^poll_option => poll_option_changeset} ->
          Utils.generate_hash_id_for_item(poll_option_changeset, "poll_option")
        end)
      end)

with

multi
|> Multi.run(poll_option, fn repo, %{poll: poll} -> 
	repo.insert(poll_option_changeset(poll, option))
end)
|> Multi.run(poll_option_with_hash, fn _repo, %{^poll_option => poll_option_changeset} ->
	Utils.generate_hash_id_for_item(poll_option_changeset, "poll_option")
end)

Nothing fancy.

5 Likes