diff --git a/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApi.ts b/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApi.ts index fddac0399c9..efe0687127a 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApi.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApi.ts @@ -41,12 +41,14 @@ function tryParsePayload(payload: Payload) { */ function getFullUrl(partialUrl: string, params: CallApi['searchParams']) { if (params) { - const url = new URL(partialUrl, window.location.href); const search = params instanceof URLSearchParams ? params : new URLSearchParams(params); - // will completely override any existing search params - url.search = search.toString(); - return url.href; + const searchString = search.toString(); + if (searchString) { + const url = new URL(partialUrl, window.location.href); + url.search = searchString; + return url.href; + } } return partialUrl; } diff --git a/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApi.test.ts b/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApi.test.ts index ad64f7fca14..8d45af31a53 100644 --- a/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApi.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApi.test.ts @@ -591,6 +591,21 @@ describe('callApi()', () => { ); }); + test('should preserve existing query params when searchParams is empty', async () => { + window.location.href = 'http://localhost'; + fetchMock.get(`glob:*/get-search*`, { yes: 'ok' }); + const response = await callApi({ + url: '/get-search?q=existing', + searchParams: {}, + method: 'GET', + }); + const result = await response.json(); + expect(result).toEqual({ yes: 'ok' }); + expect(fetchMock.callHistory.lastCall()?.url).toEqual( + `http://localhost/get-search?q=existing`, + ); + }); + test('should accept URLSearchParams', async () => { expect.assertions(2); window.location.href = 'http://localhost'; diff --git a/superset-frontend/src/hooks/apiResources/dashboards.test.ts b/superset-frontend/src/hooks/apiResources/dashboards.test.ts index 7676c5c8e87..788c4ae7486 100644 --- a/superset-frontend/src/hooks/apiResources/dashboards.test.ts +++ b/superset-frontend/src/hooks/apiResources/dashboards.test.ts @@ -18,7 +18,28 @@ */ import { renderHook } from '@testing-library/react-hooks'; import fetchMock from 'fetch-mock'; -import { useDashboardDatasets } from './dashboards'; +import { useDashboard, useDashboardDatasets } from './dashboards'; + +test('useDashboard excludes thumbnail_url from request', async () => { + fetchMock.get('glob:*/api/v1/dashboard/5?q=*', { + result: { + id: 5, + dashboard_title: 'Test', + json_metadata: '{}', + position_json: '{}', + owners: [], + }, + }); + + const { waitForNextUpdate } = renderHook(() => useDashboard(5)); + await waitForNextUpdate(); + + const calledUrl = fetchMock.callHistory.lastCall()?.url ?? ''; + expect(calledUrl).toContain('?q='); + expect(calledUrl).not.toContain('thumbnail_url'); + + fetchMock.clearHistory().removeRoutes(); +}); // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks describe('useDashboardDatasets', () => { diff --git a/superset-frontend/src/hooks/apiResources/dashboards.ts b/superset-frontend/src/hooks/apiResources/dashboards.ts index 65e4b1c0bdd..81ed364e6ea 100644 --- a/superset-frontend/src/hooks/apiResources/dashboards.ts +++ b/superset-frontend/src/hooks/apiResources/dashboards.ts @@ -17,14 +17,42 @@ * under the License. */ +import rison from 'rison'; import { Dashboard, Datasource, EmbeddedDashboard } from 'src/dashboard/types'; import { Chart } from 'src/types/Chart'; import { Currency } from '@superset-ui/core'; import { useApiV1Resource, useTransformedResource } from './apiResources'; -export const useDashboard = (idOrSlug: string | number) => - useTransformedResource( - useApiV1Resource(`/api/v1/dashboard/${idOrSlug}`), +const DASHBOARD_GET_COLUMNS = [ + 'id', + 'slug', + 'url', + 'dashboard_title', + 'published', + 'css', + 'theme', + 'json_metadata', + 'position_json', + 'certified_by', + 'certification_details', + 'changed_by_name', + 'changed_by', + 'changed_on', + 'created_by', + 'charts', + 'owners', + 'roles', + 'tags', + 'changed_on_delta_humanized', + 'created_on_delta_humanized', + 'is_managed_externally', + 'uuid', +]; + +export const useDashboard = (idOrSlug: string | number) => { + const q = rison.encode({ columns: DASHBOARD_GET_COLUMNS }); + return useTransformedResource( + useApiV1Resource(`/api/v1/dashboard/${idOrSlug}?q=${q}`), dashboard => ({ ...dashboard, // TODO: load these at the API level @@ -35,6 +63,7 @@ export const useDashboard = (idOrSlug: string | number) => owners: dashboard.owners || [], }), ); +}; // gets the chart definitions for a dashboard export const useDashboardCharts = (idOrSlug: string | number) => diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index cdbed39552b..bcb92231e56 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -22,6 +22,7 @@ from io import BytesIO from typing import Any, Callable, cast from zipfile import is_zipfile, ZipFile +import prison from flask import current_app, g, redirect, request, Response, send_file, url_for from flask_appbuilder import permission_name from flask_appbuilder.api import expose, merge_response_func, protect, rison, safe @@ -467,6 +468,12 @@ class DashboardRestApi(CustomTagsOptimizationMixin, BaseSupersetModelRestApi): type: string name: id_or_slug description: Either the id of the dashboard, or its slug + - in: query + name: q + content: + application/json: + schema: + $ref: '#/components/schemas/get_item_schema' responses: 200: description: Dashboard @@ -486,7 +493,26 @@ class DashboardRestApi(CustomTagsOptimizationMixin, BaseSupersetModelRestApi): 404: $ref: '#/components/responses/404' """ - result = self.dashboard_get_response_schema.dump(dash) + columns: list[str] | None = None + if q := request.args.get("q"): + try: + args = prison.loads(q) + except Exception: + return self.response_400(message="Invalid rison query parameter") + if isinstance(args, dict): + columns = args.get("columns") + + if columns: + schema_fields = self.dashboard_get_response_schema.fields + key_to_name = { + field.data_key or name: name for name, field in schema_fields.items() + } + only = [key_to_name[c] for c in columns if c in key_to_name] + schema = DashboardGetResponseSchema(only=only) + else: + schema = self.dashboard_get_response_schema + + result = schema.dump(dash) add_extra_log_payload( dashboard_id=dash.id, action=f"{self.__class__.__name__}.get" ) diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index ca0e8fa8968..f71af99ad40 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -569,6 +569,87 @@ class TestDashboardApi(ApiOwnersTestCaseMixin, InsertChartMixin, SupersetTestCas db.session.delete(dashboard) db.session.commit() + def test_get_dashboard_with_columns(self): + """ + Dashboard API: Test get dashboard with column selection via q param + """ + admin = self.get_user("admin") + dashboard = self.insert_dashboard( + "title", "slug1", [admin.id], created_by=admin + ) + self.login(ADMIN_USERNAME) + params = prison.dumps({"columns": ["id", "dashboard_title"]}) + uri = f"api/v1/dashboard/{dashboard.id}?q={params}" + rv = self.get_assert_metric(uri, "get") + assert rv.status_code == 200 + data = json.loads(rv.data.decode("utf-8")) + result = data["result"] + assert result["id"] == dashboard.id + assert result["dashboard_title"] == "title" + assert "thumbnail_url" not in result + assert "slug" not in result + assert "owners" not in result + # rollback changes + db.session.delete(dashboard) + db.session.commit() + + def test_get_dashboard_with_invalid_rison_q(self): + """ + Dashboard API: Test get dashboard with malformed rison returns 400 + """ + admin = self.get_user("admin") + dashboard = self.insert_dashboard( + "title", "slug1", [admin.id], created_by=admin + ) + self.login(ADMIN_USERNAME) + uri = f"api/v1/dashboard/{dashboard.id}?q=((" + rv = self.get_assert_metric(uri, "get") + assert rv.status_code == 400 + # rollback changes + db.session.delete(dashboard) + db.session.commit() + + def test_get_dashboard_with_non_dict_q(self): + """ + Dashboard API: Test get dashboard with non-dict rison returns full response + """ + admin = self.get_user("admin") + dashboard = self.insert_dashboard( + "title", "slug1", [admin.id], created_by=admin + ) + self.login(ADMIN_USERNAME) + uri = f"api/v1/dashboard/{dashboard.id}?q=a_string" + rv = self.get_assert_metric(uri, "get") + assert rv.status_code == 200 + data = json.loads(rv.data.decode("utf-8")) + # non-dict q is ignored, full response returned + assert "thumbnail_url" in data["result"] + # rollback changes + db.session.delete(dashboard) + db.session.commit() + + def test_get_dashboard_with_columns_data_key_mapping(self): + """ + Dashboard API: Test that data_key columns like changed_on_delta_humanized work + """ + admin = self.get_user("admin") + dashboard = self.insert_dashboard( + "title", "slug1", [admin.id], created_by=admin + ) + self.login(ADMIN_USERNAME) + params = prison.dumps({"columns": ["id", "changed_on_delta_humanized"]}) + uri = f"api/v1/dashboard/{dashboard.id}?q={params}" + rv = self.get_assert_metric(uri, "get") + assert rv.status_code == 200 + data = json.loads(rv.data.decode("utf-8")) + result = data["result"] + assert "id" in result + assert "changed_on_delta_humanized" in result + assert "dashboard_title" not in result + # rollback changes + db.session.delete(dashboard) + db.session.commit() + @patch("superset.dashboards.schemas.security_manager.has_guest_access") @patch("superset.dashboards.schemas.security_manager.is_guest_user") def test_get_dashboard_as_guest(self, is_guest_user, has_guest_access): diff --git a/tests/unit_tests/dashboards/api_test.py b/tests/unit_tests/dashboards/api_test.py new file mode 100644 index 00000000000..5f4c5e680d9 --- /dev/null +++ b/tests/unit_tests/dashboards/api_test.py @@ -0,0 +1,99 @@ +# 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. + +from unittest.mock import MagicMock + +import pytest + +from superset.dashboards.schemas import DashboardGetResponseSchema + + +@pytest.fixture +def mock_dashboard() -> MagicMock: + dash = MagicMock() + dash.id = 1 + dash.slug = "test-slug" + dash.url = "/superset/dashboard/test-slug/" + dash.dashboard_title = "Test Dashboard" + dash.thumbnail_url = "http://example.com/thumb.png" + dash.published = True + dash.css = "" + dash.theme = None + dash.json_metadata = "{}" + dash.position_json = "{}" + dash.certified_by = None + dash.certification_details = None + dash.changed_by_name = "admin" + dash.changed_by = MagicMock(id=1, first_name="admin", last_name="user") + dash.changed_on = None + dash.changed_on_humanized = "2 days ago" + dash.created_by = MagicMock(id=1, first_name="admin", last_name="user") + dash.created_on_humanized = "5 days ago" + dash.charts = [] + dash.owners = [] + dash.roles = [] + dash.tags = [] + dash.custom_tags = [] + dash.is_managed_externally = False + dash.uuid = None + return dash + + +def test_schema_column_selection_excludes_thumbnail( + mock_dashboard: MagicMock, +) -> None: + schema = DashboardGetResponseSchema(only=["id", "dashboard_title"]) + result = schema.dump(mock_dashboard) + assert "id" in result + assert "dashboard_title" in result + assert "thumbnail_url" not in result + assert "slug" not in result + + +def test_schema_column_selection_with_data_key( + mock_dashboard: MagicMock, +) -> None: + """Fields with data_key should work when using the internal field name.""" + schema = DashboardGetResponseSchema(only=["id", "changed_on_humanized"]) + result = schema.dump(mock_dashboard) + assert "id" in result + assert "changed_on_delta_humanized" in result + assert "dashboard_title" not in result + + +def test_schema_full_response_includes_thumbnail( + mock_dashboard: MagicMock, +) -> None: + schema = DashboardGetResponseSchema() + result = schema.dump(mock_dashboard) + assert "thumbnail_url" in result + assert "id" in result + assert "dashboard_title" in result + + +def test_data_key_mapping_logic() -> None: + """The key_to_name mapping used in the API correctly maps data_key to field name.""" + schema = DashboardGetResponseSchema() + key_to_name = { + field.data_key or name: name for name, field in schema.fields.items() + } + # changed_on_delta_humanized is the data_key for changed_on_humanized + assert key_to_name["changed_on_delta_humanized"] == "changed_on_humanized" + assert key_to_name["created_on_delta_humanized"] == "created_on_humanized" + # fields without data_key map to themselves + assert key_to_name["id"] == "id" + assert key_to_name["thumbnail_url"] == "thumbnail_url"