feat(v2): add clearer error messages when updating collaborators

[siddarth2824]

Problem

Currently the error messages when users attempt to update collaborator list on admin form are not user friendly.

Closes [#3851]

Solution

Update the client side to have a mapping between status codes and user friendly error messages displayed.

[timotheeg]

The error message will not be correct if requestEmail is an empty string. You probably need a conditional block to format 2 distinct error messages.

If requestEmail is this block can in fact never be undefined and converted to empty string, let's try to make that explicit in code.

[timotheeg]

The ! a not a non-null assertion, it’s the logical not operator. It will convert whatever truthiness requestEmail represents to its opposite true or false, and so the resulting message will be either :

true is not part of a whitelisted agency

or

false is not part of a whitelisted agency

which is not what we want 😅 .

If requestEmail is always defined on 422 errors, we can make it clear with a cast:
errorMessage = ${requestEmail as string} is not part of a whitelisted agency

If requestEmail may be undefined, we’d be better off with a conditional block:

errorMessage = requestEmail
    ? `${requestEmail} is not part of a whitelisted agency`
    : `An unexpected error 422 happened`

One small issue I have with doing a straight cast in this case is code self-consistency and implicit assumptions. Maybe we can try to make things a little clearer 🤔 . Based on the call tree of getMappedErrorMessage(), requestEmail is only supplied if formCollaboratorAction is FormCollaboratorAction.ADD. The 422 error code is not inspected in a context where formCollaboratorAction has been checked, so if we just cast requestEmail as string to show we know it's true, we've implicitly stated only FormCollaboratorAction.ADD returns 422. It may be true, but it's a bit dirty and not obvious from the code itself (the function signature shows requestEmail can be undefined).

Long story short: I'd recommend going the defensive programming route here. Either do a check to verify requestEmail is not undefined like above, and have an error message for unexpected cases. Or check on formCollaboratorAction first, and then cast to string, and you likely still need an error message for unexpected cases.

[siddarth2824]

Thanks for the detailed explanation. I initially assumed that it was okay to add an empty string if in the case requestEmail is empty because the 422 error thrown only occurs for FormCollaboratorAction.Add. I do realise now that it is not clear in the code that that is the case and it is better just to do a simple non-null check and throw a different error if requestEmail is null.

[timotheeg]

👍