You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by rb...@apache.org on 2026/09/03 10:32:08 UTC
(superset) branch master updated: fix(charts): sort chart types by display name (#43634)
This is an automated email from the ASF dual-hosted git repository.
rebenitez1802 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new bf12a21367c fix(charts): sort chart types by display name (#43634)
bf12a21367c is described below
commit bf12a21367ca10f6f29e553b3245f7d888d543f5
Author: Mafi <ma...@gmail.com>
AuthorDate: Thu Sep 3 20:32:01 2026 +1000
fix(charts): sort chart types by display name (#43634)
Co-authored-by: Matt Fitzgerald <ma...@preset.io>
Co-authored-by: Amin Ghadersohi <am...@gmail.com>
---
superset-frontend/src/components/ListView/types.ts | 2 +
.../pages/ChartList/ChartList.listview.test.tsx | 76 ++++++++++-
superset-frontend/src/pages/ChartList/index.tsx | 28 +++-
superset-frontend/src/views/CRUD/hooks.test.tsx | 102 ++++++++++++++
superset-frontend/src/views/CRUD/hooks.ts | 17 +++
superset/charts/api.py | 146 ++++++++++++++++++++-
superset/charts/schemas.py | 21 +++
tests/integration_tests/charts/api_tests.py | 114 ++++++++++++++++
tests/unit_tests/charts/test_schemas.py | 42 ++++++
9 files changed, 544 insertions(+), 4 deletions(-)
diff --git a/superset-frontend/src/components/ListView/types.ts b/superset-frontend/src/components/ListView/types.ts
index 6a07082ed1e..571b02306f3 100644
--- a/superset-frontend/src/components/ListView/types.ts
+++ b/superset-frontend/src/components/ListView/types.ts
@@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
+import { type JsonObject } from '@superset-ui/core';
import { type ReactNode } from 'react';
export interface SortColumn {
@@ -102,6 +103,7 @@ export interface ListViewFetchDataConfig {
pageSize: number;
sortBy: SortColumn[];
filters: ListViewFilterValue[];
+ extraQueryParams?: JsonObject;
}
export interface InternalFilter extends ListViewFilterValue {
diff --git a/superset-frontend/src/pages/ChartList/ChartList.listview.test.tsx b/superset-frontend/src/pages/ChartList/ChartList.listview.test.tsx
index 69ff7e21f7b..d04d9a56ba8 100644
--- a/superset-frontend/src/pages/ChartList/ChartList.listview.test.tsx
+++ b/superset-frontend/src/pages/ChartList/ChartList.listview.test.tsx
@@ -20,7 +20,12 @@ import fetchMock from 'fetch-mock';
import { mockUserSubjectsBootstrapData } from 'spec/helpers/mockBootstrapData';
import { screen, waitFor, within } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
-import { isFeatureEnabled } from '@superset-ui/core';
+import rison from 'rison';
+import {
+ ChartMetadata,
+ getChartMetadataRegistry,
+ isFeatureEnabled,
+} from '@superset-ui/core';
import {
mockCharts,
mockHandleResourceExport,
@@ -279,6 +284,75 @@ test('sorts table when clicking column headers', async () => {
});
});
+test('sends display-ordered chart type slugs only for Type sorting', async () => {
+ const registry = getChartMetadataRegistry();
+ const customVizTypes = Array.from(
+ { length: 260 },
+ (_, index) => `custom_display_order_${index}`,
+ );
+ registry
+ .registerValue(
+ 'slug_a',
+ new ChartMetadata({ name: '001 Zulu', thumbnail: '', behaviors: [] }),
+ )
+ .registerValue(
+ 'slug_z',
+ new ChartMetadata({ name: '000 Alpha', thumbnail: '', behaviors: [] }),
+ );
+ customVizTypes.forEach((vizType, index) =>
+ registry.registerValue(
+ vizType,
+ new ChartMetadata({
+ name: `Plugin ${index}`,
+ thumbnail: '',
+ behaviors: [],
+ }),
+ ),
+ );
+
+ try {
+ renderChartList(mockUser);
+
+ const table = await screen.findByTestId('listview-table');
+ const initialCall = fetchMock.callHistory
+ .calls(/chart\/\?q/)
+ .find(call => !call.url.includes('order_column:viz_type'));
+ expect(initialCall).toBeDefined();
+ const initialQuery = new URL(
+ initialCall!.url,
+ 'http://localhost',
+ ).searchParams.get('q');
+ expect(rison.decode(initialQuery!)).not.toHaveProperty('viz_type_order');
+
+ await userEvent.click(within(table).getByTitle('Type'));
+
+ await waitFor(() => {
+ const typeSortCall = fetchMock.callHistory
+ .calls(/chart\/\?q/)
+ .find(call => call.url.includes('order_column:viz_type'));
+ expect(typeSortCall).toBeDefined();
+
+ const query = new URL(
+ typeSortCall!.url,
+ 'http://localhost',
+ ).searchParams.get('q');
+ const decoded = rison.decode(query!) as {
+ order_column: string;
+ viz_type_order: string[];
+ };
+ expect(decoded.order_column).toBe('viz_type');
+ expect(decoded.viz_type_order).toHaveLength(256);
+ expect(decoded.viz_type_order.indexOf('slug_z')).toBeLessThan(
+ decoded.viz_type_order.indexOf('slug_a'),
+ );
+ });
+ } finally {
+ registry.remove('slug_a');
+ registry.remove('slug_z');
+ customVizTypes.forEach(vizType => registry.remove(vizType));
+ }
+});
+
test('displays chart data correctly in table rows', async () => {
/**
* @todo Implement test logic for tagging.
diff --git a/superset-frontend/src/pages/ChartList/index.tsx b/superset-frontend/src/pages/ChartList/index.tsx
index 16c4d40c62c..7d2f90f4bff 100644
--- a/superset-frontend/src/pages/ChartList/index.tsx
+++ b/superset-frontend/src/pages/ChartList/index.tsx
@@ -71,6 +71,7 @@ import {
ListViewFilterOperator as FilterOperator,
DashboardCrossLinks,
type ListViewProps,
+ type ListViewFetchDataConfig,
type ListViewFilters,
type ListViewFilter,
} from 'src/components';
@@ -198,6 +199,7 @@ const CONFIRM_OVERWRITE_MESSAGE = t(
);
const registry = getChartMetadataRegistry();
+const MAX_VIZ_TYPE_ORDER_LENGTH = 256;
const createFetchDatasets = async (
filterValue = '',
@@ -260,11 +262,35 @@ function ChartList(props: ChartListProps) {
},
setResourceCollection: setCharts,
hasPerm,
- fetchData,
+ fetchData: fetchChartData,
toggleBulkSelect,
refreshData,
} = useListViewResource<Chart>('chart', t('chart'), addDangerToast);
+ const fetchData = useCallback(
+ (config: ListViewFetchDataConfig) =>
+ fetchChartData({
+ ...config,
+ ...(config.sortBy[0]?.id === 'viz_type'
+ ? {
+ extraQueryParams: {
+ ...config.extraQueryParams,
+ viz_type_order: registry
+ .keys()
+ .sort((left, right) => {
+ const nameComparison = (
+ registry.get(left)?.name || left
+ ).localeCompare(registry.get(right)?.name || right);
+ return nameComparison || left.localeCompare(right);
+ })
+ .slice(0, MAX_VIZ_TYPE_ORDER_LENGTH),
+ },
+ }
+ : {}),
+ }),
+ [fetchChartData],
+ );
+
const chartIds = useMemo(() => charts.map(c => c.id), [charts]);
const { roles } = useSelector<any, UserWithPermissionsAndRoles>(
state => state.user,
diff --git a/superset-frontend/src/views/CRUD/hooks.test.tsx b/superset-frontend/src/views/CRUD/hooks.test.tsx
index 179970fefb4..dbe44e83b81 100644
--- a/superset-frontend/src/views/CRUD/hooks.test.tsx
+++ b/superset-frontend/src/views/CRUD/hooks.test.tsx
@@ -17,6 +17,7 @@
* under the License.
*/
import { renderHook, act } from '@testing-library/react';
+import rison from 'rison';
import { waitFor } from 'spec/helpers/testing-library';
import { JsonResponse, SupersetClient } from '@superset-ui/core';
@@ -551,6 +552,107 @@ test('useListViewResource: uses desc sort direction when desc is true', async ()
expect(endpoint).toContain('order_direction:desc');
});
+test('useListViewResource: includes extra list query parameters', async () => {
+ const getSpy = jest.spyOn(SupersetClient, 'get').mockResolvedValue({
+ json: { result: [], count: 0 },
+ } as unknown as JsonResponse);
+
+ const { result } = renderHook(() =>
+ useListViewResource('chart', 'Charts', jest.fn()),
+ );
+
+ await act(async () => {
+ await result.current.fetchData({
+ pageIndex: 0,
+ pageSize: 25,
+ sortBy: [{ id: 'viz_type' }],
+ filters: [],
+ extraQueryParams: {
+ viz_type_order: ['slug_z', 'slug_a'],
+ },
+ });
+ });
+
+ const endpoint = findEndpoint(getSpy, '/api/v1/chart/?q=');
+ const query = new URL(endpoint, 'http://localhost').searchParams.get('q');
+ expect(rison.decode(query!)).toMatchObject({
+ order_column: 'viz_type',
+ viz_type_order: ['slug_z', 'slug_a'],
+ });
+});
+
+test('useListViewResource: refresh reuses extra list query parameters', async () => {
+ const getSpy = jest.spyOn(SupersetClient, 'get').mockResolvedValue({
+ json: { result: [], count: 0 },
+ } as unknown as JsonResponse);
+
+ const { result } = renderHook(() =>
+ useListViewResource('chart', 'Charts', jest.fn()),
+ );
+
+ await act(async () => {
+ await result.current.fetchData({
+ pageIndex: 0,
+ pageSize: 25,
+ sortBy: [{ id: 'viz_type' }],
+ filters: [],
+ extraQueryParams: { viz_type_order: ['slug_z', 'slug_a'] },
+ });
+ await result.current.refreshData();
+ });
+
+ const listQueries = getSpy.mock.calls
+ .map(call => (call[0] as { endpoint: string }).endpoint)
+ .filter(endpoint => endpoint.includes('/api/v1/chart/?q='))
+ .map(endpoint => {
+ const query = new URL(endpoint, 'http://localhost').searchParams.get('q');
+ return rison.decode(query!);
+ });
+ expect(listQueries).toHaveLength(2);
+ expect(listQueries).toEqual([
+ expect.objectContaining({ viz_type_order: ['slug_z', 'slug_a'] }),
+ expect.objectContaining({ viz_type_order: ['slug_z', 'slug_a'] }),
+ ]);
+});
+
+test('useListViewResource: extra parameters cannot replace list controls', async () => {
+ const getSpy = jest.spyOn(SupersetClient, 'get').mockResolvedValue({
+ json: { result: [], count: 0 },
+ } as unknown as JsonResponse);
+
+ const { result } = renderHook(() =>
+ useListViewResource('chart', 'Charts', jest.fn()),
+ );
+
+ await act(async () => {
+ await result.current.fetchData({
+ pageIndex: 0,
+ pageSize: 25,
+ sortBy: [{ id: 'viz_type' }],
+ filters: [],
+ extraQueryParams: {
+ custom_param: 'preserved',
+ filters: [{ col: 'slice_name', opr: 'eq', value: 'injected' }],
+ order_column: 'slice_name',
+ order_direction: 'desc',
+ page: 99,
+ page_size: 1,
+ select_columns: ['slice_name'],
+ },
+ });
+ });
+
+ const endpoint = findEndpoint(getSpy, '/api/v1/chart/?q=');
+ const query = new URL(endpoint, 'http://localhost').searchParams.get('q');
+ expect(rison.decode(query!)).toEqual({
+ custom_param: 'preserved',
+ order_column: 'viz_type',
+ order_direction: 'asc',
+ page: 0,
+ page_size: 25,
+ });
+});
+
// useSingleViewResource
test('useSingleViewResource: initial state has loading false and null resource', () => {
const { result } = renderHook(() =>
diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts
index ea320372e58..631dcc72a12 100644
--- a/superset-frontend/src/views/CRUD/hooks.ts
+++ b/superset-frontend/src/views/CRUD/hooks.ts
@@ -61,6 +61,15 @@ interface ListViewResourceState<D extends object = any> {
lastFetched?: string;
}
+const reservedListQueryParams = new Set([
+ 'filters',
+ 'order_column',
+ 'order_direction',
+ 'page',
+ 'page_size',
+ 'select_columns',
+]);
+
const parsedErrorMessage = (
errorMessage: Record<string, string[] | string> | string,
) => {
@@ -156,6 +165,7 @@ export function useListViewResource<D extends object = any>(
pageSize,
sortBy,
filters: filterValues,
+ extraQueryParams,
}: FetchDataConfig) => {
const requestId = latestRequestIdRef.current + 1;
latestRequestIdRef.current = requestId;
@@ -165,6 +175,7 @@ export function useListViewResource<D extends object = any>(
pageIndex,
pageSize,
sortBy,
+ extraQueryParams,
};
lastFetchDataConfigRef.current = config;
// set loading state, cache the last config for refreshing data.
@@ -186,7 +197,13 @@ export function useListViewResource<D extends object = any>(
: value,
}));
+ const safeExtraQueryParams = Object.fromEntries(
+ Object.entries(extraQueryParams ?? {}).filter(
+ ([key]) => !reservedListQueryParams.has(key),
+ ),
+ );
const queryParams = rison.encode_uri({
+ ...safeExtraQueryParams,
order_column: sortBy[0].id,
order_direction: sortBy[0].desc ? 'desc' : 'asc',
page: pageIndex,
diff --git a/superset/charts/api.py b/superset/charts/api.py
index 605bd2cf7da..0e17924d25c 100644
--- a/superset/charts/api.py
+++ b/superset/charts/api.py
@@ -16,17 +16,35 @@
# under the License.
# pylint: disable=too-many-lines
import logging
+from contextvars import ContextVar
from datetime import datetime
from io import BytesIO
from typing import Any, cast, Optional
from zipfile import is_zipfile, ZipFile
from flask import current_app, redirect, request, Response, url_for
-from flask_appbuilder.api import expose, protect, rison as parse_rison, safe
+from flask_appbuilder import permission_name
+from flask_appbuilder.api import (
+ expose,
+ merge_response_func,
+ protect,
+ rison as parse_rison,
+ safe,
+)
+from flask_appbuilder.const import (
+ API_DESCRIPTION_COLUMNS_RIS_KEY,
+ API_LABEL_COLUMNS_RIS_KEY,
+ API_LIST_COLUMNS_RIS_KEY,
+ API_LIST_TITLE_RIS_KEY,
+ API_ORDER_COLUMNS_RIS_KEY,
+)
from flask_appbuilder.hooks import before_request
from flask_appbuilder.models.sqla.interface import SQLAInterface
from flask_babel import ngettext
from marshmallow import ValidationError
+from sqlalchemy import asc, case, desc
+from sqlalchemy.orm import Query
+from sqlalchemy.orm.util import AliasedClass
from werkzeug.wrappers import Response as WerkzeugResponse
from werkzeug.wsgi import FileWrapper
@@ -46,6 +64,7 @@ from superset.charts.filters import (
ChartTagNameFilter,
)
from superset.charts.schemas import (
+ chart_get_list_schema,
CHART_SCHEMAS,
ChartCacheWarmUpRequestSchema,
ChartGetResponseSchema,
@@ -142,9 +161,49 @@ _CHART_PURGE_BINDING = SoftDeleteBinding(
delete_failed=ChartDeleteFailedError,
)
+_viz_type_order: ContextVar[dict[str, int] | None] = ContextVar(
+ "chart_viz_type_order", default=None
+)
+
+
+class ChartSQLAInterface(SQLAInterface):
+ """Chart model interface with request-scoped display viz type ordering."""
+
+ def apply_order_by(
+ self,
+ query: Query,
+ order_column: str,
+ order_direction: str,
+ aliases_mapping: dict[str, AliasedClass] | None = None,
+ bypass_many_to_many: bool = False,
+ add_pk: bool = False,
+ ) -> Query:
+ viz_type_order = _viz_type_order.get()
+ if order_column != "viz_type" or not viz_type_order:
+ return super().apply_order_by(
+ query,
+ order_column,
+ order_direction,
+ aliases_mapping=aliases_mapping,
+ bypass_many_to_many=bypass_many_to_many,
+ add_pk=add_pk,
+ )
+
+ order_expression = case(
+ viz_type_order,
+ value=Slice.viz_type,
+ else_=len(viz_type_order),
+ )
+ direction = asc if order_direction == "asc" else desc
+ order_by_columns = [direction(order_expression), direction(Slice.viz_type)]
+ primary_key = self.get_pk()
+ if add_pk and primary_key is not None:
+ order_by_columns.append(direction(primary_key))
+ return query.order_by(*order_by_columns)
+
class ChartRestApi(SoftDeleteApiMixin, BaseSupersetModelRestApi):
- datamodel = SQLAInterface(Slice)
+ datamodel = ChartSQLAInterface(Slice)
resource_name = "chart"
allow_browser_login = True
@@ -312,6 +371,7 @@ class ChartRestApi(SoftDeleteApiMixin, BaseSupersetModelRestApi):
openapi_spec_component_schemas = CHART_SCHEMAS + (VersionListItemSchema,)
apispec_parameter_schemas = {
+ "chart_get_list_schema": chart_get_list_schema,
"screenshot_query_schema": screenshot_query_schema,
"get_delete_ids_schema": get_delete_ids_schema,
"get_export_ids_schema": get_export_ids_schema,
@@ -422,6 +482,88 @@ class ChartRestApi(SoftDeleteApiMixin, BaseSupersetModelRestApi):
if row_id in extra_editors_by_id:
row["extra_editors"] = extra_editors_by_id[row_id]
+ @expose("/", methods=("GET",))
+ @protect()
+ @safe
+ @permission_name("get")
+ @parse_rison(chart_get_list_schema)
+ @merge_response_func(
+ BaseSupersetModelRestApi.merge_order_columns, API_ORDER_COLUMNS_RIS_KEY
+ )
+ @merge_response_func(
+ BaseSupersetModelRestApi.merge_list_label_columns, API_LABEL_COLUMNS_RIS_KEY
+ )
+ @merge_response_func(
+ BaseSupersetModelRestApi.merge_description_columns,
+ API_DESCRIPTION_COLUMNS_RIS_KEY,
+ )
+ @merge_response_func(
+ BaseSupersetModelRestApi.merge_list_columns, API_LIST_COLUMNS_RIS_KEY
+ )
+ @merge_response_func(
+ BaseSupersetModelRestApi.merge_list_title, API_LIST_TITLE_RIS_KEY
+ )
+ def get_list(self, **kwargs: Any) -> Response:
+ """Get a list of charts.
+ ---
+ get:
+ summary: Get a list of charts
+ parameters:
+ - in: query
+ name: q
+ description: >-
+ Rison-encoded list query. viz_type_order may contain up to 256
+ unique visualization type slugs, each at most 250 characters,
+ in the display-name order to use when sorting by viz_type.
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/chart_get_list_schema'
+ responses:
+ 200:
+ description: Charts
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ ids:
+ type: array
+ items:
+ type: integer
+ count:
+ type: integer
+ result:
+ type: array
+ items:
+ $ref: >-
+ #/components/schemas/{{self.__class__.__name__}}.get_list
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 422:
+ $ref: '#/components/responses/422'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ return self.get_list_headless(**kwargs)
+
+ def get_list_headless(self, **kwargs: Any) -> Response:
+ """Apply client display ordering before list pagination."""
+ args = kwargs.get("rison", {})
+ viz_types = args.get("viz_type_order")
+ if args.get("order_column") != "viz_type" or not viz_types:
+ return super().get_list_headless(**kwargs)
+
+ token = _viz_type_order.set(
+ {viz_type: index for index, viz_type in enumerate(viz_types)}
+ )
+ try:
+ return super().get_list_headless(**kwargs)
+ finally:
+ _viz_type_order.reset(token)
+
@expose("/<pk>/deck_layers/", methods=("GET",))
@protect()
@safe
diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py
index 3c6465fbbaf..458320aac48 100644
--- a/superset/charts/schemas.py
+++ b/superset/charts/schemas.py
@@ -20,6 +20,7 @@ from __future__ import annotations
from typing import Any, TYPE_CHECKING
from flask import current_app
+from flask_appbuilder.api.schemas import get_list_schema
from flask_babel import gettext as _
from marshmallow import (
EXCLUDE,
@@ -113,6 +114,26 @@ def validate_prophet_periods(value: int) -> None:
#
# RISON/JSON schemas for query parameters
#
+MAX_VIZ_TYPE_ORDER_LENGTH = 256
+MAX_VIZ_TYPE_LENGTH = 250
+
+chart_get_list_schema = {
+ **get_list_schema,
+ "properties": {
+ **get_list_schema["properties"],
+ "viz_type_order": {
+ "type": "array",
+ "items": {"type": "string", "maxLength": MAX_VIZ_TYPE_LENGTH},
+ "maxItems": MAX_VIZ_TYPE_ORDER_LENGTH,
+ "uniqueItems": True,
+ "description": (
+ "Visualization type slugs in display-name order. Used only when "
+ "order_column is viz_type."
+ ),
+ },
+ },
+}
+
get_delete_ids_schema = {
"type": "array",
"items": {"type": "integer"},
diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py
index 9338121c357..71a0aefaa3d 100644
--- a/tests/integration_tests/charts/api_tests.py
+++ b/tests/integration_tests/charts/api_tests.py
@@ -28,6 +28,7 @@ from parameterized import parameterized
from sqlalchemy import and_
from sqlalchemy.sql import func
+from superset.charts.schemas import chart_get_list_schema
from superset.commands.chart.data.get_data_command import ChartDataCommand
from superset.commands.chart.exceptions import ChartDataQueryFailedError
from superset.connectors.sqla.models import SqlaTable
@@ -1545,6 +1546,119 @@ class TestChartApi(ApiEditorsTestCaseMixin, InsertChartMixin, SupersetTestCase):
data = json.loads(rv.data.decode("utf-8"))
assert data["count"] == 5
+ def test_chart_list_openapi_documents_viz_type_order(self):
+ """Chart API: display ordering is part of the documented list contract."""
+ self.login(ADMIN_USERNAME)
+ rv = self.client.get("api/v1/_openapi")
+
+ assert rv.status_code == 200
+ spec = json.loads(rv.data.decode("utf-8"))
+ query_parameter = spec["paths"]["/api/v1/chart/"]["get"]["parameters"][0]
+ assert query_parameter["content"]["application/json"]["schema"] == {
+ "$ref": "#/components/schemas/chart_get_list_schema"
+ }
+ viz_type_order = spec["components"]["schemas"]["chart_get_list_schema"][
+ "properties"
+ ]["viz_type_order"]
+ assert viz_type_order == chart_get_list_schema["properties"]["viz_type_order"]
+
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_get_charts_orders_display_viz_types_before_pagination(self):
+ """Chart API: display chart type ordering happens before pagination."""
+ admin = self.get_user("admin")
+ charts = [
+ self.insert_chart("display_type_sort_a", [admin.id], 1, viz_type="slug_a"),
+ self.insert_chart(
+ "display_type_sort_middle", [admin.id], 1, viz_type="middle"
+ ),
+ self.insert_chart("display_type_sort_z", [admin.id], 1, viz_type="slug_z"),
+ self.insert_chart(
+ "display_type_sort_unknown", [admin.id], 1, viz_type="unknown"
+ ),
+ ]
+ self.login(ADMIN_USERNAME)
+
+ arguments = {
+ "filters": [
+ {
+ "col": "slice_name",
+ "opr": "sw",
+ "value": "display_type_sort_",
+ }
+ ],
+ "order_column": "viz_type",
+ "order_direction": "asc",
+ "page_size": 2,
+ "viz_type_order": ["slug_z", "middle", "slug_a"],
+ }
+
+ try:
+ for direction, expected in (
+ ("asc", ["slug_z", "middle", "slug_a", "unknown"]),
+ ("desc", ["unknown", "slug_a", "middle", "slug_z"]),
+ ):
+ arguments["order_direction"] = direction
+ pages = []
+ for page in (0, 1):
+ arguments["page"] = page
+ uri = f"api/v1/chart/?q={rison.dumps(arguments)}"
+ rv = self.get_assert_metric(uri, "get_list")
+ assert rv.status_code == 200
+ data = json.loads(rv.data.decode("utf-8"))
+ assert data["count"] == 4
+ pages.extend(item["viz_type"] for item in data["result"])
+
+ assert pages == expected
+
+ arguments.update(
+ {
+ "order_column": "slice_name",
+ "order_direction": "asc",
+ "page": 0,
+ "page_size": 4,
+ }
+ )
+ uri = f"api/v1/chart/?q={rison.dumps(arguments)}"
+ rv = self.get_assert_metric(uri, "get_list")
+ assert rv.status_code == 200
+ data = json.loads(rv.data.decode("utf-8"))
+ assert [item["slice_name"] for item in data["result"]] == sorted(
+ chart.slice_name for chart in charts
+ )
+
+ arguments.update(
+ {
+ "order_column": "viz_type",
+ "viz_type_order": [],
+ }
+ )
+ uri = f"api/v1/chart/?q={rison.dumps(arguments)}"
+ rv = self.get_assert_metric(uri, "get_list")
+ assert rv.status_code == 200
+ data = json.loads(rv.data.decode("utf-8"))
+ assert [item["viz_type"] for item in data["result"]] == [
+ "middle",
+ "slug_a",
+ "slug_z",
+ "unknown",
+ ]
+
+ arguments.pop("viz_type_order")
+ uri = f"api/v1/chart/?q={rison.dumps(arguments)}"
+ rv = self.get_assert_metric(uri, "get_list")
+ assert rv.status_code == 200
+ data = json.loads(rv.data.decode("utf-8"))
+ assert [item["viz_type"] for item in data["result"]] == [
+ "middle",
+ "slug_a",
+ "slug_z",
+ "unknown",
+ ]
+ finally:
+ for chart in charts:
+ db.session.delete(chart)
+ db.session.commit()
+
@pytest.fixture
def load_energy_charts(self):
with app.app_context():
diff --git a/tests/unit_tests/charts/test_schemas.py b/tests/unit_tests/charts/test_schemas.py
index 153e8a9734b..e22d8cb928c 100644
--- a/tests/unit_tests/charts/test_schemas.py
+++ b/tests/unit_tests/charts/test_schemas.py
@@ -18,10 +18,13 @@
import pandas as pd
import pytest
from flask import current_app
+from jsonschema import validate as validate_json_schema
+from jsonschema.exceptions import ValidationError as JSONSchemaValidationError
from marshmallow import ValidationError
from pytest_mock import MockerFixture
from superset.charts.schemas import (
+ chart_get_list_schema,
ChartDataAdhocMetricSchema,
ChartDataExtrasSchema,
ChartDataPostProcessingOperationSchema,
@@ -35,9 +38,48 @@ from superset.charts.schemas import (
DEFAULT_MAX_PROPHET_PERIODS,
get_max_prophet_periods,
get_time_grain_choices,
+ MAX_VIZ_TYPE_LENGTH,
+ MAX_VIZ_TYPE_ORDER_LENGTH,
)
+def test_chart_get_list_schema_accepts_viz_type_display_order() -> None:
+ validate_json_schema(
+ instance={
+ "order_column": "viz_type",
+ "viz_type_order": ["slug_z", "slug_a"],
+ },
+ schema=chart_get_list_schema,
+ )
+ validate_json_schema(
+ instance={"order_column": "viz_type", "viz_type_order": []},
+ schema=chart_get_list_schema,
+ )
+
+
+@pytest.mark.parametrize(
+ "viz_type_order",
+ [
+ "slug_a",
+ [1],
+ ["slug_a", "slug_a"],
+ ["a" * (MAX_VIZ_TYPE_LENGTH + 1)],
+ [f"slug_{index}" for index in range(MAX_VIZ_TYPE_ORDER_LENGTH + 1)],
+ ],
+)
+def test_chart_get_list_schema_rejects_invalid_viz_type_display_order(
+ viz_type_order: object,
+) -> None:
+ with pytest.raises(JSONSchemaValidationError):
+ validate_json_schema(
+ instance={
+ "order_column": "viz_type",
+ "viz_type_order": viz_type_order,
+ },
+ schema=chart_get_list_schema,
+ )
+
+
def test_get_time_grain_choices(app_context: None) -> None:
"""Test that get_time_grain_choices returns values with config addons"""
# Save original config