Merge pull request #3 from mbramson/improve-address-validation-error-…

…structure

Improve address validation failure error structure

lib/catalog_api/address.ex

@@ -153,7 +153,7 @@ defmodule CatalogApi.Address do
       ...>   postal_code: "44444",
       ...>   country: "AJ"}
       ...> CatalogApi.Address.validate(address)
-      {:error, {:invalid_address, [{:country, ["country code must be valid ISO 3166-1 alpha 2 country code"]}, {:city, ["cannot be blank"]}]}}
+      {:error, {:invalid_address, %{country: ["country code must be valid ISO 3166-1 alpha 2 country code"], city: ["cannot be blank"]}}}
 
   """
   @spec validate(t) ::
@@ -162,12 +162,13 @@ defmodule CatalogApi.Address do
   def validate(%Address{} = address) do
     {:ok, allowed_fields} = StructHelper.allowed_fields(Address)
     errors = allowed_fields
-      |> Enum.reduce([], fn field, acc ->
-           validate_field(field, Map.fetch!(address, field)) ++ acc
+      |> Enum.reduce(%{}, fn field, acc ->
+           maybe_errors = validate_field(field, Map.fetch!(address, field))
+           Map.merge(acc, maybe_errors)
          end)
 
     case errors do
-      [] -> :ok
+      map when map == %{} -> :ok
       errors -> {:error, {:invalid_address, errors}}
     end
   end
@@ -182,74 +183,74 @@ defmodule CatalogApi.Address do
   Validates a specific address field in the context of what is valid as input
   to a CatalogApi address.
   """
-  @spec validate_field(atom(), any()) :: list()
-  def validate_field(:first_name, ""), do: [{:first_name, ["cannot be blank"]}]
+  @spec validate_field(atom(), any()) :: map()
+  def validate_field(:first_name, ""), do: %{first_name: ["cannot be blank"]}
   def validate_field(:first_name, first_name) when is_binary(first_name) do
     validate_field_length(:first_name, first_name, 40)
   end
-  def validate_field(:first_name, _), do: [{:first_name, ["must be a string"]}]
-  def validate_field(:last_name, ""), do: [{:last_name, ["cannot be blank"]}]
+  def validate_field(:first_name, _), do: %{first_name: ["must be a string"]}
+  def validate_field(:last_name, ""), do: %{last_name: ["cannot be blank"]}
   def validate_field(:last_name, last_name) when is_binary(last_name) do
     validate_field_length(:last_name, last_name, 40)
   end
-  def validate_field(:last_name, _), do: [{:last_name, ["must be a string"]}]
-  def validate_field(:address_1, ""), do: [{:address_1, ["cannot be blank"]}]
+  def validate_field(:last_name, _), do: %{last_name: ["must be a string"]}
+  def validate_field(:address_1, ""), do: %{address_1: ["cannot be blank"]}
   def validate_field(:address_1, address_1) when is_binary(address_1) do
     validate_field_length(:address_1, address_1, 75)
   end
-  def validate_field(:address_1, _), do: [{:address_1, ["must be a string"]}]
+  def validate_field(:address_1, _), do: %{address_1: ["must be a string"]}
   def validate_field(:address_2, address_2) when is_binary(address_2) do
     validate_field_length(:address_2, address_2, 60)
   end
-  def validate_field(:address_2, _), do: [{:address_2, ["must be a string"]}]
+  def validate_field(:address_2, _), do: %{address_2: ["must be a string"]}
   def validate_field(:address_3, address_3) when is_binary(address_3) do
     validate_field_length(:address_3, address_3, 60)
   end
-  def validate_field(:address_3, _), do: [{:address_3, ["must be a string"]}]
-  def validate_field(:city, ""), do: [{:city, ["cannot be blank"]}]
+  def validate_field(:address_3, _), do: %{address_3: ["must be a string"]}
+  def validate_field(:city, ""), do: %{city: ["cannot be blank"]}
   def validate_field(:city, city) when is_binary(city) do
     validate_field_length(:city, city, 40)
   end
-  def validate_field(:city, _), do: [{:city, ["must be a string"]}]
-  def validate_field(:state_province, ""), do: [{:state_province, ["cannot be blank"]}]
+  def validate_field(:city, _), do: %{city: ["must be a string"]}
+  def validate_field(:state_province, ""), do: %{state_province: ["cannot be blank"]}
   def validate_field(:state_province, state_province) when is_binary(state_province) do
     validate_field_length(:state_province, state_province, 50)
   end
-  def validate_field(:state_province, _), do: [{:state_province, ["must be a string"]}]
-  def validate_field(:postal_code, ""), do: [{:postal_code, ["cannot be blank"]}]
+  def validate_field(:state_province, _), do: %{state_province: ["must be a string"]}
+  def validate_field(:postal_code, ""), do: %{postal_code: ["cannot be blank"]}
   def validate_field(:postal_code, postal_code) when is_binary(postal_code) do
     validate_field_length(:postal_code, postal_code, 15)
   end
-  def validate_field(:postal_code, _), do: [{:postal_code, ["must be a string"]}]
-  def validate_field(:country, ""), do: [{:country, ["cannot be blank"]}]
+  def validate_field(:postal_code, _), do: %{postal_code: ["must be a string"]}
+  def validate_field(:country, ""), do: %{country: ["cannot be blank"]}
   def validate_field(:country, country) when is_binary(country) do
     case Iso3166.validate(country) do
-      :ok -> []
-      :error -> [{:country, ["country code must be valid ISO 3166-1 alpha 2 country code"]}]
+      :ok -> %{}
+      :error -> %{country: ["country code must be valid ISO 3166-1 alpha 2 country code"]}
     end
   end
-  def validate_field(:country, _), do: [{:country, ["must be a string"]}]
-  def validate_field(:email, ""), do: []
+  def validate_field(:country, _), do: %{country: ["must be a string"]}
+  def validate_field(:email, ""), do: %{}
   def validate_field(:email, email) when is_binary(email) do
     cond do
-      String.length(email) > 254 -> [{:email, ["cannot be longer than 254 characters"]}]
-      Email.valid?(email) -> []
-      true -> [{:email, ["must be a valid email"]}]
+      String.length(email) > 254 -> %{email: ["cannot be longer than 254 characters"]}
+      Email.valid?(email) -> %{}
+      true -> %{email: ["must be a valid email"]}
     end
   end
-  def validate_field(:email, _), do: [{:email, ["must be a string"]}]
-  def validate_field(:phone_number, ""), do: []
+  def validate_field(:email, _), do: %{email: ["must be a string"]}
+  def validate_field(:phone_number, ""), do: %{}
   def validate_field(:phone_number, phone_number) when is_binary(phone_number) do
     validate_field_length(:phone_number, phone_number, 20)
   end
-  def validate_field(:phone_number, _), do: [{:phone_number, ["must be a string"]}]
-  def validate_field(_field, _value), do: []
+  def validate_field(:phone_number, _), do: %{phone_number: ["must be a string"]}
+  def validate_field(_field, _value), do: %{}
 
   defp validate_field_length(field, value, max_length) when is_binary(value) do
     cond do
       String.length(value) > max_length ->
-        [{field, ["cannot be longer than #{max_length} characters"]}]
-      true -> []
+        %{field => ["cannot be longer than #{max_length} characters"]}
+      true -> %{}
     end
   end
 end

test/catalog_api/address_test.exs

@@ -44,7 +44,8 @@ defmodule CatalogApi.AddressTest do
 
     test "returns an error when given invalid address params" do
       params = Map.put(@valid_address_params, "city", "")
-      assert {:error, {:invalid_address, _}} = Address.validate_params(params)
+      assert {:error, {:invalid_address, errors}} = Address.validate_params(params)
+      assert %{city: ["cannot be blank"]} = errors
     end
 
     test "returns an error when given an empty map" do
@@ -62,22 +63,22 @@ defmodule CatalogApi.AddressTest do
     test "returns an error tuple if first_name is blank" do
       address = Map.put(@valid_address, :first_name, "")
       error_message = "cannot be blank"
-      assert {:error, {:invalid_address, [{:first_name, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{first_name: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if first_name is longer than 40 characters" do
       first_name = "<  ten   ><  ten   ><  ten   ><  ten   >1"
       address = Map.put(@valid_address, :first_name, first_name)
       error_message = "cannot be longer than 40 characters"
-      assert {:error, {:invalid_address, [{:first_name, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{first_name: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if first_name is not a string" do
       address = Map.put(@valid_address, :first_name, 123)
       error_message = "must be a string"
-      assert {:error, {:invalid_address, [{:first_name, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{first_name: [^error_message]}}} =
         Address.validate(address)
     end
 
@@ -86,22 +87,22 @@ defmodule CatalogApi.AddressTest do
     test "returns an error tuple if last_name is blank" do
       address = Map.put(@valid_address, :last_name, "")
       error_message = "cannot be blank"
-      assert {:error, {:invalid_address, [{:last_name, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{last_name: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if last_name is longer than 40 characters" do
       last_name = "<  ten   ><  ten   ><  ten   ><  ten   >1"
       address = Map.put(@valid_address, :last_name, last_name)
       error_message = "cannot be longer than 40 characters"
-      assert {:error, {:invalid_address, [{:last_name, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{last_name: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if last_name is not a string" do
       address = Map.put(@valid_address, :last_name, 123)
       error_message = "must be a string"
-      assert {:error, {:invalid_address, [{:last_name, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{last_name: [^error_message]}}} =
         Address.validate(address)
     end
 
@@ -110,22 +111,22 @@ defmodule CatalogApi.AddressTest do
     test "returns an error tuple if address_1 is blank" do
       address = Map.put(@valid_address, :address_1, "")
       error_message = "cannot be blank"
-      assert {:error, {:invalid_address, [{:address_1, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{address_1: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if address_1 is longer than 75 characters" do
       address_1 = "<  ten   ><  ten   ><  ten   ><  ten   ><  ten   ><  ten   ><  ten   >123456"
       address = Map.put(@valid_address, :address_1, address_1)
       error_message = "cannot be longer than 75 characters"
-      assert {:error, {:invalid_address, [{:address_1, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{address_1: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if address_1 is not a string" do
       address = Map.put(@valid_address, :address_1, 123)
       error_message = "must be a string"
-      assert {:error, {:invalid_address, [{:address_1, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{address_1: [^error_message]}}} =
         Address.validate(address)
     end
 
@@ -140,14 +141,14 @@ defmodule CatalogApi.AddressTest do
       address_2 = "<  ten   ><  ten   ><  ten   ><  ten   ><  ten   ><  ten   >1"
       address = Map.put(@valid_address, :address_2, address_2)
       error_message = "cannot be longer than 60 characters"
-      assert {:error, {:invalid_address, [{:address_2, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{address_2: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if address_2 is not a string" do
       address = Map.put(@valid_address, :address_2, 123)
       error_message = "must be a string"
-      assert {:error, {:invalid_address, [{:address_2, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{address_2: [^error_message]}}} =
         Address.validate(address)
     end
 
@@ -162,14 +163,14 @@ defmodule CatalogApi.AddressTest do
       address_3 = "<  ten   ><  ten   ><  ten   ><  ten   ><  ten   ><  ten   >1"
       address = Map.put(@valid_address, :address_3, address_3)
       error_message = "cannot be longer than 60 characters"
-      assert {:error, {:invalid_address, [{:address_3, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{address_3: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if address_3 is not a string" do
       address = Map.put(@valid_address, :address_3, 123)
       error_message = "must be a string"
-      assert {:error, {:invalid_address, [{:address_3, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{address_3: [^error_message]}}} =
         Address.validate(address)
     end
 
@@ -178,22 +179,22 @@ defmodule CatalogApi.AddressTest do
     test "returns an error tuple if city is blank" do
       address = Map.put(@valid_address, :city, "")
       error_message = "cannot be blank"
-      assert {:error, {:invalid_address, [{:city, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{city: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if city is longer than 40 characters" do
       city = "<  ten   ><  ten   ><  ten   ><  ten   >1"
       address = Map.put(@valid_address, :city, city)
       error_message = "cannot be longer than 40 characters"
-      assert {:error, {:invalid_address, [{:city, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{city: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if city is not a string" do
       address = Map.put(@valid_address, :city, 123)
       error_message = "must be a string"
-      assert {:error, {:invalid_address, [{:city, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{city: [^error_message]}}} =
         Address.validate(address)
     end
 
@@ -202,22 +203,22 @@ defmodule CatalogApi.AddressTest do
     test "returns an error tuple if state_province is blank" do
       address = Map.put(@valid_address, :state_province, "")
       error_message = "cannot be blank"
-      assert {:error, {:invalid_address, [{:state_province, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{state_province: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if state_province is longer than 50 characters" do
       state_province = "<  ten   ><  ten   ><  ten   ><  ten   ><  ten   >1"
       address = Map.put(@valid_address, :state_province, state_province)
       error_message = "cannot be longer than 50 characters"
-      assert {:error, {:invalid_address, [{:state_province, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{state_province: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if state_province is not a string" do
       address = Map.put(@valid_address, :state_province, 123)
       error_message = "must be a string"
-      assert {:error, {:invalid_address, [{:state_province, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{state_province: [^error_message]}}} =
         Address.validate(address)
     end
 
@@ -226,22 +227,22 @@ defmodule CatalogApi.AddressTest do
     test "returns an error tuple if postal_code is blank" do
       address = Map.put(@valid_address, :postal_code, "")
       error_message = "cannot be blank"
-      assert {:error, {:invalid_address, [{:postal_code, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{postal_code: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if postal_code is longer than 15 characters" do
       postal_code = "<  ten   >123456"
       address = Map.put(@valid_address, :postal_code, postal_code)
       error_message = "cannot be longer than 15 characters"
-      assert {:error, {:invalid_address, [{:postal_code, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{postal_code: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if postal_code is not a string" do
       address = Map.put(@valid_address, :postal_code, 123)
       error_message = "must be a string"
-      assert {:error, {:invalid_address, [{:postal_code, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{postal_code: [^error_message]}}} =
         Address.validate(address)
     end
 
@@ -250,21 +251,21 @@ defmodule CatalogApi.AddressTest do
     test "returns an error tuple if country is blank" do
       address = Map.put(@valid_address, :country, "")
       error_message = "cannot be blank"
-      assert {:error, {:invalid_address, [{:country, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{country: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if country is not an ISO-3166-1 Alpha 2 Country Code" do
       address = Map.put(@valid_address, :country, "AP")
       error_message = "country code must be valid ISO 3166-1 alpha 2 country code"
-      assert {:error, {:invalid_address, [{:country, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{country: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if country is not a string" do
       address = Map.put(@valid_address, :country, 123)
       error_message = "must be a string"
-      assert {:error, {:invalid_address, [{:country, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{country: [^error_message]}}} =
         Address.validate(address)
     end
 
@@ -279,22 +280,22 @@ defmodule CatalogApi.AddressTest do
       email = String.pad_trailing("[email protected]", 255, "<  ten   >")
       address = Map.put(@valid_address, :email, email)
       error_message = "cannot be longer than 254 characters"
-      assert {:error, {:invalid_address, [{:email, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{email: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if email is invalid" do
       email = "no_at_sign"
       address = Map.put(@valid_address, :email, email)
       error_message = "must be a valid email"
-      assert {:error, {:invalid_address, [{:email, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{email: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if email is not a string" do
       address = Map.put(@valid_address, :email, 123)
       error_message = "must be a string"
-      assert {:error, {:invalid_address, [{:email, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{email: [^error_message]}}} =
         Address.validate(address)
     end
 
@@ -309,14 +310,14 @@ defmodule CatalogApi.AddressTest do
       phone_number = String.pad_trailing("", 21, "<  ten   >")
       address = Map.put(@valid_address, :phone_number, phone_number)
       error_message = "cannot be longer than 20 characters"
-      assert {:error, {:invalid_address, [{:phone_number, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{phone_number: [^error_message]}}} =
         Address.validate(address)
     end
 
     test "returns an error tuple if phone_number is not a string" do
       address = Map.put(@valid_address, :phone_number, 123)
       error_message = "must be a string"
-      assert {:error, {:invalid_address, [{:phone_number, [^error_message]}]}} =
+      assert {:error, {:invalid_address, %{phone_number: [^error_message]}}} =
         Address.validate(address)
     end
   end