Repository navigation
fix(reports): export full paginated table results - #44218
Conversation
Code Review Agent Run #cc7dadActionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #44218 +/- ##
=======================================
Coverage 81.06% 81.06%
=======================================
Files 2955 2955
Lines 178300 178300
Branches 41307 41307
=======================================
Hits 144533 144533
Misses 31056 31056
Partials 2711 2711
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
EnxDev
left a comment
There was a problem hiding this comment.
Reviewed the current diff and traced the rebuilt query through the Excel export task. All 49 tests in tests/unit_tests/tasks/test_export_dashboard_excel.py pass at 9a07679. I didn't find any blocking issues. One small test suggestion below.
The current diff only adds documentation and regression coverage; the backend already builds a single full-limit query. Could we update the PR summary to reflect that scope?
|
The suggestion to use a nonzero tests/unit_tests/tasks/test_export_dashboard_excel.py |
Code Review Agent Run #ab7165Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
EnxDev
left a comment
There was a problem hiding this comment.
Second pass complete at 1714632. Thanks for adding the nonzero offset; that addresses my earlier suggestion. I checked the aggregate/raw query construction, pagination handling, and the downstream defaults, and ran the export-task and shared query-builder suites together: 89 tests passed. I didn't find any additional code issues.
One wording detail in the summary: ChartDataQueryContextSchema and ChartDataCommand are mocked in this test, so it verifies the payload handed to them, rather than actual schema loading or database execution. Also, the current diff has no production-code changes, so the reference to removing the form_data.result_format assignment can be dropped.
Code Review Agent Run #733d03Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
1714632 to
f3764bb
Compare
Code Review Agent Run #e039f7Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
(cherry picked from commit ffe9a00)
SUMMARY
Extracts self-contained documentation and regression coverage for dashboard Excel pagination from #43771 so that the Sunburst feature PR has a smaller, more reviewable scope.
The backend already rebuilds a saved paginated Table as one full-limit query instead of the interactive page and row-count pair. This PR documents and locks in that behavior through the real schema/
ChartDataCommandpath, including resetting a nonzero input offset, and removes an ineffectiveform_data.result_formatassignment.This change is independent of Sunburst and can land before #43771. The other independent extraction is #44219. Once merged, #43771 can absorb master normally and its corresponding diff disappears without rewriting either branch.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable; backend export behavior only.
TESTING INSTRUCTIONS
Regression coverage verifies aggregate and raw paginated Tables export the configured 1,000-row limit rather than a 10-row interactive page, reset the offset, and omit the count query.
Exact-head CI:
1714632f9541a2220cc1edf17bad4608b6b6cecfis mergeable. Focused tests and pre-commit pass; integration checks are blocked by the two migration heads present on the currentmastermerge result (c7f53d184ea2and88a01c781622).ADDITIONAL INFORMATION