You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "AoLiGei1221 (via GitHub)" <gi...@apache.org> on 2025/11/26 04:23:23 UTC
[PR] feat: Export table data with "Search box" enabled #35141 [superset]
AoLiGei1221 opened a new pull request, #36281:
URL: https://github.com/apache/superset/pull/36281
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
According to the [issue#35141](https://github.com/apache/superset/issues/35141), the export functionality for table chart when the search box enabled and pagination is disabled has some unexpected behaviours for it. We fixed Table chart export by passing filtered rows from DataTable to TableChart using a new callback and storing them in ownState.clientView, allowing frontend exports without backend queries.
Unexpected Behaviour: When the search box is enabled, it will export all data if we do export .CSV, Excel, .JSON
Expected Behaviour: We should export the data appeared in the current view (i.e. filtered data)
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
The before screenshot can be checked in [issue#35141](https://github.com/apache/superset/issues/35141). More details in the issue page.
The after screenshot:
<img width="356" height="67" alt="截屏2025-11-25 23 08 18" src="https://github.com/user-attachments/assets/51fb21b8-2f12-4c71-8a2e-5c914acf0cba" />
<img width="329" height="106" alt="截屏2025-11-25 23 08 30" src="https://github.com/user-attachments/assets/0c10b237-6bd9-4de7-b922-1aa8d9c39e66" />
<img width="564" height="261" alt="截屏2025-11-25 23 08 52" src="https://github.com/user-attachments/assets/33061037-e4fc-4351-896e-56ad0f65fe63" />
### TESTING INSTRUCTIONS
1. Click the **Charts** on the top of the superset page
2. Open an existing table chart or create a chart with type **Table**.
3. Click "Customize" and turn on the check box of "Search Box"
4. Click the "three dot" option on the right hand side of Save button.
5. Click "Data Export Options"
6. Choose the option between "Export All Data" and "Export Current View"
7. Choose your desired way to export data.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: https://github.com/apache/superset/issues/35141#issuecomment-3577980613
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [x] Introduces new feature or API
- [ ] Removes existing feature or API
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #36281:
URL: https://github.com/apache/superset/pull/36281
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "AoLiGei1221 (via GitHub)" <gi...@apache.org>.
AoLiGei1221 commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3610263964
<img width="1113" height="453" alt="截屏2025-12-04 00 24 49" src="https://github.com/user-attachments/assets/0d977b37-0cc6-41fa-a55c-d3a14e5c372d" />
@rusackas All good locally now in my side! When you have a moment, please rerun CI. Thanks!
--
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] feat: Export table data with "Search box" enabled #35141 [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3579004692
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "AoLiGei1221 (via GitHub)" <gi...@apache.org>.
AoLiGei1221 commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3604821158
I try to modify the test. Hopefully, it fixes the e2e. :)
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3598409102
Looks like pre-commit needs to be run, and that _might_ unblock some of the other failing CI jobs.
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "AoLiGei1221 (via GitHub)" <gi...@apache.org>.
AoLiGei1221 commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3597767916
@rusackas Hey Evan! Thank you for communicating with one of my teammate Rebecca. Could you please run the test again?
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3608455690
CodeAnt AI is running Incremental review
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3604779609
CodeAnt AI is running Incremental review
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3603117600
CodeAnt AI is reviewing your 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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3603154436
CodeAnt AI finished reviewing your 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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "AoLiGei1221 (via GitHub)" <gi...@apache.org>.
AoLiGei1221 commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3604650826
> It looks like the E2E tests are failing, and might be related to these changes. Not sure about the Frontend Build failure so I restarted that one 🤞
@rusackas Hey Evan! I am not sure about E2E tests since last time CI also fail E2E. It looks like we need to modify one test case and run Cypress but it has some issues run locally. Do you have any suggestions?
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3610358222
CodeAnt AI is running Incremental review
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3583511318
Love it! I think the change I'd want to see (at least, as of when we chatted) would be to only show this new menu option when it's relevant. We might want to just _enable_ it for charts where it's relevant, via an entry in the chart's metadata. In this case, that would be `superset-frontend/plugins/plugin-chart-table/src/index.ts`. Then it wouldn't appear for charts where it's _not_ relevant (pie chart, for example) but could be _added_ for charts where it _is_ relevant (besides this one, maybe the line chart when it has the "Data Zoom" feature enabled).
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "AoLiGei1221 (via GitHub)" <gi...@apache.org>.
AoLiGei1221 commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3604125528
> @AoLiGei1221 looks like some file changes aren't related to this pr, can we clean this up such that this PR contains only the changes mentioned in the feature?
Yes, I think now is good. I add some test cases for the new feature. Please re-run the CI. Hopefully, this time. it works. :)
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "AoLiGei1221 (via GitHub)" <gi...@apache.org>.
AoLiGei1221 commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3628341159
Thank you @msyavuz for running the test for me. Now, all good. Evan @rusackas ,do you mind helping us to merge this PR please? Thank you so much!
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "AoLiGei1221 (via GitHub)" <gi...@apache.org>.
AoLiGei1221 commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3599616997
Hey @rusackas!
I’ve just run the pre-commit hooks locally. Please take a look and re-run on your end when you get a chance.
I noticed that mypy (main) reported a few Unused "type: ignore" warnings during git commit.
Code:
> mypy (main)..............................................................Failed
> - hook id: mypy
> - exit code: 1
>
> superset/core/api/core_api_injection.py:58: error: Unused "type: ignore[assignment, misc]" comment [unused-ignore]
> superset/core/api/core_api_injection.py:59: error: Unused "type: ignore[assignment, misc]" comment [unused-ignore]
> superset/core/api/core_api_injection.py:60: error: Unused "type: ignore[assignment, misc]" comment [unused-ignore]
> superset/core/api/core_api_injection.py:61: error: Unused "type: ignore[assignment, misc]" comment [unused-ignore]
> superset/core/api/core_api_injection.py:62: error: Unused "type: ignore[assignment, misc]" comment [unused-ignore]
> superset/core/api/core_api_injection.py:63: error: Unused "type: ignore[assignment, misc]" comment [unused-ignore]
> superset/core/api/core_api_injection.py:64: error: Unused "type: ignore[assignment, misc]" comment [unused-ignore]
> superset/core/api/core_api_injection.py:65: error: Unused "type: ignore[assignment, misc]" comment [unused-ignore]
> superset/core/api/core_api_injection.py:66: error: Unused "type: ignore[assignment, misc]" comment [unused-ignore]
but when I ran:
`pre-commit run --all-files`
everything passed successfully with no errors.
I believe this should be fine since pre-commit run --all-files is consistent with the checks performed in CI.
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3603405916
@AoLiGei1221 looks like some file changes aren't related to this pr, can we clean this up such that this PR contains only the changes mentioned in the feature?
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "AoLiGei1221 (via GitHub)" <gi...@apache.org>.
AoLiGei1221 commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3608461617
@rusackas Hey Evan! Sorry to bother you again. I modified the test and please run the CI again. Hopefully, this time is ok. :)
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #36281:
URL: https://github.com/apache/superset/pull/36281#discussion_r2578304589
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx:
##########
@@ -619,6 +873,7 @@ export const useExploreAdditionalActionsMenu = (
showDashboardSearch,
slice,
theme.sizeUnit,
+ ownState,
Review Comment:
The `hasExportCurrentView` variable is used in the menu construction (line 749) but is not included in the `useMemo` dependency array. This could cause the menu to show incorrect export options when the viz_type changes. Add `hasExportCurrentView` to the dependency array at line 854.
```suggestion
ownState,
hasExportCurrentView,
```
##########
superset-frontend/plugins/plugin-chart-table/src/index.ts:
##########
@@ -39,6 +39,7 @@ const metadata = new ChartMetadata({
Behavior.InteractiveChart,
Behavior.DrillToDetail,
Behavior.DrillBy,
+ 'EXPORT_CURRENT_VIEW' as any,
],
Review Comment:
[nitpick] Using `as any` to add a custom behavior bypasses TypeScript's type safety. Consider either:
1. Adding `EXPORT_CURRENT_VIEW` to the `Behavior` enum in `@superset-ui/core` if this is intended to be a standard behavior, or
2. Extending the behaviors array type to support string literals for custom behaviors.
This would make the implementation more type-safe and maintainable.
```suggestion
'EXPORT_CURRENT_VIEW',
] as (Behavior | 'EXPORT_CURRENT_VIEW')[],
```
##########
superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx:
##########
@@ -424,6 +429,45 @@ export default typedMemo(function DataTable<D extends object>({
onServerPaginationChange(pageNumber, serverPageSize);
}
+ // Emit filtered rows to parent in client-side mode (debounced via RAF)
+ const isMountedRef = useRef(true);
+ useEffect(() => {
+ isMountedRef.current = true;
+ return () => {
+ isMountedRef.current = false;
+ };
+ }, []);
+
+ const rafRef = useRef<number | null>(null);
+ const lastSigRef = useRef<string>('');
+
+ useEffect(() => {
+ if (serverPagination || typeof onFilteredRowsChange !== 'function') {
+ return;
+ }
+
+ const filtered = rows.map(r => r.original as D);
+ const len = filtered.length;
+ const first = len ? Object.values(filtered[0] as any)[0] : '';
+ const last = len ? Object.values(filtered[len - 1] as any)[0] : '';
+ const sig = `${len}|${String(first)}|${String(last)}`;
Review Comment:
[nitpick] The signature generation logic uses `Object.values(filtered[0] as any)[0]` to get the first value of the first row. This approach is fragile because:
1. If the first column value is `null`, `undefined`, or an empty string, the signature might incorrectly indicate no change
2. Column order changes would not be detected if the first value remains the same
3. The `as any` type assertion bypasses type safety
Consider using a more robust approach, such as computing a hash of the entire filtered dataset or using a combination of row count and a stable identifier (e.g., first row's ID field if available).
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx:
##########
@@ -462,7 +633,7 @@ export const useExploreAdditionalActionsMenu = (
},
{
key: MENU_KEYS.DOWNLOAD_AS_IMAGE,
- label: t('Download as image'),
+ label: t('Export screenshot (jpeg)'),
Review Comment:
[nitpick] The label text was changed from "Download as image" to "Export screenshot (jpeg)", which is more descriptive. However, this menu item remains in the `currentViewChildren` array (used for "Export Current View") even though screenshots capture the entire visible chart and are not filtered by the current data view. Consider moving this item to a separate menu section or clarifying that it captures the visual representation rather than being data-dependent.
##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -1320,6 +1321,49 @@ export default function TableChart<D extends DataRecord = DataRecord>(
}
};
+ // collect client-side filtered rows for export & push snapshot to ownState (guarded)
+ const [clientViewRows, setClientViewRows] = useState<DataRecord[]>([]);
+
+ const exportColumns = useMemo(
+ () =>
+ visibleColumnsMeta.map(col => ({
+ key: col.key,
+ label: col.config?.customColumnName || col.originalLabel || col.key,
+ })),
+ [visibleColumnsMeta],
+ );
+
+ const clientViewSigRef = useRef<string>('');
+ useEffect(() => {
+ if (serverPagination) return; // only for client-side mode
+
+ const len = clientViewRows.length;
+ const first = len ? clientViewRows[0]?.[exportColumns[0]?.key ?? ''] : '';
+ const last = len
+ ? clientViewRows[len - 1]?.[exportColumns[0]?.key ?? '']
+ : '';
+ const colSig = exportColumns.map(c => c.key).join(',');
+ const sig = `${len}|${String(first)}|${String(last)}|${colSig}`;
+
+ if (sig !== clientViewSigRef.current) {
+ clientViewSigRef.current = sig;
+ updateTableOwnState(setDataMask, {
+ ...(serverPaginationData || {}),
+ clientView: {
+ rows: clientViewRows,
+ columns: exportColumns,
+ count: len,
Review Comment:
[nitpick] Similar to the issue in DataTable.tsx, the signature generation in TableChart uses a simple concatenation of first/last values and column keys. This approach has similar fragility issues:
1. Changes to middle rows won't be detected
2. Column value changes in non-first/non-last rows won't trigger updates
3. The signature could have false positives if different datasets happen to have the same first/last values
While this optimization helps avoid unnecessary state updates, consider documenting these limitations in a comment or using a more comprehensive approach (e.g., shallow equality check on the arrays themselves).
```suggestion
// Use a ref to store previous clientViewRows and exportColumns for robust change detection
const prevClientViewRef = useRef<{ rows: DataRecord[]; columns: typeof exportColumns } | null>(null);
useEffect(() => {
if (serverPagination) return; // only for client-side mode
const prev = prevClientViewRef.current;
const rowsChanged = !prev || !isEqual(prev.rows, clientViewRows);
const columnsChanged = !prev || !isEqual(prev.columns, exportColumns);
if (rowsChanged || columnsChanged) {
prevClientViewRef.current = { rows: clientViewRows, columns: exportColumns };
updateTableOwnState(setDataMask, {
...(serverPaginationData || {}),
clientView: {
rows: clientViewRows,
columns: exportColumns,
count: clientViewRows.length,
```
##########
superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx:
##########
@@ -424,6 +429,45 @@ export default typedMemo(function DataTable<D extends object>({
onServerPaginationChange(pageNumber, serverPageSize);
}
+ // Emit filtered rows to parent in client-side mode (debounced via RAF)
+ const isMountedRef = useRef(true);
+ useEffect(() => {
+ isMountedRef.current = true;
+ return () => {
+ isMountedRef.current = false;
+ };
+ }, []);
+
+ const rafRef = useRef<number | null>(null);
+ const lastSigRef = useRef<string>('');
+
+ useEffect(() => {
+ if (serverPagination || typeof onFilteredRowsChange !== 'function') {
+ return;
+ }
+
+ const filtered = rows.map(r => r.original as D);
+ const len = filtered.length;
+ const first = len ? Object.values(filtered[0] as any)[0] : '';
+ const last = len ? Object.values(filtered[len - 1] as any)[0] : '';
+ const sig = `${len}|${String(first)}|${String(last)}`;
+
+ if (sig !== lastSigRef.current) {
+ lastSigRef.current = sig;
+
+ rafRef.current = requestAnimationFrame(() => {
+ if (isMountedRef.current) onFilteredRowsChange(filtered);
+ });
+ }
Review Comment:
There's a potential race condition with the `requestAnimationFrame` scheduling. When the signature changes (line 455), a new RAF is scheduled without canceling the previous one first. This could result in multiple pending RAF callbacks. Consider canceling any existing `rafRef.current` before scheduling a new one:
```javascript
if (sig !== lastSigRef.current) {
lastSigRef.current = sig;
if (rafRef.current != null) {
cancelAnimationFrame(rafRef.current);
}
rafRef.current = requestAnimationFrame(() => {
if (isMountedRef.current) onFilteredRowsChange(filtered);
});
}
```
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx:
##########
@@ -307,6 +319,116 @@ export const useExploreAdditionalActionsMenu = (
}
}, [addDangerToast, addSuccessToast, latestQueryFormData]);
+ // Minimal client-side CSV builder used for "Current View" when pagination is disabled
+ const downloadClientCSV = (rows, columns, filename) => {
+ if (!rows?.length || !columns?.length) return;
+ const esc = v => {
+ if (v === null || v === undefined) return '';
+ const s = String(v);
+ const wrapped = /[",\n]/.test(s) ? `"${s.replace(/"/g, '""')}"` : s;
+ return wrapped;
+ };
+ const header = columns.map(c => esc(c.label ?? c.key ?? '')).join(',');
+ const body = rows
+ .map(r => columns.map(c => esc(r[c.key])).join(','))
+ .join('\n');
+ const csv = `${header}\n${body}`;
+ const blob = new Blob([csv], { type: 'text/csv;charset=utf-8;' });
+ const link = document.createElement('a');
+ link.href = URL.createObjectURL(blob);
+ link.download = `${filename || 'current_view'}.csv`;
+ document.body.appendChild(link);
+ link.click();
+ document.body.removeChild(link);
+ URL.revokeObjectURL(link.href);
+ };
+
+ // Robust client-side JSON for "Current View"
+ const downloadClientJSON = (rows, columns, filename) => {
+ if (!rows?.length || !columns?.length) return;
+
+ const norm = v => {
+ if (v instanceof Date) return v.toISOString();
+ if (v && typeof v === 'object' && 'input' in v && 'formatter' in v) {
+ const dv = v.input ?? v.value ?? v.toString?.() ?? '';
+ return dv instanceof Date ? dv.toISOString() : dv;
+ }
+ return v;
+ };
+
+ const data = rows.map(r => {
+ const out = {};
+ columns.forEach(c => {
+ out[c.key] = norm(r[c.key]);
+ });
+ return out;
+ });
+
+ const meta = {
+ columns: columns.map(c => ({
+ key: c.key,
+ label: c.label ?? c.key,
+ })),
+ count: rows.length,
+ };
+
+ const payload = { meta, data };
+ const blob = new Blob([JSON.stringify(payload, null, 2)], {
+ type: 'application/json;charset=utf-8;',
+ });
+ const link = document.createElement('a');
+ link.href = URL.createObjectURL(blob);
+ link.download = `${filename || 'current_view'}.json`;
+ document.body.appendChild(link);
+ link.click();
+ document.body.removeChild(link);
+ URL.revokeObjectURL(link.href);
+ };
+
+ // NEW: Client-side XLSX for "Current View" (uses 'xlsx' already in deps)
+ const downloadClientXLSX = async (rows, columns, filename) => {
+ if (!rows?.length || !columns?.length) return;
+ try {
+ const XLSX = (await import(/* webpackChunkName: "xlsx" */ 'xlsx'))
+ .default;
+
+ // Build a flat array of objects keyed by backend column key
+ const data = rows.map(r => {
+ const o = {};
+ columns.forEach(c => {
+ const v = r[c.key];
+ o[c.label ?? c.key] =
+ v && typeof v === 'object' && 'input' in v && 'formatter' in v
+ ? v.input instanceof Date
+ ? v.input.toISOString()
+ : (v.input ?? v.value ?? '')
+ : v instanceof Date
+ ? v.toISOString()
+ : v;
+ });
+ return o;
+ });
+
+ const ws = XLSX.utils.json_to_sheet(data, { skipHeader: false });
+ const wb = XLSX.utils.book_new();
+ XLSX.utils.book_append_sheet(wb, ws, 'Current View');
+
+ // Autosize columns (roughly) by header length
+ const colWidths = Object.keys(data[0] || {}).map(h => ({
+ wch: Math.max(10, String(h).length + 2),
+ }));
+ ws['!cols'] = colWidths;
+
+ XLSX.writeFile(wb, `${filename || 'current_view'}.xlsx`);
+ } catch (e) {
+ // If xlsx isn’t available for some reason, fall back to CSV
+ downloadClientCSV(rows, columns, filename || 'current_view');
+ addDangerToast?.(
+ t('Falling back to CSV; Excel export library not available.'),
+ );
+ }
+ };
Review Comment:
[nitpick] The functions `downloadClientCSV`, `downloadClientJSON`, and `downloadClientXLSX` are defined inside the hook component but are not wrapped in `useCallback`. While they're not included in the `useMemo` dependencies, they're used directly in the menu item onClick handlers (lines 583, 617, 667). Consider wrapping these functions in `useCallback` to prevent unnecessary re-creation on every render and ensure stable function references.
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3602534267
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/36281?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 44.08%. Comparing base ([`6e0960c`](https://app.codecov.io/gh/apache/superset/commit/6e0960c3f53f79ed3a986c7a0ee6b7c3d02240e3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`6ae8833`](https://app.codecov.io/gh/apache/superset/commit/6ae88335fa867bffd67fe4cf66e7568737b8b403?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 3 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #36281 +/- ##
===========================================
+ Coverage 0 44.08% +44.08%
===========================================
Files 0 639 +639
Lines 0 47138 +47138
Branches 0 5083 +5083
===========================================
+ Hits 0 20782 +20782
- Misses 0 26014 +26014
- Partials 0 342 +342
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/36281/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/36281/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.74% <ø> (?)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/36281/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.74% <ø> (?)` | |
| [superset-extensions-cli](https://app.codecov.io/gh/apache/superset/pull/36281/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `94.90% <ø> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/36281?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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] feat(table): Export table data with "Search box" enabled [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #36281:
URL: https://github.com/apache/superset/pull/36281#issuecomment-3603145579
## Nitpicks 🔍
<table>
<tr><td>🔒 <strong>No security issues identified</strong></td></tr>
<tr><td>⚡ <strong>Recommended areas for review</strong><br><br>
- [ ] <a href='https://github.com/apache/superset/pull/36281/files#diff-888cc9d72e504a2c52077bddadbadbcbce423d45adffa6b53e0bd8888589a330R324-R345'><strong>Data Export Accuracy</strong></a><br>The client-side "Current View" CSV export (`downloadClientCSV`) directly stringifies cell values. For table cells that are objects (e.g., with `input`/`formatter` like those handled in `downloadClientJSON` and `downloadClientXLSX`), this can export `[object Object]` instead of the underlying value, leading to incorrect CSV content compared to JSON/XLSX and server-side exports. Consider normalizing values similarly to the JSON/XLSX paths.<br>
- [ ] <a href='https://github.com/apache/superset/pull/36281/files#diff-888cc9d72e504a2c52077bddadbadbcbce423d45adffa6b53e0bd8888589a330R606-R684'><strong>Possible Bug</strong></a><br>In the "Export Current View" submenu, the JSON and Excel handlers fall back to calling `exportJson()`/`exportExcel()` without passing `ownState` when `server_pagination` is enabled or `clientView` is missing, while the CSV handler explicitly passes `ownState` in its server path. This may cause JSON/XLSX "current view" exports to ignore some client-side state (e.g., search box filters) or behave differently from the CSV export for the same menu section. It's worth verifying whether `ownState` should also be forwarded in these fallback branches.<br>
</td></tr>
</table>
--
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