Validate association fields

I have a User schema:

  schema "users" do
    field :username, :string
    belongs_to :invite, App.Admin.Invite

    timestamps()
  end

and a Invite schema:

  schema "invites" do
    field :code, :string
    field :expire, Ecto.Date
    field :used, :boolean, default: false

    has_one :user, App.User, on_delete: :nothing

    timestamps()
  end

The template part:

  <div class="form-group">
    <%= label f, :invite, gettext("Invite Code"), class: "control-label control-label--required" %>
    <%= inputs_for f, :invite, fn fi -> %>
      <%= text_input fi, :code, class: "form-control" %>
      <%= error_tag fi, :code %>
      <%= error_tag fi, :used %>
      <%= error_tag fi, :expire %>
    <% end %>
  </div>

When inserting a user, I would love to check the associated invite:

  1. does the code existed in invites table?
  2. is the used value false now?
  3. is the expire value larger than today?

If all the checks pass, then insert the user and update the associated invite.

At first, I tried with cast_assoc in user.ex:

def changeset(struct, params \\ %{}) do
  struct
  |> cast_assoc(:invite, with: &App.Admin.Invite.update_changeset/2)

In App.Admin.Invite.update_changeset/2 I defined the checks. But when inserting user, I found that it tried to insert another invite, the action of associated invite changeset was set to insert - it’s not what I want. We just want to update the invite’s used to true. Does it mean cast_assoc can only be used when inserting association?

I also tried to check it in user_controller.ex file, but it became a mess quickly:

  def create(conn, %{"user" => user_params}) do

    changeset = User.changeset(%User{}, user_params)

    %{"invide" => invite} = user_params
    %{"code" => code} = invite

    invite = Repo.get_by(Invite, code: code)
    changeset = if is_nil(invite) do
      Ecto.Changeset.add_error(changeset, :invite, gettext("code is invalid"))
    else
      if invite.used == true do
        Ecto.Changeset.add_error(changeset, :invite, gettext("code is used"))
      else
        case Ecto.Date.compare(invite.expire, Ecto.Date.utc) do
          :lt -> Ecto.Changeset.add_error(changeset, :invite, gettext("code is expired"))
          _ ->
            Ecto.Changeset.put_assoc(changeset, :invite, invite) |> Ecto.Changeset.put_change(:invite_id, invite.id)
        end
      end
    end

So I’m wondering if there is any better way to handle this use case?

Update

I found this ecto issue https://github.com/elixir-ecto/ecto/issues/1850 similar to my problem.

2 Likes

I would suggest splitting this code up into several individual functions. Here’s what I came up with after a bit of tinkering:

defmodule Invites.Accounts.Users do
  alias Invites.Accounts.Invites
  alias Invites.Accounts.User

  def create(code) do
    changeset = User.changeset(%User{})

    case Invites.find_by_code(code) do
      nil ->
        Ecto.Changeset.add_error(changeset, :invite, "code is invalid")
      invite ->
        changeset
        |> check_invite_usage(invite)
        |> check_invite_expiry(invite)
    end
  end

  defp check_invite_usage(changeset, %{used: used}) when used do
    Ecto.Changeset.add_error(changeset, :invite, "code is used")
  end

  defp check_invite_usage(changeset, _invite), do: changeset

  defp check_invite_expiry(changeset, invite = %{expire: expire}) when is_nil(expire) do
    add_invite(changeset, invite)
  end

  defp check_invite_expiry(changeset, invite = %{expire: expire}) do
    case Date.compare(expire, Date.utc_today) do
      :lt ->
        changeset
        |> Ecto.Changeset.add_error(:invite, "code is expired")
      _ ->
        add_invite(changeset, invite)
    end
  end

  defp add_invite(changeset, invite) do
    changeset
    |> Ecto.Changeset.put_assoc(:invite, invite)
    |> Ecto.Changeset.put_change(:invite_id, invite.id)
  end
end

I feel with this code that the second check_invite_expiry function should be split up somehow so that we pattern match on :lt, rather than having that function’s body be a big case statement, but I can’t figure out that cleaner way.

Would love to see other people’s attempts!

4 Likes

Thanks for your idea, here’s what I got finally:

  defp code_required(code) do
    if String.length(code) > 0 do
      {:ok, code}
    else
      {:error, :invite, "invite is required"}
    end
  end

  defp invite_existed(code) do
    case Repo.get_by Invite, code: code do
      nil -> {:error, :invite, "invite is invalid"}
      invite -> {:ok, invite}
    end
  end

  defp invite_available(invite) do
    if invite.used do
      {:error, :invite, "invite is used"}
    else
      {:ok, invite}
    end
  end

  defp invite_not_expired(invite) do
    case Ecto.Date.compare(invite.expire, Ecto.Date.utc) do
      :lt ->
        {:error, :invite, "invite is expired"}
      _ ->
        {:ok, invite}
    end
  end

  def create(conn, %{"user" => user_params}) do
    %{"invite" => %{
      "code" => code
    }} = user_params

    user_changeset =
      conn.assigns.role_customer
      |> build_assoc(:users)
      |> User.changeset(user_params)
    
    with {:ok, code} <- code_required(code),
         {:ok, invite} <- invite_existed(code),
         {:ok, invite} <- invite_available(invite),
         {:ok, invite} <- invite_not_expired(invite)
    do
      # invite is ok
      multi =
        Multi.new
        |> Multi.insert(:user, user_changeset |> Ecto.Changeset.put_change(:invite_id, invite.id))
        |> Multi.update(:invite, Invite.update_changeset(invite, %{used: true}))

      case Repo.transaction(multi) do
        {:ok, %{user: user, invite: _invite}} ->
          conn
          |> put_session(:user_id, user.id)
          |> put_flash(:info, gettext("User created successfully."))
          |> configure_session(renew: true)
          |> redirect(to: user_path(conn, :show, user))
        {:error, :user, changeset, _} ->
          render(conn, "new.html", changeset: changeset)
        {:error, :invite, changeset, _} ->
          render(conn, "new.html", changeset: changeset)
      end
    else
      {:error, :invite, msg} ->
        user_changeset =
          user_changeset
          |> Ecto.Changeset.add_error(:invite, msg)
        render conn, "new.html", changeset: %{user_changeset | action: "insert"}
    end       
  end

I think the with used here is much easy to reason about, although I need to wrap the condition in functions.

1 Like