-
Notifications
You must be signed in to change notification settings - Fork 1
MAGE-1459 Total records resolver #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
damcou
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job ! 🙌
Tests are passing and I think I get most of the code, had a few comments but nothing blocking, this can be tackled later in your next ticket.
This still hard to guess what belongs to our codebase and what is still from the core modules with all the mocked stuff but we are on a good path I believe :)
|
Yeah the mock stuff is just a frame of reference to help me understand what I actually need to build. I will rip all that out at the end. |
|
@damcou Thanks for the review! I've made some updates based on your feedback if you wouldn't mind please having another look. |
damcou
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀 🙌
Thanks for making the changes !
Summary
This PR aims to provide a more robust pagination mechanism for backend search:
algoliaengineTotalRecordsResolverQueryMapperandDocumentMapperNOTE: This depends on changes from core module under algolia/algoliasearch-magento-2#1860
Result
