diff --git a/UPDATING.md b/UPDATING.md index fb37e3bcc04..e8f24360d99 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -24,6 +24,16 @@ assists people when migrating to a new version. ## Next +### `thumbnail_url` removed from dashboard list API response + +The `thumbnail_url` field has been removed from `GET /api/v1/dashboard/` list responses. External consumers relying on this field must now construct the thumbnail URL client-side using `id` and `changed_on_utc`: + +``` +/api/v1/dashboard/{id}/thumbnail/{changed_on_utc}/ +``` + +The thumbnail endpoint redirects to the current digest URL regardless of whether the supplied digest is exact. If the image is not yet cached, that digest URL may return `202` and trigger async generation. Using `changed_on_utc` as the digest is sufficient for cache-busting purposes. + ### Webhook alerts/reports block private/internal hosts by default Webhook alert/report dispatch (`WebhookNotification.send`) now validates the target URL's host against the same private/internal-IP block applied to dataset import URLs. If the resolved host is in a loopback, link-local, private (RFC-1918), shared-CGNAT, or multicast range, the webhook is rejected with `NotificationParamException`. diff --git a/superset-frontend/src/features/dashboards/DashboardCard.test.tsx b/superset-frontend/src/features/dashboards/DashboardCard.test.tsx index 0d944234030..65dec65d004 100644 --- a/superset-frontend/src/features/dashboards/DashboardCard.test.tsx +++ b/superset-frontend/src/features/dashboards/DashboardCard.test.tsx @@ -18,13 +18,9 @@ */ import { MemoryRouter } from 'react-router-dom'; -import { - JsonResponse, - SupersetClient, - isFeatureEnabled, -} from '@superset-ui/core'; +import { isFeatureEnabled } from '@superset-ui/core'; -import { render, screen, waitFor } from 'spec/helpers/testing-library'; +import { render, screen } from 'spec/helpers/testing-library'; import DashboardCard from './DashboardCard'; @@ -35,7 +31,7 @@ const mockDashboard = { certification_details: 'Certified on 2022-01-01', published: true, url: '/dashboard/1', - thumbnail_url: '/thumbnails/1.png', + changed_on_utc: '2024-01-01T00:00:00', changed_on_delta_humanized: '2 days ago', owners: [ { id: 1, name: 'Alice', first_name: 'Alice', last_name: 'Doe' }, @@ -104,57 +100,81 @@ test('Renders the modified date', () => { expect(modifiedDateElement).toBeInTheDocument(); }); -test('should fetch thumbnail when dashboard has no thumbnail URL and feature flag is enabled', async () => { - const mockGet = jest.spyOn(SupersetClient, 'get').mockResolvedValue({ - json: { result: { thumbnail_url: '/new-thumbnail.png' } }, - } as unknown as JsonResponse); +describe('thumbnail URL construction', () => { + let fetchSpy: jest.SpyInstance; - const { rerender } = render( - true} - bulkSelectEnabled={false} - loading={false} - saveFavoriteStatus={() => {}} - favoriteStatus={false} - handleBulkDashboardExport={() => {}} - onDelete={() => {}} - />, - ); - await waitFor(() => { - expect(mockGet).toHaveBeenCalledWith({ - endpoint: '/api/v1/dashboard/1', + beforeEach(() => { + fetchSpy = jest.spyOn(global, 'fetch').mockResolvedValue({ + blob: () => Promise.resolve(new Blob([''], { type: 'image/png' })), + } as Response); + }); + + afterEach(() => { + fetchSpy.mockRestore(); + }); + + const renderCard = (dashboard: object) => + render( + + true} + bulkSelectEnabled={false} + loading={false} + showThumbnails + saveFavoriteStatus={() => {}} + favoriteStatus={false} + handleBulkDashboardExport={() => {}} + onDelete={() => {}} + /> + , + ); + + test('constructs thumbnail URL from dashboard id and changed_on_utc', () => { + renderCard({ + id: 2, + changed_by_name: '', + changed_by: '', + dashboard_title: 'UTC Dashboard', + published: false, + url: '/dashboard/2', + owners: [], + changed_on_utc: '2024-01-01T00:00:00', }); + + expect(fetchSpy).toHaveBeenCalledWith( + '/api/v1/dashboard/2/thumbnail/2024-01-01T00%3A00%3A00/', + ); + }); + + test('falls back to changed_on when changed_on_utc is absent', () => { + renderCard({ + id: 3, + changed_by_name: '', + changed_by: '', + dashboard_title: 'Fallback Dashboard', + published: false, + url: '/dashboard/3', + owners: [], + changed_on: '2024-06-01T12:00:00', + }); + + expect(fetchSpy).toHaveBeenCalledWith( + '/api/v1/dashboard/3/thumbnail/2024-06-01T12%3A00%3A00/', + ); + }); + + test('renders no thumbnail when both changed_on_utc and changed_on are absent', () => { + renderCard({ + id: 4, + changed_by_name: '', + changed_by: '', + dashboard_title: 'No Timestamp Dashboard', + published: false, + url: '/dashboard/4', + owners: [], + }); + + expect(fetchSpy).not.toHaveBeenCalled(); }); - rerender( - true} - bulkSelectEnabled={false} - loading={false} - saveFavoriteStatus={() => {}} - favoriteStatus={false} - handleBulkDashboardExport={() => {}} - onDelete={() => {}} - />, - ); - mockGet.mockRestore(); }); diff --git a/superset-frontend/src/features/dashboards/DashboardCard.tsx b/superset-frontend/src/features/dashboards/DashboardCard.tsx index 5793e0e1bc0..959797853f6 100644 --- a/superset-frontend/src/features/dashboards/DashboardCard.tsx +++ b/superset-frontend/src/features/dashboards/DashboardCard.tsx @@ -16,14 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import { useEffect, useState } from 'react'; import { Link, useHistory } from 'react-router-dom'; import { t } from '@apache-superset/core/translation'; -import { - isFeatureEnabled, - FeatureFlag, - SupersetClient, -} from '@superset-ui/core'; +import { isFeatureEnabled, FeatureFlag } from '@superset-ui/core'; import { CardStyles } from 'src/views/CRUD/utils'; import { Dropdown, @@ -69,33 +64,11 @@ function DashboardCard({ const canEdit = hasPerm('can_write'); const canDelete = hasPerm('can_write'); const canExport = hasPerm('can_export'); - const [thumbnailUrl, setThumbnailUrl] = useState(null); - const [fetchingThumbnail, setFetchingThumbnail] = useState(false); - - useEffect(() => { - // fetch thumbnail only if it's not already fetched - if ( - !fetchingThumbnail && - dashboard.id && - (thumbnailUrl === undefined || thumbnailUrl === null) && - isFeatureEnabled(FeatureFlag.Thumbnails) - ) { - // fetch thumbnail - if (dashboard.thumbnail_url) { - // set to empty string if null so that we don't - // keep fetching the thumbnail - setThumbnailUrl(dashboard.thumbnail_url || ''); - return; - } - setFetchingThumbnail(true); - SupersetClient.get({ - endpoint: `/api/v1/dashboard/${dashboard.id}`, - }).then(({ json = {} }) => { - setThumbnailUrl(json.result?.thumbnail_url || ''); - setFetchingThumbnail(false); - }); - } - }, [dashboard, thumbnailUrl]); + const digest = dashboard.changed_on_utc || dashboard.changed_on; + const thumbnailUrl = + isFeatureEnabled(FeatureFlag.Thumbnails) && dashboard.id && digest + ? `/api/v1/dashboard/${dashboard.id}/thumbnail/${encodeURIComponent(digest)}/` + : ''; const menuItems: MenuItem[] = []; diff --git a/superset-frontend/src/pages/DashboardList/index.tsx b/superset-frontend/src/pages/DashboardList/index.tsx index a12fc03bcd5..13a3e44dd00 100644 --- a/superset-frontend/src/pages/DashboardList/index.tsx +++ b/superset-frontend/src/pages/DashboardList/index.tsx @@ -106,11 +106,11 @@ export interface Dashboard { changed_by_name: string; changed_on_delta_humanized: string; changed_by: string; + changed_on?: string; dashboard_title: string; id: number; published: boolean; url: string; - thumbnail_url: string; owners: Owner[]; tags: TagType[]; created_by: object; diff --git a/superset-frontend/src/views/CRUD/types.ts b/superset-frontend/src/views/CRUD/types.ts index e781a282910..dce68699370 100644 --- a/superset-frontend/src/views/CRUD/types.ts +++ b/superset-frontend/src/views/CRUD/types.ts @@ -55,6 +55,7 @@ export interface Dashboard { certified_by?: string; certification_details?: string; changed_by_name: string; + changed_on?: string; changed_on_delta_humanized?: string; changed_on_utc?: string; changed_by: string; @@ -63,7 +64,7 @@ export interface Dashboard { id: number; published: boolean; url: string; - thumbnail_url: string; + thumbnail_url?: string | null; owners: Owner[]; loading?: boolean; } diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index d67a45c8d27..fe3d81820df 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -187,7 +187,6 @@ BASE_LIST_COLUMNS = [ "status", "slug", "url", - "thumbnail_url", "certified_by", "certification_details", "changed_by.first_name", diff --git a/tests/integration_tests/thumbnails_tests.py b/tests/integration_tests/thumbnails_tests.py index 81596bc186c..b29f6ce71b7 100644 --- a/tests/integration_tests/thumbnails_tests.py +++ b/tests/integration_tests/thumbnails_tests.py @@ -67,11 +67,14 @@ class TestThumbnailsSeleniumLive(LiveServerTestCase): """ Thumbnails: Simple get async dashboard screenshot """ - with patch("superset.dashboards.api.DashboardRestApi.get") as mock_get: # noqa: F841 - rv = self.client.get(DASHBOARD_URL) - resp = json.loads(rv.data.decode("utf-8")) - thumbnail_url = resp["result"][0]["thumbnail_url"] + rv = self.client.get(DASHBOARD_URL) + resp = json.loads(rv.data.decode("utf-8")) + obj_id = resp["result"][0]["id"] + rv = self.client.get(f"{DASHBOARD_URL}{obj_id}") + resp = json.loads(rv.data.decode("utf-8")) + thumbnail_url = resp["result"]["thumbnail_url"] + with patch("superset.dashboards.api.DashboardRestApi.get"): response = self.url_open_auth( ADMIN_USERNAME, thumbnail_url, @@ -194,11 +197,31 @@ class TestThumbnails(SupersetTestCase): # SHA-256 hash of "foo_bar" (default HASH_ALGORITHM is sha256) digest_hash = "4928cae8b37b3d1113f5e01e60c967df6c2b9e826dc7d91488d23a62fec715ba" + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + def test_dashboard_list_omits_thumbnail_url(self): + """ + Thumbnails: dashboard list response must not include thumbnail_url + """ + self.login(ADMIN_USERNAME) + rv = self.client.get(DASHBOARD_URL) + resp = json.loads(rv.data.decode("utf-8")) + assert rv.status_code == 200 + assert len(resp["result"]) > 0 + for dashboard in resp["result"]: + assert "thumbnail_url" not in dashboard, ( + "thumbnail_url should not appear in list responses; " + "it is only available on the detail endpoint" + ) + def _get_id_and_thumbnail_url(self, url: str) -> tuple[int, str]: rv = self.client.get(url) resp = json.loads(rv.data.decode("utf-8")) - obj = resp["result"][0] - return obj["id"], obj["thumbnail_url"] + obj_id = resp["result"][0]["id"] + # Fetch thumbnail_url from the detail endpoint since it's + # not included in list responses + rv = self.client.get(f"{url}{obj_id}") + resp = json.loads(rv.data.decode("utf-8")) + return obj_id, resp["result"]["thumbnail_url"] @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") @with_feature_flags(THUMBNAILS=False)