Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support Ecto Embed #112

Merged
merged 1 commit into from
Dec 18, 2020
Merged

Support Ecto Embed #112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 46 additions & 3 deletions lib/paper_trail/serializer.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ defmodule PaperTrail.Serializer do
Dumps changes using Ecto fields
"""
@spec serialize_changes(Ecto.Changeset.t()) :: map()
def serialize_changes(%Ecto.Changeset{data: %schema{}, changes: changes}) do
changes
|> schema.__struct__()
def serialize_changes(%Ecto.Changeset{changes: changes} = changeset) do
changeset
|> serialize_model_changes()
|> serialize()
|> Map.take(Map.keys(changes))
end
Expand Down Expand Up @@ -147,4 +147,47 @@ defmodule PaperTrail.Serializer do
"#{model_id}"
end
end

@spec serialize_model_changes(Ecto.Changeset.t()) :: map()
defp serialize_model_changes(%Ecto.Changeset{data: %schema{}} = changeset) do
field_values = serialize_model_field_changes(changeset)
embed_values = serialize_model_embed_changes(changeset)

field_values
|> Map.merge(embed_values)
|> schema.__struct__()
end

defp serialize_model_field_changes(%Ecto.Changeset{data: %schema{}, changes: changes}) do
change_keys = changes |> Map.keys() |> MapSet.new()

field_keys =
:fields
|> schema.__schema__()
|> MapSet.new()
|> MapSet.intersection(change_keys)
|> MapSet.to_list()

Map.take(changes, field_keys)
end

defp serialize_model_embed_changes(%Ecto.Changeset{data: %schema{}, changes: changes}) do
change_keys = changes |> Map.keys() |> MapSet.new()

embed_keys =
:embeds
|> schema.__schema__()
|> MapSet.new()
|> MapSet.intersection(change_keys)
|> MapSet.to_list()

changes
|> Map.take(embed_keys)
|> Map.new(fn {key, value} ->
case schema.__schema__(:embed, key) do
%Ecto.Embedded{cardinality: :one} -> {key, serialize_model_changes(value)}
%Ecto.Embedded{cardinality: :many} -> {key, Enum.map(value, &serialize_model_changes/1)}
Copy link
Owner

@izelnakri izelnakri Dec 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm currently trying to understand this line. Could you explain this further @maennchen , are these the only cases the code can ever hit? is there a _ -> _ case as well that allows certain keys to be ignored? or do they get ignored during embed_keys or maps with normal fields hit this %Ecto.Embedded{cardinality: :one} -> {key, serialize_model_changes(value)} per each/every key? Generally its a nice PR, but would have helped if we also published the changes in few PRs as I see few changes are bit unrelated. Regardless Im currently reading up on everything, thanks for the very interesting work! We should document this functionality in the README.md as well before merging anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's only two cases: embeds_one and embeds_many. Therefore no catch-all is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's four commits in this PR on purpose. I can open them in as many PRs as you like. (The order is kind of important though.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About documentation I don't think there's much to say. The library supports ectos features. The features are already documented at ecto.

What would you like me to mention in the README?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@izelnakri I split the PR apart into #122, #123, #124

Copy link
Owner

@izelnakri izelnakri Dec 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Merged the first two, I'll also merge the third one. Could you make github clean the diff for the embed changes afterwards? I'll try to review in the next 10 days. Thanks again for all the work!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@izelnakri Sure, I'll rebase it as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@izelnakri Rebased as well.

end
end)
end
end
2 changes: 2 additions & 0 deletions priv/repo/migrations/20160619190938_add_simple_people.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ defmodule Repo.Migrations.CreateSimplePeople do
add :visit_count, :integer
add :gender, :boolean
add :birthdate, :date
add :singular, :map
add :plural, {:array, :map}

add :company_id, references(:simple_companies), null: false

Expand Down
129 changes: 123 additions & 6 deletions test/paper_trail/bang_functions_simple_mode_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,9 @@ defmodule PaperTrailTest.SimpleModeBangFunctions do
gender: true,
visit_count: nil,
birthdate: nil,
company_id: second_company.id
company_id: second_company.id,
plural: [],
singular: nil
}

assert Map.drop(version, [:id, :inserted_at]) == %{
Expand All @@ -342,6 +344,39 @@ defmodule PaperTrailTest.SimpleModeBangFunctions do
assert person == first(Person, :id) |> repo().one |> serialize
end

test "creating a person with embeds creates a person version with correct attributes" do
company = create_company_with_version()

%Person{
id: person_id,
plural: [%{id: _, name: "Plural"}],
singular: %{id: _, name: "Singular"}
} =
person =
%Person{}
|> Person.changeset(%{
first_name: "Izel",
last_name: "Nakri",
gender: true,
company_id: company.id,
plural: [%{name: "Plural"}],
singular: %{name: "Singular"}
})
|> PaperTrail.insert!()

version = PaperTrail.get_version(person)
person_change = person |> serialize() |> convert_to_string_map

assert %{
event: "insert",
item_type: "SimplePerson",
item_id: ^person_id,
item_changes: ^person_change
} = version

assert person == first(Person, :id) |> repo().one
end

test "updating a person creates a person version with correct attributes" do
inserted_initial_company =
create_company_with_version(%{
Expand Down Expand Up @@ -391,7 +426,9 @@ defmodule PaperTrailTest.SimpleModeBangFunctions do
visit_count: 10,
birthdate: ~D[1992-04-01],
last_name: "Nakri",
gender: true
gender: true,
plural: [],
singular: nil
}

assert Map.drop(version, [:id, :inserted_at]) == %{
Expand All @@ -413,6 +450,44 @@ defmodule PaperTrailTest.SimpleModeBangFunctions do
assert person == first(Person, :id) |> repo().one |> serialize
end

test "updating a person with embeds creates a person version with correct attributes" do
company = create_company_with_version()

%Person{
id: person_id,
plural: [%{id: _, name: "Plural"}],
singular: %{id: _, name: "Singular"}
} =
person =
%Person{}
|> Person.changeset(%{
first_name: "Izel",
last_name: "Nakri",
gender: true,
company_id: company.id
})
|> PaperTrail.insert!()
|> Person.changeset(%{
plural: [%{name: "Plural"}],
singular: %{name: "Singular"}
})
|> PaperTrail.update!()

version = PaperTrail.get_version(person)

assert %{
event: "update",
item_type: "SimplePerson",
item_id: ^person_id,
item_changes: %{
"plural" => [%{"id" => _, "name" => "Plural"}],
"singular" => %{"id" => _, "name" => "Singular"}
}
} = version

assert person == first(Person, :id) |> repo().one
end

test "deleting a person creates a person version with correct attributes" do
create_company_with_version(%{name: "Acme LLC", website: "http://www.acme.com"})

Expand Down Expand Up @@ -475,7 +550,9 @@ defmodule PaperTrailTest.SimpleModeBangFunctions do
gender: true,
visit_count: 10,
birthdate: ~D[1992-04-01],
company_id: inserted_target_company.id
company_id: inserted_target_company.id,
plural: [],
singular: nil
}),
originator_id: nil,
origin: "admin",
Expand All @@ -485,6 +562,40 @@ defmodule PaperTrailTest.SimpleModeBangFunctions do
assert old_person == person_before_deletion
end

test "deleting a person with embeds creates a person version with correct attributes" do
company = create_company_with_version()

%Person{
id: person_id,
plural: [%{id: _, name: "Plural"}],
singular: %{id: _, name: "Singular"}
} =
person =
%Person{}
|> Person.changeset(%{
first_name: "Izel",
last_name: "Nakri",
gender: true,
company_id: company.id,
plural: [%{name: "Plural"}],
singular: %{name: "Singular"}
})
|> PaperTrail.insert!()
|> PaperTrail.delete!()

version = PaperTrail.get_version(person)
person_change = person |> serialize() |> convert_to_string_map

assert %{
event: "delete",
item_type: "SimplePerson",
item_id: ^person_id,
item_changes: ^person_change
} = version

assert is_nil(first(Person, :id) |> repo().one)
end

# Multi tenant tests
test "[multi tenant] creating a company creates a company version with correct attributes" do
tenant = MultiTenant.tenant()
Expand Down Expand Up @@ -791,7 +902,9 @@ defmodule PaperTrailTest.SimpleModeBangFunctions do
gender: true,
visit_count: nil,
birthdate: nil,
company_id: second_company.id
company_id: second_company.id,
plural: [],
singular: nil
}

assert Map.drop(version, [:id, :inserted_at]) == %{
Expand Down Expand Up @@ -863,7 +976,9 @@ defmodule PaperTrailTest.SimpleModeBangFunctions do
visit_count: 10,
birthdate: ~D[1992-04-01],
last_name: "Nakri",
gender: true
gender: true,
plural: [],
singular: nil
}

assert Map.drop(version, [:id, :inserted_at]) == %{
Expand Down Expand Up @@ -955,7 +1070,9 @@ defmodule PaperTrailTest.SimpleModeBangFunctions do
gender: true,
visit_count: 10,
birthdate: ~D[1992-04-01],
company_id: inserted_target_company.id
company_id: inserted_target_company.id,
plural: [],
singular: nil
}),
originator_id: nil,
origin: "admin",
Expand Down
12 changes: 9 additions & 3 deletions test/paper_trail/base_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,9 @@ defmodule PaperTrailTest do
gender: true,
visit_count: nil,
birthdate: nil,
company_id: new_company_result[:model].id
company_id: new_company_result[:model].id,
plural: [],
singular: nil
}

assert Map.drop(version, [:id, :inserted_at]) == %{
Expand Down Expand Up @@ -475,7 +477,9 @@ defmodule PaperTrailTest do
visit_count: 10,
birthdate: ~D[1992-04-01],
last_name: "Nakri",
gender: true
gender: true,
plural: [],
singular: nil
}

assert Map.drop(version, [:id, :inserted_at]) == %{
Expand Down Expand Up @@ -557,7 +561,9 @@ defmodule PaperTrailTest do
gender: true,
visit_count: 10,
birthdate: ~D[1992-04-01],
company_id: target_company_insertion[:model].id
company_id: target_company_insertion[:model].id,
plural: [],
singular: nil
},
originator_id: nil,
origin: "admin",
Expand Down
20 changes: 20 additions & 0 deletions test/support/simple_models.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ defmodule SimplePerson do

belongs_to(:company, SimpleCompany, foreign_key: :company_id)

embeds_one(:singular, SimpleEmbed)
embeds_many(:plural, SimpleEmbed)

timestamps()
end

Expand All @@ -113,6 +116,8 @@ defmodule SimplePerson do
model
|> cast(params, @optional_fields)
|> foreign_key_constraint(:company_id)
|> cast_embed(:singular)
|> cast_embed(:plural)
end

def count do
Expand All @@ -125,3 +130,18 @@ defmodule SimplePerson do
|> PaperTrail.RepoClient.repo().one
end
end

defmodule SimpleEmbed do
use Ecto.Schema

import Ecto.Changeset

embedded_schema do
field(:name, :string)
end

def changeset(model, params \\ %{}) do
model
|> cast(params, [:name])
end
end