Skip to content

ETT-1332 My Collections won't display page 2#193

Merged
moseshll merged 1 commit intomainfrom
ETT-1332_collections_pagination
Mar 10, 2026
Merged

ETT-1332 My Collections won't display page 2#193
moseshll merged 1 commit intomainfrom
ETT-1332_collections_pagination

Conversation

@moseshll
Copy link
Contributor

@moseshll moseshll commented Mar 9, 2026

  • This bug affects all "lists of collections" (NOT "lists of collection items" i.e., items in a single collection)
  • The pagination math behind how the results array is sliced, is incorrect in that it does not multiply by items per page
  • Since we have a nice Data::Page object created at the end of the routine, I just move it ahead of the slicing and adjust for 0-based vs 1-based indexing.
  • The Data::Page code uses a config value for items per page; the bug locale uses hardcoded 100; I swapped in the former.

Reviewer:
Compare production vs dev-2 (or your own version):

  • Choose any criteria for a list of collections > 100, as a not-logged-in user the simplest way is to use the 9700-odd-member Shared Collections, https://babel.hathitrust.org/cgi/mb?a=listcs;colltype=all
  • The list starts with -------2024_REFERENCES followed by 'Generations', continuing on down to the last entry 1934.
  • Go to page 2 of the results: on production the page starts with 'Generations' but the dev-2 version starts with 1835 which I think we can be confident is the beginning of the correct slice.

- This bug affects all "lists of collections" (NOT "lists of collection items" i.e., items in a single collection)
- The pagination math behind how the results array is sliced, is incorrect in that it does not multiply by items per page
- Since we have a nice `Data::Page` object created at the end of the routine, I just move it ahead of the slicing and
adjust for 0-based vs 1-based indexing.
- The `Data::Page` code uses a config value for items per page; the bug locale uses hardcoded `100`; I swapped in the former.
@moseshll moseshll requested a review from carylwyatt March 9, 2026 20:34
Copy link
Member

@carylwyatt carylwyatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's truly amazing that no one has reported this bug before now! Excellent sleuthing and fix.

I tested jumping to page 47 of results and paging my way back and forth. It all looks in order! Approve!

@moseshll moseshll merged commit b5d8073 into main Mar 10, 2026
2 checks passed
@moseshll moseshll deleted the ETT-1332_collections_pagination branch March 10, 2026 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants