Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PISTON-1082: Crossbar validation errors with the same top level key will no longer overwrite each other but instead will now merge the keys where possible #6699

Merged

Conversation

bradfordben
Copy link
Contributor

@bradfordben bradfordben commented Feb 11, 2021

Kazoo 5pr: https://github.com/2600hz/kazoo-crossbar/pull/140

When CB builds the error response from multiple validation errors it would do a top level merge only resulting in errors regularly overwriting each other in the response.

EG:
Take the error list below

[{<<"numbers">>,<<"type">>,
  {[{<<"message">>,<<"Value did not match type(s): array">>},
    {<<"target">>,<<"array">>},
    {<<"value">>,<<"12345">>}]}},
 {<<"numbers">>,<<"required">>,
  {[{<<"message">>,
     <<"Callflows must be assigned at least one number or pattern">>}]}}]

With the current merge CB would return:

{[{<<"numbers">>,
   {[{<<"required">>,
      {[{<<"message">>,
         <<"Callflows must be assigned at least one number or pattern">>}]}}]}}]}

With this solution, CB will respond with:

{[{<<"numbers">>,
   {[{<<"type">>,
      {[{<<"value">>,<<"12345">>},
        {<<"target">>,<<"array">>},
        {<<"message">>,<<"Value did not match type(s): array">>}]}},
     {<<"required">>,
      {[{<<"message">>,
         <<"Callflows must be assigned at least one number or pattern">>}]}}]}}]}

This still does not handle the the case where the keys exactly match but the only way to do this would be to change the response type of the error to an array (Breaking change) but maybe its worth considering for a future version.

{
    "numbers": {
        "unique": [
            {
                "value": "12345",
                "message": "number is not unique"
            },
            {
                "value": "54321",
                "message": "number is not unique"
            }
        ]
    }
}

no longer overwrite each other but instead will now merge the keys where
possible
@jamesaimonetti jamesaimonetti merged commit 70372ba into 2600hz:4.3 Jun 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants