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

Use EEX for changeset error messages #921

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@kurtisnelson

kurtisnelson commented Sep 4, 2015

Right now serializing errors in JSONAPI format is painful, the detail field ends up being "must be at least %{count} characters" and there is no easy way to sub in the count value. If the error message strings instead use EEX, it is trivial to output them correctly, as shown below.

defmodule App.ChangesetView do
  use App.Web, :view

  def render("error.json", %{changeset: changeset}) do
    errors = Enum.map(changeset.errors, fn {field, detail} ->
      %{
        source: %{ pointer: "/data/attributes/#{field}" },
        title: "Invalid Attribute",
        detail: render_detail(detail)
      }
    end)

    %{errors: errors}
  end

  def render_detail({message, values}) do
    EEx.eval_string message, values
  end

  def render_detail(message) do
    message
  end
end
@josevalim

This comment has been minimized.

Member

josevalim commented Sep 4, 2015

Thank you but we should not use a template language for interpolation in error messages. Besides being a possible security issue in the future (if you allow those messages to come from an i18n system), it will also be slow. Please use String.replace/3.

@josevalim josevalim closed this Sep 4, 2015

@josevalim

This comment has been minimized.

Member

josevalim commented Sep 4, 2015

If you want to do it programmatically:

{message, values} = error_message_in_a_tuple
Enum.reduce values, message, fn {k, v}, acc ->
  String.replace(acc, "%{#{k}}", to_string(v))
end

It is only 4 LOC. :)

@kurtisnelson kurtisnelson deleted the kurtisnelson:kn/changeset-errors branch Sep 4, 2015

@michalmuskala

This comment has been minimized.

Member

michalmuskala commented Sep 4, 2015

@kurtisnelson You can also look at the Poison.Encoder protocol implementation for Ecto.Changeset in phoenix_ecto: https://github.com/phoenixframework/phoenix_ecto/blob/master/lib/phoenix_ecto/json.ex#L2-L45

@wojtekmach

This comment has been minimized.

Member

wojtekmach commented Sep 4, 2015

An alternative approach would be to expose a public API in Ecto to do the interpolation, perhaps something like this?

  test "error_messages" do
    changeset =
      changeset(%{"title" => "hello"})
      |> validate_length(:title, max: 3)

    assert changeset.errors == [title: {"should be at most %{count} characters", count: 3}]
    assert Ecto.Changeset.error_messages(changeset) == [title: "should be at most 3 characters"]
  end

If this seems like a good idea I can open up a PR for this

@josevalim

This comment has been minimized.

Member

josevalim commented Sep 4, 2015

I want to wait with that until we figure out how I18n is going to work out. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment