Skip to content

Commit

Permalink
Skip map inputs in generated forms (#5582)
Browse files Browse the repository at this point in the history
Previously when using the html and live generators, generating a map
field would result in a text input field. There is no trivial way to
input map data in a text field. The map input is now skipped when
generating the html and live forms.

This introduces a new issue where the field is missing, but marked as
required on the schema. To fix this, a new `optionals` value is
available on the generator schema, maps are automatically included into
this schema. Any optionals are now skipped when generating
`validate_required` in the schema file.
  • Loading branch information
Gazler authored Sep 30, 2023
1 parent 2df444b commit 991f25d
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 8 deletions.
13 changes: 12 additions & 1 deletion lib/mix/phoenix/schema.ex
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ defmodule Mix.Phoenix.Schema do
migration_defaults: nil,
migration?: false,
params: %{},
optionals: [],
sample_id: nil,
web_path: nil,
web_namespace: nil,
Expand Down Expand Up @@ -86,7 +87,6 @@ defmodule Mix.Phoenix.Schema do
api_prefix = Application.get_env(otp_app, :generators)[:api_prefix] || "/api"
embedded? = Keyword.get(opts, :embedded, false)
generate? = Keyword.get(opts, :schema, true)

singular =
module
|> Module.split()
Expand All @@ -97,6 +97,8 @@ defmodule Mix.Phoenix.Schema do
string_attr = string_attr(types)
create_params = params(attrs, :create)

optionals = for {key, :map} <- types, do: key, into: []

default_params_key =
case Enum.at(create_params, 0) do
{key, _} -> key
Expand All @@ -118,6 +120,7 @@ defmodule Mix.Phoenix.Schema do
plural: schema_plural,
singular: singular,
collection: collection,
optionals: optionals,
assocs: assocs,
types: types,
defaults: schema_defaults(attrs),
Expand Down Expand Up @@ -250,6 +253,14 @@ defmodule Mix.Phoenix.Schema do
end)
end

@doc """
Return the required fields in the schema. Anything not in the `optionals` list
is considered required.
"""
def required_fields(schema) do
Enum.reject(schema.attrs, fn {key, _} -> key in schema.optionals end)
end

def type_and_opts_for_schema({:enum, opts}),
do: ~s|Ecto.Enum, values: #{inspect(Keyword.get(opts, :values))}|

Expand Down
7 changes: 6 additions & 1 deletion lib/mix/tasks/phx.gen.html.ex
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,9 @@ defmodule Mix.Tasks.Phx.Gen.Html do

@doc false
def inputs(%Schema{} = schema) do
Enum.map(schema.attrs, fn
schema.attrs
|> Enum.reject(fn {_key, type} -> type == :map end)
|> Enum.map(fn
{key, :integer} ->
~s(<.input field={f[#{inspect(key)}]} type="number" label="#{label(key)}" />)

Expand Down Expand Up @@ -246,6 +248,9 @@ defmodule Mix.Tasks.Phx.Gen.Html do
lines = input |> String.split("\n") |> Enum.reject(&(&1 == ""))

case lines do
[] ->
[]

[line] ->
[columns, line]

Expand Down
4 changes: 3 additions & 1 deletion lib/mix/tasks/phx.gen.live.ex
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,9 @@ defmodule Mix.Tasks.Phx.Gen.Live do

@doc false
def inputs(%Schema{} = schema) do
Enum.map(schema.attrs, fn
schema.attrs
|> Enum.reject(fn {_key, type} -> type == :map end)
|> Enum.map(fn
{_, {:references, _}} ->
nil

Expand Down
2 changes: 1 addition & 1 deletion priv/templates/phx.gen.schema/schema.ex
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ defmodule <%= inspect schema.module %> do
def changeset(<%= schema.singular %>, attrs) do
<%= schema.singular %>
|> cast(attrs, [<%= Enum.map_join(schema.attrs, ", ", &inspect(elem(&1, 0))) %>])
|> validate_required([<%= Enum.map_join(schema.attrs, ", ", &inspect(elem(&1, 0))) %>])
|> validate_required([<%= Enum.map_join(Mix.Phoenix.Schema.required_fields(schema), ", ", &inspect(elem(&1, 0))) %>])
<%= for k <- schema.uniques do %> |> unique_constraint(<%= inspect k %>)
<% end %> end
end
2 changes: 2 additions & 0 deletions test/mix/tasks/phx.gen.html_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ defmodule Mix.Tasks.Phx.Gen.HtmlTest do
alarm:time
alarm_usec:time_usec
secret:uuid:redact announcement_date:date alarm:time
metadata:map
weight:float user_id:references:users))

assert_file("lib/phoenix/blog/post.ex")
Expand Down Expand Up @@ -167,6 +168,7 @@ defmodule Mix.Tasks.Phx.Gen.HtmlTest do
assert file =~ ~s(<.input field={f[:announcement_date]} type="date")
assert file =~ ~s(<.input field={f[:alarm]} type="time")
assert file =~ ~s(<.input field={f[:secret]} type="text" label="Secret" />)
refute file =~ ~s(field={f[:metadata]})

assert file =~ """
<.input
Expand Down
2 changes: 2 additions & 0 deletions test/mix/tasks/phx.gen.live_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ defmodule Mix.Tasks.Phx.Gen.LiveTest do
alarm:time
alarm_usec:time_usec
secret:uuid:redact announcement_date:date alarm:time
metadata:map
weight:float user_id:references:users))

assert_file "lib/phoenix/blog/post.ex"
Expand Down Expand Up @@ -116,6 +117,7 @@ defmodule Mix.Tasks.Phx.Gen.LiveTest do
assert file =~ ~s(<.input field={@form[:announcement_date]} type="date")
assert file =~ ~s(<.input field={@form[:alarm]} type="time")
assert file =~ ~s(<.input field={@form[:secret]} type="text" label="Secret" />)
refute file =~ ~s(<field={@form[:metadata]})
assert file =~ """
<.input
field={@form[:status]}
Expand Down
19 changes: 15 additions & 4 deletions test/mix/tasks/phx.gen.schema_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ defmodule Mix.Tasks.Phx.Gen.SchemaTest do

test "build" do
in_tmp_project "build", fn ->
schema = Gen.Schema.build(~w(Blog.Post posts title:string), [])
schema = Gen.Schema.build(~w(Blog.Post posts title:string tags:map), [])

assert %Schema{
alias: Post,
Expand All @@ -28,10 +28,11 @@ defmodule Mix.Tasks.Phx.Gen.SchemaTest do
singular: "post",
human_plural: "Posts",
human_singular: "Post",
attrs: [title: :string],
types: %{title: :string},
attrs: [title: :string, tags: :map],
types: %{title: :string, tags: :map},
optionals: [:tags],
route_helper: "post",
defaults: %{title: ""},
defaults: %{title: "", tags: ""},
} = schema
assert String.ends_with?(schema.file, "lib/phoenix/blog/post.ex")
end
Expand Down Expand Up @@ -112,6 +113,16 @@ defmodule Mix.Tasks.Phx.Gen.SchemaTest do
end
end

test "does not add maps to the required list", config do
in_tmp_project config.test, fn ->
Gen.Schema.run(~w(Blog.Post blog_posts title:string tags:map published_at:naive_datetime))
assert_file "lib/phoenix/blog/post.ex", fn file ->
assert file =~ "cast(attrs, [:title, :tags, :published_at]"
assert file =~ "validate_required([:title, :published_at]"
end
end
end

test "generates nested schema", config do
in_tmp_project config.test, fn ->
Gen.Schema.run(~w(Blog.Admin.User users name:string))
Expand Down

0 comments on commit 991f25d

Please sign in to comment.