Only include form errors if an action was set

lib/phoenix_ecto/html.ex

@@ -1,6 +1,7 @@
 if Code.ensure_loaded?(Phoenix.HTML) do
   defimpl Phoenix.HTML.FormData, for: Ecto.Changeset do
-    def to_form(%Ecto.Changeset{params: params, data: data} = changeset, opts) do
+    def to_form(changeset, opts) do
+      %{params: params, data: data} = changeset
       {name, opts} = Keyword.pop(opts, :as)
       name = to_string(name || form_for_name(data))
 
@@ -9,15 +10,15 @@ if Code.ensure_loaded?(Phoenix.HTML) do
         impl: __MODULE__,
         id: name,
         name: name,
-        errors: changeset.errors,
+        errors: form_for_errors(changeset),
         data: data,
         params: params || %{},
         hidden: form_for_hidden(data),
         options: Keyword.put_new(opts, :method, form_for_method(data))
       }
     end
 
-    def to_form(source, form, field, opts) do
+    def to_form(%{action: parent_action} = source, form, field, opts) do
       if Keyword.has_key?(opts, :default) do
         raise ArgumentError, ":default is not supported on inputs_for with changesets. " <>
                              "The default value must be set in the changeset data"
@@ -41,16 +42,17 @@ if Code.ensure_loaded?(Phoenix.HTML) do
             end
 
           for changeset <- skip_replaced(changesets) do
-            %{data: data} = changeset = to_changeset(changeset, module, cast)
+            %{data: data, params: params} = changeset =
+              to_changeset(changeset, parent_action, module, cast)
 
             %Phoenix.HTML.Form{
               source: changeset,
               impl: __MODULE__,
               id: id,
               name: name,
-              errors: changeset.errors,
+              errors: form_for_errors(changeset),
               data: data,
-              params: changeset.params || %{},
+              params: params || %{},
               hidden: form_for_hidden(data),
               options: opts
             }
@@ -72,7 +74,8 @@ if Code.ensure_loaded?(Phoenix.HTML) do
           changesets = skip_replaced(changesets)
 
           for {changeset, index} <- Enum.with_index(changesets) do
-            %{data: data} = changeset = to_changeset(changeset, module, cast)
+            %{data: data, params: params} = changeset =
+              to_changeset(changeset, parent_action, module, cast)
             index_string = Integer.to_string(index)
 
             %Phoenix.HTML.Form{
@@ -81,9 +84,9 @@ if Code.ensure_loaded?(Phoenix.HTML) do
               id: id <> "_" <> index_string,
               name: name <> "[" <> index_string <> "]",
               index: index,
-              errors: changeset.errors,
+              errors: form_for_errors(changeset),
               data: data,
-              params: changeset.params || %{},
+              params: params || %{},
               hidden: form_for_hidden(data),
               options: opts
             }
@@ -118,15 +121,14 @@ if Code.ensure_loaded?(Phoenix.HTML) do
     defp assoc_from_data(data, field) do
       assoc_from_data(data, Map.fetch!(data, field), field)
     end
-
-    defp assoc_from_data(%{__meta__: %{state: :built}}, %Ecto.Association.NotLoaded{}, _field), do: nil
-
+    defp assoc_from_data(%{__meta__: %{state: :built}}, %Ecto.Association.NotLoaded{}, _field) do
+      nil
+    end
     defp assoc_from_data(%{__struct__: struct}, %Ecto.Association.NotLoaded{}, field) do
       raise ArgumentError, "using inputs_for for association `#{field}` " <>
         "from `#{inspect struct}` but it was not loaded. Please preload your " <>
         "associations before using them in inputs_for"
     end
-
     defp assoc_from_data(_data, value, _field) do
       value
     end
@@ -202,11 +204,19 @@ if Code.ensure_loaded?(Phoenix.HTML) do
       end
     end
 
-    defp to_changeset(%Ecto.Changeset{} = changeset, _module, _cast), do: changeset
-    defp to_changeset(%{} = data, _module, cast) when is_function(cast, 2),
-      do: cast.(data, :invalid)
-    defp to_changeset(%{} = data, _module, nil),
-      do: Ecto.Changeset.change(data)
+    defp to_changeset(%Ecto.Changeset{} = changeset, parent_action, _module, _cast),
+      do: apply_action(changeset, parent_action)
+    defp to_changeset(%{} = data, parent_action, _module, cast) when is_function(cast, 2),
+      do: apply_action(cast.(data, %{}), parent_action)
+    defp to_changeset(%{} = data, parent_action, _module, nil),
+      do: apply_action(Ecto.Changeset.change(data), parent_action)
+
+    # If the parent changeset had no action, we need to remove the action
+    # from children changeset so we ignore all errors accordingly.
+    defp apply_action(changeset, nil),
+      do: %{changeset | action: nil}
+    defp apply_action(changeset, _action),
+      do: changeset
 
     defp validate_list!(value, _what) when is_list(value) or is_nil(value), do: value
     defp validate_list!(value, what) do
@@ -218,6 +228,9 @@ if Code.ensure_loaded?(Phoenix.HTML) do
       raise ArgumentError, "expected #{what} to be a map/struct, got: #{inspect value}"
     end
 
+    defp form_for_errors(%{action: nil}), do: []
+    defp form_for_errors(%{errors: errors}), do: errors
+
     defp form_for_hidden(data) do
       # Since they are primary keys, we should ignore nil values.
       for {k, v} <- Ecto.primary_key(data), v != nil, do: {k, v}

test/phoenix_ecto/html_test.exs

@@ -5,6 +5,10 @@ defmodule PhoenixEcto.HTMLTest do
   import Phoenix.HTML
   import Phoenix.HTML.Form
 
+  defp safe_form_for(changeset, opts \\ [], function) do
+    safe_to_string(form_for(changeset, "/", opts, function))
+  end
+
   test "converts decimal to safe" do
     assert html_escape(Decimal.new("1.0")) == {:safe, "1.0"}
   end
@@ -21,35 +25,35 @@ defmodule PhoenixEcto.HTMLTest do
   end
 
   test "form_for/4 with new changeset" do
-    changeset = cast(%User{}, :invalid, ~w(), ~w())
+    changeset = cast(%User{}, :invalid, ~w())
                 |> validate_length(:name, min: 3)
 
-    form = safe_to_string(form_for(changeset, "/", fn f ->
+    form = safe_form_for(changeset, fn f ->
       assert f.id == "user"
       assert f.name == "user"
       assert f.impl == Phoenix.HTML.FormData.Ecto.Changeset
       assert f.source == changeset
       assert f.params == %{}
       assert f.hidden == []
       "FROM FORM"
-    end))
+    end)
 
     assert form =~ ~s(<form accept-charset="UTF-8" action="/" method="post">)
     assert form =~ "FROM FORM"
   end
 
   test "form_for/4 with loaded changeset" do
     changeset = cast(%User{__meta__: %{state: :loaded}, id: 13},
-                     %{"foo" => "bar"}, ~w(), ~w())
+                     %{"foo" => "bar"}, ~w())
 
-    form = safe_to_string(form_for(changeset, "/", fn f ->
+    form = safe_form_for(changeset, fn f ->
       assert f.id == "user"
       assert f.name == "user"
       assert f.source == changeset
       assert f.params == %{"foo" => "bar"}
       assert f.hidden == [id: 13]
       "FROM FORM"
-    end))
+    end)
 
     assert form =~ ~s(<form accept-charset="UTF-8" action="/" method="post">)
     assert form =~ ~s(<input name="_method" type="hidden" value="put">)
@@ -58,14 +62,14 @@ defmodule PhoenixEcto.HTMLTest do
   end
 
   test "form_for/4 with custom options" do
-    changeset = cast(%User{}, :invalid, ~w(), ~w())
+    changeset = cast(%User{}, :invalid, ~w())
 
-    form = safe_to_string(form_for(changeset, "/", [as: "another", multipart: true], fn f ->
+    form = safe_form_for(changeset, [as: "another", multipart: true], fn f ->
       assert f.id == "another"
       assert f.name == "another"
       assert f.source == changeset
       "FROM FORM"
-    end))
+    end)
 
     assert form =~ ~s(<form accept-charset="UTF-8" action="/" enctype="multipart/form-data" method="post">)
     assert form =~ "FROM FORM"
@@ -74,26 +78,33 @@ defmodule PhoenixEcto.HTMLTest do
   test "form_for/4 with errors" do
     changeset =
       %User{}
-      |> cast(%{"name" => "JV"}, ~w(name), ~w())
+      |> cast(%{"name" => "JV"}, ~w(name))
       |> validate_length(:name, min: 3)
       |> add_error(:score, {"must be greater than %{count}", count: Decimal.new(18)})
 
-    form = safe_to_string(form_for(changeset, "/", [as: "another", multipart: true], fn f ->
+    safe_form_for(changeset, [as: "another", multipart: true], fn f ->
+      assert f.errors == []
+      "FROM FORM"
+    end)
+
+    changeset = %{changeset | action: :insert}
+
+    form = safe_form_for(changeset, [as: "another", multipart: true], fn f ->
       assert f.errors == [score: {"must be greater than %{count}", count: Decimal.new(18)},
                           name: {"should be at least %{count} character(s)", count: 3}]
       "FROM FORM"
-    end))
+    end)
 
     assert form =~ ~s(<form accept-charset="UTF-8" action="/" enctype="multipart/form-data" method="post">)
     assert form =~ "FROM FORM"
   end
 
   test "form_for/4 with inputs" do
-    changeset = cast(%User{}, %{"name" => "JV"}, ~w(name), ~w())
+    changeset = cast(%User{}, %{"name" => "JV"}, ~w(name))
 
-    form = safe_to_string(form_for(changeset, "/", [as: "another", multipart: true], fn f ->
+    form = safe_form_for(changeset, [as: "another", multipart: true], fn f ->
       [text_input(f, :name), text_input(f, :other)]
-    end))
+    end)
 
     assert form =~ ~s(<input id="another_name" name="another[name]" type="text" value="JV">)
     assert form =~ ~s(<input id="another_other" name="another[other]" type="text">)
@@ -115,9 +126,9 @@ defmodule PhoenixEcto.HTMLTest do
   end
 
   test "input types" do
-    changeset = cast(%Custom{}, :invalid, [], [])
+    changeset = cast(%Custom{}, :invalid, ~w())
 
-    form_for(changeset, "/", fn f ->
+    safe_form_for(changeset, fn f ->
       assert input_type(f, :integer) == :number_input
       assert input_type(f, :float) == :number_input
       assert input_type(f, :decimal) == :number_input
@@ -132,12 +143,13 @@ defmodule PhoenixEcto.HTMLTest do
 
   test "input validations" do
     changeset =
-      cast(%Custom{}, :invalid, ~w(integer string), ~w())
+      cast(%Custom{}, :invalid, ~w(), ~w(integer string))
+      |> validate_required([:integer, :string])
       |> validate_number(:integer, greater_than: 0, less_than: 100)
       |> validate_number(:float, greater_than_or_equal_to: 0)
       |> validate_length(:string, min: 0, max: 100)
 
-    form_for(changeset, "/", fn f ->
+    safe_form_for(changeset, fn f ->
       assert input_validations(f, :integer) == [required: true, step: 1, min: 1, max: 99]
       assert input_validations(f, :float)   == [required: false, step: "any", min: 0]
       assert input_validations(f, :decimal) == [required: false]

test/phoenix_ecto/inputs_for_test.exs

@@ -51,6 +51,16 @@ defmodule PhoenixEcto.InputsForTest do
 
     contents =
       safe_inputs_for(changeset, :comment, fn f ->
+        assert f.errors == []
+        assert f.source.validations == [body: {:length, min: 3}]
+        text_input f, :body
+      end)
+
+    assert contents ==
+           ~s(<input id="user_comment_body" name="user[comment][body]" type="text" value="ht">)
+
+    contents =
+      safe_inputs_for(Map.put(changeset, :action, :insert), :comment, fn f ->
         assert f.errors == [body: {"should be at least %{count} character(s)", count: 3}]
         assert f.source.validations == [body: {:length, min: 3}]
         text_input f, :body
@@ -129,6 +139,7 @@ defmodule PhoenixEcto.InputsForTest do
       %User{comments: [%Comment{body: "def1"}, %Comment{body: "def2"}]}
       |> cast(:invalid, [], [])
       |> cast_assoc(:comments)
+      |> Map.put(:action, :insert)
 
     contents =
       safe_inputs_for(changeset, :comments, [
@@ -151,6 +162,7 @@ defmodule PhoenixEcto.InputsForTest do
       %User{comments: [%Comment{id: "a", body: "data1"}, %Comment{id: "b", body: "data2"}]}
       |> cast(:invalid, [], [])
       |> cast_assoc(:comments)
+      |> Map.put(:action, :insert)
 
     contents =
       safe_inputs_for(changeset, :comments,
@@ -177,6 +189,7 @@ defmodule PhoenixEcto.InputsForTest do
       |> cast(%{"comments" => [%{"id" => "1", "body" => "h1"},
                                %{"id" => "2", "body" => "h2"}]}, [], [])
       |> cast_assoc(:comments)
+      |> Map.put(:action, :insert)
 
     contents =
       safe_inputs_for(changeset, :comments,
@@ -270,6 +283,16 @@ defmodule PhoenixEcto.InputsForTest do
 
     contents =
       safe_inputs_for(changeset, :permalink, fn f ->
+        assert f.errors == []
+        assert f.source.validations == [url: {:length, min: 3}]
+        text_input f, :url
+      end)
+
+    assert contents ==
+           ~s(<input id="user_permalink_url" name="user[permalink][url]" type="text" value="ht">)
+
+    contents =
+      safe_inputs_for(Map.put(changeset, :action, :insert), :permalink, fn f ->
         assert f.errors == [url: {"should be at least %{count} character(s)", count: 3}]
         assert f.source.validations == [url: {:length, min: 3}]
         text_input f, :url
@@ -348,6 +371,7 @@ defmodule PhoenixEcto.InputsForTest do
       %User{permalinks: [%Permalink{url: "def1"}, %Permalink{url: "def2"}]}
       |> cast(:invalid, [], [])
       |> cast_embed(:permalinks)
+      |> Map.put(:action, :insert)
 
     contents =
       safe_inputs_for(changeset, :permalinks, [
@@ -370,6 +394,7 @@ defmodule PhoenixEcto.InputsForTest do
       %User{permalinks: [%Permalink{id: "a", url: "data1"}, %Permalink{id: "b", url: "data2"}]}
       |> cast(:invalid, [], [])
       |> cast_embed(:permalinks)
+      |> Map.put(:action, :insert)
 
     contents =
       safe_inputs_for(changeset, :permalinks,
@@ -396,6 +421,7 @@ defmodule PhoenixEcto.InputsForTest do
       |> cast(%{"permalinks" => [%{"id" => "a", "url" => "h1"},
                                  %{"id" => "b", "url" => "h2"}]}, [], [])
       |> cast_embed(:permalinks)
+      |> Map.put(:action, :insert)
 
     contents =
       safe_inputs_for(changeset, :permalinks,