Merge pull request #1580 from nulib/1246-controlled-term-bug

Handle ControlledTerm errors in ChangesetErrors.error_details/1

lib/meadow/data/schemas/types/coded_term.ex

@@ -32,7 +32,7 @@ defmodule Meadow.Data.Types.CodedTerm do
   defp retrieve_term(%{id: id, scheme: scheme}) do
     case CodedTerms.get_coded_term(id, scheme) do
       nil ->
-        {:error, message: "#{id} is invalid coded term for scheme #{String.upcase(scheme)}"}
+        {:error, message: "#{id} is an invalid coded term for scheme #{String.upcase(scheme)}"}
 
       {{:ok, _}, %{id: id, scheme: scheme, label: label}} ->
         {:ok, %{id: id, scheme: scheme, label: label}}

lib/meadow_web/resolvers/controlled_vocabulary.ex

@@ -10,7 +10,7 @@ defmodule MeadowWeb.Resolvers.Data.ControlledVocabulary do
   def fetch_coded_term_label(_, %{id: id, scheme: scheme}, _) do
     case CodedTerms.get_coded_term(id, scheme) do
       nil ->
-        {:error, message: "#{id} is invalid coded term for scheme #{String.upcase(scheme)}"}
+        {:error, message: "#{id} is an invalid coded term for scheme #{String.upcase(scheme)}"}
 
       {{:ok, _}, term} ->
         {:ok, term}

lib/meadow_web/schema/changeset_errors.ex

@@ -6,21 +6,38 @@ defmodule MeadowWeb.Schema.ChangesetErrors do
   %{title: ["can't be blank"], accession_number: ["must be unique"]}
   """
   def error_details(changeset) do
-    Ecto.Changeset.traverse_errors(changeset, fn {msg, opts} ->
-      Enum.reduce(opts, msg, fn {key, value}, acc ->
-        value =
-          try do
-            case {key, value} do
-              {:type, {aggregate, type}} when is_atom(type) -> "#{aggregate} of #{type}"
-              {:type, type} when is_atom(type) -> to_string(type)
-              other -> to_string(other)
-            end
-          rescue
-            Protocol.UndefinedError -> inspect(value)
+    Ecto.Changeset.traverse_errors(changeset, &format_error/1)
+  end
+
+  defp format_error({404, _opts}) do
+    "identifier not found in authority"
+  end
+
+  defp format_error({msg, opts}) when is_atom(msg) do
+    with msg <- msg |> to_string() |> String.replace("_", " ") do
+      format_error({msg, opts})
+    end
+  end
+
+  defp format_error({msg, opts}) do
+    Enum.reduce(opts, msg, fn {key, value}, acc ->
+      value =
+        try do
+          case {key, value} do
+            {:type, {aggregate, type}} when is_atom(type) ->
+              "#{aggregate} of #{type}"
+
+            {:type, type} when is_atom(type) ->
+              to_string(type)
+
+            other ->
+              to_string(other)
           end
+        rescue
+          Protocol.UndefinedError -> inspect(value)
+        end
 
-        String.replace(acc, "%{#{key}}", value)
-      end)
+      String.replace(acc, "%{#{key}}", value)
     end)
   end
 end

mix.exs

@@ -48,7 +48,7 @@ defmodule Meadow.MixProject do
       {:absinthe_plug, "~> 1.5.0"},
       {:absinthe_phoenix, "~> 2.0.0"},
       {:assertions, "~> 0.18.0", only: :test},
-      {:authoritex, "~> 0.4.1"},
+      {:authoritex, "~> 0.5.0"},
       {:briefly, "~> 0.3.0", only: :test},
       {:cachex, "~> 3.2"},
       {:configparser_ex, "~> 4.0.0"},

mix.lock

@@ -4,7 +4,7 @@
   "absinthe_plug": {:hex, :absinthe_plug, "1.5.0", "018ef544cf577339018d1f482404b4bed762e1b530c78be9de4bbb88a6f3a805", [:mix], [{:absinthe, "~> 1.5.0", [hex: :absinthe, repo: "hexpm", optional: false]}, {:plug, "~> 1.3.2 or ~> 1.4", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "4c160f4ce9a1233a4219a42de946e4e05d0e8733537cd5d8d20e7d4ef8d4b7c7"},
   "assertions": {:hex, :assertions, "0.18.1", "a7e1e0c65dacac7d6c010345f2831e63e1897ba0c028c608fc508677c662d3d4", [:mix], [], "hexpm", "3b0479514b8f5ef5cb2a193202e792f60f87554a72d3b9c6fd791da0d81ae1e1"},
   "atomic_map": {:hex, :atomic_map, "0.9.3", "3c7f1302e0590164732d08ca999708efbb2cd768abf2911cf140280ce2dc499d", [:mix], [], "hexpm", "c237babf301bd2435bd85b96cffc973022b4cbb7721537059ee0dd3bb74938d2"},
-  "authoritex": {:hex, :authoritex, "0.4.1", "6c959ba79845e5c4e1600b2625b73500b7189e23356e27f4a1cdfea25c02faf6", [:mix], [{:httpoison, "~> 1.6.2", [hex: :httpoison, repo: "hexpm", optional: false]}, {:httpoison_retry, "~> 1.0.0", [hex: :httpoison_retry, repo: "hexpm", optional: false]}, {:jason, "~> 1.2.1", [hex: :jason, repo: "hexpm", optional: false]}, {:sweet_xml, "~> 0.6", [hex: :sweet_xml, repo: "hexpm", optional: false]}], "hexpm", "1e10979a7ccd343bd9b170279866dafe9b8928983c9ba01b63718f6b801320c3"},
+  "authoritex": {:hex, :authoritex, "0.5.0", "681dacab66302bff5f20e7e33047050f1dd96c941eef86544bc7dda2ebae237f", [:mix], [{:httpoison, "~> 1.6.2", [hex: :httpoison, repo: "hexpm", optional: false]}, {:httpoison_retry, "~> 1.0.0", [hex: :httpoison_retry, repo: "hexpm", optional: false]}, {:jason, "~> 1.2.1", [hex: :jason, repo: "hexpm", optional: false]}, {:sweet_xml, "~> 0.6", [hex: :sweet_xml, repo: "hexpm", optional: false]}], "hexpm", "c67f6cfbfeed046022d9ef970ea6e0da6e24128df7bcf780ed4ab47fa83afd5b"},
   "briefly": {:hex, :briefly, "0.3.0", "16e6b76d2070ebc9cbd025fa85cf5dbaf52368c4bd896fb482b5a6b95a540c2f", [:mix], [], "hexpm", "c6ebf8fc3dcd4950dd10c03e953fb4f553a8bcf0ff4c8c40d71542434cd7e046"},
   "broadway": {:hex, :broadway, "0.6.2", "ef8e0d257420c72f0e600958cf95556835d9921ad14be333493083226458791a", [:mix], [{:gen_stage, "~> 1.0", [hex: :gen_stage, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "f4f93704304a736c984cd6ed884f697415f68eb50906f4dc5d641926366ad8fa"},
   "broadway_sqs": {:hex, :broadway_sqs, "0.6.1", "be9717c2e1b6356145304c4c6900076c712a2e1b74bfed681fd88fd102018752", [:mix], [{:broadway, "~> 0.6.0", [hex: :broadway, repo: "hexpm", optional: false]}, {:ex_aws_sqs, "~> 3.2.1 or ~> 3.3", [hex: :ex_aws_sqs, repo: "hexpm", optional: false]}, {:saxy, "~> 1.1", [hex: :saxy, repo: "hexpm", optional: false]}], "hexpm", "a17981e2f76535db8dc9b77e94b047ee397f06bcda575257908acd7d5a6407df"},

test/meadow/data/controlled_terms_test.exs

@@ -33,7 +33,7 @@ defmodule Meadow.Data.ControlledTermsTest do
     end
 
     test "invalid term" do
-      assert {:error, 404} == ControlledTerms.fetch("mock0:result0")
+      assert {:error, :unknown_authority} == ControlledTerms.fetch("mock0:result0")
     end
   end
 

test/meadow/data/schemas/types/coded_term_test.exs

@@ -21,13 +21,14 @@ defmodule Meadow.Data.Types.CodedTermTest do
       assert {:ok, @coded_term_custom_type} == CodedTerm.cast(@coded_term_db_type)
       assert CodedTerm.cast(1234) == {:error, [message: "Invalid coded term type"]}
 
-      assert {:error, [message: "totallywrong is invalid coded term for scheme RIGHTS_STATEMENT"]} ==
+      assert {:error,
+              [message: "totallywrong is an invalid coded term for scheme RIGHTS_STATEMENT"]} ==
                CodedTerm.cast(%{id: "totallywrong", scheme: "rights_statement"})
 
       assert {:error,
               [
                 message:
-                  "http://rightsstatements.org/vocab/CNE/1.0/ is invalid coded term for scheme LICENSE"
+                  "http://rightsstatements.org/vocab/CNE/1.0/ is an invalid coded term for scheme LICENSE"
               ]} ==
                CodedTerm.cast(%{
                  id: "http://rightsstatements.org/vocab/CNE/1.0/",

test/meadow/data/schemas/types/controlled_term_test.exs

@@ -16,7 +16,7 @@ defmodule Meadow.Data.Types.ControlledTermTest do
       assert {:ok, @controlled_term} == ControlledTerm.cast(@controlled_term.id)
       assert ControlledTerm.cast(1234) == {:error, [message: "Invalid controlled term type"]}
 
-      assert {:error, [message: 404]} ==
+      assert {:error, [message: :unknown_authority]} ==
                ControlledTerm.cast("totallywrong")
     end
 

test/meadow_web/schema/changeset_errors_test.exs

@@ -0,0 +1,69 @@
+defmodule MeadowWeb.Schema.ChangesetErrorsTest do
+  use Meadow.AuthorityCase
+  use Meadow.DataCase
+
+  alias Meadow.Data.ControlledTerms
+  alias Meadow.Data.Schemas.Work
+  alias MeadowWeb.Schema.ChangesetErrors
+
+  setup do
+    ControlledTerms.fetch("unknown:nb2015010626")
+    :ok
+  end
+
+  test "formats errors for human readability" do
+    invalid_attrs = %{
+      accession_number: "",
+      descriptive_metadata: %{
+        contributor: [
+          %{
+            term: "missing_id_authority:123",
+            role: %{
+              id: "aut",
+              scheme: "marc_relator"
+            }
+          }
+        ],
+        genre: [
+          %{term: "wrong"}
+        ],
+        subject: [
+          %{
+            term: "http://id.loc.gov/authorities/names/nb2015010626",
+            role: %{
+              id: "wrong_coded_term_id",
+              scheme: "subject_role"
+            }
+          }
+        ]
+      }
+    }
+
+    changeset = Work.changeset(%Work{}, invalid_attrs)
+    error_details = ChangesetErrors.error_details(changeset)
+
+    assert get_in(error_details, [:accession_number]) == ["can't be blank"]
+
+    assert List.first(get_in(error_details, [:descriptive_metadata, :contributor]))
+           |> Map.get(:term) == ["identifier not found in authority"]
+
+    assert List.first(get_in(error_details, [:descriptive_metadata, :genre]))
+           |> Map.get(:term) == ["unknown authority"]
+
+    assert List.first(get_in(error_details, [:descriptive_metadata, :subject]))
+           |> Map.get(:role) == [
+             "wrong_coded_term_id is an invalid coded term for scheme SUBJECT_ROLE"
+           ]
+  end
+
+  test "handles changesets without errors" do
+    valid_attrs = %{
+      accession_number: "12345",
+      descriptive_metadata: %{title: "Test"}
+    }
+
+    changeset = Work.changeset(%Work{}, valid_attrs)
+
+    assert ChangesetErrors.error_details(changeset) == %{}
+  end
+end

test/meadow_web/schema/query/controlled_types/fetch_coded_term_label_test.exs

@@ -31,7 +31,7 @@ defmodule MeadowWeb.Schema.Query.FetchCodedTermLabelTest do
       assert get_in(query_data, ["fetchCodedTermLabel"]) |> is_nil()
 
       assert get_in(List.first(errors), [:message]) ==
-               "http://wrongsstatements.org/vocab/InC/1.0/ is invalid coded term for scheme RIGHTS_STATEMENT"
+               "http://wrongsstatements.org/vocab/InC/1.0/ is an invalid coded term for scheme RIGHTS_STATEMENT"
     end
   end
 end

test/meadow_web/schema/query/controlled_types/fetch_controlled_term_label_test.exs

@@ -20,7 +20,7 @@ defmodule MeadowWeb.Schema.Query.FetchControlledTermLabelTest do
       result = query_gql(variables: %{"id" => "mock0:result0"}, context: gql_context())
 
       assert {:ok, %{errors: [error]}} = result
-      assert error.message == "404"
+      assert error.message == "unknown_authority"
     end
   end
 end