How to properly validate uniqueness based on multiple fields?

Phoenix Forum Questions / Help
, ,
1

Hey there. I come from a Ruby on Rails background and was trying to figure out how to do things in Phoenix similar to how they’re done in Rails.

I have a setup where a User can have Categories, but a specific User can only have one Category with a given name. For instance, Steve can have one “Default” category, but not two, and Tom can have a “Default” category that is not the same as Steve’s.

In Rails, this can be done kinda like this:

def cat_name_unique_for_user?(user_id, cat_name)
  return false unless Category.find_by(user_id: user_id, name: cat_name).nil?

  true
end

The closest equivalent I could come up with without trying to unravel the mysteries of how to actually access the return of Ecto.query() was this validator

  defp validate_name_unique_for_user(changeset, user_id, name) do
    query = from(cat in "categories", where: cat.user_id == ^user_id, where: cat.name == ^name)

    unless Repo.exists?(query) do
      add_error(changeset, :name, "Name is not unique for current user!")
    end

    changeset
  end

This gets called in changesets by:

     category
     ...
     |> validate_name_unique_for_user(:user_id, :name)

The error I get back when trying this approach is
** (DBConnection.EncodeError) Postgrex expected an integer in -9223372036854775808..9223372036854775807, got :user_id. Please make sure the value you are passing matches the definition in your table or in your query or convert the value accordingly.

Given I’m interpolating the user_id, I would expect that to only ever be an integer, not an atom. Can someone please explain to me where I’ve gone wrong here? These speedbumps with Ecto are proving obnoxious. Thanks!

2

Have you got the right constraints in the database? If so, see unsafe_validate_unique and unique_constraint. If not, do so and return to step 1.

https://hexdocs.pm/ecto/Ecto.Changeset.html#unsafe_validate_unique/4

Also, be mindful that if you want to change a value within a function, you need to reassign it. You’re not in the icky world of mutation anymore and doing the below is not going to do what you want (pretty sure it’s backwards too and this is not how you check unique constraints outside of a transaction).

If you want the updated changeset, reassign it. This isn’t mutating the changeset, it is saying the variable changeset now points to this new changeset.

changeset = 
  if Repo.exists?(query) do
      add_error(changeset, :name, "Name is not unique for current user!")
  else 
    changeset
  end

You might want to read the getting started guide on the elixir website before tackling Ecto.

1 Like
3

So there’s three approaches:

  1. Check the constraint within elixir with runtime accessible information
  2. Check the constraint within the db with transactional db accessible information
  3. Check the constraint within the db with non-transactional db accessible information

The third is generally avoided (or called unsafe) due to the race conditions involved between checking the db and actually writing data after validation was successful. That’s what you attempted with the ecto query.

The second would be possible if the foreign key linking to users would be on the same table as the category name using a unique constraint across both columns. Your post would suggest those to be on a single column, but they not always are.

The first approach can be used if you handle all categories assigned to a user as a complete set (using cast_assoc or put_assoc). In that case you can validate the set to not contain duplicates using a custom changeset validation (using get_assoc to get all the assoc items).

Generally I’d suggest the 2. approach if possible (and reasonable) to do.

2 Likes
4

Aha, so this does indeed resolve into a skill issue on my part. I combed over several parts of the Ecto docs, but it didn’t occur to me that you could pass an array of fields to the unique_constraint method, or that, in the simplest case, specify the field and the specific constraint within the database to check against. This is indeed the solution.

  def new_category_changeset(category, attrs \\ %{}) do
    category
    |> cast(attrs, [:name, :description, :user_id])
    |> validate_required([:name, :description, :user_id])
    |> unique_constraint(:name, name: :categories_user_id_name_index)
  end

This is a good callout too. I’m used to immutable things yelling if you try to poke them (Rust) rather than silently doing something you don’t expect, and this has tripped me up a couple times before. The syntax being so similar to Ruby doesn’t help, lol.

Thanks for the assistance with my growing pains, and here’s hoping the gotchas thin out as I go further in and build up this app’s complexity. (or at least familiarity grows in tandem and allows me to not spend hours on simple problems like this)

5

The compiler would need to be certain the code within that block is free from side effects to warn you because it’s perfectly valid to write code similar to what you had, e.g

if ... do
  some_function_that_does_something_but_the_return_value_is_not_used()
end

There might be a credo rule to make you discard the value with _ = ... :person_shrugging: