Is this the proper way of creating errors inside the multi in a loop?

Hey there,
I am wondering if is this the recommended way of having errors in an Ecto.Multi in a loop?

def remove_admin_from_event(user, event, admins) do
    multi =
      Enum.reduce(admins, Multi.new(), fn hash_id, multi ->
        admin = Accounts.get_user_by_hash_id(hash_id)
        user_event = get_user_event(admin.id, event.id)

        number_of_admins = Enum.count(event.users, fn user -> user.role == "admin" end)

        user_event_changeset =
          UserEvent.changeset(user_event, admin, event, %{
            role: "participant",
            status: user_event.status
          })

        cond do
          user.id == admin.id ->
            changeset =
              add_error(
                user_event_changeset,
                :user_event,
                "You can't remove your own admin rights!"
              )

            multi
            |> Multi.error("remove_admin_#{hash_id}", changeset)

          number_of_admins <= 1 ->
            changeset =
              add_error(
                user_event_changeset,
                :user_event,
                "A dotoo needs at least one admin, you are the last one currently!"
              )

            multi
            |> Multi.error("remove_admin_#{hash_id}", changeset)

          true ->
            multi
            |> Multi.update(
              "user_event_#{admin.id}_#{event.id}",
              user_event_changeset
            )
        end
      end)

    Repo.transaction(multi)
  end
1 Like

I’d have that logic directly applied to the changesets and let multi simply try to insert those. It’ll fail if the changeset is invalid anyways. I’ve not yet had the need to manually add errors to the multi itself.

1 Like

We do stuff exactly like this when the validation control can’t be represented well in a changeset function. We try to keep our changeset functions pure, which sometimes means that we have to add errors like this from our context modules. Sometimes though it is better to add virtual fields in the schema, that you populate from other data sources and use in your changeset validation functions.

3 Likes

Yes, I was thinking to try to put it into changeset functions, but then it is not wrong, or terrible to do it this way, I will take a look at refactoring it though to changeset functions.

Thanks for the replies

I ended up with this:

def remove_admin_from_event(user, event, admins) do
    multi =
      Enum.reduce(admins, Multi.new(), fn hash_id, multi ->
        admin = Accounts.get_user_by_hash_id(hash_id)
        user_event = get_user_event(admin.id, event.id)

        user_event_changeset =
          UserEvent.changeset_remove_admin(user_event, admin, user, event, %{
            role: "participant",
            status: user_event.status
          })

        multi
        |> Multi.update(
          "user_event_#{admin.id}_#{event.id}",
          user_event_changeset
        )
      end)

    Repo.transaction(multi)
  end

if someone in the future would be interested :grin:

1 Like

I handle errors in loops with multi by just changing it to something like:

Enum.reduce(admins, Multi.new(), fn
  hash_id, %Ecto.Multi{} = multi ->
    ... normal code body here ...
  _hash_id, error -> error

And then anytime there is an error in the normal code body then I just return an {:error, reason}. :slight_smile:

2 Likes

*so the errors are handled in changeset functions