# Need help to refactor the resource creation code

**URL:** <https://forum.elixirforum.com/t/need-help-to-refactor-the-resource-creation-code/24784>\
**Category:** Questions\
**Tags:** ecto, phoenix, phoenix\_ecto\
**Created:** [August 19, 2019, 4:12pm UTC](https://forum.elixirforum.com/t/need-help-to-refactor-the-resource-creation-code/24784 "2019-08-19T16:12:05Z")\
**Posts on this page:** 6\
**Page:** 1

<div class="post-metadata">

**Author:** ![achempion](https://forum.elixirforum.com/user_avatar/forum.elixirforum.com/achempion/32/21179_2.png) [@achempion](https://forum.elixirforum.com/u/achempion)\
**Post date:** [August 19, 2019, 4:12pm UTC](https://forum.elixirforum.com/t/need-help-to-refactor-the-resource-creation-code/24784/1 "2019-08-19T16:12:05Z")

</div>

Hi,

I’m trying to create a form with file field to upload a file. In order to store file into the proper folder I need to know resourse’s `id` in advance. So, I’ve split the creation transaction to two separate change-sets: `changeset` for the form fields and `audio_changeset` to upload the file.

When I call `audio_changeset` to update changeset and upload the file with invalid data, it changes the form state to update which changes form rendering as `put` action. This all happen inside the creating resource transaction. To overcome this and reset form status back to creating resource state I need to reset `changeset.data. __meta__.state` to nil.

It works but I think it has a better and prettier solution 🙂

```elixir
  def create_recording(attrs \\ %{}) do
    Repo.transaction(fn ->
      %Recording{}
      |> Recording.changeset(attrs)
      |> Repo.insert()
      |> case do
           {:ok, recording} ->
             Recording.audio_changeset(recording, attrs)
             |> Repo.update()
             |> case do
                  {:ok, recording_with_audio} -> recording_with_audio
                  {:error, changeset} ->
                    # make the form to a "new" state insted of "update"
                    meta = Map.put(changeset.data. __meta__ , :state, nil)
                    data = Map.put(changeset.data, : __meta__ , meta)

                    changeset
                    |> Map.put(:data, data)
                    |> Repo.rollback()
                end

           {:error, error} -> Repo.rollback(error)
      end
    end)
  end

```

---

<div class="post-metadata">

**Author:** ![david\_ex](https://forum.elixirforum.com/user_avatar/forum.elixirforum.com/david_ex/32/9498_2.png) [@david\_ex](https://forum.elixirforum.com/u/david_ex)\
**Post date:** [August 19, 2019, 4:22pm UTC](https://forum.elixirforum.com/t/need-help-to-refactor-the-resource-creation-code/24784/2 "2019-08-19T16:22:04Z")

</div>

One easy thing that will make this much more readable is

```elixir
a
|> b()
|> c()
|> case do
  :foo -> nil
  :bar -> nil
end

```

instead of

```elixir
case a |> b() |> c() do
  :foo -> nil
  :bar -> nil
end

```

---

<div class="post-metadata">

**Author:** ![StefanL](https://forum.elixirforum.com/user_avatar/forum.elixirforum.com/stefanl/32/6566_2.png) [@StefanL](https://forum.elixirforum.com/u/StefanL)\
**Post date:** [August 19, 2019, 5:49pm UTC](https://forum.elixirforum.com/t/need-help-to-refactor-the-resource-creation-code/24784/3 "2019-08-19T17:49:23Z")

</div>

You could use `with` statements if there are multiple failures possible.

However i think this does not fundamentally solves what you are trying to achieve. Maybe [Ecto.Multi](https://hexdocs.pm/ecto/Ecto.Multi.html) would be a better choice if you need to move files around as part of the transaction.

Its kind of hard to understand what is going on exactly in the snippet you posted. If it is one form, i guess you probably should have 1 changeset, also [prepare\_changes/2](https://hexdocs.pm/ecto/Ecto.Changeset.html#prepare_changes/2) allows you to run code in the transaction.

Using with instead of nested case statements could look like this: (i did not mess with the changeset)

```elixir
    def create_recording(attrs \\ %{}) do
      Repo.transaction(fn ->
        with {:ok, recording} <- insert_recording(attrs),
             {:ok, with_audio} <- update_recording(recording) do
          with_audio
        else
          {:error, error} ->
            Repo.rollback(error)
        end
      end)
    end

    defp insert_recording(attrs \\ %{}) do
      %Recording{}
      |> Recording.changeset(attrs)
      |> Repo.insert()
    end

    defp update_recording(recording, attrs \\ %{}) do
      recording
      |> Recording.audio_changeset(attrs)
      |> Repo.update()
    end

```

If you are uploading a file and don’t want to depend on ids from the db, generate a random path for the uploaded file (eg uuid as filename). Maybe this helps you to get rid of the second changeset … You would probably still need a transaction to move/upload the file. Just be aware that storing a lot of files inside 1 directory is a performance killer.

---

<div class="post-metadata">

**Author:** ![achempion](https://forum.elixirforum.com/user_avatar/forum.elixirforum.com/achempion/32/21179_2.png) [@achempion](https://forum.elixirforum.com/u/achempion)\
**Post date:** [August 19, 2019, 6:22pm UTC](https://forum.elixirforum.com/t/need-help-to-refactor-the-resource-creation-code/24784/4 "2019-08-19T18:22:22Z")

</div>

Thank you for the answer. Your code looks great, I think `with` is the way to go. Also, we still need to reset the `state` filed to make the form render properly.

I can’t use [prepare\_changes/2](https://hexdocs.pm/ecto/Ecto.Changeset.html#prepare_changes/2) because resource’s id is not available on this stage yet.

---

<div class="post-metadata">

**Author:** ![fuelen](https://forum.elixirforum.com/user_avatar/forum.elixirforum.com/fuelen/32/9870_2.png) [@fuelen](https://forum.elixirforum.com/u/fuelen)\
**Post date:** [August 19, 2019, 7:18pm UTC](https://forum.elixirforum.com/t/need-help-to-refactor-the-resource-creation-code/24784/5 "2019-08-19T19:18:06Z")

</div>

try this:

```elixir
  def create_recording(attrs \\ %{}) do
    Ecto.Multi.new()
    |> Ecto.Multi.insert(:recording, Recording.changeset(%Recording{}, attrs))
    |> Ecto.Multi.update(:recording_with_audio, &Recording.audio_changeset(&1.recording, attrs))
    |> Repo.transaction()
    |> case do
      {:ok, %{recording_with_audio: recording}} -> {:ok, recording}
      {:error, _operation, changeset, _} -> Ecto.Changeset.apply_action(changeset, :insert)
    end
  end

```

---

<div class="post-metadata">

**Author:** ![achempion](https://forum.elixirforum.com/user_avatar/forum.elixirforum.com/achempion/32/21179_2.png) [@achempion](https://forum.elixirforum.com/u/achempion)\
**Post date:** [September 3, 2019, 2:22pm UTC](https://forum.elixirforum.com/t/need-help-to-refactor-the-resource-creation-code/24784/6 "2019-09-03T14:22:31Z")

</div>

final version

```elixir
  def create_recording(attrs \\ %{}) do
    Ecto.Multi.new()
    |> Ecto.Multi.insert(:recording, Recording.changeset(%Recording{}, attrs))
    |> Ecto.Multi.update(:recording_with_audio, &Recording.audio_changeset(&1.recording, attrs))
    |> Repo.transaction()
    |> case do
         {:ok, %{recording_with_audio: recording}} -> {:ok, recording}
         {:error, _operation, changeset, _} ->
           # make the form to a "new" state insted of "update"
           # https://github.com/phoenixframework/phoenix_ecto/blob/master/lib/phoenix_ecto/html.ex#L300
           meta = Map.put(changeset.data. __meta__ , :state, nil)
           data = Map.put(changeset.data, : __meta__ , meta)

           Map.put(changeset, :data, data)
           |> Ecto.Changeset.apply_action(:insert)
       end
  end

```