course.quiz.listSubmissions(options) doesn't return results with multi-page results set

[kevintechie]

The course.quiz.listSubmissions(options) method doesn't return any results when visitEndpoint returns multi-page results. This happens because, with multi-page result sets, visitEndpoint() returns an array of objects and not an object that contains a quiz_submissions key.

See code at line:

caccl-api/endpoints/API/Course/Quiz.js

Line 587 in 08b290f

return Promise.resolve(response.quiz_submissions);

I haven't checked other methods that check multi-page results return results properly, but suspect the listSubmissions() method is not the only one.

The root of the problem is that genVisitEndpoint() doesn't return an array if there is only one page of results. This is unfortunate as it complicates how to handle its return value by calling code.

See code starting at line:

caccl-api/classes/instantiateEndpoint/genVisitEndpoint.js

Line 208 in 08adfb2

const allData = (

If genVisitEndpoint() always returned an array, calling code could easily flatten single or multi-page results (e.g.: [].concat(...response.map(({ quiz_submissions }) => quiz_submissions))) or unwrap when a single result is expected (e.g.: response[0].key).

[gabeabrams]

Hi Kevin, thank you for bringing this to my attention!

Most of this stems from Canvas being really strange with this endpoint. They usually follow a straightforward standard: if the response can be paged, it will always be of type array. When we get a multi-page response, we check if each page is an array and we concatenate them. For this endpoint, they break their convention and give us a very weird response. In some wiki, they say that this endpoint might be changed in the future (they don't like this format either), but for now, we're stuck with this strange break from convention. All this means is that we can't auto-detect quirky breaks from convention, so we do need a fix to the issue you're mentioning. Am I making sense? Or is there something I'm missing?

Now let's fix this problem!

Does this new code for line 587 fix what you're talking about?

// Make sure the response is an array
const responseAsArray = (
  Array.isArray(response)
    ? response
    : [response]
);

// Extract submissions
const submissionLists = responseAsArray.map((page) => {
  return page.quiz_submissions;
});
const submissions = [].concat(...submissionLists);

// Resolve
return submissions;

[kevintechie]

It's good to know that the other endpoints aren't as goofy. I agree that having a single key object that holds an array is a strange response. We we know we asked for submissions in the first place, after all. I digress...

I haven't tested your fix, but the 'Extract submissions' section seems to do the exact same thing as my implicit return one-liner (which I have tested). Wrapping the object response in an array is the key to make it work for single and multi-page responses.

[gabeabrams]

I'll have to double check on this and test it on an assignment with many many pages (and an assignment with a couple submissions).

I'm going to be out of the country until next week, but please feel free to submit a PR, or you can hang tight until I get back and I can work on it.

[gabeabrams]

I ran some tests on large numbers of submissions and also ran tests by setting itemsPerPage to a small number, forcing the response to be paged. I was able to reproduce the issue you brought up.

Our fix can be seen here: #24

I tested that fix against the same two cases: large number of submissions and small itemsPerPage and found that the issue was fixed. Also, I tested against a single-paged response to make sure we weren't breaking the usual case.

I'm going to ask for review from the team at Harvard, but if you have some time and want to contribute, it would be great if you could pull that branch (bugfix/subs-multipage-results) and see if it works for your case.