michael-s-molina commented on code in PR #42088:
URL: https://github.com/apache/superset/pull/42088#discussion_r3624005449
##########
superset/migrations/shared/migrate_viz/base.py:
##########
@@ -40,6 +40,8 @@ class Slice(Base): # type: ignore
viz_type = Column(String(250))
params = Column(Text)
query_context = Column(Text)
+ datasource_id = Column(Integer)
+ datasource_type = Column(String(200))
Review Comment:
Fixed in 4292578 — added explicit type annotations (`datasource_id: int`,
`datasource_type: str`) to the two new attributes.
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx:
##########
@@ -368,6 +378,114 @@ 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,
+ });
Review Comment:
Looked into this — the `any` cast is needed because `'IS NULL'` is a
`UnaryOperator`, but `ContextMenuFilters.drillToDetail` (a shared core type
used across the whole chart context-menu system) is typed strictly as
`BinaryQueryObjectFilterClause[]`. The identical cast already exists in the V1
table plugin (`plugin-chart-table/TableChart.tsx:674`), so this mirrors
established precedent rather than introducing something new. A real fix means
widening that core type app-wide, which felt out of scope for this PR — leaving
as-is.
--
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]