VincentVrijburg / moneybird-dotnet

A wrapper for the Moneybird API.
MIT License
6 stars 3 forks source link

Deserialize error objects with different value types other than string/ #88

Closed mikevmil closed 3 months ago

mikevmil commented 9 months ago

The current code assumes that the error property always is a string, but the error object returned by Moneybird can have different value types. In order to get at least some useful information within the exception, we need to check the value type and then try to extract the information out if it.

Currently, it would only correctly extract the error response:

{
  "error": "Contact is required",
  "symbolic": {
    "contact": "required"
  }
}

But other error responses such as the following:

{
  "error": {
    "delivery_method": [
      "The sender address must contain an email address"
    ]
  }
}

and

{
  "error": {
    "firstname": [
      "is required"
    ],
    "lastname": [
      "is required"
    ],
    "company_name": [
      "is required"
    ]
  }
}

will not be extracted correctly.

This change will make sure that these messages will be shown.

VincentVrijburg commented 9 months ago

Thanks for your contribution! Would you be able to add unit tests to this? I don't think we test the HttpRequesterBase right now but maybe you could make a start or move this piece of code to an extension class/method so we can unit test the logic which retrieves the error code.

Let me know if you're able to do it.

mikevmil commented 8 months ago

Unfortunately, I am currently unable to write this, but if existing unit tests have already been created or are being created, I am willing to expand on them.

VincentVrijburg commented 3 months ago

I have used your suggestion to create a pr with error deserialization improvements as well as unit tests: #108 Will be included in the next minor release.