You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "abhyudaytomar (via GitHub)" <gi...@apache.org> on 2026/04/11 16:52:38 UTC

[PR] fix(drill-detail): restrict page size options to match fixed PAGE_SIZE [superset]

abhyudaytomar opened a new pull request, #39295:
URL: https://github.com/apache/superset/pull/39295

   ## Summary
   
   Fixes #36267
   
   The drill-to-details table uses a fixed page size of 50 (`PAGE_SIZE` constant) for all backend API calls, but the frontend Table component displayed a page size selector with options `[5, 15, 25, 50, 100]`. This mismatch caused bugs when users selected a different page size:
   
   - **Sizes > 50**: Still only showed 50 rows per page
   - **Sizes < 50**: Could result in empty pages or database query errors when navigating to the "last" page
   
   ## Changes
   
   Added `pageSizeOptions={[\`${PAGE_SIZE}\`]}` to the Table component in `DrillDetailPane.tsx`, restricting the page size selector to only show the valid option (50). This is a minimal, safe fix that aligns the UI with the backend's fixed page size.
   
   ## Test Plan
   
   - [ ] Existing DrillDetailPane tests pass (7/7)
   - [ ] Open drill-to-details on any chart and verify the page size selector only shows "50"
   - [ ] Verify pagination still works correctly with 50 rows per page
   - [ ] Verify no visual regression in the drill-detail table
   
   Made with [Cursor](https://cursor.com)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(drill-detail): restrict page size options to match fixed PAGE_SIZE [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas closed pull request #39295: fix(drill-detail): restrict page size options to match fixed PAGE_SIZE
URL: https://github.com/apache/superset/pull/39295


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(drill-detail): restrict page size options to match fixed PAGE_SIZE [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39295:
URL: https://github.com/apache/superset/pull/39295#issuecomment-4381684795

   <!-- Bito Reply -->
   The change adds pageSizeOptions to fix the selector to only show the fixed PAGE_SIZE. To add a regression test, extend the existing DrillDetailPane test file (likely DrillDetailPane.test.tsx) with a test that renders the component and asserts the page size selector options are limited to [PAGE_SIZE], and that selecting other sizes isn't possible (e.g., by checking the select element's options and simulating user interaction). However, since the test file isn't in the provided PR context, I can't generate the exact code snippet.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(drill-detail): restrict page size options to match fixed PAGE_SIZE [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39295:
URL: https://github.com/apache/superset/pull/39295#issuecomment-4229793812

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(drill-detail): restrict page size options to match fixed PAGE_SIZE [superset]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #39295:
URL: https://github.com/apache/superset/pull/39295#discussion_r3190458229


##########
superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx:
##########
@@ -314,6 +314,7 @@ export default function DrillDetailPane({
           columns={mappedColumns}
           size={TableSize.Small}
           defaultPageSize={PAGE_SIZE}
+          pageSizeOptions={[`${PAGE_SIZE}`]}

Review Comment:
   This change modifies visible pagination behavior (the page-size selector options) but doesn’t appear to add a regression test for it. Since DrillDetailPane already has tests per the PR description, add/extend a test to assert the page size selector only offers the fixed PAGE_SIZE (and that selecting other sizes is not possible). This will prevent reintroducing the UI/backend mismatch.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(drill-detail): restrict page size options to match fixed PAGE_SIZE [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #39295:
URL: https://github.com/apache/superset/pull/39295#issuecomment-4815997382

   Thanks @abhyudaytomar, and sorry, this got overtaken. #36267 was fixed by #37975, which landed in May and made the page-size selector functionally adjustable rather than locking it to 50. That covers the bug from the other direction, so I think we'll close this one, but let me know I'm missing something and you/we can reopen it. Appreciate this (and any future) work!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org