vitalik
Add cursor pagination implementation
This is a direct port of DRF's CursorPagination to django-ninja. It is designed to be completely API- and behaviorally-compatible with the DRF version; even opaque cursor URLs generated with DRF should still work under this implementation, which was a design goal as the downstream project I initially created this on was switching from DRF to ninja.
If the ninja maintainers approve of the general concept, I can add tests and whatever else is required to get this merged.
Hi @zachmullen
that's a good start, but would be nice to add some test coverage and fix failing tests on old python versions
I've got the tests passing and added some of my own, but mypy is unhappy, and the coverage isn't at 100% and I don't feel comfortable inline-disabling it. This is all the work I will be able to do for a while, so let me know if you want to take over this branch, or else I can release this paginator as its own third party package.
@vitalik I would like to help get this MR up to your standards so it can be merged. From some local testing the basic functionality seems to work fine. So it seems like the main issue will be getting more testing coverage. Before I proceed, however, I was wondering if you could tell me whether the current testing for cursor pagination in this branch is up to your standards. It adds endpoints to the demo_project and runs tests through there. Looking at other testing in the main repo, however, I got the impression you might want something a bit more abstracted. Very few tests for other parts of django-ninja use the demo_project. Should I build on the current cursor pagination testing here or try to overhaul it before building out more coverage? Any input would be appreciated.
I noticed that the existing pagination works with both querysets and lists. See existing pagination. This one does not support lists. It looks like it would be have be more careful around using .count and perhaps avoid attempting to order lists.
Hi @zachmullen
sorry for delay, we just got async pagination support so this PR got a bit spoiled with conflicts
Thanks for the update @vitalk . What needs to be done to get this merged?
Hi @vitalik , is this something you intend to get merged into django-ninja, or would it make more sense for me to publish a separate package for this feature?
Thanks @bufke . FWIW I am planning to publish this functionality as a 3rd party package in the coming days or weeks. I'll ping this thread when it's available.
I have published this capability as a supplementary package: https://pypi.org/project/django-ninja-cursor-pagination/
I'll close this issue for now, but the maintainers should feel free to let me know if they'd like to upstream this into django-ninja and I'd be happy to help.
(FYI @bufke )
I'm reopening this in case the maintainers want to keep this record. Feel free to close it.