-
Notifications
You must be signed in to change notification settings - Fork 33k
feat(core): Dynamic max page size limit for pagination dto #16100
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
feat(core): Dynamic max page size limit for pagination dto #16100
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
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.
cubic found 4 issues across 3 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai
to give specific feedback.
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.
Just an optional small comment, looks good otherwise 👍
@@ -30,7 +32,8 @@ const sortByValidator = z | |||
.optional(); | |||
|
|||
export class ListInsightsWorkflowQueryDto extends Z.class({ | |||
...paginationSchema, | |||
skip: skipValidator, |
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.
Would it make sense to include the full paginationSchema and just overwrite the take validator? For example:
...paginationSchema,
take: createTakeValidator(MAX_ITEMS_PER_PAGE),
This way if the paginationSchema changes otherwise, we will automatically get these changes.
|
ce8ac42
to
165e9d8
Compare
|
165e9d8
to
572be8f
Compare
|
2 similar comments
|
|
✅ All Cypress E2E specs passed |
Got released with |
Summary
Make the max page size dynamic when parsing the pagination DTO, because insights require a max page size of 100.
Related Linear tickets, Github issues, and Community forum posts
https://linear.app/n8n/issue/PAY-2924/insights-table-page-size-does-not-work
Review / Merge checklist
release/backport
(if the PR is an urgent fix that needs to be backported)