diff --git a/lib/meadow/data/schemas/types/coded_term.ex b/lib/meadow/data/schemas/types/coded_term.ex index 4ac0efb30..eb8b248b2 100644 --- a/lib/meadow/data/schemas/types/coded_term.ex +++ b/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}} diff --git a/lib/meadow_web/resolvers/controlled_vocabulary.ex b/lib/meadow_web/resolvers/controlled_vocabulary.ex index 47d3fba1b..8cae76e11 100644 --- a/lib/meadow_web/resolvers/controlled_vocabulary.ex +++ b/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} diff --git a/lib/meadow_web/schema/changeset_errors.ex b/lib/meadow_web/schema/changeset_errors.ex index 84e3de03a..a869f5670 100644 --- a/lib/meadow_web/schema/changeset_errors.ex +++ b/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 diff --git a/mix.exs b/mix.exs index ef3f7b458..5d3486d82 100644 --- a/mix.exs +++ b/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"}, diff --git a/mix.lock b/mix.lock index 0688a2bbf..325325914 100644 --- a/mix.lock +++ b/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"}, diff --git a/test/meadow/data/controlled_terms_test.exs b/test/meadow/data/controlled_terms_test.exs index 918fa7cad..171f381e4 100644 --- a/test/meadow/data/controlled_terms_test.exs +++ b/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 diff --git a/test/meadow/data/schemas/types/coded_term_test.exs b/test/meadow/data/schemas/types/coded_term_test.exs index 9636cc86f..b0d0a2060 100644 --- a/test/meadow/data/schemas/types/coded_term_test.exs +++ b/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/", diff --git a/test/meadow/data/schemas/types/controlled_term_test.exs b/test/meadow/data/schemas/types/controlled_term_test.exs index 6d69e0ddf..a51d26c3b 100644 --- a/test/meadow/data/schemas/types/controlled_term_test.exs +++ b/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 diff --git a/test/meadow_web/schema/changeset_errors_test.exs b/test/meadow_web/schema/changeset_errors_test.exs new file mode 100644 index 000000000..3513c2f66 --- /dev/null +++ b/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 diff --git a/test/meadow_web/schema/query/controlled_types/fetch_coded_term_label_test.exs b/test/meadow_web/schema/query/controlled_types/fetch_coded_term_label_test.exs index 59a1b5d5b..4c6357c95 100644 --- a/test/meadow_web/schema/query/controlled_types/fetch_coded_term_label_test.exs +++ b/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 diff --git a/test/meadow_web/schema/query/controlled_types/fetch_controlled_term_label_test.exs b/test/meadow_web/schema/query/controlled_types/fetch_controlled_term_label_test.exs index bd8f87830..7f1a642cd 100644 --- a/test/meadow_web/schema/query/controlled_types/fetch_controlled_term_label_test.exs +++ b/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