scottquach / Canvas-Assignments-Transfer-For-Todoist

Transfer your school assignments from Canvas to Todoist
MIT License
41 stars 19 forks source link

No Pagination - 100 item limit #34

Closed stacksjb closed 1 year ago

stacksjb commented 1 year ago

The Canvas API has pagination when calling items/assignments. This is shown by the return of the "link" item in the header, which informs the next page to query. (see https://canvas.instructure.com/doc/api/file.pagination.html)

However, this script doesn't handle that, meaning we only process the first 100 items.

Need to add logic to handle pagination (one option may be this link https://community.canvaslms.com/t5/Canvas-Developers-Group/Handling-Pagination/td-p/51450/page/2)

Is there someone who might be able to help with this?

stacksjb commented 1 year ago

I hacked together a solution from this post https://community.canvaslms.com/t5/Canvas-Developers-Group/Pagination-makes-endless-api-calls-when-retrieving-page-views/td-p/151089/page/2 and it works, but would love some review/input from @Nassuel who has better understanding of Python than I.

Nassuel commented 1 year ago

It looks fine to me. Other than the if statement on line 193 using the response weirdly due to its scope. What's the purpose of that if statement?

For example, if there were no course_ids, then you'd run into a referenced before assignment error on the response variable.

stacksjb commented 1 year ago

Gotcha, good catch. When I was debugging, I ran into errors at first because I didn't' include the headers the second time, so I got a 401 on the paginated response (having only authenticated the initial request).

I'll pull that out - I don't think we need that line there since the two places we can get a 401/bad key would be on initial course selection (in iniitalize_api) or on initial course pull (if you're running it when using previously selected courses)