-
Notifications
You must be signed in to change notification settings - Fork 6
feat: Add paginated loader to entity-database-adapter-knex #422
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
base: wschurman/02-09-fix_move_entityprivacyutils_back_into_core_package
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## wschurman/02-09-fix_move_entityprivacyutils_back_into_core_package #422 +/- ##
=====================================================================================================
Coverage 100.00% 100.00%
=====================================================================================================
Files 108 108
Lines 15073 15616 +543
Branches 1320 1388 +68
=====================================================================================================
+ Hits 15073 15616 +543
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5d121ae to
aabff5d
Compare
aabff5d to
da0673d
Compare
75f7914 to
4be6e84
Compare
6164b63 to
fbce886
Compare
586186d to
806058c
Compare
29a3d8c to
0c6daeb
Compare
806058c to
11222bc
Compare
11222bc to
33e8e6f
Compare
0c6daeb to
64712a7
Compare
b347527 to
0b4f87d
Compare
0b4f87d to
1218be7
Compare
quinlanj
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, but will need to account for the tricky Date case in the future, if we want to support that
| private encodeCursor(fieldObject: Readonly<TFields>, fields: readonly (keyof TFields)[]): string { | ||
| const cursorData: Partial<TFields> = {}; | ||
| for (const field of fields) { | ||
| cursorData[field] = fieldObject[field]; |
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.
This works for most fields but if we want to support the Date field it will be trickier because Postgres stores dates with a finer granularity, and it gets converted to the JS date when it comes out of the DB at a rougher granularity. This is a big problem when you rely on the Date field to order the objects.
We'll either have to encode the date as a string to preserve granularity or have an option to encode the cursor with just the entity ID and lookup the cursor in a WITH .... statement appended to the main SQL so we never expose the Date in JS.

Why
This is the main goal of the stack that started in #407. It adds a paginated loader for entities for applications that use the knex postgres database adapter.
How
Many many iterations of claude.
The general principle is hoisted from Expo server application code: fetch
limit + 1entities to know if there are more pages.This makes use of the sql loader added in #414 to do the actual page load. Exact implementation described in docblocks.
Next PRs:
includeTotaloption and use postgres windows if possibleSELECT *, COUNT(*) OVER() as total_countTest Plan
Full test coverage and example usage in tests.