fix pagination InvalidPage handling for PaginatedCollection

PaginatedCollection doesn't currently conform to the full will_paginate
API, and can't easily because it wraps a deeper customer pagination
handler that it doesn't have any awareness of. If that deeper pagination
handler raises Folio::InvalidPage, we can't currently query
a PaginationColelction for whether it uses ordinal pages or not, so if
not, just default to forwarding the error rather (which is handled as
a 404), rather than dying (which becomes a 500)

This is currently not a problem in core canvas-lms, but is a problem
with student_summary handling in the analytics plugin.

closes FOO-2251
flag=none

test plan:
- with the analytics plugin, pass an invalid page number to the
  `course_student_summaries` action.
- it should return a 404

Change-Id: I2d8cc6f8771e892fec82389bdb1bd60da76dbe48
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/270977
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
This commit is contained in:
Simon Williams 2021-08-09 11:44:45 -05:00
parent 566d6c29af
commit 6d76608e40
1 changed files with 3 additions and 1 deletions

View File

@ -372,7 +372,9 @@ module Api
begin
paginated = collection.paginate(pagination_args)
rescue Folio::InvalidPage
if pagination_args[:page].to_s =~ /\d+/ && pagination_args[:page].to_i > 0 && collection.build_page.ordinal_pages?
# Have to .try(:build_page) because we use some collections (like
# PaginatedCollection) that do not conform to the full will_paginate API.
if pagination_args[:page].to_s =~ /\d+/ && pagination_args[:page].to_i > 0 && collection.try(:build_page)&.ordinal_pages?
# for backwards compatibility we currently require returning [] for
# pages beyond the end of an ordinal collection, rather than a 404.
paginated = Folio::Ordinal::Page.create