Phoenix: Model generators not consistent with given command

[conradwt]

Environment

• Elixir version (elixir -v):

Elixir 1.4.0-dev (27c350d)

• Phoenix version (mix deps):
  phoenix 1.2.0
• NodeJS version (node -v):

v6.2.2

• NPM version (npm -v):

3.9.5

• Operating system:

macOS 10.12 Beta 3

Expected behavior

When I execute the following command:

$ mix phoenix.gen.model Friendship friendships person_id:references:people  friend_id:references:people

I would expected that the model generated file would have looked as follows:

defmodule Testprod.Friendship do
  use Testprod.Web, :model

  schema "friendships" do
    belongs_to :person, Testprod.Person
    belongs_to :friend, Testprod.Person

    timestamps()
  end

  @doc """
  Builds a changeset based on the `struct` and `params`.
  """
  def changeset(struct, params \\ %{}) do
    struct
    |> cast(params, [:person_id, :friend_id])
    |> validate_required([:person_id, :friend_id])
  end
end

Actual behavior

defmodule Testprod.Friendship do
  use Testprod.Web, :model

  schema "friendships" do
    belongs_to :person, Testprod.Person
    belongs_to :friend, Testprod.Friend

    timestamps()
  end

  @doc """
  Builds a changeset based on the `struct` and `params`.
  """
  def changeset(struct, params \\ %{}) do
    struct
    |> cast(params, [])
    |> validate_required([])
  end
end

[josevalim]

We don't include any association in the changeset because we don't know if
we will change ids or submit the whole children as cast_assoc. there are
other things you may want to do constraint wise. Maybe we should do those
things, maybe we should let the user decide. Given the purpose of the
generator is to guide, we should like add the field to the changeset with
the proper constraints.

On Saturday, July 30, 2016, Conrad Taylor [email protected] wrote:

Environment

• Elixir version (elixir -v):

Elixir 1.4.0-dev (27c350d)

-

Phoenix version (mix deps):
phoenix 1.2.0
-

NodeJS version (node -v):

v6.2.2

• NPM version (npm -v):

3.9.5

• Operating system:

macOS 10.12 Beta 3
Expected behavior

When I execute the following command:

$ mix phoenix.gen.model Friendship friendships person_id:references:people friend_id:references:people

I would expected that the model generated file would have looked as
follows:

defmodule Testprod.Friendship do
use Testprod.Web, :model

schema "friendships" do
belongs_to :person, Testprod.Person
belongs_to :friend, Testprod.Person

timestamps()

end

@doc """ Builds a changeset based on the struct and params. """
def changeset(struct, params \ %{}) do
struct
|> cast(params, [:person_id, :friend_id])
|> validate_required([:person_id, :friend_id])
endend

Actual behavior

defmodule Testprod.Friendship do
use Testprod.Web, :model

schema "friendships" do
belongs_to :person, Testprod.Person
belongs_to :friend, Testprod.Friend

timestamps()

end

@doc """ Builds a changeset based on the struct and params. """
def changeset(struct, params \ %{}) do
struct
|> cast(params, [])
|> validate_required([])
endend

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/phoenixframework/phoenix/issues/1842, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AAAlbtCroVMxLTFfcKGVzpzqoNBkOWjyks5qa0q7gaJpZM4JYzUZ
.

_José Valim_
www.plataformatec.com.br
Skype: jv.ptec
Founder and Director of R&D

[scrogson]

@josevalim also, the model module for the friend relationship should be Person not Friend

[josevalim]

We inflect based on the association key, so manual changes will be required.

_José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D_

[josevalim]

We could try to check if the module is available though and print a warning
if it is not.

_José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D_

[agonzalezro]

I was trying to implement the warning with this piece of code:

defp validate_module(module) do
  case Code.ensure_loaded? String.to_atom(module) do
    true ->
      :ok
    false ->
      IO.write :stderr, """
      warning: the module #{module} does not exist. Probably you will need to change that by hand
      """
      :error
  end
end

defp assocs(assocs) do
  Enum.map assocs, fn {key_id, {:references, source}} ->
    key    = String.replace(Atom.to_string(key_id), "_id", "")
    module = Mix.Phoenix.inflect(key)[:module]
    validate_module module
    {String.to_atom(key), key_id, module, source}
  end
end

but the models are not loaded at the time of execution. Could you please point me to some place where I can read the code that loads them?

I can workaround this with something like:

alias = assocs[:alias]
Code.load_file("web/models/#{alias}.ex")

But it's wrong and I am pretty sure that it's not what you will want to see on the code :)

Sorry for the questions, just trying to get my hands dirty on your codebase.

[teamon]

Are you sure it is a good idea to include *_id's by default? IMO if the purpose of generator is to guide people it should state that the allowed keys should be picked up carefully.

I'd be more than happy to discuss this further.

(And yes, this is entirely based on attr_accessible etc. madness in rails world :) ).

[kelvinst]

@agonzalezro @josevalim I have another idea for this, that would be changing the assocs definition on the model generator to member_id:references:User:users. My point is: if we do not inflect the source table from de association name, why the hell we inflect the source module? This way, it feels even a little bit more like the model generator itself, which you must pass the singular and plural forms to it.

As for "backwards compatibility", I guess people that are upgrading to phoenix 1.3 can get used to it, and we can raise to them a very clear message, as we already do for this.

[michalmuskala]

Is that level of complication in a generator really worth it? I would actually go another way and consider removing the references from generators completely if they cause that much trouble.

[kelvinst]

@michallepicki well, I agree this is a little bit more complicated than it could be. Maybe leave as it is and just forget about generating the belongs_to in the model?

[kelvinst]

Well, I was thinking a little bit about it, and came to the conclusion that, at least for me member_id:references:User:users is not that much more complicated than member_id:references:users. My main argue for it is that, this way we would keep a standard of always, when you need to put the plural name for the model, you put the singular name too.

If your point is removing completely the possibility to create references on the generators, I guess it would add extra work to create associations, even the very simple ones. I really don't disagree totally with you on this, but I'm trying to measure the pros and cons of each option.

[yordis]

I would say just add the module name.

What happen with people that use a different code structure. In my case, I use App.Entity.* (for example) so this generator is not useful at that moment for generate belongs_to in the model.

foreign_key_name:references:table_name Optional Module, do not generate belongs to
foreign_key_name:references:table_name:module_name It generates the belong to

[chrismccord]

After thinking about this more, I agree with the points @michalmuskala made. I don't believe these additions are worth the complexity they introduce. In the case you need to build associations, you are going to necessarily have to touch code in your templates and in your functions that build changesets. Thanks for the idea and discussion!