Handle form submission for associations

Hi,

I’m wiring a small movie library application. A movie has a title, synopsis and whatnot, but also a poster. I store posters separately in my application. The problem I’m having is with what should happen when a user fills out a form with movie details, and adds an image with an upload form, and there are errors (i.e., invalid changeset for either image or movie).

The way I see it, you either first create the movie, and then try to store the image, or the other way around.
My confusion is in what should happen when either of both fail.

In case creating the movie fails, you redirect the user to the movie form with the movie changeset and you can show nice errors and whatnot. But if the image fails, you cannot redirect with a movie changeset, because you have either already added the movie (and dont have a changeset), or you have not inserted it yet (you tried image first) and don’t have a changeset yet.

I have shown my code below of what I have at the moment.

 # controller
  def create(conn, %{"movie" => %{"image" => img} = movie_params}, _user) do
    # Create the image (assume read will succeed)
    bin = File.read!(img.path)

    case Media.create_image(%{:data => bin}) do
      {:ok, image} ->
        case Media.create_movie(image, movie_params) do
          {:ok, movie} ->
            conn
            |> put_flash(:info, "Movie created successfully.")
            |> redirect(to: Routes.movie_path(conn, :show, movie))

          {:error, %Ecto.Changeset{} = changeset} ->
            render(conn, "new.html", changeset: changeset)
        end

      # {:error, %Ecto.Changeset{}} ->
      _ ->
        render(conn, "new.html", changeset: Media.change_image(%Image{}))
    end
  end

My form looks like this.

<%= form_for @changeset, @action,[multipart: true], fn f -> %>
  <%= if @changeset.action do %>
    <div class="alert alert-danger">
      <p>Oops, something went wrong! Please check the errors below.</p>
    </div>
  <% end %>

  <%= label f, :title %>
  <%= text_input f, :title %>
  <%= error_tag f, :title %>

  <%= label f, :synopsis %>
  <%= text_input f, :synopsis %>
  <%= error_tag f, :synopsis %>

  <%= label f, :tmdb %>
  <%= text_input f, :tmdb %>
  <%= error_tag f, :tmdb %>

  <%= label f, :imdb %>
  <%= text_input f, :imdb %>
  <%= error_tag f, :imdb %>

  <%= label f, :release %>
  <%= date_select f, :release %>
  <%= error_tag f, :release %>

  <%= label f, :image %>
  <%= file_input f, :image %>
  <%= error_tag f, :image %>

  <div>
    <%= submit "Save" %>
  </div>
<% end %>

As you can see, in the outer case I only have access to an Image changeset, and not a movie changeset. So I should just redirect to an empty form?

Thanks in advance,

Edit

I have been playing around with transactions, as they allow me to roll back changes made to the database. And I think I have a partial solution to the problem.

So in short: to create a movie I need to create an image, and then create a movie (or the other way around).
When the movie fails, I can use the pre-existing mechanics to show errors in the changeset, but not with the image. So I’m still looking for a way to fix that.

My controller now looks like this:

  def create(conn, %{"movie" => %{"image" => img} = movie_params}, _user) do
    # Create the image (assume read will succeed)
    bin = File.read!(img.path)

    result =
      Repo.transaction(fn ->
        # Create the movie first, because we need to return this changeset in case of an error!
        case Media.create_movie(movie_params) do
          {:ok, movie} ->
            case Media.create_image(%{:data => bin}) do
              {:ok, image} ->
                Media.add_poster_to_movie(movie, image)
                {:ok, movie, image}

              {:error, %Ecto.Changeset{} = _imagechangeset} ->
                Repo.rollback(Media.change_movie(movie))
            end

          {:error, %Ecto.Changeset{} = moviechangeset} ->
            Repo.rollback({:error, moviechangeset})
        end
      end)

    case result do
      {:ok, movie, _image} ->
        conn
        |> put_flash(:info, "Movie created successfully.")
        |> redirect(to: Routes.movie_path(conn, :show, movie))

      {:error, moviechangeset} ->
        render(conn, "new.html", changeset: moviechangeset)
    end
  end

So if there is an error with the image in any way, the changeset for the created movie will be shown. This results in a form showing all the previously filled data, but not the image. The only thing missing now is how I can show an error to the user in the form saying that the image data was not good for some reason.

1 Like

Sorry I can’t be of more help but have you looked at waffle and waffle_ecto?

1 Like

Might something like this work?

Note usually when I have nested case clauses, I find that is a signal to use with instead. I’ve wrapped the expected results of each with clause in a tuple to make it easier to identify which step it fails on. If the :image step fails, I just add a generic message to the movie changeset (but you could make it more specific by getting information from image_changeset

Finally it would probably be better to put the repo transaction logic in a separate module/function and call that function from your controller instead.

def create(conn, %{"movie" => %{"image" => img} = movie_params}, _user) do
  # Create the image (assume read will succeed)
  bin = File.read!(img.path)

  result =
    Repo.transaction(fn ->
      # Create the movie first, because we need to return this changeset in case of an error!

      with {:movie, {:ok, movie}} <- Media.create_movie(movie_params),
           {:image, {:ok, image}, movie} <- Media.create_image(%{:data => bin}),
           {:poster, {:ok, _movie}} <- Media.add_poster_to_movie(movie, image) do
            {movie, image}
      else
        {:movie, {:error, changeset}} ->
          Repo.rollback(changeset)

        {:image, {:error, _image_changeset}, movie} ->
          movie
          |> Media.change_movie()
          |> Ecto.Changeset.add_error(:image, "Invalid image data")
          |> Repo.rollback()

      end
    end)

  case result do
    {:ok, {movie, _image}} ->
      conn
      |> put_flash(:info, "Movie created successfully.")
      |> redirect(to: Routes.movie_path(conn, :show, movie))

    {:error, moviechangeset} ->
      render(conn, "new.html", changeset: moviechangeset)
  end
end
1 Like

You might consider wrapping all of that up into a single function on your context and using a Multi to automatically rollback on failure.

def create_movie(params)
  Multi.new()
  |> Multi.run(:create_movie...
  |> Multi.run(:create_image...
  |> Multi.run(:create_poster...
  |> Repo.transaction()
end

You might also consider using a form module dedicated to movie creation rather than relying on changesets for multiple resources. That way you can manage validations in any custom way you deem necessary.

It’s hard to say exactly what is best in your situation though based on the user experience you are looking for. When I hear movies and images, I immediately think about client-side uploads rather than hitting my server with them, but that rabbit hole can go pretty deep.

2 Likes