Ecto: on_replace: :update ignores association with put_assoc

[shulhi]

Environment

• Elixir version (elixir -v): 1.5.2
• Database and version (PostgreSQL 9.4, MongoDB 3.2, etc.): 9.6
• Ecto version (mix deps): 9a45b6c
• Database adapter and version (mix deps): postgrex 0.13.3
• Operating system: Ubuntu 16.04

Current behavior

  # Schedule schema
  schema "schedules" do
    field :name, :string
    has_many :shifts, Shift
  end

  def changeset(%Schedule{} = shift, attrs \\ %{}) do
    shift
    |> cast(attrs, [:name])
    |> cast_assoc(:shifts)
  end

  # Shift schema
  schema "shifts" do
    field :day_of_the_week, :integer
    belongs_to :schedule, Schedule, on_replace: :update
  end

  def changeset(%Shift{} = shift, attrs \\ %{}) do
    shift
    |> cast(attrs, [:day_of_the_week])
    |> assoc_constraint(:schedule)
  end

  # Contexts
  def update_shift(%Shift{} = shift, attrs \\ %{}, opts \\ []) do
    schedule = Keyword.get(opts, :schedule, :empty)

    shift
    |> Shift.changeset(attrs)
    |> update_shift_with(schedule)
    |> Repo.update()
  end

  defp update_shift_with(changeset, :empty), do: changeset
  defp update_shift_with(changeset, schedule) do
    changeset
    |> Ecto.Changeset.put_assoc(:schedule, schedule)
  end

  # Tests
  schedule = Repo.get!(Schedule, schedule_id)
  shift = Repo.get!(Shift, shift_id) |> Repo.preload(:schedule)

  shift_attrs = %{"day_of_the_week" => 2}

  {:ok, updated_shift} = Operators.update_shift(shift, shift_attrs, schedule: schedule)

  assert updated_shift.day_of_the_week == 2 # passes
  assert updated_shift.schedule_id == schedule.id # failed
  assert updated_shift.schedule.id == schedule.id # failed

updated_shift.schedule still refers to the old schedule

Expected behavior

I expect updated_shift.schedule will be replaced by given schedule.
The documentation mentions,

:update - updates the association, available only for has_one and belongs_to. This option will update all the fields given to the changeset including the id for the association

[josevalim]

Can you provide a test case that runs in our test suite? It will make our lives much easier as we can then focus on the fix. Thanks!

[shulhi]

@josevalim you can checkout my branch here - https://github.com/shulhi/ecto/tree/onreplace_update_bug, and run the test with MIX_ENV=pg mix test --only maybe_bug

The test is here https://github.com/shulhi/ecto/blob/onreplace_update_bug/integration_test/cases/assoc.exs#L674, I am not sure if it is a bug or my lack of understanding. I couldn't really figure out the correct behavior of :update from the docs and existing tests. Thanks!

[jayjun]

If I’m not mistaken, put_assoc/3 only changes foreign keys while on_replace: :update ignores foreign key changes, so they cancel each other out.

If that's true, documentation should be improved or an error be raised? (similar to on_replace: :raise)

[shulhi]

@josevalim do you had a chance to look at this? I am curious if this really a bug or it is intended behavior like explained by @jayjun. I can help with improving the documentation if so.

[josevalim]

Sorry for the delay @shulhi. There is definitely a bug. If you inspect the changeset after put_assoc, it says there are no changes, while they are most definitely there. And you are right, it is something that happens only on :update.

[josevalim]

After a conversation with @ericmj we have decided to raise since, if you specify on_replace: :update and then you give something that cannot be updated, we should not silently replace it but instead update it.

[shulhi]

Thank you @josevalim for the quick fix!

[dustinfarris]

Sounds like Ecto needs an :update_or_replace option for this