Skip to content

Commit

Permalink
Always return single error message (closes #41)
Browse files Browse the repository at this point in the history
Instead of sometimes returning a list and sometimes a single error, this
standardizes so that the format will be the following:

    {:error, "message 1, message 2, etc."}
  • Loading branch information
rcdilorenzo committed Aug 25, 2017
1 parent 81cbc70 commit 625f128
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 18 deletions.
20 changes: 10 additions & 10 deletions lib/filtrex.ex
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ defmodule Filtrex do
@type t :: Filtrex.t

@doc """
Parses a filter expression and returns errors or the parsed filter with
Parses a filter expression and returns an error or the parsed filter with
the appropriate parsed sub-structures.
The `configs` option is a list of type configs (See `Filtrex.Type.Config`)
Expand All @@ -35,7 +35,7 @@ defmodule Filtrex do
[%Filtrex.Type.Config{type: :text, keys: ~w(title comments)}]
```
"""
@spec parse([Filtrex.Type.Config.t], Map.t) :: {:errors, List.t} | {:ok, Filtrex.t}
@spec parse([Filtrex.Type.Config.t], Map.t) :: {:error, string} | {:ok, Filtrex.t}
def parse(configs, map) do
with {:ok, sanitized} <- Filtrex.Params.sanitize(map, @whitelist),
{:ok, valid_structured_map} <- validate_structure(sanitized),
Expand Down Expand Up @@ -122,31 +122,31 @@ defmodule Filtrex do
def validate_structure(map) do
case map do
%{filter: %{type: type}} when not type in ~w(all any none) ->
{:errors, ["Invalid filter type '#{type}'"]}
{:error, "Invalid filter type '#{type}'"}
%{filter: %{conditions: conditions}} when conditions == [] or not is_list(conditions) ->
{:errors, ["One or more conditions required to filter"]}
{:error, "One or more conditions required to filter"}
%{filter: %{sub_filters: sub_filters}} when not is_list(sub_filters) ->
{:errors, ["Sub-filters must be a valid list of filters"]}
{:error, "Sub-filters must be a valid list of filters"}
validated = %{filter: params} ->
sub_filters = Map.get(params, :sub_filters, [])
result = Enum.reduce_while(sub_filters, {:ok, []}, fn (sub_map, {:ok, acc}) ->
case validate_structure(sub_map) do
{:errors, errors} -> {:halt, {:errors, errors}}
{:ok, sub_validated} -> {:cont, {:ok, acc ++ [sub_validated]}}
{:error, error} -> {:halt, {:error, error}}
end
end)
with {:ok, validated_sub_filters} <- result,
do: {:ok, put_in(validated.filter[:sub_filters], validated_sub_filters)}
_ ->
{:errors, ["Invalid filter structure"]}
{:error, "Invalid filter structure"}
end
end

defp parse_validated_structure(configs, %{filter: params}) do
parsed_filters = Enum.reduce_while(params[:sub_filters], {:ok, []}, fn (to_parse, {:ok, acc}) ->
case parse(configs, to_parse) do
{:ok, filter} -> {:cont, {:ok, acc ++ [filter]}}
{:errors, errors} -> {:halt, {:errors, errors}}
{:error, error} -> {:halt, {:error, error}}
end
end)
with {:ok, filters} <- parsed_filters,
Expand All @@ -169,7 +169,7 @@ defmodule Filtrex do
{:ok, %Filtrex{type: type, conditions: conditions, sub_filters: parsed_filters}}
end
defp parse_condition_results(%{errors: errors}, _, _) do
{:errors, errors}
{:error, Enum.join(errors, ", ")}
end

defp parse_params_filter_union(params) do
Expand All @@ -179,7 +179,7 @@ defmodule Filtrex do
:error ->
{:ok, {"all", params}}
_ ->
{:errors, ["Invalid filter union"]}
{:error, "Invalid filter union"}
end
end

Expand Down
14 changes: 6 additions & 8 deletions test/filtrex_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ defmodule FiltrexTest do
assert FilterParams.build(:all)
|> FilterParams.type("dash-combine")
|> Filtrex.validate_structure ==
{:errors, ["Invalid filter type 'dash-combine'"]}
{:error, "Invalid filter type 'dash-combine'"}
end

@tag :validate_structure
test "requiring more than one condition" do
assert FilterParams.build(:all)
|> Filtrex.validate_structure ==
{:errors, ["One or more conditions required to filter"]}
{:error, "One or more conditions required to filter"}
end

@tag :validate_structure
Expand All @@ -27,7 +27,7 @@ defmodule FiltrexTest do
|> FilterParams.conditions([ConditionParams.build(:text)])
|> FilterParams.sub_filters(%{})
|> Filtrex.validate_structure ==
{:errors, ["Sub-filters must be a valid list of filters"]}
{:error, "Sub-filters must be a valid list of filters"}
end

@tag :validate_structure
Expand All @@ -39,7 +39,7 @@ defmodule FiltrexTest do
|> FilterParams.type("blah")
|> FilterParams.conditions([ConditionParams.build(:text)])])
|> Filtrex.validate_structure ==
{:errors, ["Invalid filter type 'blah'"]}
{:error, "Invalid filter type 'blah'"}
end

test "trickling up errors from conditions" do
Expand All @@ -48,10 +48,8 @@ defmodule FiltrexTest do
ConditionParams.build(:text, comparator: "invalid")
])

assert Filtrex.parse(@config, params) == {:errors, [
"Unknown column 'wrong_column'",
"Invalid text comparator 'invalid'"
]}
assert Filtrex.parse(@config, params) ==
{:error, "Unknown column 'wrong_column', Invalid text comparator 'invalid'"}
end

test "creating an 'all' ecto filter query" do
Expand Down

0 comments on commit 625f128

Please sign in to comment.