E2E Framework: fix empty array check in RestAPIClient. #74741
Conversation
- fix empty array check
matticbot
added
the
[Status] Needs Review
|
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
- fix another instance of comparison
Done - thanks for the heads up! |
| @@ -360,7 +360,7 @@ export class RestAPIClient { | |||
| ); | |||
| } | |||
|
|
|||
| if ( response.errors === [] ) { | |||
| if ( response.errors.length === 0 ) { | |||
| console.log( response ); | |||
| throw new Error( `Failed to create invite: ${ response.errors }` ); | |||
There was a problem hiding this comment.
I think we need to check if there are errors here!
There was a problem hiding this comment.
Right, I didn't read the rest of the lines before making the change, but am a little confused at this logic now - I think we handled the error scenario in Line 357-360, so not sure why we're checking another error here.
Going to play with the console for a bit to see what is up.
There was a problem hiding this comment.
Ah okay, I should have written some comments because it isn't straightforward at first glance.
The first hasOwnProperty is checking for API errors.
The second error check is checking for errors relating to the invite itself.
I've added comment, fixed the logic for the if clause and added a helpful console.error level output.
- fix if logic. - add output. - add comments.
| for ( const err of response.errors ) { | ||
| console.error( `${ err.code }: ${ err.message }` ); | ||
| } | ||
| throw new Error( `Failed to create invite due to ${ response.errors.length } errors.` ); |
There was a problem hiding this comment.
Nice change, this makes a lot more sense to me 👍
| throw new Error( `Failed to create invite: ${ response.errors }` ); | ||
| // This handles "errors" relating to the invite itself and can be an array. | ||
| // For instance, if a user tries to invite itself, or invite an already added user. | ||
| if ( response.errors.length !== 0 ) { |
There was a problem hiding this comment.
nitpicky review comment alert :D
I kinda prefer > 0 to !== 0, but it doesn't make a practical difference. Or, you can even just do if ( response.errors.length ) for a quick truthy check
There was a problem hiding this comment.
True, I like that.
- truthy check
github-actions
bot
removed
the
[Status] Needs Review
Related to #74479 (comment).
Proposed Changes
This PR fixes the empty array check introduced in #74479.
Testing Instructions
Pre-merge Checklist