Conversation
adunn49
commented
Feb 23, 2026
| // And then just a quick extra sanity check that fetching 2 Documents only returns 2 Documents | ||
| $documents = $fetcher->fetch(2); | ||
| // make sure we fetched all the documents | ||
| $this->assertCount($totalDocuments, $fetchedDocumentIDs); |
Contributor
Author
There was a problem hiding this comment.
Without the ->sort('ID') fix, the fetched document count for this test is about 75.
adunn49
commented
Feb 23, 2026
| PageFake::class, | ||
| PageFakeVersioned::class, | ||
| DataObjectFake::class, | ||
| TagFake::class, |
Contributor
Author
There was a problem hiding this comment.
I was having trouble running the module tests locally from within my skeleton project. Adding these seemed to fix the issue.
adunn49
changed the title
blueo
requested changes
Feb 26, 2026
Contributor
blueo
left a comment
blueo
left a comment
There was a problem hiding this comment.
thanks for picking this up!
| { | ||
| $list = DataList::create($this->dataObjectClass); | ||
| // sort records by id so that their order can be guaranteed across multiple fetches | ||
| $list = DataList::create($this->dataObjectClass)->sort('ID'); |
Contributor
There was a problem hiding this comment.
this is a great pickup as the issue here is subtle and only appears with a large number of pages. The only thing I'd add is could we make this sort configurable? Sorting by ID will mean the oldest created pages will be indexed first. This is mostly fine but potentially it would be useful to provide a different order (eg newest first).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes
#58
Description
Details about the issue and how to replicate it are added to #58
The simple fix here is to add
->sort('ID')rather than fetching objects where the default 'Sort' is applied. This ensures that it is consistent and reliably indexes all expected documents.Added unit test to cover this scenario.