Repository: elixir-ecto/ecto
Pull request #2113 (2017-07-01 15:19:19)

Stop using `__schema__` to resolve assocs in Ecto.Changeset
Ecto.Changeset should use information from type when available.

This is a first step in implementing Dynamic Embeds as discussed in #1763. I'm splitting all work to have many small changes, rather than one big.

In next PR I'll try to reduce the number of `__schema__` calls in Ecto.Repo.Schema, so Changesets that were dynamically created will be able to be inserted into DB.

ebert[bot]: Ebert has finished reviewing this Pull Request and has found:

* 1 possible new issue (including those that may have been commented here).
* 1 fixed issue! 🎉

But beware that this branch is **[3 commits behind the `elixir-ecto:master` branch](https://github.com/elixir-ecto/ecto/compare/AndrewDryga:schemaless-changesets...elixir-ecto:master)**, and a review of an up to date branch would produce more accurate results.

You can see more details about this review at https://ebertapp.io/github/elixir-ecto/ecto/pulls/2113.

<!-- Ebert auto generated comment -->

ebert[bot]: Ebert has finished reviewing this Pull Request and has found:

* 1 fixed issue! 🎉

You can see more details about this review at https://ebertapp.io/github/elixir-ecto/ecto/pulls/2113.

<!-- Ebert auto generated comment -->


Patch:
diff --git a/lib/ecto/changeset.ex b/lib/ecto/changeset.ex
index 60bf40ea..e1faf33d 100644
--- a/lib/ecto/changeset.ex
+++ b/lib/ecto/changeset.ex
@@ -1902,7 +1902,7 @@ defmodule Ecto.Changeset do
       create unique_index(:users, [:email, :company_id])
 
   Because such indexes have usually more complex names, we need
-  to explicitly tell the changeset which constraint name to use (here we're 
+  to explicitly tell the changeset which constraint name to use (here we're
   using the naming convention that `unique_index` uses):
 
       cast(user, params, [:email])
@@ -2132,7 +2132,6 @@ defmodule Ecto.Changeset do
   defp no_assoc_message(:one), do: "is still associated with this entry"
   defp no_assoc_message(:many), do: "are still associated with this entry"
 
-
   defp add_constraint(changeset, type, constraint, match, field, error)
        when is_binary(constraint) and is_atom(field) and is_tuple(error) and is_atom(match)  do
     unless match in @match_types, do: raise(ArgumentError, "invalid match type: #{inspect match}. Allowed match types: #{inspect @match_types}")
@@ -2145,16 +2144,19 @@ defmodule Ecto.Changeset do
   defp get_source(%{data: data}), do:
     raise(ArgumentError, "cannot add constraint to changeset because it does not have a source, got: #{inspect data}")
 
-  defp get_assoc(%{data: %{__struct__: schema}}, assoc) do
-    schema.__schema__(:association, assoc) || raise_invalid_assoc(schema, assoc)
+  defp get_assoc(%{types: types}, assoc) do
+    case Map.fetch(types, assoc) do
+      {:ok, {:assoc, association}} ->
+        association
+      _ ->
+        raise_invalid_assoc(types, assoc)
+    end
   end
 
-  defp raise_invalid_assoc(schema, assoc) do
-    associations = Enum.map_join(schema.__schema__(:associations), ", ", fn(association) ->
-      "`#{association}`"
-    end)
-    msg = "cannot add constraint to changeset because association `#{assoc}` does not exist. Did you mean one of #{associations}?"
-    raise(ArgumentError, msg)
+  defp raise_invalid_assoc(types, assoc) do
+    associations = for {_key, {:assoc, %{field: field}}} <- types, do: field
+    raise ArgumentError, "cannot add constraint to changeset because association `#{assoc}` does not exist. " <>
+                         "Did you mean one of `#{Enum.join(associations, "`, `")}`?"
   end
 
   @doc ~S"""
diff --git a/lib/mix/tasks/ecto.dump.ex b/lib/mix/tasks/ecto.dump.ex
index 6e539c62..285cbdc1 100644
--- a/lib/mix/tasks/ecto.dump.ex
+++ b/lib/mix/tasks/ecto.dump.ex
@@ -12,6 +12,13 @@ defmodule Mix.Tasks.Ecto.Dump do
   The repository must be set under `:ecto_repos` in the
   current app configuration or given via the `-r` option.
 
+  This task needs some shell utility to be present on the machine running the task.
+
+   Database   | Utility needed
+   :--------- | :-------------
+   PostgreSQL | pg_dump
+   MySQL      | mysqldump
+
   ## Example
 
       mix ecto.dump
diff --git a/lib/mix/tasks/ecto.load.ex b/lib/mix/tasks/ecto.load.ex
index 13c3d2a5..d7ba45c6 100644
--- a/lib/mix/tasks/ecto.load.ex
+++ b/lib/mix/tasks/ecto.load.ex
@@ -12,6 +12,13 @@ defmodule Mix.Tasks.Ecto.Load do
   The repository must be set under `:ecto_repos` in the
   current app configuration or given via the `-r` option.
 
+  This task needs some shell utility to be present on the machine running the task.
+
+   Database   | Utility needed
+   :--------- | :-------------
+   PostgreSQL | psql
+   MySQL      | mysql
+
   ## Example
 
       mix ecto.load


Test patch:
diff --git a/test/ecto/changeset_test.exs b/test/ecto/changeset_test.exs
index 046bd612..36eaf6c4 100644
--- a/test/ecto/changeset_test.exs
+++ b/test/ecto/changeset_test.exs
@@ -2,6 +2,20 @@ defmodule Ecto.ChangesetTest do
   use ExUnit.Case, async: true
   import Ecto.Changeset
 
+  defmodule SocialSource do
+    use Ecto.Schema
+
+    @primary_key false
+    embedded_schema do
+      field :origin
+      field :url
+    end
+
+    def changeset(schema \\ %SocialSource{}, params) do
+      cast(schema, params, ~w(origin url))
+    end
+  end
+
   defmodule Comment do
     use Ecto.Schema
 
@@ -22,6 +36,7 @@ defmodule Ecto.ChangesetTest do
       field :topics, {:array, :string}
       field :virtual, :string, virtual: true
       field :published_at, :naive_datetime
+      field :source, :map
       has_many :comments, Ecto.ChangesetTest.Comment, on_replace: :delete
       has_one :comment, Ecto.ChangesetTest.Comment
     end
@@ -99,6 +114,44 @@ defmodule Ecto.ChangesetTest do
     assert apply_changes(changeset) == %{title: "world", upvotes: 0}
   end
 
+  test "cast/4: with dynamic embed" do
+    data = {
+      %{
+        title: "hello"
+      },
+      %{
+        title: :string,
+        source: {
+          :embed,
+          %Ecto.Embedded{
+            cardinality: :one,
+            field: :source,
+            on_cast: &SocialSource.changeset(&1, &2),
+            on_replace: :raise,
+            owner: nil,
+            related: SocialSource,
+            unique: true
+          }
+        }
+      }
+    }
+
+    params = %{"title" => "world", "source" => %{"origin" => "facebook", "url" => "http://example.com/social"}}
+
+    changeset =
+      data
+      |> cast(params, ~w(title))
+      |> cast_embed(:source, required: true)
+
+    assert changeset.params == params
+    assert changeset.data  == %{title: "hello"}
+    assert %{title: "world", source: %Ecto.Changeset{}} = changeset.changes
+    assert changeset.errors == []
+    assert changeset.valid?
+    assert apply_changes(changeset) ==
+      %{title: "world", source: %Ecto.ChangesetTest.SocialSource{origin: "facebook", url: "http://example.com/social"}}
+  end
+
   test "cast/4: with changeset" do
     base_changeset = cast(%Post{title: "valid"}, %{}, ~w(title))
                      |> validate_required(:title)
@@ -1064,7 +1117,7 @@ defmodule Ecto.ChangesetTest do
   end
 
   test "assoc_constraint/3 with errors" do
-    message = ~r"cannot add constraint to changeset because association `unknown` does not exist. Did you mean one of `comments`, `comment`?"
+    message = ~r"cannot add constraint to changeset because association `unknown` does not exist. Did you mean one of `comment`, `comments`?"
     assert_raise ArgumentError, message, fn ->
       change(%Post{}) |> assoc_constraint(:unknown)
     end