Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Repo.insert/2 Struct converted to Changeset #2197

Closed
alenm opened this issue Aug 31, 2017 · 3 comments
Closed

Repo.insert/2 Struct converted to Changeset #2197

alenm opened this issue Aug 31, 2017 · 3 comments

Comments

@alenm
Copy link

alenm commented Aug 31, 2017

Environment

Current Version

Elixir 1.4.1
Erlang/OTP 19
defp deps do
  [
    {:phoenix, "~> 1.3.0"},
    {:phoenix_pubsub, "~> 1.0"},
    {:phoenix_ecto, "~> 3.2"},
    {:postgrex, ">= 0.0.0"},
    {:phoenix_html, "~> 2.10"},
    {:phoenix_live_reload, "~> 1.0", only: :dev},
    {:gettext, "~> 0.11"},
    {:cowboy, "~> 1.0"},
    {:comeonin, "~> 4.0"},
    {:bcrypt_elixir, "~> 0.12"},
    {:guardian, "~> 0.14"}
  ]
end

Current behavior

Ecto.Repo.insert/2 states

...In case a struct is given, the struct is converted into a changeset with all non-nil fields as part of the changeset...

When I fire up the console in my Phoenix 1.3 app my changeset fails (which is correct because of validation) but my struct passes and is inserted in the DB.

# With changeset it return :error
iex(15)> Repo.insert User.changeset(%User{}, %{email: "test@gmail.com"})
{:error,
 #Ecto.Changeset<action: :insert, changes: %{email: "test@gmail.com"},
  errors: [name: {"can't be blank", [validation: :required]},
   password: {"can't be blank", [validation: :required]}],
  data: #Try.Accounts.User<>, valid?: false>}

# Struct it passes :ok
iex(16)> Repo.insert %User{email: "test@gmail.com"} 
[debug] QUERY OK db=2.7ms
INSERT INTO "users" ("email","inserted_at","updated_at") VALUES ($1,$2,$3) RETURNING "id" ["test@gmail.com", {{2017, 8, 31}, {0, 56, 23, 5376}}, {{2017, 8, 31}, {0, 56, 23, 5389}}]
{:ok,
 %Try.Accounts.User{__meta__: #Ecto.Schema.Metadata<:loaded, "users">,
  email: "test@gmail.com", id: 151, inserted_at: ~N[2017-08-31 00:56:23.005376],
  name: nil, password: nil, password_hash: nil,
  updated_at: ~N[2017-08-31 00:56:23.005389]}}

Here is the User Schema I'm using...

defmodule Try.Accounts.User do
  use Ecto.Schema
  import Ecto.Changeset
  alias Try.Accounts.User

  schema "users" do
    field :email, :string
    field :name, :string
    field :password_hash, :string
    field :password, :string, virtual: :string
    timestamps()
  end

  @doc false
  def changeset(%User{} = user, attrs) do
    user
    |> cast(attrs, [:name, :email, :password])
    |> validate_required([:name, :email, :password])
    |> unique_constraint(:email)
    |> hash_password()
  end

  defp hash_password(changeset) do
    case changeset do
      %Ecto.Changeset{valid?: true, changes: %{password: password}} ->
        changeset
        |> put_change(:password_hash, Comeonin.Bcrypt.hashpwsalt(password))
        |> delete_change(:password) # remove the virtual password
        #put_change(changeset, :password_hash, Comeonin.Bcrypt.hashpwsalt(password))
      _ ->
        changeset
    end
  end

end

Expected behavior

I expected the struct to fail if the documentation is saying that the struct will be converted into a changeset.

In other documentation:

What's new in Ecto 2.1 PDF that Plataformatec provides as a download. It also has under a section called Less Changesets pg 38 and it states:

"..Ecto 2.0 goes away with all of those limitations. You can now also pass structs to the repository and Ecto will take care of building the changesets for you behind the scenes..."

I did get some feedback on Slack in the Ecto Channel that its probably an error in documentation and the idea that a struct will be converted into a changeset automatically is unlikely because you may have multiple changesets for that schema and how would it know.

What am I doing wrong?

@josevalim
Copy link
Member

josevalim commented Aug 31, 2017 via email

@alenm
Copy link
Author

alenm commented Aug 31, 2017

I think I understand the documentation now based on your feedback.

iex(18)> Ecto.Changeset.change %User{email: "test@gmail.com"} 
#Ecto.Changeset<action: nil, changes: %{}, errors: [],
data: #Try.Accounts.User<>, valid?: true>

Example 1
Ecto.Changeset.change/2 returns just an Ecto.Changeset with the data: containing the struct.

iex(19)> User.changeset(%User{}, %{email: "test@gmail.com"})  
#Ecto.Changeset<action: nil, changes: %{email: "test@gmail.com"},
errors: [name: {"can't be blank", [validation: :required]},
password: {"can't be blank", [validation: :required]}],
data: #Try.Accounts.User<>, valid?: false>

Example 2
This returns an Ecto.Changeset but this was built up and returned by my User.changeset/2 function so it has gone through validation etc.

When I read this part of the documentation

Inserts a struct defined via Ecto.Schema or a changeset.

I thought since my User schema has a changeset defined User.changeset/2 when it's passed into Repo.insert/2 it would insert a struct defined by User.changeset/2.

@josevalim
Copy link
Member

josevalim commented Aug 31, 2017 via email

@alenm alenm closed this as completed Sep 1, 2017
matiasgarciaisaia added a commit to instedd/surveda that referenced this issue Dec 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants