stavro / arc_ecto

An integration with Arc and Ecto.
255 stars 149 forks source link

Ecto 3.0 emits warnings when using arc_ecto #102

Closed dbernheisel closed 5 years ago

dbernheisel commented 5 years ago

When using Ecto 3.0 and arc_ecto, I see a lot of warnings about arc_ecto passing strings into cast() as the allowed list of param keys.

image

Here are the ecto versions in my mix.lock

"ecto": {:hex, :ecto, "3.0.1", "a26605ee7b243a754e6609d1c23da27bcb22823659b07bf03f9020da92a8e4f4" ....
"ecto_sql": {:hex, :ecto_sql, "3.0.0", "8d1883376bee02a0e76b5ef797e39d04333c34b9935d0b4785dbf3cbdb571e2a" ....
OvermindDL1 commented 5 years ago

This warning is still occurring with 0.11.1.

It looks like this comment in arc_ecto may not be correct, and thus it's related code is not correct: https://github.com/stavro/arc_ecto/blob/master/lib/arc_ecto/schema.ex#L20

      # Cast supports both atom and string keys, ensure we're matching on both.
dbernheisel commented 5 years ago

@OvermindDL1 what's your changeset logic look like? I've been using 0.11.1 and haven't seen the warnings lately.

Mine looks something like this:

  @required_fields ~w(first_name last_name)a
  @optional_fields ~w(user_id company phone job_title linkedin_url job_description supervisor_name other)a

  @doc false
  def changeset(struct, attrs) do
    struct
    |> cast(attrs, @required_fields ++ @optional_fields)
    |> ensure_uuid(:id)
    |> cast_attachments(attrs, [:avatar, :other_file, :job_description_file])
    |> ...
OvermindDL1 commented 5 years ago

Figured out the reason, it was caused by having the file field in the cast instead of cast_attachments (perhaps an error should be thrown instead when that is done, easy to detect because scope is nil).