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

benonymus

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}?

8 comments

#ecto_multi #multi

0 1850 8

2019-04-03 12:17:15 UTC

Showing Posts 1 to 8

Show Best Posts
Show All (oldest first)
Show All (newest first)

LostKobrakai

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.

Post #1

benonymus

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()

Post #2

LostKobrakai

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

Post #3

benonymus

Does that have any noticeable upsides?

Post #4

LostKobrakai

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

Post #5

benonymus

I see, thanks!

Post #6

Qqwy

TypeCheck Core Team

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

Post #7

LostKobrakai

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.

Post #8

— All posts loaded —