Ecto: Parent updated_at not changing when adding children

[hisapy]

Precheck

This is considered a possible bug, based on conversation with @michalmuskala on #ecto Slack channel.

Environment

• Elixir version (elixir -v): 1.4.2
• Database and version (PostgreSQL 9.4, MongoDB 3.2, etc.): PostgreSQL 9.6.3
• Ecto version (mix deps): 2.2.6
• Database adapter and version (mix deps): postgrex 0.13.3
• Operating system: macOS Sierra 10.12.6, Ubuntu 10.04 LTS

Current behavior

Given an existing record associated with zero or more children, adding children to the association via the parent's changeset, doesn't change updated_at on parent.

For example, given the following schema

# parent
defmodule Webapp.Company do 
  schema "companies" do 
    field :name, :string
    has_many :members, Webapp.CompanyMember, on_replace: :delete
    # other fields ...
    timestamps()
  end

  def changeset(struct, params \\ %{}) do
    struct
    |> cast(params, [:name])
    |> validate_required(:name)
    |> cast_assoc(:members)
  end
end

# child
defmodule Webapp.CompanyMember do
  use Webapp.Web, :model

  schema "company_members" do
    belongs_to :company, Webapp.Company
    field :is_executive, :boolean
    # other fields ...
    timestamps()
  end

  def changeset(model, params \\ %{}) do
    model
    |> cast(params, [:company_id, :is_executive])
    |> assoc_constraint(:company)
  end
end 

... and given an existing Company, adding new members to it, doesn't update its updated_at.

# args is coming from the outside (client)
args = %{"members" => [%{"isExecutive" => false}]} 
company
 |> Repo.preload(:members)
 |> Company.changeset(args)
 |> Repo.update 

Expected behavior

Given an existing Company adding members to it should update its updated_at. A behavior similar to passing args like above but with data for :name - or any other Company _specific_ field, or like the following:

company
 |> Repo.preload(:members)
 |> Company.changeset(args)
 |> Repo.update(force: true)

but without _bypassing the logic to skip query if there are no changes in the changeset_

[josevalim]

Thank you for the report @hisapy. I would like to hear why changing the child should update the parent. Is it because we are using cast_assoc? Should that also apply to put_assoc? Wouldn't it be better to do it under a specific flag?

[hisapy]

Hi @josevalim. The logic I would like to enforce is that the Company should be marked as updated when adding members to it. Or speaking generically, if you cast_assoc or put_assoc on the parent to create/update children, the parent should be marked as updated.

I believe that it would be better to have an specific flag to _opt-out_ from the expected behavior. For example, if you were to add a Comment to a Post, you'd normally use a Comment.changeset, but most of the times when you want to add children via its parent it's because you're updating the parent, like in a form with nested fields, i.e.: user roles, invoice rows, post tags, etc.

[josevalim]

I believe that it would be better to have an specific flag to opt-out from the expected behavior.

The issue is that this behaviour may make sense for cast_assoc but not necessarily for put_assoc. And if we have the defaults for those being different, it will be confusing. That's why I am more inclined to be opt-in.

[AndrewDryga]

I think this issue depends on a stance of view. If you think about Ecto as "standalone" solution - it's probably okay to assume that it would update parent structs, but it has a database underneath. And when you add an item to an associated table, the database won't update referenced row unless you have an explicitly defined trigger for that.

So I think we should do the same and make it opt-in, or even leave as-is.

[hisapy]

Yeah, it makes more sense for cast_assoc and not so much for put_assoc. Anyway, I think the objections are fair enough so I'm ok with the _opt-in_ behavior.

EDIT: Actually, I have a doubt. Repo.update inserts/updates children rows given a changeset. There is no explicit flag to achieve this. So I think that instead of focusing on cast_assoc and put_assoc, I believe that the logic to set the updated_at on the parent should be the responsibility of the Repo.update function.

[josevalim]

Actually, I have a doubt. Repo.update inserts/updates children rows given a changeset. There is no explicit flag to achieve this. So I think that instead of focusing on cast_assoc and put_assoc, I believe that the logic to set the updated_at on the parent should be the responsibility of the Repo.update function.

The thing is that update is mostly a "dumb layer", when it handles associations, it is simply because those instructions are present in the changeset. So the changeset is the one responsible for figuring everything out, "update" just executes it.

[josevalim]

Here is my proposal: we add a :force_update_on_change to cast_assoc and cast_embed. When that flag is true and the association/embed change, we add the force: true flag to repo_opts. PR are appreciated.

[hisapy]

Ok. So in this sense, I think that the default value of force_update_on_change should be true. Otherwise, it will be easier to just put a force: true on update, specially if you have more than one cast_assoc and/or cast_embed piped in a given changeset. Does this make sense @josevalim ?

[josevalim]

That's ok by me.