CODEWITHSUNDEEP
validationelixirectochangeset
harryg
harryg

Reputation: 24107

With Ecto, validate that a changeset with 2 different related models have the same parent model

In my app I have a method to create a new response. A response has a belongs_to relationship to both a player and match.

In addition player and match both have a belongs_to relationship to a team.

It looks like this:

schema

When inserting a new response I want to validate that the player and match having the player_id and match_id foreign keys in the changeset belong to the same team.

Currently I'm achieving this as follows. First, define a custom validation that checks the records belonging to the foreign keys:

def validate_match_player(changeset) do
  player_team =
    Player
    |> Repo.get(get_field(changeset, :player_id))
    |> Map.get(:team_id)

  match_team =
    Match
    |> Repo.get(get_field(changeset, :match_id))
    |> Map.get(:team_id)

  cond do
    match_team == player_team -> changeset
    true -> changeset |> add_error(:player, "does not belong to the same team as the match")
  end
end

and the use the validation as part of the changeset:

def changeset(model, params \\ %{}) do
  model
  |> cast(params, [:player_id, :match_id, :message])
  |> validate_required([:player_id, :match_id, :message])
  |> foreign_key_constraint(:match_id)
  |> foreign_key_constraint(:player_id)
  |> validate_match_player()
  |> unique_constraint(
    :player,
    name: :responses_player_id_match_id_unique,
    message: "already has an response for this match"
  )
end

This works fine but involves a couple of extra SQL queries to look up the related records in order to get their team_id foreign keys to compare them.

Is there a nicer way to do this, perhaps using constraints, that avoids the extra queries?

Upvotes: 3

Views: 1531

Answers (1)

Alex de Sousa
Alex de Sousa

Reputation: 1611

I have two possible improvements:

  • Application level solution: instead of two queries, you just query once.
  • Database level solution: you create a trigger for the check in the database.

Application Level Solution

Right now you have two queries for checking that player and match belong to the same team. That means two round trips to the database. You could reduce this by half if you use just one query e.g. given the following query:

    SELECT COUNT(*)
      FROM players AS p
INNER JOIN matches AS m
        ON p.team_id = m.team_id
     WHERE p.id = NEW.player_id AND m.id = NEW.match_id

you would change your function as follows:

def validate_match_player(changeset) do
  player_id = get_field(changeset, :player_id)
  match_id = get_field(changeset, :match_id)

  [result] =
    Player
    |> join(:inner, [p], m in Match, on: p.team_id == m.team_id)
    |> where([p, m], p.id == ^player_id and m.id == ^match_id)
    |> select([p, m], %{count: count(p.id)})
    |> Repo.all()

  case result do
    %{count: 0} ->
      add_error(changeset, :player, "does not belong to the same team as the match")
    _ ->
      changeset
  end
end

Database Level Solution

I'm assuming you're using PostgreSQL, so my answer will correspond to what you can find in the PostgreSQL manual.

There's no (clean) way to define a constraint in the table that does this. Constraints can only access the table where they're defined. Some constraints can only access the column from what they're defined and nothing more (CHECK CONSTRAINT).

The best approach would be writing a trigger for validating both fields e.g:

CREATE OR REPLACE FUNCTION trigger_validate_match_player()
  RETURNS TRIGGER AS $$
    IF (
         SELECT COUNT(*)
         FROM players AS p
         INNER JOIN matches AS m
         ON p.team_id = m.team_id
         WHERE p.id = NEW.player_id AND m.id = NEW.match_id
       ) = 0
    THEN
      RAISE 'does not belong to the same team as the match'
        USING ERRCODE 'invalid_match_player';
    END IF;

    RETURN NEW;
  $$ LANGUAGE plpgsql;

CREATE TRIGGER responses_validate_match_player
  BEFORE INSERT OR UPDATE ON responses
  FOR EACH ROW
  EXECUTE PROCEDURE trigger_validate_match_player();

The previous trigger will raise an exception when it fails. This also means Ecto will raise an exception. You can see how to handle this exception here.

In the end, maintaining triggers is not easy unless you're using something like sqitch for database migrations.

PS: If you're curious, the very dirty way of doing this in a CHECK constraint is by defining a PostgreSQL function that basically bypasses the limitation. I wouldn't recommend it.

I hope this helps :)

Upvotes: 2

Related Questions