LevisNgigi commented on code in PR #35343:
URL: https://github.com/apache/superset/pull/35343#discussion_r2411579882
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx:
##########
@@ -257,6 +318,24 @@ const AgGridDataTable: FunctionComponent<AgGridTableProps>
= memo(
const onGridReady = (params: GridReadyEvent) => {
// This will make columns fill the grid width
params.api.sizeColumnsToFit();
+
+ // Restore saved AG Grid state from permalink if available
+ if (savedAgGridState && params.api) {
+ try {
+ if (savedAgGridState.columnState) {
+ params.api.applyColumnState?.({
+ state: savedAgGridState.columnState as any,
Review Comment:
Curious if there is a better type definition that we can use for this
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -174,10 +174,11 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug
}: PageProps) => {
// activeTabs is initialized with undefined so that it doesn't override
// the currently stored value when hydrating
let activeTabs: string[] | undefined;
+ let chartStates: Record<string, any> | undefined;
Review Comment:
same here for the type definition?
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts:
##########
@@ -292,6 +356,23 @@ const buildQuery: BuildQuery<TableChartFormData> = (
}
}
+ if (
+ isDownloadQuery &&
+ Array.isArray(ownState.agGridFilters) &&
+ ownState.agGridFilters.length > 0
+ ) {
+ const agGridQueryFilters = ownState.agGridFilters.map((filter: any) => ({
Review Comment:
same here might there be a better type definition?
##########
superset-frontend/src/dashboard/types.ts:
##########
@@ -123,6 +123,7 @@ export type DashboardState = {
dashboardId: number;
data: JsonObject;
};
+ chartStates?: Record<string, any>;
Review Comment:
same here if there is a better type definition
##########
superset-frontend/src/dashboard/types/chartState.ts:
##########
@@ -0,0 +1,65 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+export interface AgGridColumnState {
+ colId: string;
+ width?: number;
+ hide?: boolean;
+ pinned?: 'left' | 'right' | null;
+ sort?: 'asc' | 'desc' | null;
+ sortIndex?: number;
+ aggFunc?: string;
+}
+
+export interface AgGridSortModel {
+ colId: string;
+ sort: 'asc' | 'desc';
+ sortIndex?: number;
+}
+
+export interface AgGridFilterModel {
+ [colId: string]: {
+ filterType: string;
+ type?: string;
+ filter?: any;
+ condition1?: any;
+ condition2?: any;
Review Comment:
I think it would be great to clean the types?
##########
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.jsx:
##########
@@ -378,20 +392,139 @@ const Chart = props => {
slice_id: slice.slice_id,
is_cached: isCached,
});
+
+ let ownState = dataMask[props.id]?.ownState || {};
+
+ // For AG Grid tables, convert AG Grid state to backend-compatible format
+ if (slice.viz_type === 'ag-grid-table' && chartStates[props.id]?.state) {
+ const agGridState = chartStates[props.id].state;
+
+ // Convert AG Grid sortModel to backend sortBy format
+ if (agGridState.sortModel && agGridState.sortModel.length > 0) {
+ const sortItem = agGridState.sortModel[0];
+ ownState = {
+ ...ownState,
+ sortBy: [
+ {
+ id: sortItem.colId,
+ key: sortItem.colId,
+ desc: sortItem.sort === 'desc',
+ },
+ ],
+ };
+ }
+
+ // Store column order for backend processing
+ if (agGridState.columnState && agGridState.columnState.length > 0) {
+ ownState = {
+ ...ownState,
+ columnOrder: agGridState.columnState.map(col => col.colId),
+ };
+ }
+
+ if (
+ agGridState.filterModel &&
+ Object.keys(agGridState.filterModel).length > 0
+ ) {
+ const agGridFilters = [];
+
+ Object.keys(agGridState.filterModel).forEach(colId => {
+ const filter = agGridState.filterModel[colId];
+
+ // Text filter
+ if (filter.filterType === 'text' && filter.filter) {
+ const clause = {
+ expressionType: 'SIMPLE',
+ subject: colId,
+ operator:
+ filter.type === 'equals'
+ ? '=='
+ : filter.type === 'notEqual'
+ ? '!='
+ : filter.type === 'contains'
+ ? 'ILIKE'
+ : filter.type === 'notContains'
+ ? 'NOT ILIKE'
+ : filter.type === 'startsWith'
+ ? 'ILIKE'
+ : filter.type === 'endsWith'
+ ? 'ILIKE'
+ : 'ILIKE',
+ comparator:
+ filter.type === 'contains' || filter.type === 'notContains'
+ ? `%${filter.filter}%`
+ : filter.type === 'startsWith'
+ ? `${filter.filter}%`
+ : filter.type === 'endsWith'
+ ? `%${filter.filter}`
+ : filter.filter,
+ };
+ agGridFilters.push(clause);
+ } else if (
+ filter.filterType === 'number' &&
+ filter.filter !== undefined
+ ) {
+ const clause = {
+ expressionType: 'SIMPLE',
+ subject: colId,
+ operator:
+ filter.type === 'equals'
+ ? '=='
+ : filter.type === 'notEqual'
+ ? '!='
+ : filter.type === 'lessThan'
+ ? '<'
+ : filter.type === 'lessThanOrEqual'
+ ? '<='
+ : filter.type === 'greaterThan'
+ ? '>'
+ : filter.type === 'greaterThanOrEqual'
+ ? '>='
+ : '==',
+ comparator: filter.filter,
+ };
+ agGridFilters.push(clause);
+ } else if (
+ filter.filterType === 'set' &&
+ Array.isArray(filter.values) &&
+ filter.values.length > 0
+ ) {
+ const clause = {
+ expressionType: 'SIMPLE',
+ subject: colId,
+ operator: 'IN',
+ comparator: filter.values,
+ };
+ agGridFilters.push(clause);
+ }
+ });
+
+ if (agGridFilters.length > 0) {
+ ownState = {
+ ...ownState,
+ agGridFilters,
+ };
+ }
+ }
+ }
+
exportChart({
formData: isFullCSV ? { ...formData, row_limit: maxRows } : formData,
resultType: isPivot ? 'post_processed' : 'full',
resultFormat: format,
force: true,
- ownState: dataMask[props.id]?.ownState,
Review Comment:
is the dataMask[props.id]?.ownState, no longer needed?
##########
superset-frontend/src/visualizations/presets/MainPreset.js:
##########
@@ -99,6 +99,11 @@ export default class MainPreset extends Preset {
? [new AgGridTableChartPlugin().configure({ key: VizType.TableAgGrid })]
: [];
+ console.log(
+ 'AG_GRID_TABLE_ENABLED',
+ isFeatureEnabled(FeatureFlag.AgGridTableEnabled),
+ );
Review Comment:
is this intentional?
--
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]
