amaannawab923 commented on code in PR #42088:
URL: https://github.com/apache/superset/pull/42088#discussion_r3646227269
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx:
##########
@@ -368,75 +395,202 @@ export default function TableChart<D extends DataRecord
= DataRecord>(
[emitCrossFilters, setDataMask, timeGrain, timestampFormatter],
);
+ const drillColumns = isUsingTimeComparison
+ ? (filteredColumns as InputColumn[])
+ : (columns as InputColumn[]);
+
+ const handleContextMenu = useCallback(
+ (event: CellContextMenuEvent) => {
+ if (!onContextMenu || isRawRecords || !event.column || !event.data) {
+ return;
+ }
+ const nativeEvent = event.event as MouseEvent | null | undefined;
+ if (!nativeEvent) return;
+ nativeEvent.preventDefault();
+ nativeEvent.stopPropagation();
+
+ const rowData = event.data as Record<string, DataRecordValue>;
+ const key = event.column.getColId();
+ const cellValue = event.value as DataRecordValue;
+ const colDef = event.column.getColDef();
+ const isMetric = Boolean(
+ colDef.context?.isMetric || colDef.context?.isPercentMetric,
+ );
+
+ const drillToDetailFilters: BinaryQueryObjectFilterClause[] = [];
+ drillColumns.forEach(col => {
+ if (col.isMetric || col.isPercentMetric) return;
+ const dataRecordValue = rowData[col.key];
+
+ if (
+ dataRecordValue == null ||
+ (dataRecordValue instanceof DateWithFormatter &&
+ dataRecordValue.input == null)
+ ) {
+ drillToDetailFilters.push({
+ col: col.key,
+ op: 'IS NULL' as any,
+ val: null,
+ });
+ } else if (col.dataType === GenericDataType.Temporal && timeGrain) {
+ const startTime =
+ dataRecordValue instanceof Date
+ ? dataRecordValue
+ : new Date(dataRecordValue as string | number);
+
+ if (Number.isNaN(startTime.getTime())) {
+ // Malformed temporal value: fall back to an equality filter
+ // instead of building a TEMPORAL_RANGE, since toISOString()
+ // throws on an Invalid Date and would crash the context menu.
+ const sanitizedValue = extractTextFromHTML(dataRecordValue);
+ drillToDetailFilters.push({
+ col: col.key,
+ op: '==',
+ val: sanitizedValue as string | number | boolean,
+ formattedVal: formatColumnValue(col, sanitizedValue)[1],
+ });
+ } else {
+ const [rangeStartTime, rangeEndTime] = getTimeRangeFromGranularity(
+ startTime,
+ timeGrain,
+ );
+ const timeRangeValue = `${rangeStartTime.toISOString()} :
${rangeEndTime.toISOString()}`;
+
+ drillToDetailFilters.push({
+ col: col.key,
+ op: 'TEMPORAL_RANGE',
+ val: timeRangeValue,
+ grain: timeGrain,
+ formattedVal: formatColumnValue(col, dataRecordValue)[1],
+ });
+ }
+ } else {
+ const sanitizedValue = extractTextFromHTML(dataRecordValue);
+ drillToDetailFilters.push({
+ col: col.key,
+ op: '==',
+ val: sanitizedValue as string | number | boolean,
+ formattedVal: formatColumnValue(col, sanitizedValue)[1],
+ });
+ }
+ });
+
+ const isCellValueNull =
+ cellValue == null ||
+ (cellValue instanceof DateWithFormatter && cellValue.input == null);
+
+ onContextMenu(nativeEvent.clientX, nativeEvent.clientY, {
+ drillToDetail: drillToDetailFilters,
+ crossFilter: isMetric
+ ? undefined
+ : getCrossFilterDataMask({
+ key,
+ value: cellValue,
+ filters,
+ timeGrain,
+ isActiveFilterValue,
+ timestampFormatter,
+ }),
+ drillBy: isMetric
+ ? undefined
+ : {
+ filters: [
+ isCellValueNull
+ ? { col: key, op: 'IS NULL' as any, val: null }
+ : {
+ col: key,
+ op: '==' as any,
+ val: extractTextFromHTML(cellValue),
+ },
+ ],
+ groupbyFieldName: 'groupby',
+ },
+ });
+ },
+ [
+ onContextMenu,
+ isRawRecords,
+ drillColumns,
+ timeGrain,
+ filters,
+ isActiveFilterValue,
+ timestampFormatter,
+ ],
+ );
+
const handleServerPaginationChange = useCallback(
(pageNumber: number, pageSize: number) => {
- const modifiedOwnState = {
- ...serverPaginationData,
+ writeOwnState({
currentPage: pageNumber,
pageSize,
lastFilteredColumn: undefined,
lastFilteredInputPosition: undefined,
- };
- updateTableOwnState(setDataMask, modifiedOwnState);
+ });
},
- [setDataMask],
+ [writeOwnState],
);
const handlePageSizeChange = useCallback(
(pageSize: number) => {
- const modifiedOwnState = {
- ...serverPaginationData,
+ writeOwnState({
currentPage: 0,
pageSize,
lastFilteredColumn: undefined,
lastFilteredInputPosition: undefined,
- };
- updateTableOwnState(setDataMask, modifiedOwnState);
+ });
},
- [setDataMask],
+ [writeOwnState],
);
const handleChangeSearchCol = (searchCol: string) => {
- if (!isEqual(searchCol, serverPaginationData?.searchColumn)) {
- const modifiedOwnState = {
- ...serverPaginationData,
+ if (!isEqual(searchCol, ownStateRef.current?.searchColumn)) {
+ writeOwnState({
searchColumn: searchCol,
searchText: '',
lastFilteredColumn: undefined,
lastFilteredInputPosition: undefined,
- };
- updateTableOwnState(setDataMask, modifiedOwnState);
+ });
}
};
const handleSearch = useCallback(
(searchText: string) => {
- const modifiedOwnState = {
- ...serverPaginationData,
+ writeOwnState({
searchColumn:
- serverPaginationData?.searchColumn || searchOptions[0]?.value,
+ (ownStateRef.current?.searchColumn as string | undefined) ||
+ searchOptions[0]?.value,
searchText,
currentPage: 0, // Reset to first page when searching
lastFilteredColumn: undefined,
lastFilteredInputPosition: undefined,
- };
- updateTableOwnState(setDataMask, modifiedOwnState);
+ });
},
- [setDataMask, searchOptions],
+ [writeOwnState, searchOptions],
);
const handleSortByChange = useCallback(
(sortBy: SortByItem[]) => {
if (!serverPagination) return;
- const modifiedOwnState = {
- ...serverPaginationData,
+ writeOwnState({
sortBy,
lastFilteredColumn: undefined,
lastFilteredInputPosition: undefined,
- };
- updateTableOwnState(setDataMask, modifiedOwnState);
+ });
+ },
+ [writeOwnState, serverPagination],
+ );
+
+ // Feeds the "Export Current View" menu item (EXPORT_CURRENT_VIEW behavior),
+ // mirroring Table V1's clientView snapshot on ownState. Written through
+ // writeOwnState (rather than spreading serverPaginationData directly)
+ // because onModelUpdated can fire with a stale closure relative to other
+ // ownState writers (e.g. a just-applied filter), and updateTableOwnState
+ // replaces ownState wholesale.
+ const handleClientViewChange = useCallback(
+ (clientView: ClientViewSnapshot) => {
+ writeOwnState({ clientView });
Review Comment:
in client mode this `clientView` write ends up breaking column filters in
Explore. `handleModelUpdated` is the client-only path and it fires on every
filter/operator change, so this writes `clientView` into ownState and flips
`dataMask.ownState` from `undefined` to `{clientView}`. `ExploreViewContainer`
tries to ignore clientView in its requery guard (`omit(s, ['clientView'])`),
but on that first write it compares `undefined` vs `{}`, which `isEqual` treats
as different, so it fires a requery. that requery remounts the grid, and on the
re-render `transformProps`'s `savedFilterAppliedSet` strips the just-applied
filter (`filterModel: {}`), so the filter is lost and the whole table reloads.
probable fix is to not route clientView through the requery-triggering
ownState path in client mode since it's only needed for Export Current View, or
normalize the `ExploreViewContainer` strip so `undefined` and `{}` compare
equal.
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx:
##########
@@ -493,6 +662,7 @@ export default function TableChart<D extends DataRecord =
DataRecord>(
width={width}
onColumnStateChange={handleColumnStateChange}
Review Comment:
on dashboards there's a second path to the same symptom. this handler
persists the grid filter/sort into chartState, and
`createOwnStateWithChartState` (both the dashboard and explore consumers) folds
chartState into ownState unconditionally. so in client mode a filter operator
change produces a new ownState, which triggers a requery, remount, and the same
`savedFilterAppliedSet` filter strip. the fold mechanism is pre-existing (came
in with #35343) and is already reachable with the opt-in V2 table, but since
this PR makes V2 the default it now hits every dashboard using the table.
probable fix is to gate the ownState fold on `server_pagination`, since in
client mode AG Grid handles sort/filter locally and none of it needs to reach
the backend query, while server pagination still needs that state.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
