improvement: properly set path into error instead of on error messages

TBD: this may break some people's tests, and so may need to be
reverted or released as part of 3.0
This commit is contained in:
Zach Daniel 2023-08-15 16:23:06 -07:00
parent 8c1f334075
commit 8233634bb1
8 changed files with 128 additions and 87 deletions

View file

@ -111,20 +111,17 @@ defmodule Ash.EmbeddableType do
vars
|> Keyword.put(:field, field)
|> Keyword.put(:message, message)
|> add_index()
end
defp do_handle_errors(%{message: message, vars: vars, field: field}) do
vars
|> Keyword.put(:message, message)
|> Keyword.put(:field, field)
|> add_index()
end
defp do_handle_errors(%{message: message, vars: vars}) do
vars
|> Keyword.put(:message, message)
|> add_index()
end
defp do_handle_errors(%{field: field} = exception) do
@ -143,20 +140,6 @@ defmodule Ash.EmbeddableType do
[message: "Something went wrong"]
end
defp add_index(opts) do
opts
# cond do
# opts[:index] && opts[:field] ->
# Keyword.put(opts, :field, "#{opts[:field]}[#{opts[:index]}]")
# opts[:index] ->
# Keyword.put(opts, :field, "[#{opts[:index]}]")
# true ->
# opts
# end
end
defmacro single_embed_implementation do
# credo:disable-for-next-line Credo.Check.Refactor.LongQuoteBlocks
quote location: :keep do
@ -625,7 +608,9 @@ defmodule Ash.EmbeddableType do
error
|> Ash.EmbeddableType.handle_errors()
|> Enum.map(fn keyword ->
Keyword.put(keyword, :index, index)
keyword
|> Keyword.put(:index, index)
|> Keyword.update(:path, [index], &[index | &1])
end)
{:halt, {:error, errors}}

View file

@ -519,7 +519,7 @@ defmodule Ash.Error do
class_error ->
breadcrumb(class_error.error_context) <>
"* #{Exception.message(class_error)}"
"* #{Exception.message(class_error)}" <> path(class_error)
end)
end
end)

View file

@ -71,7 +71,7 @@ defmodule Ash.Type.Helpers do
keyword
|> Keyword.put(
:message,
add_index(keyword[:message], keyword)
keyword[:message]
)
|> Keyword.put(:field, attribute.name)
]
@ -79,10 +79,12 @@ defmodule Ash.Type.Helpers do
fields ->
Enum.map(
fields,
&Keyword.merge(message,
field: attribute.name,
message: add_index(add_field(keyword[:message], "#{&1}"), keyword)
)
fn field ->
keyword
|> Keyword.put(:field, field)
|> Keyword.delete(:fields)
|> Keyword.update(:path, [attribute.name], &[attribute.name | &1])
end
)
end
@ -90,24 +92,18 @@ defmodule Ash.Type.Helpers do
[[field: attribute.name, message: message]]
value when is_exception(value) ->
value
|> Ash.Error.to_ash_error()
|> Map.put(:field, attribute.name)
exception =
value
|> Ash.Error.to_ash_error()
if Map.get(exception, :field) || Map.get(exception, :fields) do
Ash.Error.set_path(exception, attribute.name)
else
Map.put(exception, :field, attribute.name)
end
_ ->
[[field: attribute.name]]
end
end
defp add_field(message, field) do
"at field #{field} " <> (message || "")
end
defp add_index(message, opts) do
if opts[:index] do
"at index #{opts[:index]} " <> (message || "")
else
message
end
end
end

View file

@ -446,7 +446,8 @@ defmodule Ash.Type do
|> Enum.reduce_while({:ok, []}, fn {item, index}, {:ok, casted} ->
case cast_input(type, item, single_constraints) do
:error ->
{:halt, {:error, message: "invalid value at %{index}", index: index}}
{:halt,
{:error, message: "invalid value at %{index}", index: index, path: [index]}}
{:error, keyword} ->
errors =
@ -458,7 +459,9 @@ defmodule Ash.Type do
[message: message, index: index, path: [index]]
keyword ->
Keyword.merge(keyword, index: index, path: [index])
keyword
|> Keyword.put(:index, index)
|> Keyword.update(:path, [index], &[index | &1])
end)
{:halt, {:error, errors}}

View file

@ -237,7 +237,7 @@ defmodule Ash.Test.Changeset.EmbeddedResourceTest do
test "embedded resources run validations on create" do
msg =
~r/Invalid value provided for profile: at field last_name exactly 2 of first_name,last_name must be present/
~r/Invalid value provided for last_name: exactly 2 of first_name,last_name must be present/
assert_raise Ash.Error.Invalid,
msg,
@ -272,7 +272,7 @@ defmodule Ash.Test.Changeset.EmbeddedResourceTest do
input = %{counter: author.profile.counter - 1}
assert_raise Ash.Error.Invalid,
~r/Invalid value provided for profile: at field counter must be increasing/,
~r/Invalid value provided for counter: must be increasing/,
fn ->
Changeset.for_update(
author,
@ -384,7 +384,7 @@ defmodule Ash.Test.Changeset.EmbeddedResourceTest do
|> Api.create!()
assert_raise Ash.Error.Invalid,
~r/Invalid value provided for tags: at index 0 at field score must be present/,
~r/Invalid value provided for score: must be present/,
fn ->
Changeset.for_update(
author,
@ -399,7 +399,7 @@ defmodule Ash.Test.Changeset.EmbeddedResourceTest do
end
assert_raise Ash.Error.Invalid,
~r/Invalid value provided for tags: at index 0 at field score must be absent/,
~r/Invalid value provided for score: must be absent/,
fn ->
Changeset.for_update(
author,
@ -428,20 +428,23 @@ defmodule Ash.Test.Changeset.EmbeddedResourceTest do
)
|> Api.create!()
assert_raise Ash.Error.Invalid,
~r/Invalid value provided for tags_with_id: at index 0 at field score must be increasing/,
fn ->
Changeset.for_update(
author,
:update,
%{
tags_with_id: [
%{id: tag.id, score: 1}
]
}
)
|> Api.update!()
end
exception =
assert_raise Ash.Error.Invalid,
~r/Invalid value provided for score: must be increasing/,
fn ->
Changeset.for_update(
author,
:update,
%{
tags_with_id: [
%{id: tag.id, score: 1}
]
}
)
|> Api.update!()
end
assert Enum.at(exception.errors, 0).path == [:tags_with_id, 0]
Changeset.for_update(
author,

View file

@ -115,15 +115,15 @@ defmodule Type.KeywordTest do
assert [
%Ash.Error.Changes.InvalidAttribute{
field: :metadata,
message: "at field foo field must be present",
field: :foo,
message: "field must be present",
private_vars: nil,
value: [bar: 1],
changeset: nil,
query: nil,
error_context: [],
vars: [field: :metadata, message: "at field foo field must be present"],
path: []
vars: [field: :foo, message: "field must be present", path: [:metadata]],
path: [:metadata]
}
] = changeset.errors
end
@ -139,18 +139,19 @@ defmodule Type.KeywordTest do
assert [
%Ash.Error.Changes.InvalidAttribute{
field: :metadata,
message: "at field bar must be more than or equal to %{min}",
field: :bar,
message: "must be more than or equal to %{min}",
private_vars: nil,
value: [foo: "hello", bar: -1],
changeset: nil,
query: nil,
error_context: [],
vars: [
field: :metadata,
message: "at field bar must be more than or equal to %{min}"
field: :bar,
message: "must be more than or equal to %{min}",
path: [:metadata]
],
path: []
path: [:metadata]
}
] = changeset.errors
end
@ -192,15 +193,15 @@ defmodule Type.KeywordTest do
assert [
%Ash.Error.Changes.InvalidAttribute{
field: :metadata,
message: "at field foo value must not be nil",
field: :foo,
message: "value must not be nil",
private_vars: nil,
value: [foo: "", bar: "2"],
changeset: nil,
query: nil,
error_context: [],
vars: [field: :metadata, message: "at field foo value must not be nil"],
path: []
vars: [field: :foo, message: "value must not be nil", path: [:metadata]],
path: [:metadata]
}
] = changeset.errors
end

View file

@ -117,15 +117,15 @@ defmodule Type.MapTest do
assert [
%Ash.Error.Changes.InvalidAttribute{
field: :metadata,
message: "at field foo field must be present",
field: :foo,
message: "field must be present",
private_vars: nil,
value: %{bar: 1},
changeset: nil,
query: nil,
error_context: [],
vars: [field: :metadata, message: "at field foo field must be present"],
path: []
vars: [field: :foo, message: "field must be present", path: [:metadata]],
path: [:metadata]
}
] = changeset.errors
end
@ -141,18 +141,19 @@ defmodule Type.MapTest do
assert [
%Ash.Error.Changes.InvalidAttribute{
field: :metadata,
message: "at field bar must be more than or equal to %{min}",
field: :bar,
message: "must be more than or equal to %{min}",
private_vars: nil,
value: %{bar: -1, foo: "hello"},
changeset: nil,
query: nil,
error_context: [],
vars: [
field: :metadata,
message: "at field bar must be more than or equal to %{min}"
field: :bar,
message: "must be more than or equal to %{min}",
path: [:metadata]
],
path: []
path: [:metadata]
}
] = changeset.errors
end
@ -194,15 +195,15 @@ defmodule Type.MapTest do
assert [
%Ash.Error.Changes.InvalidAttribute{
field: :metadata,
message: "at field foo value must not be nil",
field: :foo,
message: "value must not be nil",
private_vars: nil,
value: %{:bar => "2", "foo" => ""},
changeset: nil,
query: nil,
error_context: [],
vars: [field: :metadata, message: "at field foo value must not be nil"],
path: []
vars: [field: :foo, message: "value must not be nil", path: [:metadata]],
path: [:metadata]
}
] = changeset.errors
end

View file

@ -28,6 +28,39 @@ defmodule Ash.Test.Type.UnionTest do
end
end
defmodule Example do
use Ash.Resource, data_layer: Ash.DataLayer.Ets
ets do
private? true
end
actions do
defaults [:create, :read, :update, :destroy]
end
attributes do
uuid_primary_key :id
attribute :things, {:array, :union} do
constraints items: [
types: [
foo: [
type: Foo,
tag: :type,
tag_value: :foo
],
bar: [
type: Bar,
tag: :type,
tag_value: :bar
]
]
]
end
end
end
test "it handles simple types" do
constraints = [
types: [
@ -107,7 +140,26 @@ defmodule Ash.Test.Type.UnionTest do
assert {:ok, [%Ash.Union{value: %{type: "foo", foo: "foo"}, type: :foo}]} =
Ash.Type.cast_input(type, [%{type: :foo, foo: "foo"}], constraints)
# assert {:ok, [%Ash.Union{value: %{type: "foo", foo: "foo"}, type: :foo}]} =
Ash.Type.cast_input(type, [%{type: :foo, foo: "bar"}], constraints) |> IO.inspect()
assert {:error,
[
error
]} =
Ash.Type.cast_input(type, [%{type: :foo, foo: "bar"}], constraints)
for {key, val} <- [
message: "must match the pattern %{regex}",
field: :foo,
regex: "~r/foo/",
index: 0,
path: [0]
] do
assert error[key] == val
end
end
test "it handles paths on a resource" do
Example
|> Ash.Changeset.for_create(:create, %{things: [%{type: :foo, foo: "bar"}]})
|> Ash.Test.AnyApi.create()
end
end