Test example solutions endpoint & refactor

[maureensturgeon]

Background: a week ago I merged in updates for loading level examples async but I made a mistake with the endpoint and it didn't load, I merged a fix but didn't add tests at that time. Also having the request as part of the promise.all wasn't the right move because we still want app options to load even if level examples don't load.

• Add tests for updates made to example solutions endpoint made a week ago
• Refactor how promises are handled in loadApp

Testing story

Tested locally

PR Checklist:

• Tests provide adequate coverage
• Privacy and Security impacts have been assessed
• Code is well-commented
• New features are translatable or updates will not break translations
• Relevant documentation has been added or updated
• User impact is well-understood and desirable
• Pull Request is labeled appropriately
• Follow-up work items (including potential tech debt) are tracked and linked

[jamescodeorg]

I'm ok with this as-is but left a style-related comment below.

This is going to conflict with #43422 in kind of an annoying way. Would it be ok if I went first?

[jamescodeorg]

(minor) I feel like this is somehow obscuring the logic a bit (possibly even more so than we just used a .then block). Would it be simpler if we just had two separate await blocks instead of using Promise.allSettled?

Side note: It looks like the handling is currently the same for both requests -- if the requests fails, we silently fail but continue to load the page. Long-term, I think we'll want to show an error to the user if userAppOptionsRequest fails (but maybe not if exampleSolutionsRequest fails), right?

[maureensturgeon]

If we have two separate awaits then the requests will not run in parallel, each await will wait for the response before continuing. I can try refactoring to use .then, I generally prefer await because I think it's easier to read.

The way I wrote this, if example solutions request fails, we'll do a console log and continue to execute. If app options fails I return the app options that we have, which is how it was handled before if the app options request failed. I agree we should probably re-think what we want to happen if that request fails

[jamescodeorg]

Oh, I thought the requests get initiated (i.e. sent) when $.ajax is called and the await just waits for it to finish? (In general, I agree that await is a more modern pattern and we should favor it. I was just surprised that for some reason this feels harder to read.)

[maureensturgeon]

When you use await the next line isn't executed until the the request finishes.

const responseA = await RequestA();
const responseB = await RequestB();
doOtherStuff()

would be the same as

RequestA.then(responseA => {
  RequestB.then(responseB => {
    doOtherStuff()
  })
})

Let me know if that helps, also happy to do a call and discuss this more!

I think the reason this one is harder to read is because instead of getting a direct result from allSettled it returns objects with statuses and results which means we need to check another layer.

[maureensturgeon]

Let me see if I can replace allSettled with something that's easier to work with

[jamescodeorg]

Oh, I was thinking more along the lines of leaving everything before line 318 as-is and then replacing the rest of the function with something like:

try {
    exampleSolutions = await exampleSolutionsRequest
} catch (err) {
}
...
try {
    userAppOptions = await userAppOptionsRequest
} catch (err) {
}

(Not at all sure about this, just throwing it out there.)

[maureensturgeon]

So if it was written like that, the example solutions request would be made and then after it returns and the stuff in between executes, then the request to appOptions would be made. The requests would not be made in parallel.. wait now I'm going to check that statement..

[maureensturgeon]

Yes, even if the result is assigned in a separate try block the execution is still blocked until the promise resolves

[maureensturgeon]

Following up on this after a conversation James and I had in slack. These requests will run in parallel if ajax is called for each before the await statements, I've updated the PR

[maureensturgeon]

I'm ok with this as-is but left a style-related comment below.

This is going to conflict with #43422 in kind of an annoying way. Would it be ok if I went first?

Go ahead and merge yours first