improvement: make generators more consistent

lib/ash_phoenix/form/form.ex

@@ -2385,9 +2385,8 @@ defmodule AshPhoenix.Form do
   defp get_non_attribute_non_argument_param(changeset, form, field) do
     if Ash.Resource.Info.attribute(changeset.resource, field) ||
          Enum.any?(changeset.action.arguments, &(&1.name == field)) do
-      with :error <- Map.fetch(changeset.params, field),
-           :error <- Map.fetch(changeset.params, to_string(field)) do
-        :error
+      with :error <- Map.fetch(changeset.params, field) do
+        Map.fetch(changeset.params, to_string(field))
       end
     else
       Map.fetch(AshPhoenix.Form.params(form), Atom.to_string(field))

lib/ash_phoenix/gen/gen.ex

@@ -0,0 +1,79 @@
+defmodule AshPhoenix.Gen do
+  @moduledoc false
+
+  def docs do
+    """
+    ## Positional Arguments
+
+    - `api` - The API (e.g. "Shop").
+    - `resource` - The resource (e.g. "Product").
+
+    ## Options
+
+    - `--resource-plural` - The plural resource name (e.g. "products")
+    """
+  end
+
+  def parse_opts(argv) do
+    {api, resource, rest} =
+      case argv do
+        [api, resource | rest] ->
+          {api, resource, rest}
+
+        argv ->
+          raise "Not enough arguments. Expected 2, got #{Enum.count(argv)}"
+      end
+
+    if String.starts_with?(api, "-") do
+      raise "Expected first argument to be an api module, not an option"
+    end
+
+    if String.starts_with?(resource, "-") do
+      raise "Expected second argument to be a resource module, not an option"
+    end
+
+    {parsed, _, _} =
+      OptionParser.parse(rest,
+        strict: [resource_plural: :string, actor: :string, no_actor: :boolean]
+      )
+
+    api = Module.concat([api])
+    resource = Module.concat([resource])
+
+    parsed =
+      Keyword.put_new_lazy(rest, :resource_plural, fn ->
+        plural_name!(resource, parsed)
+      end)
+
+    {api, resource, parsed, rest}
+  end
+
+  defp plural_name!(resource, opts) do
+    plural_name =
+      opts[:resource_plural] ||
+        Ash.Resource.Info.plural_name(resource) ||
+        Mix.shell().prompt(
+          """
+          Please provide a plural_name for #{inspect(resource)}. For example the plural of tweet is tweets.
+
+          This can also be configured on the resource. To do so, press enter to abort,
+          and add the following configuration to your resource (using the proper plural name)
+
+              resource do
+                plural_name :tweets
+              end
+          >
+          """
+          |> String.trim()
+        )
+        |> String.trim()
+
+    case plural_name do
+      empty when empty in ["", nil] ->
+        raise("Must configure `plural_name` on resource or provide --resource-plural")
+
+      plural_name ->
+        to_string(plural_name)
+    end
+  end
+end

lib/ash_phoenix/gen/live.ex

@@ -2,38 +2,12 @@ defmodule AshPhoenix.Gen.Live do
   @moduledoc false
 
   def generate_from_cli(argv) do
-    if Mix.Project.umbrella?() do
-      Mix.raise(
-        "mix phx.gen.live must be invoked from within your *_web application root directory"
-      )
-    end
-
-    {api, resource, rest} =
-      case argv do
-        [api, resource | rest] ->
-          {api, resource, rest}
-
-        argv ->
-          raise "Not enough arguments. Expected 2, got #{Enum.count(argv)}"
-      end
-
-    if String.starts_with?(api, "-") do
-      raise "Expected first argument to be an api module, not an option"
-    end
-
-    if String.starts_with?(resource, "-") do
-      raise "Expected second argument to be a resource module, not an option"
-    end
-
-    {parsed, _, _} =
-      OptionParser.parse(rest,
-        strict: [resource_plural: :string, actor: :string, no_actor: :boolean]
-      )
+    {api, resource, opts, _rest} = AshPhoenix.Gen.parse_opts(argv)
 
     generate(
-      Module.concat([api]),
-      Module.concat([resource]),
-      Keyword.put(parsed, :interactive?, true)
+      api,
+      resource,
+      Keyword.put(opts, :interactive?, true)
     )
   end
 
@@ -47,11 +21,11 @@ defmodule AshPhoenix.Gen.Live do
              "Would you like to name your actor? For example: `current_user`. If you choose no, we will not add any actor logic."
            ) do
           actor =
-            Mix.shell().prompt("What would you like to name it? For example: `current_user`")
+            Mix.shell().prompt("What would you like to name it? Default: `current_user`")
             |> String.trim()
 
           if actor == "" do
-            opts
+            Keyword.put(opts, :actor, "current_user")
           else
             Keyword.put(opts, :actor, actor)
           end
@@ -172,7 +146,7 @@ defmodule AshPhoenix.Gen.Live do
       |> Ash.Resource.Info.short_name()
       |> to_string()
 
-    plural_name = plural_name!(resource, opts)
+    plural_name = opts[:resource_plural]
 
     pkey =
       case Ash.Resource.Info.primary_key(resource) do
@@ -331,33 +305,6 @@ defmodule AshPhoenix.Gen.Live do
     end
   end
 
-  defp plural_name!(resource, opts) do
-    plural_name =
-      opts[:resource_plural] ||
-        Ash.Resource.Info.plural_name(resource) ||
-        Mix.shell().prompt(
-          """
-          Please provide a plural_name. For example the plural of tweet is tweets.
-          You can press enter to abort, and then configure one on the resource, for example:
-
-              resource do
-                plural_name :tweets
-              end
-          >
-          """
-          |> String.trim()
-        )
-        |> String.trim()
-
-    case plural_name do
-      empty when empty in ["", nil] ->
-        raise("Must configure `plural_name` on resource or provide --resource-plural")
-
-      plural_name ->
-        to_string(plural_name)
-    end
-  end
-
   defp web_path do
     web_module().module_info[:compile][:source]
     |> Path.relative_to(root_path())

lib/mix/tasks/ash_phoenix.gen.html.ex

@@ -6,74 +6,85 @@ defmodule Mix.Tasks.AshPhoenix.Gen.Html do
   @moduledoc """
   This task renders .ex and .heex templates and copies them to specified directories.
 
-  ## Arguments
+  #{AshPhoenix.Gen.docs()}
 
-  api         The API (e.g. "Shop").
-  resource    The resource (e.g. "Product").
-  plural      The plural schema name (e.g. "products").
-
-  ## Example
-
-  mix ash_phoenix.gen.html Shop Product products
+  mix ash_phoenix.gen.html MyApp.Shop MyApp.Shop.Product --plural-name products
   """
 
   def run([]) do
+    not_umbrella!()
+
     Mix.shell().info("""
     #{Mix.Task.shortdoc(__MODULE__)}
 
     #{Mix.Task.moduledoc(__MODULE__)}
     """)
   end
 
-  def run(args) when length(args) == 3 do
+  def run(args) do
+    not_umbrella!()
     Mix.Task.run("compile")
 
-    [api, resource, plural] = args
-    singular = String.downcase(resource)
+    {api, resource, opts, _} = AshPhoenix.Gen.parse_opts(args)
+
+    singular = to_string(Ash.Resource.Info.short_name(resource))
 
     opts = %{
-      api: api,
-      resource: resource,
+      resource: List.last(Module.split(resource)),
+      full_resource: resource,
+      full_api: api,
       singular: singular,
-      plural: plural
+      plural: opts[:resource_plural]
     }
 
-    if Code.ensure_loaded?(resource_module(opts)) do
+    if Code.ensure_loaded?(resource) do
       source_path = Application.app_dir(:ash_phoenix, "priv/templates/ash_phoenix.gen.html")
-      resource_html_dir = Macro.underscore(opts[:resource]) <> "_html"
+      resource_html_dir = to_string(opts[:singular]) <> "_html"
 
       template_files(resource_html_dir, opts)
-      |> generate_files(assigns([:api, :resource, :singular, :plural], opts), source_path)
+      |> generate_files(
+        assigns([:api, :full_resource, :full_api, :resource, :singular, :plural], resource, opts),
+        source_path
+      )
 
-      print_shell_instructions(opts[:resource], opts[:plural])
+      print_shell_instructions(opts)
     else
       Mix.shell().info(
-        "The resource #{app_name()}.#{opts[:api]}.#{opts[:resource]} does not exist."
+        "The resource #{inspect(opts[:api])}.#{inspect(opts[:resource])} does not exist."
       )
     end
   end
 
-  defp assigns(keys, opts) do
+  defp not_umbrella! do
+    if Mix.Project.umbrella?() do
+      Mix.raise(
+        "mix phx.gen.html must be invoked from within your *_web application root directory"
+      )
+    end
+  end
+
+  defp assigns(keys, resource, opts) do
     binding = Enum.map(keys, fn key -> {key, opts[key]} end)
-    binding = [{:route_prefix, Macro.underscore(opts[:plural])} | binding]
+    binding = [{:route_prefix, to_string(opts[:plural])} | binding]
     binding = [{:app_name, app_name()} | binding]
-    binding = [{:attributes, attributes(opts)} | binding]
+    binding = [{:attributes, attributes(resource)} | binding]
+    binding = [{:update_attributes, update_attributes(resource)} | binding]
+    binding = [{:create_attributes, create_attributes(resource)} | binding]
     Enum.into(binding, %{})
   end
 
   defp template_files(resource_html_dir, opts) do
-    app_web_path = "lib/#{Macro.underscore(app_name())}_web"
+    app_web_path = "lib/#{app_name_underscore()}_web"
 
     %{
       "index.html.heex" => "#{app_web_path}/controllers/#{resource_html_dir}/index.html.heex",
       "show.html.heex" => "#{app_web_path}/controllers/#{resource_html_dir}/show.html.heex",
       "resource_form.html.heex" =>
-        "#{app_web_path}/controllers/#{resource_html_dir}/#{Macro.underscore(opts[:resource])}_form.html.heex",
+        "#{app_web_path}/controllers/#{resource_html_dir}/#{opts[:singular]}_form.html.heex",
       "new.html.heex" => "#{app_web_path}/controllers/#{resource_html_dir}/new.html.heex",
       "edit.html.heex" => "#{app_web_path}/controllers/#{resource_html_dir}/edit.html.heex",
-      "controller.ex" =>
-        "#{app_web_path}/controllers/#{Macro.underscore(opts[:resource])}_controller.ex",
-      "html.ex" => "#{app_web_path}/controllers/#{Macro.underscore(opts[:resource])}_html.ex"
+      "controller.ex" => "#{app_web_path}/controllers/#{opts[:singular]}_controller.ex",
+      "html.ex" => "#{app_web_path}/controllers/#{opts[:singular]}_html.ex"
     }
   end
 
@@ -86,36 +97,63 @@ defmodule Mix.Tasks.AshPhoenix.Gen.Html do
     end)
   end
 
+  defp app_name_underscore do
+    Mix.Project.config()[:app]
+  end
+
   defp app_name do
     app_name_atom = Mix.Project.config()[:app]
     Macro.camelize(Atom.to_string(app_name_atom))
   end
 
-  defp print_shell_instructions(resource, plural) do
+  defp print_shell_instructions(opts) do
     Mix.shell().info("""
 
-      Add the resource to your browser scope in lib/#{Macro.underscore(resource)}_web/router.ex:
+      Add the resource to your browser scope in lib/#{opts[:singular]}_web/router.ex:
 
-        resources "/#{plural}", #{resource}Controller
+        resources "/#{opts[:plural]}", #{opts[:resource]}Controller
     """)
   end
 
-  defp resource_module(opts) do
-    Module.concat(["#{app_name()}.#{opts[:api]}.#{opts[:resource]}"])
+  defp attributes(resource) do
+    resource
+    |> Ash.Resource.Info.public_attributes()
+    |> Enum.reject(&(&1.type == Ash.Type.UUID))
+    |> Enum.map(&attribute_map/1)
   end
 
-  defp attributes(opts) do
-    resource_module(opts)
-    |> Ash.Resource.Info.attributes()
+  defp create_attributes(resource) do
+    create_action = Ash.Resource.Info.primary_action!(resource, :create)
+
+    attrs =
+      create_action.accept
+      |> Enum.map(&Ash.Resource.Info.attribute(resource, &1))
+      |> Enum.filter(& &1.writable?)
+
+    create_action.arguments
+    |> Enum.concat(attrs)
     |> Enum.map(&attribute_map/1)
-    |> Enum.reject(&reject_attribute?/1)
   end
 
-  defp attribute_map(attr) do
-    %{name: attr.name, type: attr.type, writable?: attr.writable?, private?: attr.private?}
+  defp update_attributes(resource) do
+    update_action = Ash.Resource.Info.primary_action!(resource, :update)
+
+    attrs =
+      update_action.accept
+      |> Enum.map(&Ash.Resource.Info.attribute(resource, &1))
+      |> Enum.filter(& &1.writable?)
+
+    update_action.arguments
+    |> Enum.concat(attrs)
+    |> Enum.map(&attribute_map/1)
   end
 
-  defp reject_attribute?(%{name: :id, type: Ash.Type.UUID}), do: true
-  defp reject_attribute?(%{private?: true}), do: true
-  defp reject_attribute?(_), do: false
+  defp attribute_map(attr) do
+    %{
+      name: attr.name,
+      type: attr.type,
+      writable?: Map.get(attr, :writable?, true),
+      private?: attr.private?
+    }
+  end
 end