diff --git a/CHANGELOG.md b/CHANGELOG.md index fa2f25c..55d0206 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased][] -- Nothing yet +- Implement diffing for has_many through associations, resolves associated error. ## [0.5.0][] diff --git a/lib/ecto_diff.ex b/lib/ecto_diff.ex index 1f5b95e..49f2fb2 100644 --- a/lib/ecto_diff.ex +++ b/lib/ecto_diff.ex @@ -367,7 +367,40 @@ defmodule EctoDiff do end end - defp diff_association(previous, current, %{cardinality: :many, field: field, related: struct}, acc, opts) do + # The association likely already has the diff for the many through association, + # Attempt to get the diff through the association, calculate the diff if not found. + defp diff_association( + previous, + current, + %{cardinality: :many, through: [through, assoc_field], owner: owner}, + acc, + opts + ) do + diff = + case get_association_through_diff(acc, through, assoc_field) do + [%EctoDiff{} | _] = diff -> + diff + + [] -> + through_association = owner.__schema__(:association, through).related + association = through_association.__schema__(:association, assoc_field) + + diff = diff_association(previous, current, association, acc, opts) + get_association_through_diff(diff, through, assoc_field) + end + + if diff == [], do: acc, else: [{assoc_field, diff} | acc] + end + + defp diff_association( + previous, + current, + %{cardinality: :many, field: field} = association, + acc, + opts + ) do + %{related: struct} = association + primary_key_fields = get_primary_key_fields(struct, opts) if primary_key_fields == [], @@ -404,6 +437,17 @@ defmodule EctoDiff do end end + # returns the diff for an association through for assoc_field, or an empty list + # removes [nil] to return empty list when `assoc_field` is not present + # removes diffs with no changes + defp get_association_through_diff(diff, through, assoc_field) do + with [assoc_diffs] <- Keyword.get_values(diff, through) do + assoc_diffs + |> Enum.flat_map(& &1[:changes][assoc_field]) + |> Enum.reject(&(&1 == nil or no_changes?(&1))) + end + end + defp get_primary_key_fields(struct, opts) do overrides = Keyword.get(opts, :overrides, []) @@ -428,7 +472,7 @@ defmodule EctoDiff do keys end - defp no_changes?(%{effect: :changed, changes: map}) when map == %{}, do: true + defp no_changes?(%{effect: effect, changes: map}) when map == %{} and effect in [:added, :changed], do: true defp no_changes?(_), do: false defp primary_key_nil?(key), do: Enum.all?(key, fn {_key, value} -> is_nil(value) end) diff --git a/priv/repo/migrations/20230915120413_create_resources_toys.exs b/priv/repo/migrations/20230915120413_create_resources_toys.exs new file mode 100644 index 0000000..becd3e5 --- /dev/null +++ b/priv/repo/migrations/20230915120413_create_resources_toys.exs @@ -0,0 +1,18 @@ +defmodule EctoDiff.Repo.Migrations.CreateResources do + use Ecto.Migration + + def change do + create table(:resources) do + add :pet_id, references(:pets) + add :refid, :uuid + end + + create table(:toys) do + add :name, :string + add :type, :string + add :quantity, :integer, default: 1 + add :resource_id, references(:resources) + add :refid, :uuid + end + end +end diff --git a/test/ecto_diff_test.exs b/test/ecto_diff_test.exs index 89d0b94..54922c8 100644 --- a/test/ecto_diff_test.exs +++ b/test/ecto_diff_test.exs @@ -333,7 +333,7 @@ defmodule EctoDiffTest do } = diff end - # TODO: the following case only tests belongs_to with on_replace: :update + # the following case only tests belongs_to with on_replace: :update # other cases should be written for on_replace: :nilify and :delete test "update a belongs_to using on_replace: :update" do @@ -614,6 +614,325 @@ defmodule EctoDiffTest do } = diff end + # Has Many through polymorphic Association + + test "no changes with has_many through" do + {:ok, pet} = + %{ + name: "Spot", + skills: [%{name: "Eating"}, %{name: "Sleeping"}], + resources: [ + %{toys: [%{name: "ball", type: "play", quantity: 1}, %{name: "mouse", type: "play", quantity: 2}]} + ] + } + |> Pet.new() + |> Repo.insert() + + assert EctoDiff.diff(pet, pet) == {:ok, :unchanged} + end + + test "insert with has_many through" do + {:ok, pet} = + %{ + name: "Spot", + resources: [ + %{toys: [%{name: "ball", type: "play", quantity: 1}, %{name: "mouse", type: "play", quantity: 2}]} + ] + } + |> Pet.new() + |> Repo.insert() + + pet = Repo.preload(pet, :toys) + + id = pet.id + [%{id: resource_id}] = pet.resources + [ball_id, mouse_id] = Enum.map(pet.toys, & &1.id) + + {:ok, diff} = EctoDiff.diff(nil, pet) + [%{changes: %{toys: resource_toys}}] = diff.changes.resources + + assert resource_toys == diff.changes.toys + + assert %EctoDiff{ + effect: :added, + primary_key: %{id: ^id}, + changes: %{ + id: {nil, ^id}, + name: {nil, "Spot"}, + resources: [ + %EctoDiff{ + effect: :added, + changes: %{ + pet_id: {nil, ^id} + } + } + ], + toys: [ + %EctoDiff{ + effect: :added, + primary_key: %{id: ^ball_id}, + changes: %{ + id: {nil, ^ball_id}, + resource_id: {nil, ^resource_id}, + name: {nil, "ball"} + } + }, + %EctoDiff{ + effect: :added, + primary_key: %{id: ^mouse_id}, + changes: %{ + id: {nil, ^mouse_id}, + resource_id: {nil, ^resource_id}, + name: {nil, "mouse"} + } + } + ] + } + } = diff + end + + test "insert with empty has_many through" do + {:ok, pet} = + %{ + name: "Spot", + resources: [ + %{toys: []} + ] + } + |> Pet.new() + |> Repo.insert() + + id = pet.id + + {:ok, diff} = EctoDiff.diff(nil, pet) + + assert %EctoDiff{ + effect: :added, + primary_key: %{id: ^id}, + changes: %{ + id: {nil, ^id}, + name: {nil, "Spot"}, + resources: [ + %EctoDiff{ + effect: :added, + changes: %{ + pet_id: {nil, ^id} + } + } + ] + } + } = diff + end + + test "update with adding new has_many through records" do + {:ok, pet} = %{name: "Spot", resources: []} |> Pet.new() |> Repo.insert() + + {:ok, updated_pet} = + pet + |> Pet.update(%{ + resources: [ + %{toys: [%{name: "ball", type: "play", quantity: 1}, %{name: "mouse", type: "play", quantity: 2}]} + ] + }) + |> Repo.update() + + updated_pet = Repo.preload(updated_pet, :toys) + + id = pet.id + [%{id: resource_id}] = updated_pet.resources + %{"ball" => ball_id, "mouse" => mouse_id} = Enum.into(updated_pet.toys, %{}, &{&1.name, &1.id}) + + {:ok, diff} = EctoDiff.diff(pet, updated_pet) + [%{changes: %{toys: resource_toys}}] = diff.changes.resources + + assert resource_toys == diff.changes.toys + + assert %EctoDiff{ + effect: :changed, + primary_key: %{id: ^id}, + changes: %{ + resources: [ + %EctoDiff{ + effect: :added, + changes: %{ + pet_id: {nil, ^id}, + toys: [ + %EctoDiff{ + effect: :added, + changes: %{} + }, + %EctoDiff{ + effect: :added, + changes: %{} + } + ] + } + } + ], + toys: [ + %EctoDiff{ + effect: :added, + primary_key: %{id: ^ball_id}, + changes: %{ + id: {nil, ^ball_id}, + resource_id: {nil, ^resource_id}, + name: {nil, "ball"} + } + }, + %EctoDiff{ + effect: :added, + primary_key: %{id: ^mouse_id}, + changes: %{ + id: {nil, ^mouse_id}, + resource_id: {nil, ^resource_id}, + name: {nil, "mouse"} + } + } + ] + } + } = diff + end + + test "update with has_many through, updating one of many records" do + {:ok, pet} = + %{ + name: "Spot", + resources: [ + %{toys: [%{name: "ball", type: "play", quantity: 1}, %{name: "mouse", type: "play", quantity: 2}]} + ] + } + |> Pet.new() + |> Repo.insert() + + id = pet.id + pet = Repo.preload(pet, :toys) + + %{"ball" => ball_id, "mouse" => mouse_id} = Enum.into(pet.toys, %{}, &{&1.name, &1.id}) + [%{id: resource_id}] = pet.resources + + {:ok, updated_pet} = + pet + |> Pet.update(%{ + resources: [ + %{id: resource_id, toys: [%{id: ball_id, quantity: 5}, %{id: mouse_id}]} + ] + }) + |> Repo.update() + + {:ok, diff} = EctoDiff.diff(pet, updated_pet) + [%{changes: %{toys: resource_toys}}] = diff.changes.resources + + assert resource_toys == diff.changes.toys + + assert %EctoDiff{ + effect: :changed, + primary_key: %{id: ^id}, + changes: %{ + resources: [ + %EctoDiff{ + effect: :changed, + primary_key: %{id: ^resource_id}, + changes: %{ + toys: [ + %EctoDiff{ + effect: :changed, + primary_key: %{id: ^ball_id}, + changes: %{ + quantity: {1, 5} + } + } + ] + } + } + ], + toys: [ + %EctoDiff{ + effect: :changed, + primary_key: %{id: ^ball_id}, + changes: %{ + quantity: {1, 5} + } + } + ] + } + } = diff + end + + test "update with has_many through, removing one of many records using on_replace: :delete" do + {:ok, pet} = + %{name: "Spot", resources: [%{toys: [%{name: "ball", type: "play", quantity: 1}]}]} + |> Pet.new() + |> Repo.insert() + + pet = Repo.preload(pet, :toys) + id = pet.id + [%{id: resource_id}] = pet.resources + [ball_id] = Enum.map(pet.toys, & &1.id) + + {:ok, updated_pet} = pet |> Pet.update(%{resources: [%{id: resource_id, toys: []}]}) |> Repo.update() + + {:ok, diff} = EctoDiff.diff(pet, updated_pet) + [%{changes: %{toys: resource_toys}}] = diff.changes.resources + + assert resource_toys == diff.changes.toys + + assert %EctoDiff{ + effect: :changed, + primary_key: %{id: ^id}, + changes: %{ + resources: [ + %EctoDiff{ + effect: :changed, + primary_key: %{id: ^resource_id}, + changes: %{ + toys: [ + %EctoDiff{ + effect: :deleted, + primary_key: %{id: ^ball_id}, + current: nil, + changes: %{} + } + ] + } + } + ], + toys: [ + %EctoDiff{ + effect: :deleted, + primary_key: %{id: ^ball_id}, + current: nil, + changes: %{} + } + ] + } + } = diff + end + + test "update that doesn't change a loaded has_many through does not include it in diff" do + {:ok, pet} = + %{name: "Spot", resources: [%{toys: [%{name: "ball", type: "play", quantity: 1}]}]} + |> Pet.new() + |> Repo.insert() + + pet = Repo.preload(pet, :toys) + + {:ok, updated_pet} = pet |> Pet.update(%{name: "McFluffFace"}) |> Repo.update() + id = pet.id + + {:ok, diff} = EctoDiff.diff(pet, updated_pet) + + assert %EctoDiff{ + effect: :changed, + struct: Pet, + primary_key: %{id: id}, + previous: pet, + current: updated_pet, + changes: %{ + name: {"Spot", "McFluffFace"} + } + } == diff + end + # Embeds One Association test "no changes with embeds_one" do @@ -671,7 +990,7 @@ defmodule EctoDiffTest do } = diff end - # TODO: the following case only tests embeds_one with on_replace: :update + # the following case only tests embeds_one with on_replace: :update # other cases should be written for other options to on_replace test "update an embeds_one using on_replace: :update" do diff --git a/test/support/test_schemas/pet.ex b/test/support/test_schemas/pet.ex index 8a40e68..045702f 100644 --- a/test/support/test_schemas/pet.ex +++ b/test/support/test_schemas/pet.ex @@ -5,15 +5,16 @@ defmodule EctoDiff.Pet do import Ecto.Changeset schema("pets") do - field :name, :string - field :type, :string, default: "Cat" - field :refid, Ecto.UUID, autogenerate: true - field :owner_email, :string, virtual: true - belongs_to :owner, EctoDiff.Owner, on_replace: :update has_many :skills, EctoDiff.Skill, on_replace: :delete + has_many :resources, EctoDiff.Resource, on_replace: :delete + has_many :toys, through: [:resources, :toys], on_replace: :update embeds_one :details, EctoDiff.PetDetails, on_replace: :update embeds_many :quotes, EctoDiff.PetQuote, on_replace: :delete + field :name, :string + field :type, :string, default: "Cat" + field :refid, Ecto.UUID, autogenerate: true + field :owner_email, :string, virtual: true end def new(params), do: changeset(%__MODULE__{}, params) @@ -24,6 +25,7 @@ defmodule EctoDiff.Pet do |> cast(params, [:name, :type]) |> cast_assoc(:owner) |> cast_assoc(:skills) + |> cast_assoc(:resources) |> cast_embed(:details) |> cast_embed(:quotes) end diff --git a/test/support/test_schemas/resource.ex b/test/support/test_schemas/resource.ex new file mode 100644 index 0000000..01b14b5 --- /dev/null +++ b/test/support/test_schemas/resource.ex @@ -0,0 +1,19 @@ +defmodule EctoDiff.Resource do + @moduledoc false + + use Ecto.Schema + import Ecto.Changeset + + schema("resources") do + belongs_to :pet, EctoDiff.Pet + has_many :toys, EctoDiff.Toy, on_replace: :delete + + field :refid, Ecto.UUID, autogenerate: true + end + + def changeset(item, params \\ %{}) do + item + |> cast(params, [:pet_id]) + |> cast_assoc(:toys) + end +end diff --git a/test/support/test_schemas/toy.ex b/test/support/test_schemas/toy.ex new file mode 100644 index 0000000..b3bce60 --- /dev/null +++ b/test/support/test_schemas/toy.ex @@ -0,0 +1,18 @@ +defmodule EctoDiff.Toy do + @moduledoc false + + use Ecto.Schema + import Ecto.Changeset + + schema("toys") do + belongs_to :resource, EctoDiff.Resource, type: :id + + field :name, :string + field :type, :string + field :quantity, :integer, default: 1 + end + + def changeset(toy, params) do + toy |> cast(params, [:name, :type, :quantity, :resource_id]) + end +end