ex_admin icon indicating copy to clipboard operation
ex_admin copied to clipboard

Crash on changeset validation with decimals

Open goldfish15 opened this issue 8 years ago • 4 comments

Hi there, I think I have found a bug for ex_admin which happens when a changeset fails and it has decimal types.

Way to reproduce.

schema "bookings" do
    field :name, :string
    field :total, :decimal
end
def changeset(struct, params \\ %{}) do
    struct
    |> cast(params, [:name, :total])
    |> validate_required([:name])
end

If this is our structure and you go to create a new booking from ex_admin. If you type anything in to the total field and leave name blank, it will give this error.

[error] Task #PID<0.1759.0> started from #PID<0.1198.0> terminating
** (ArgumentError) argument error
    lib/honeybadger/client.ex:37: Honeybadger.Client.do_send_notice/4
    (elixir) lib/task/supervised.ex:85: Task.Supervised.do_apply/2
    (stdlib) proc_lib.erl:247: :proc_lib.init_p_do_apply/3
Function: #Function<2.20169725/0 in Leap.Router.handle_errors/2>
    Args: []
[error] #PID<0.1198.0> running Leap.Endpoint terminated
Server: localhost:4000 (http)
Request: POST /admin/businesses
** (exit) an exception was raised:
    ** (Protocol.UndefinedError) protocol Enumerable not implemented for #Decimal<0.0>
        (elixir) lib/enum.ex:1: Enumerable.impl_for!/1
        (elixir) lib/enum.ex:116: Enumerable.reduce/3
        (elixir) lib/enum.ex:1767: Enum.map/2
        (elixir) lib/enum.ex:1233: anonymous fn/3 in Enum.map/2
        (stdlib) lists.erl:1263: :lists.foldl/3
        (elixir) lib/enum.ex:1772: Enum.map/2
        web/ex_admin/errors_helper.ex:36: ExAdmin.ErrorsHelper.flatten_errors/3
        web/ex_admin/errors_helper.ex:29: ExAdmin.ErrorsHelper.create_errors/2
        web/controllers/admin_resource_controller.ex:5: ExAdmin.AdminResourceController.handle_changeset_error/4
        web/controllers/admin_resource_controller.ex:1: ExAdmin.AdminResourceController.action/2
        web/controllers/admin_resource_controller.ex:1: ExAdmin.AdminResourceController.phoenix_controller_pipeline/2
        (leap) lib/leap/endpoint.ex:1: Leap.Endpoint.instrument/4
        (leap) lib/phoenix/router.ex:261: Leap.Router.dispatch/2
        (leap) web/router.ex:1: Leap.Router.do_call/2
        (leap) web/router.ex:5: Leap.Router.call/2
        (leap) lib/leap/endpoint.ex:1: Leap.Endpoint.phoenix_pipeline/1
        (leap) lib/plug/debugger.ex:123: Leap.Endpoint."call (overridable 3)"/2
        (leap) lib/leap/endpoint.ex:1: Leap.Endpoint.call/2
        (plug) lib/plug/adapters/cowboy/handler.ex:15: Plug.Adapters.Cowboy.Handler.upgrade/4
        (cowboy) /Users/goldfish/Projects/leap-booking-pheonix/deps/cowboy/src/cowboy_protocol.erl:442: :cowboy_protocol.execute/4

The required field was just an example error, this same error occurs for any validation fails on an ex_admin form as long as a decimal field is present and has a value in it.

Hopefully this is useful, first time bug reporting. :)

EDIT: This same behaviour occurs for :time types as well.

goldfish15 avatar Jul 03 '17 23:07 goldfish15

This error happens randomly to me.

  1. Inserting a new changeset works fine and as expected. Changeset has cast(attrs, [:name, :price, :description]) and validate_required([:name, :price, :description])
  2. After some development, inserting the same changeset to the database causes the protocol Enumerable not implemented for #Decimal<0.0> exception.
  3. Removing :price from the validate_required() list makes the insertion occur successfully.
  4. Adding :price back again to the validate_required() list doesn't raise an exception anymore and everything works once again as expected.

aesmail avatar Jul 15 '17 16:07 aesmail

I'm experiencing the same problem.

This is caused by this pattern match: https://github.com/smpallen99/ex_admin/blob/18b0435d640eb501bc1c86937bafaf5204c1724d/web/ex_admin/errors_helper.ex#L52 It matches on %{} in order to traverse nested has_many/belongs_to associations for errors, however Decimal (as used by ecto) is a struct %Decimal{} which in essence is a map.

I solved it by adding the following code before defp flatten_errors(%{} = errors_map, assoc_prefixes, prefix) do line:

defp flatten_errors(%{__struct__: _}, _, _), do: nil

ku1ik avatar Aug 18 '17 09:08 ku1ik

I'd be happy to send PR with this fix :)

ku1ik avatar Aug 18 '17 09:08 ku1ik

#381 should solve this one too.

ku1ik avatar Oct 10 '17 11:10 ku1ik