Repository: phoenixframework/phoenix
Pull request #4600 (2021-11-16 02:42:09)

fix: `Ecto.Enum` field creation with `mix phx.gen.embedded`
Fixes: #4593



Patch:
diff --git a/guides/contexts.md b/guides/contexts.md
index 673bfae1b..355f967ca 100644
--- a/guides/contexts.md
+++ b/guides/contexts.md
@@ -262,7 +262,7 @@ Intuitively, you would assume the following events:
   1. User 1 loads the product page with count of 13
   2. User 1 saves the product page with count of 14
   3. User 2 loads the product page with count of 14
-  4. User 2 loads the product page with count of 15
+  4. User 2 saves the product page with count of 15
 
 While in practice this would happen:
 
@@ -273,7 +273,7 @@ While in practice this would happen:
 
 The race conditions would make this an unreliable way to update the existing table since multiple callers may be updating out of date view values. There's a better way.
 
-Let's think of a function describes what we want to accomplish. Here's how we would like to use it:
+Let's think of a function that describes what we want to accomplish. Here's how we would like to use it:
 
     product = Catalog.inc_page_views(product)
 
@@ -752,7 +752,7 @@ We added a new `:fetch_current_user` and `:fetch_current_cart` plug to our brows
 
 We'll need to implement a cart controller for handling cart operations like viewing a cart, updating quantities, and initiating the checkout process, as well as a cart items controller for adding and removing individual items to and from the cart. Add the following routes to your router in `lib/hello_web/router.ex`:
 
-```elixir
+```diff
   scope "/", HelloWeb do
     pipe_through :browser
 
diff --git a/guides/controllers.md b/guides/controllers.md
index 0a69a443b..089283be7 100644
--- a/guides/controllers.md
+++ b/guides/controllers.md
@@ -262,9 +262,10 @@ def index(conn, _params) do
 end
 ```
 
-Reloading [http://localhost:4000](http://localhost:4000) should show us a completely blank page. The network tab of our browser's developer tools should show a response status of "201" (Created).
+Reloading [http://localhost:4000](http://localhost:4000) should show us a completely blank page. The network tab of our browser's developer tools should show a response status of "201" (Created). Some browsers (Safari) will download the response, as the content type is not set.
+
+To be specific about the content type, we can use [`put_resp_content_type/2`] in conjunction with [`send_resp/3`].
 
-If we would like to be really specific about the content type, we can use [`put_resp_content_type/2`] in conjunction with [`send_resp/3`].
 
 ```elixir
 def index(conn, _params) do
diff --git a/lib/mix/phoenix/schema.ex b/lib/mix/phoenix/schema.ex
index 571ee90d4..4b2004b21 100644
--- a/lib/mix/phoenix/schema.ex
+++ b/lib/mix/phoenix/schema.ex
@@ -255,6 +255,12 @@ defmodule Mix.Phoenix.Schema do
   def type_for_migration({:enum, _}), do: :string
   def type_for_migration(other), do: other
 
+  def format_fields_for_schema(schema) do
+    Enum.map_join(schema.types, "\n", fn {k, v} ->
+      "    field #{inspect(k)}, #{type_and_opts_for_schema(v)}#{schema.defaults[k]}#{maybe_redact_field(k in schema.redacts)}"
+    end)
+  end
+
   def type_and_opts_for_schema({:enum, opts}), do: ~s|Ecto.Enum, values: #{inspect Keyword.get(opts, :values)}|
   def type_and_opts_for_schema(other), do: inspect other
 
diff --git a/lib/phoenix/controller.ex b/lib/phoenix/controller.ex
index 2c79ed20c..e8155d84b 100644
--- a/lib/phoenix/controller.ex
+++ b/lib/phoenix/controller.ex
@@ -861,7 +861,7 @@ defmodule Phoenix.Controller do
   `Routes.some_route_path` helpers, as those are always relative.
   """
   def put_router_url(conn, %URI{} = uri) do
-    put_private(conn, :phoenix_router_url, uri)
+    put_private(conn, :phoenix_router_url, URI.to_string(uri))
   end
   def put_router_url(conn, url) when is_binary(url) do
     put_private(conn, :phoenix_router_url, url)
@@ -875,7 +875,7 @@ defmodule Phoenix.Controller do
   endpoint configuration (much like `put_router_url/2` but for static URLs).
   """
   def put_static_url(conn, %URI{} = uri) do
-    put_private(conn, :phoenix_static_url, uri)
+    put_private(conn, :phoenix_static_url, URI.to_string(uri))
   end
   def put_static_url(conn, url) when is_binary(url) do
     put_private(conn, :phoenix_static_url, url)
diff --git a/lib/phoenix/router/helpers.ex b/lib/phoenix/router/helpers.ex
index 443b8527e..7e65dfaaf 100644
--- a/lib/phoenix/router/helpers.ex
+++ b/lib/phoenix/router/helpers.ex
@@ -10,7 +10,6 @@ defmodule Phoenix.Router.Helpers do
   """
   def url(_router, %Conn{private: private}) do
     case private do
-      %{phoenix_router_url: %URI{} = uri} -> URI.to_string(uri)
       %{phoenix_router_url: url} when is_binary(url) -> url
       %{phoenix_endpoint: endpoint} -> endpoint.url()
     end
@@ -237,7 +236,6 @@ defmodule Phoenix.Router.Helpers do
       """
       def static_url(%Conn{private: private}, path) do
         case private do
-          %{phoenix_static_url: %URI{} = uri} -> URI.to_string(uri) <> path
           %{phoenix_static_url: url} when is_binary(url) -> url <> path
           %{phoenix_endpoint: endpoint} -> static_url(endpoint, path)
         end
@@ -418,13 +416,13 @@ defmodule Phoenix.Router.Helpers do
   end
 
   defp expand_segments([{:|, _, [h, t]}], acc),
-    do: quote(do: unquote(expand_segments([h], acc)) <> "/" <> Enum.map_join(unquote(t), "/", fn(s) -> URI.encode(s, &URI.char_unreserved?/1) end))
+    do: quote(do: unquote(expand_segments([h], acc)) <> "/" <> Enum.map_join(unquote(t), "/", &unquote(__MODULE__).encode_param/1))
 
   defp expand_segments([h|t], acc) when is_binary(h),
     do: expand_segments(t, quote(do: unquote(acc) <> unquote("/" <> h)))
 
   defp expand_segments([h|t], acc),
-    do: expand_segments(t, quote(do: unquote(acc) <> "/" <> URI.encode(to_param(unquote(h)), &URI.char_unreserved?/1)))
+    do: expand_segments(t, quote(do: unquote(acc) <> "/" <> unquote(__MODULE__).encode_param(to_param(unquote(h)))))
 
   defp expand_segments([], acc),
     do: acc
diff --git a/priv/templates/phx.gen.embedded/embedded_schema.ex b/priv/templates/phx.gen.embedded/embedded_schema.ex
index 03c4b6fd1..e3acfb81a 100644
--- a/priv/templates/phx.gen.embedded/embedded_schema.ex
+++ b/priv/templates/phx.gen.embedded/embedded_schema.ex
@@ -4,8 +4,8 @@ defmodule <%= inspect schema.module %> do
   alias <%= inspect schema.module %>
 
   embedded_schema do
-<%= for {k, v} <- schema.types do %>    field <%= inspect k %>, <%= inspect v %><%= schema.defaults[k] %>
-<% end %>  end
+<%= Mix.Phoenix.Schema.format_fields_for_schema(schema) %>
+  end
 
   @doc false
   def changeset(%<%= inspect schema.alias %>{} = <%= schema.singular %>, attrs) do
diff --git a/priv/templates/phx.gen.schema/schema.ex b/priv/templates/phx.gen.schema/schema.ex
index 63a19d420..fc1d4788e 100644
--- a/priv/templates/phx.gen.schema/schema.ex
+++ b/priv/templates/phx.gen.schema/schema.ex
@@ -6,8 +6,8 @@ defmodule <%= inspect schema.module %> do
   @primary_key {:id, :binary_id, autogenerate: true}
   @foreign_key_type :binary_id<% end %>
   schema <%= inspect schema.table %> do
-<%= for {k, v} <- schema.types do %>    field <%= inspect k %>, <%= Mix.Phoenix.Schema.type_and_opts_for_schema(v) %><%= schema.defaults[k] %><%= Mix.Phoenix.Schema.maybe_redact_field(k in schema.redacts) %>
-<% end %><%= for {_, k, _, _} <- schema.assocs do %>    field <%= inspect k %>, <%= if schema.binary_id do %>:binary_id<% else %>:id<% end %>
+<%= Mix.Phoenix.Schema.format_fields_for_schema(schema) %>
+<%= for {_, k, _, _} <- schema.assocs do %>    field <%= inspect k %>, <%= if schema.binary_id do %>:binary_id<% else %>:id<% end %>
 <% end %>
     timestamps()
   end


Test patch:
diff --git a/test/mix/tasks/phx.gen.embedded_test.exs b/test/mix/tasks/phx.gen.embedded_test.exs
index 088e572af..e0526f8d7 100644
--- a/test/mix/tasks/phx.gen.embedded_test.exs
+++ b/test/mix/tasks/phx.gen.embedded_test.exs
@@ -1,4 +1,4 @@
-Code.require_file "../../../installer/test/mix_helper.exs", __DIR__
+Code.require_file("../../../installer/test/mix_helper.exs", __DIR__)
 
 defmodule Mix.Tasks.Phx.Gen.EmbeddedTest do
   use ExUnit.Case
@@ -12,57 +12,83 @@ defmodule Mix.Tasks.Phx.Gen.EmbeddedTest do
   end
 
   test "build" do
-    in_tmp_project "embedded build", fn ->
+    in_tmp_project("embedded build", fn ->
       schema = Gen.Embedded.build(~w(Blog.Post title:string))
 
       assert %Schema{
-        alias: Post,
-        module: Phoenix.Blog.Post,
-        repo: Phoenix.Repo,
-        migration?: false,
-        migration_defaults: %{title: ""},
-        plural: nil,
-        singular: "post",
-        human_plural: "Nil",
-        human_singular: "Post",
-        attrs: [title: :string],
-        types: %{title: :string},
-        embedded?: true,
-        defaults: %{title: ""},
-      } = schema
+               alias: Post,
+               module: Phoenix.Blog.Post,
+               repo: Phoenix.Repo,
+               migration?: false,
+               migration_defaults: %{title: ""},
+               plural: nil,
+               singular: "post",
+               human_plural: "Nil",
+               human_singular: "Post",
+               attrs: [title: :string],
+               types: %{title: :string},
+               embedded?: true,
+               defaults: %{title: ""}
+             } = schema
+
       assert String.ends_with?(schema.file, "lib/phoenix/blog/post.ex")
-    end
+    end)
   end
 
   test "generates embedded schema", config do
-    in_tmp_project config.test, fn ->
+    in_tmp_project(config.test, fn ->
       Gen.Embedded.run(~w(Blog.Post title:string))
-      assert_file "lib/phoenix/blog/post.ex", fn file ->
+
+      assert_file("lib/phoenix/blog/post.ex", fn file ->
         assert file =~ "embedded_schema do"
-      end
-    end
+      end)
+    end)
   end
 
   test "generates nested embedded schema", config do
-    in_tmp_project config.test, fn ->
+    in_tmp_project(config.test, fn ->
       Gen.Embedded.run(~w(Blog.Admin.User name:string))
 
-      assert_file "lib/phoenix/blog/admin/user.ex", fn file ->
+      assert_file("lib/phoenix/blog/admin/user.ex", fn file ->
         assert file =~ "defmodule Phoenix.Blog.Admin.User do"
         assert file =~ "embedded_schema do"
-      end
-    end
+      end)
+    end)
   end
 
   test "generates embedded schema with proper datetime types", config do
-    in_tmp_project config.test, fn ->
-      Gen.Embedded.run(~w(Blog.Comment title:string drafted_at:datetime published_at:naive_datetime edited_at:utc_datetime))
+    in_tmp_project(config.test, fn ->
+      Gen.Embedded.run(
+        ~w(Blog.Comment title:string drafted_at:datetime published_at:naive_datetime edited_at:utc_datetime)
+      )
 
-      assert_file "lib/phoenix/blog/comment.ex", fn file ->
+      assert_file("lib/phoenix/blog/comment.ex", fn file ->
         assert file =~ "field :drafted_at, :naive_datetime"
         assert file =~ "field :published_at, :naive_datetime"
         assert file =~ "field :edited_at, :utc_datetime"
-      end
-    end
+      end)
+    end)
+  end
+
+  test "generates embedded schema with enum", config do
+    in_tmp_project(config.test, fn ->
+      Gen.Embedded.run(
+        ~w(Blog.Comment comments title:string status:enum:unpublished:published:deleted)
+      )
+
+      assert_file("lib/phoenix/blog/comment.ex", fn file ->
+        assert file =~ "field :status, Ecto.Enum, values: [:unpublished, :published, :deleted]"
+      end)
+    end)
+  end
+
+  test "generates embedded schema with redact option", config do
+    in_tmp_project(config.test, fn ->
+      Gen.Embedded.run(~w(Blog.Comment comments title:string secret:redact))
+
+      assert_file("lib/phoenix/blog/comment.ex", fn file ->
+        assert file =~ "field :secret, :string, redact: true"
+      end)
+    end)
   end
 end