Legal API returns incorrect error structure

[riyazuddinsyed]

Describe the bug in current situation When a user starts filing the IA and say some mistakes have been made in the application (eg, invalid company address) then upon clicking "File and Pay" the UI throws a error dialog display with incorrect error message.

Impact of this bug Users when mistake is made while filing the incorporation application

Steps to Reproduce Steps to reproduce the behavior:

1. Go to 'https://test.bcregistry.ca/cooperatives/' 2.Start an Incorporation Application. 3.Leave few steps incomplete
2. See error

Screenshots/ Visual Reference/ Source

[severinbeauvais]

I have tested/investigated this...

It is a valid bug as the UI is failing to display the errors returned by the API <<< this is what needs to be fixed in this ticket. Update: see comment below.

The API is correctly validating the company addresses and identifying errors:

This will not happen if the UI prevents invalid addresses (which is a separate bug). This is good case study on why the API should protect itself from bad data (even when we control the UI, but especially when we don't control the UI, ie future third party UI).

Whether this ticket's priority is appropriate depends on whether all save errors/warning are not being displayed. (Either way, this bug does not lead to bad data.) I'm investigating this now...

[severinbeauvais]

It is a valid bug as the UI is failing to display the errors returned by the API <<< this is what needs to be fixed in this ticket.

I'm a bit surprised to see that the error object returned by the UI is a nested array of array of errors (eg, first error is data.errors[0][0]).

My working hypothesis is that the BE is not returning these validation errors correctly (although it might be returning schema errors correctly).

Need someone familiar with BE to look into this...

[severinbeauvais]

@peter-freshworks just had a quick look at this and also believes the Legal API is returning the error structure incorrectly.

The Filings UI also expects errors to be an array (not an array-in-an-array). Notice that there is no array index in this code:

} else if (error && error.response && error.response.status === BAD_REQUEST) {
   if (error.response.data.errors) {
      this.saveErrors = error.response.data.errors
   }
   if (error.response.data.warnings) {
      this.saveWarnings = error.response.data.warnings
   }
...

[severinbeauvais]

@lmcclung @Kaineatthelab Do you think P2 is appropriate for this ticket? Although no bad data will result from this bug, some errors are not being reported to the user -- and thus preventing them from fixing them and filing successfully.

[Kaineatthelab]

@severinbeauvais this was pending the other address issue resolution. If the address fix is completed then this is a p2, if it is not then it is a p1

[severinbeauvais]

@Kaineatthelab The other bug is now fixed and awaiting review/merge!

On the one hand, the problem in this ticket is more general than just the addresses. It seems that, in some cases, the Legal API returns an incorrect error structure. So far we have only seen it with address validation because of the UI bug that failed to catch it.

On the other hand, it seems we don't have a ton of UI bugs that file incorrrect data, so I'm less concerned about this bug (which is really about the API protecting itself from bad data -- which it does -- and reporting that correctly -- which it doesn't).

So I think P2 is appropriate.

[lmcclung]

Agreed P2 works.

[severinbeauvais]

I just finished triaging this ticket (same as we did for the others).

[severinbeauvais]

I just had a quick look at the code (incorporation_application.py) and I think I've found the problem:

• validate_offices() appends the error message into an array (msg)
• validate() (which calls validate_offices) then appends the result (err) into another array (different msg)

The result is that errors are nested in an array-in-an-array incorrectly. There should only be one array containing the error objects.

[severinbeauvais]

Updated estimate: 1 hr for investigation + 1 hr for fix and local testing.

[severinbeauvais]

Test Notes

The UI (theoretically) does not send bad IA data to the API, so (theoretically) it is not possible to test this fix once deployed :)

Instead, I ran the UI locally (against the deployed API) with some checks disabled so that I could file bad data. This is what I got back:

I think this is enough verification/validation for this ticket. @riyazuddinsyed Please close if you agree.