Skip to content

Commit

Permalink
fix: properly return errors on invalid calculation arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
zachdaniel committed Apr 8, 2023
1 parent b5e0cbb commit 29096c2
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 4 deletions.
26 changes: 26 additions & 0 deletions lib/ash/error/query/invalid_calculation_argument.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
defmodule Ash.Error.Query.InvalidCalculationArgument do
@moduledoc "Used when an invalid value is provided for a calculation argument"
use Ash.Error.Exception

def_ash_error([:calculation, :field, :message, :value], class: :invalid)

defimpl Ash.ErrorKind do
def id(_), do: Ash.UUID.generate()

def code(_), do: "invalid_calculation_argument"

def message(error) do
"""
Invalid value provided for calculation argument #{error.field} in #{error.calculation}: #{do_message(error)}
#{inspect(error.value)}
"""
end

defp do_message(%{message: message}) when not is_nil(message) do
": #{message}."
end

defp do_message(_), do: "."
end
end
36 changes: 32 additions & 4 deletions lib/ash/query/query.ex
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ defmodule Ash.Query do
alias Ash.Error.Query.{
AggregatesNotSupported,
InvalidArgument,
InvalidCalculationArgument,
InvalidLimit,
InvalidOffset,
NoReadAction,
Expand Down Expand Up @@ -911,6 +912,9 @@ defmodule Ash.Query do
select_and_load_calc(resource_calculation, %{calculation | load: field}, query)

Map.update!(query, :calculations, &Map.put(&1, field, calculation))
else
{:error, error} ->
Ash.Query.add_error(query, :load, error)
end

true ->
Expand Down Expand Up @@ -1147,13 +1151,27 @@ defmodule Ash.Query do
)}}

expr?(value) ->
{:halt, {:error, "Argument #{argument.name} does not support expressions!"}}
{:halt,
{:error,
InvalidCalculationArgument.exception(
field: argument.name,
calculation: calculation.name,
message: "does not support expressions",
value: value
)}}

is_nil(value) && argument.allow_nil? ->
{:cont, {:ok, Map.put(arg_values, argument.name, nil)}}

is_nil(value) ->
{:halt, {:error, "Argument #{argument.name} is required"}}
{:halt,
{:error,
InvalidCalculationArgument.exception(
field: argument.name,
calculation: calculation.name,
message: "is required",
value: value
)}}

is_nil(Map.get(args, argument.name, Map.get(args, to_string(argument.name)))) &&
not is_nil(value) ->
Expand Down Expand Up @@ -1191,8 +1209,18 @@ defmodule Ash.Query do
{:cont, {:ok, Map.put(arg_values, argument.name, casted)}}
end
else
{:error, error} when is_binary(error) ->
{:halt,
{:error,
InvalidCalculationArgument.exception(
field: argument.name,
calculation: calculation.name,
message: error,
value: value
)}}

{:error, error} ->
{:halt, {:error, error}}
{:halt, {:error, Ash.Error.to_ash_error(error)}}
end
end
end)
Expand Down Expand Up @@ -1776,7 +1804,7 @@ defmodule Ash.Query do
Map.update!(query, :calculations, &Map.put(&1, as_name, calculation))
else
{:error, error} ->
Ash.Query.add_error(query, error)
Ash.Query.add_error(query, :load, error)
end
else
Ash.Query.add_error(query, "No such calculation: #{inspect(calc_name)}")
Expand Down
6 changes: 6 additions & 0 deletions test/calculation_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -873,4 +873,10 @@ defmodule Ash.Test.CalculationTest do
|> Api.read!(authorize?: true)
|> Enum.map(& &1.active)
end

test "invalid calculation arguments show errors in the query" do
assert %{valid?: false} =
User
|> Ash.Query.load(full_name: %{separator: %{foo: :bar}})
end
end

0 comments on commit 29096c2

Please sign in to comment.