This is an automated email from the ASF dual-hosted git repository. richardfogaca pushed a commit to branch feat/auto-refresh-dashboard in repository https://gitbox.apache.org/repos/asf/superset.git
commit 6c00e08fd20b29ed7c09be01d0344fbab917e0f1 Author: richard <[email protected]> AuthorDate: Tue Feb 10 20:17:38 2026 -0300 fix: address PR review - alphabetize icons and extract shared orderby helper --- .../src/components/Icons/AntdEnhanced.tsx | 16 ++++---- .../plugin-chart-echarts/src/Graph/buildQuery.ts | 20 +++++---- .../plugin-chart-echarts/src/Tree/buildQuery.ts | 19 ++------- .../plugin-chart-echarts/src/Treemap/buildQuery.ts | 17 ++------ .../plugin-chart-echarts/src/utils/orderby.ts | 47 ++++++++++++++++++++++ 5 files changed, 71 insertions(+), 48 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/components/Icons/AntdEnhanced.tsx b/superset-frontend/packages/superset-ui-core/src/components/Icons/AntdEnhanced.tsx index 58e4d8c02cd..b1914d3e267 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Icons/AntdEnhanced.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/Icons/AntdEnhanced.tsx @@ -40,9 +40,6 @@ import { CaretLeftOutlined, CaretRightOutlined, CaretRightFilled, - PauseOutlined, - PauseCircleOutlined, - PlayCircleOutlined, CalendarOutlined, CheckOutlined, CheckCircleOutlined, @@ -109,8 +106,11 @@ import { MoreOutlined, OrderedListOutlined, PartitionOutlined, - PieChartOutlined, + PauseCircleOutlined, + PauseOutlined, PicCenterOutlined, + PieChartOutlined, + PlayCircleOutlined, PlusCircleOutlined, PlusSquareOutlined, PlusOutlined, @@ -191,9 +191,6 @@ const AntdIcons = { CaretLeftOutlined, CaretRightOutlined, CaretRightFilled, - PauseOutlined, - PauseCircleOutlined, - PlayCircleOutlined, CalendarOutlined, CheckOutlined, CheckCircleOutlined, @@ -264,8 +261,11 @@ const AntdIcons = { MoreOutlined, OrderedListOutlined, PartitionOutlined, - PieChartOutlined, + PauseCircleOutlined, + PauseOutlined, PicCenterOutlined, + PieChartOutlined, + PlayCircleOutlined, PlusCircleOutlined, PlusSquareOutlined, PlusOutlined, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/buildQuery.ts index f2ee0c8be3f..78e431ac707 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/buildQuery.ts @@ -16,21 +16,19 @@ * specific language governing permissions and limitations * under the License. */ -import { buildQueryContext, QueryFormOrderBy } from '@superset-ui/core'; +import { buildQueryContext } from '@superset-ui/core'; import { EchartsGraphFormData } from './types'; +import { buildColumnsOrderBy, applyOrderBy } from '../utils/orderby'; export default function buildQuery(formData: EchartsGraphFormData) { const { source, target, source_category, target_category, row_limit } = formData; - const orderby: QueryFormOrderBy[] = []; - const shouldApplyOrderBy = - row_limit !== undefined && row_limit !== null && row_limit !== 0; - - [source, target, source_category, target_category].forEach(column => { - if (column) { - orderby.push([column, true]); - } - }); + const orderby = buildColumnsOrderBy([ + source, + target, + source_category, + target_category, + ]); return buildQueryContext(formData, { queryFields: { @@ -42,7 +40,7 @@ export default function buildQuery(formData: EchartsGraphFormData) { buildQuery: baseQueryObject => [ { ...baseQueryObject, - ...(shouldApplyOrderBy && orderby.length > 0 && { orderby }), + ...applyOrderBy(orderby, row_limit), }, ], }); diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/buildQuery.ts index d7a36e04508..09d99b28dd9 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/buildQuery.ts @@ -16,24 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import { buildQueryContext, QueryFormOrderBy } from '@superset-ui/core'; +import { buildQueryContext } from '@superset-ui/core'; import { EchartsTreeFormData } from './types'; +import { buildColumnsOrderBy, applyOrderBy } from '../utils/orderby'; export default function buildQuery(formData: EchartsTreeFormData) { const { id, parent, name, row_limit } = formData; - const orderby: QueryFormOrderBy[] = []; - const shouldApplyOrderBy = - row_limit !== undefined && row_limit !== null && row_limit !== 0; - const orderbyColumns: string[] = []; - - [parent, id, name].forEach(column => { - if (column) { - orderbyColumns.push(column); - } - }); - orderbyColumns.forEach(column => { - orderby.push([column, true]); - }); + const orderby = buildColumnsOrderBy([parent, id, name]); return buildQueryContext(formData, { queryFields: { @@ -44,7 +33,7 @@ export default function buildQuery(formData: EchartsTreeFormData) { buildQuery: baseQueryObject => [ { ...baseQueryObject, - ...(shouldApplyOrderBy && orderby.length > 0 && { orderby }), + ...applyOrderBy(orderby, row_limit), }, ], }); diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/buildQuery.ts index 84172b5b7e8..e6e76ac7ad3 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/buildQuery.ts @@ -21,31 +21,20 @@ import { QueryFormData, QueryFormOrderBy, } from '@superset-ui/core'; +import { buildColumnsOrderBy, applyOrderBy } from '../utils/orderby'; export default function buildQuery(formData: QueryFormData) { const { metric, sort_by_metric, groupby = [], row_limit } = formData; const orderby: QueryFormOrderBy[] = []; - const shouldApplyOrderBy = - row_limit !== undefined && row_limit !== null && row_limit !== 0; - if (sort_by_metric && metric) { orderby.push([metric, false]); } - if (!sort_by_metric && groupby.length > 0) { - groupby.forEach(column => { - orderby.push([column, true]); - }); - } - if (sort_by_metric && groupby.length > 0) { - groupby.forEach(column => { - orderby.push([column, true]); - }); - } + orderby.push(...buildColumnsOrderBy(groupby)); return buildQueryContext(formData, baseQueryObject => [ { ...baseQueryObject, - ...(shouldApplyOrderBy && orderby.length > 0 && { orderby }), + ...applyOrderBy(orderby, row_limit), }, ]); } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/orderby.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/orderby.ts new file mode 100644 index 00000000000..0b552adbe7d --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/orderby.ts @@ -0,0 +1,47 @@ +/** + * 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. + */ +import { QueryFormColumn, QueryFormOrderBy } from '@superset-ui/core'; + +/** + * Builds ascending orderby clauses from a list of columns, filtering out + * any non-string or nullish values. This ensures deterministic row ordering + * so that chart elements maintain stable positions across auto-refreshes. + */ +export function buildColumnsOrderBy( + columns: (QueryFormColumn | string | undefined | null)[], + ascending: boolean = true, +): QueryFormOrderBy[] { + return columns + .filter((col): col is string => typeof col === 'string' && col !== '') + .map(col => [col, ascending]); +} + +/** + * Conditionally applies orderby to a query object spread. Returns the + * orderby field only when row_limit is set (non-zero, non-null) and + * there are orderby entries to apply. + */ +export function applyOrderBy( + orderby: QueryFormOrderBy[], + rowLimit: string | number | undefined | null, +): { orderby: QueryFormOrderBy[] } | Record<string, never> { + const shouldApply = + rowLimit !== undefined && rowLimit !== null && rowLimit !== 0; + return shouldApply && orderby.length > 0 ? { orderby } : {}; +}
