This is an automated email from the ASF dual-hosted git repository. justinpark pushed a commit to branch feat--add-explore-dashboard-to-core-api-6.2 in repository https://gitbox.apache.org/repos/asf/superset.git
commit 270089bad5a194cefecfa5c5c7173560827202c1 Author: justinpark <[email protected]> AuthorDate: Wed Sep 30 15:29:15 2026 -0700 feat(extensions): add explore and dashboard to core api Adds two new public extension-API namespaces to @apache-superset/core, following the same pattern as the existing sqlLab namespace (declarations in packages/superset-core/src/*, host implementation wired through Redux in src/core/*, and exposed to extensions via window.superset in ExtensionsStartup.tsx/Namespaces.ts). explore - the chart currently open in Explore: - getChartId() - the current chart's id (undefined if unsaved or none loaded) - getControlValues() / getControlValue(name) / setControlValues(values) - generic read/write access to the chart's form data (control values) - getQuery() / getChartData() - the SQL query(ies) / result rows and columns for the chart's current control values, without running it. Return one entry per query the chart runs (more than one for a chart type like Mixed Timeseries), and throw if any of them errored. dashboard - the classic dashboard currently active: - getDashboardId() - the active dashboard's id - getLayout() / getActiveTabs() / updateLayoutNode(nodeId, meta) - read the layout tree (returned frozen, so it can't be mutated outside a reducer) / read which tabs are currently open / patch one node's meta (grid size, position, etc.) - getCss() / setCss(css) - read/apply the dashboard's custom CSS - getFilters() / updateFilters(updates) - read native filters (config + currently applied values) / apply new values to one or more filters for this session only, the same effect as interacting with the Filter Bar - saveFilters(updates, deletedFilterIds) - persist changes to filter definitions (name, targets, default value), or delete filters, so the change is visible to every future viewer - refreshChart(chartId) - re-fetch a chart's saved configuration and re-run its query, for picking up a change made outside this dashboard session, invalidating the dashboard's cached form data and awaiting the chart's re-render Every dashboard/explore getter returns undefined/an empty value, and every action throws, when the corresponding surface isn't active - including mid-navigation, before Redux has finished hydrating a newly-routed dashboard. Co-Authored-By: Claude Sonnet 5 <[email protected]> --- .../packages/superset-core/package.json | 8 + .../packages/superset-core/src/dashboard/index.ts | 325 ++++++++++ .../packages/superset-core/src/explore/index.ts | 148 +++++ .../packages/superset-core/src/index.ts | 2 + .../src/components/Chart/chartAction.ts | 20 + .../src/components/Chart/chartReducer.ts | 3 + superset-frontend/src/core/dashboard/index.test.ts | 682 +++++++++++++++++++++ superset-frontend/src/core/dashboard/index.ts | 303 +++++++++ superset-frontend/src/core/explore/index.test.ts | 320 ++++++++++ superset-frontend/src/core/explore/index.ts | 151 +++++ superset-frontend/src/core/index.ts | 2 + .../src/dashboard/actions/dashboardState.ts | 4 +- .../src/dashboard/actions/nativeFilters.ts | 43 +- .../src/dashboard/containers/DashboardPage.tsx | 7 +- superset-frontend/src/dashboard/types.ts | 2 - .../util/charts/getFormDataWithExtraFilters.ts | 15 +- .../util/getFormDataWithExtraFilters.test.ts | 36 ++ .../components/ExploreViewContainer/index.tsx | 26 +- .../controlUtils/getVisibleFormDataFromControls.ts | 46 ++ .../src/explore/controlUtils/index.ts | 1 + superset-frontend/src/explore/types.ts | 1 + .../src/extensions/ExtensionsStartup.tsx | 4 + superset-frontend/src/extensions/Namespaces.ts | 4 + 23 files changed, 2120 insertions(+), 33 deletions(-) diff --git a/superset-frontend/packages/superset-core/package.json b/superset-frontend/packages/superset-core/package.json index a8581dc55ef..c620f01b148 100644 --- a/superset-frontend/packages/superset-core/package.json +++ b/superset-frontend/packages/superset-core/package.json @@ -30,10 +30,18 @@ "types": "./lib/commands/index.d.ts", "default": "./lib/commands/index.js" }, + "./dashboard": { + "types": "./lib/dashboard/index.d.ts", + "default": "./lib/dashboard/index.js" + }, "./editors": { "types": "./lib/editors/index.d.ts", "default": "./lib/editors/index.js" }, + "./explore": { + "types": "./lib/explore/index.d.ts", + "default": "./lib/explore/index.js" + }, "./extensions": { "types": "./lib/extensions/index.d.ts", "default": "./lib/extensions/index.js" diff --git a/superset-frontend/packages/superset-core/src/dashboard/index.ts b/superset-frontend/packages/superset-core/src/dashboard/index.ts new file mode 100644 index 00000000000..2058fdd7c99 --- /dev/null +++ b/superset-frontend/packages/superset-core/src/dashboard/index.ts @@ -0,0 +1,325 @@ +/** + * 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. + */ + +/** + * @fileoverview Dashboard API for Superset extensions. + * + * Exposes the dashboard currently active on the Dashboard surface (see + * `navigation.getPage() === 'dashboard'`) so extensions can identify it and + * read/apply its layout, custom CSS, and native filter values. + */ + +/** + * Gets the ID of the dashboard currently active on the Dashboard surface. + * + * @returns The current dashboard's ID, or undefined if none is active. + * + * @example + * ```typescript + * const dashboardId = dashboard.getDashboardId(); + * if (dashboardId != null) { + * console.log(`Dashboard ID: ${dashboardId}`); + * } + * ``` + */ +export declare function getDashboardId(): number | undefined; + +/** + * One component (row, column, chart holder, tab, markdown, etc.) in a + * dashboard's layout tree, as returned by {@link getLayout}. + */ +export interface LayoutNode { + /** IDs of this node's child components, in display order. */ + children: string[]; + + /** IDs of this node's ancestor components, root first. */ + parents?: string[]; + + /** Component type, e.g. `'CHART'`, `'ROW'`, `'TABS'`, `'MARKDOWN'`. */ + type: string; + + /** This node's own ID, e.g. `'CHART-abc123'`. */ + id: string; + + /** Grid size/position and other component-specific settings. */ + meta: { + chartId?: number; + width?: number; + height?: number; + [key: string]: unknown; + }; +} + +/** + * Gets the current dashboard's full layout tree — one entry per component + * (row, column, chart holder, tab, markdown, etc.), keyed by node ID. + * + * The returned map and every node (and its nested `meta`/`children`/ + * `parents`) are frozen: mutating them has no effect on the dashboard — + * use {@link updateLayoutNode} to apply a change instead. + * + * @returns A map of node ID to layout node. + * + * @example + * ```typescript + * const layout = dashboard.getLayout(); + * const chartNode = layout['CHART-abc123']; + * console.log(chartNode.meta.width); + * ``` + */ +export declare function getLayout(): Record<string, LayoutNode>; + +/** + * Gets the IDs of the current dashboard's currently-open `TAB` layout + * nodes — one per `TABS` container, since nested tab sets can each have + * their own open tab. Use {@link getLayout} to resolve an ID to its node + * (e.g. to read its `meta.text` label). + * + * @returns The active tab node IDs, e.g. `['TAB-abc123']`. + * + * @example + * ```typescript + * const activeTabs = dashboard.getActiveTabs(); + * ``` + */ +export declare function getActiveTabs(): string[]; + +/** + * Updates a single layout node's `meta` (e.g. grid `width`/`height`, or + * other component-specific settings) on the current dashboard. Only the + * keys passed in `meta` are changed — the node's other meta fields, and the + * rest of the layout, are left as-is. + * + * @param nodeId The layout node's ID, e.g. `'CHART-abc123'`. Use + * `getLayout()` to find node IDs. + * @param meta Meta fields to merge into the node. + * @returns Promise that resolves once the layout has been updated. + * @throws If no dashboard is active, or `nodeId` doesn't exist in the + * layout. + * + * @example + * ```typescript + * await dashboard.updateLayoutNode('CHART-abc123', { width: 6, height: 50 }); + * ``` + */ +export declare function updateLayoutNode( + nodeId: string, + meta: Record<string, unknown>, +): Promise<void>; + +/** + * Gets the current dashboard's custom CSS. + * + * @returns The current CSS string (empty string if unset). + * + * @example + * ```typescript + * const css = dashboard.getCss(); + * ``` + */ +export declare function getCss(): string; + +/** + * Sets the current dashboard's custom CSS. Applies immediately to the + * rendered page — no save required. + * + * @param css The new CSS. + * @returns Promise that resolves once the CSS has been applied. + * @throws If no dashboard is active. + * + * @example + * ```typescript + * await dashboard.setCss('.dashboard-header { background: #f5f5f5; }'); + * ``` + */ +export declare function setCss(css: string): Promise<void>; + +/** + * One filter definition update, as passed to {@link saveFilters}. + */ +export interface FilterConfigUpdate { + /** + * The filter's ID. Must be an existing filter on this dashboard — + * `saveFilters` does not create new ones. Use `getFilters()` to find + * filter IDs. + */ + filterId: string; + + /** + * The filter's new display name. + */ + name?: string; + + /** + * The filter's new target columns/charts. + */ + targets?: unknown; + + /** + * The filter's new default value — applied when the dashboard next loads + * or the filter is reset — in the same shape as a filter's + * `extraFormData`/`filterState`. + */ + defaultDataMask?: { + extraFormData?: Record<string, unknown>; + filterState?: Record<string, unknown>; + }; +} + +/** + * Persists changes to the current dashboard's native filter *definitions* — + * as opposed to {@link updateFilters}, which only changes a filter's + * currently-applied value for this browser session. Changes saved here are + * visible to every future viewer of this dashboard, the same as saving + * through the Filter Bar's "Edit filters" UI. + * + * Does not create new filters or reorder existing ones — only edits or + * deletes filters that already exist on this dashboard. + * + * @param updates The filter definitions to update. + * @param deletedFilterIds IDs of filters to delete. + * @returns Promise that resolves once the changes have been saved and + * applied to this session. + * @throws If no dashboard is active, an update references a filter ID that + * doesn't exist on this dashboard, or the save fails. + * + * @example + * ```typescript + * await dashboard.saveFilters([ + * { filterId: 'NATIVE_FILTER-abc123', name: 'Region (EMEA)' }, + * ]); + * ``` + */ +export declare function saveFilters( + updates: FilterConfigUpdate[], + deletedFilterIds?: string[], +): Promise<void>; + +/** + * Represents one of the current dashboard's native filters, along with its + * currently-applied value. + */ +export interface DashboardFilter { + /** + * The filter's ID. + */ + id: string; + + /** + * The filter's display name. + */ + name?: string; + + /** + * The filter's type, e.g. `'filter_select'`, `'filter_range'`. + */ + filterType?: string; + + /** + * The columns/charts this filter targets. + */ + targets?: unknown; + + /** + * The filter's currently-applied query-modifying payload, if set. + */ + extraFormData?: Record<string, unknown>; + + /** + * The filter's currently-applied UI state (e.g. selected values), if set. + */ + filterState?: Record<string, unknown>; +} + +/** + * Gets the current dashboard's native filters, including their + * currently-applied values. + * + * @returns The dashboard's filters. + * + * @example + * ```typescript + * const filters = dashboard.getFilters(); + * ``` + */ +export declare function getFilters(): DashboardFilter[]; + +/** + * One filter value update, as passed to {@link updateFilters}. + */ +export interface FilterValueUpdate { + /** + * The filter's ID. Use `getFilters()` to find filter IDs. + */ + filterId: string; + + /** + * The new query-modifying payload to apply. + */ + extraFormData?: Record<string, unknown>; + + /** + * The new UI state (e.g. selected values) to apply. + */ + filterState?: Record<string, unknown>; +} + +/** + * Applies new values to one or more of the current dashboard's native + * filters — the same effect as a user changing a value in the Filter Bar. + * Every chart the affected filter(s) target re-queries and re-renders. + * + * @param updates The filter value updates to apply. + * @returns Promise that resolves once the updates have been applied. + * @throws If no dashboard is active. + * + * @example + * ```typescript + * await dashboard.updateFilters([ + * { filterId: 'NATIVE_FILTER-abc123', filterState: { value: ['US'] } }, + * ]); + * ``` + */ +export declare function updateFilters( + updates: FilterValueUpdate[], +): Promise<void>; + +/** + * Re-fetches the given chart's saved configuration from the server and + * re-runs its query with the dashboard's currently-applied filters, + * replacing whatever is currently rendered for it. + * + * Superset does not otherwise learn of a chart's configuration changing + * while its dashboard is open — e.g. a change made through the REST API by + * something other than this dashboard session — until the page is + * reloaded. This re-syncs a single chart without one. + * + * @param chartId The chart's ID. Use `getLayout()` to find chart IDs on + * this dashboard — each `CHART`-type node's `meta.chartId`. + * @returns Promise that resolves once the chart has been re-queried and + * re-rendered with its latest saved configuration. + * @throws If no dashboard is active, the chart isn't on this dashboard, or + * its configuration couldn't be retrieved. + * + * @example + * ```typescript + * await dashboard.refreshChart(123); + * ``` + */ +export declare function refreshChart(chartId: number): Promise<void>; diff --git a/superset-frontend/packages/superset-core/src/explore/index.ts b/superset-frontend/packages/superset-core/src/explore/index.ts new file mode 100644 index 00000000000..e2b6f9939c9 --- /dev/null +++ b/superset-frontend/packages/superset-core/src/explore/index.ts @@ -0,0 +1,148 @@ +/** + * 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. + */ + +/** + * @fileoverview Explore API for Superset extensions. + * + * Exposes the chart currently active on the Explore surface (see + * `navigation.getPage() === 'explore'`) so extensions can identify it and + * read/apply control values, including preview-only customizations that + * don't require saving the chart. + */ + +/** + * Gets the ID of the chart currently loaded in Explore. + * + * @returns The current chart's ID, or undefined if Explore has no chart + * loaded, or the chart is unsaved. + * + * @example + * ```typescript + * const chartId = explore.getChartId(); + * if (chartId != null) { + * console.log(`Chart ID: ${chartId}`); + * } + * ``` + */ +export declare function getChartId(): number | undefined; + +/** + * Gets all current control values (i.e. the chart's form data) for the + * chart currently loaded in Explore. Keys are control names, e.g. + * `viz_type`, `metrics`, `echart_options`. + * + * The returned object is a read-only snapshot: it is a shallow copy, so + * nested values (e.g. `metrics`, `adhoc_filters`) are shared references + * with Explore's internal state. Mutating them directly has no defined + * effect on the rendered chart and may corrupt that state — use + * {@link setControlValues} to apply changes instead. + * + * @returns A map of control name to its current value. + * + * @example + * ```typescript + * const values = explore.getControlValues(); + * console.log(values.viz_type, values.echart_options); + * ``` + */ +export declare function getControlValues(): Record<string, unknown>; + +/** + * Gets the current value of a single control for the chart currently + * loaded in Explore. + * + * @param name The control name, e.g. `echart_options`. + * @returns The control's current value, or undefined if unset. + * + * @example + * ```typescript + * const current = explore.getControlValue('echart_options'); + * ``` + */ +export declare function getControlValue(name: string): unknown; + +/** + * Sets one or more control values on the chart currently loaded in Explore. + * This is the same mechanism the Explore UI itself uses, so controls marked + * `renderTrigger` (e.g. `echart_options`) re-render an instant preview in + * place — no save and no new data query required. + * + * @param values A map of control name to the value it should be set to. + * @returns Promise that resolves once the values have been applied. + * + * @example + * ```typescript + * await explore.setControlValues({ + * echart_options: '{ title: { text: "Revenue" } }', + * }); + * ``` + */ +export declare function setControlValues( + values: Record<string, unknown>, +): Promise<void>; + +/** + * Gets the SQL query(ies) that would run for the chart currently loaded in + * Explore, given its current control values. Does not require the chart to + * have been run first, and does not affect the rendered chart. + * + * @returns One SQL query string per query the chart runs, in the same order + * Explore itself runs them — more than one for a chart type like Mixed + * Timeseries, which runs one query per chart layer. + * @throws If Explore has no chart loaded, or a query could not be generated. + * + * @example + * ```typescript + * const [sql] = await explore.getQuery(); + * ``` + */ +export declare function getQuery(): Promise<string[]>; + +/** + * The tabular result of a chart's data query. + */ +export interface ChartData { + /** + * Column names, in display order. + */ + columns: string[]; + + /** + * Result rows, keyed by column name. + */ + rows: Record<string, unknown>[]; +} + +/** + * Gets the underlying data for the chart currently loaded in Explore, given + * its current control values. Does not require the chart to have been run + * first, and does not affect the rendered chart. + * + * @returns One result (columns/rows) per query the chart runs, in the same + * order Explore itself runs them — more than one for a chart type like + * Mixed Timeseries, which runs one query per chart layer. + * @throws If Explore has no chart loaded, or the data could not be + * retrieved. + * + * @example + * ```typescript + * const [{ columns, rows }] = await explore.getChartData(); + * ``` + */ +export declare function getChartData(): Promise<ChartData[]>; diff --git a/superset-frontend/packages/superset-core/src/index.ts b/superset-frontend/packages/superset-core/src/index.ts index be41ac88a19..7b89db09f87 100644 --- a/superset-frontend/packages/superset-core/src/index.ts +++ b/superset-frontend/packages/superset-core/src/index.ts @@ -20,7 +20,9 @@ export * as common from './common'; export * as authentication from './authentication'; export * as chat from './chat'; export * as commands from './commands'; +export * as dashboard from './dashboard'; export * as editors from './editors'; +export * as explore from './explore'; export * as extensions from './extensions'; export * as menus from './menus'; export * as navigation from './navigation'; diff --git a/superset-frontend/src/components/Chart/chartAction.ts b/superset-frontend/src/components/Chart/chartAction.ts index 40d8ed79b16..d556878d0b4 100644 --- a/superset-frontend/src/components/Chart/chartAction.ts +++ b/superset-frontend/src/components/Chart/chartAction.ts @@ -109,6 +109,7 @@ export const DYNAMIC_PLUGIN_CONTROLS_READY = export const TRIGGER_QUERY = 'TRIGGER_QUERY' as const; export const RENDER_TRIGGERED = 'RENDER_TRIGGERED' as const; export const UPDATE_QUERY_FORM_DATA = 'UPDATE_QUERY_FORM_DATA' as const; +export const SET_CHART_FORM_DATA = 'SET_CHART_FORM_DATA' as const; export const UPDATE_CHART_ID = 'UPDATE_CHART_ID' as const; export const ADD_CHART = 'ADD_CHART' as const; @@ -200,6 +201,12 @@ export interface UpdateQueryFormDataAction { key: string | number; } +export interface SetChartFormDataAction { + type: typeof SET_CHART_FORM_DATA; + formData: QueryFormData; + key: string | number; +} + export interface UpdateChartIdAction { type: typeof UPDATE_CHART_ID; newId: number; @@ -227,6 +234,7 @@ export type ChartAction = | TriggerQueryAction | RenderTriggeredAction | UpdateQueryFormDataAction + | SetChartFormDataAction | UpdateChartIdAction | AddChartAction; @@ -677,6 +685,18 @@ export function updateQueryFormData( return { type: UPDATE_QUERY_FORM_DATA, value, key }; } +// Replaces a chart's own base form_data — the config `getFormDataWithExtraFilters` +// merges dashboard filters into before each query — as opposed to +// `updateQueryFormData`, which only records the fully-merged data from the +// last query that actually ran. Used to pick up a chart's latest saved +// configuration without a full dashboard reload. +export function setChartFormData( + formData: QueryFormData, + key: string | number, +): SetChartFormDataAction { + return { type: SET_CHART_FORM_DATA, formData, key }; +} + // in the sql lab -> explore flow, user can inline edit chart title, // then the chart will be assigned a new slice_id export function updateChartId( diff --git a/superset-frontend/src/components/Chart/chartReducer.ts b/superset-frontend/src/components/Chart/chartReducer.ts index 874c7d18cf8..ae2bcdcf39d 100644 --- a/superset-frontend/src/components/Chart/chartReducer.ts +++ b/superset-frontend/src/components/Chart/chartReducer.ts @@ -138,6 +138,9 @@ export default function chartReducer( [actions.UPDATE_QUERY_FORM_DATA](state) { return { ...state, latestQueryFormData: action.value }; }, + [actions.SET_CHART_FORM_DATA](state) { + return { ...state, form_data: action.formData }; + }, [actions.ANNOTATION_QUERY_STARTED](state) { if (state.annotationQuery?.[action.annotation.name]) { state.annotationQuery[action.annotation.name].abort(); diff --git a/superset-frontend/src/core/dashboard/index.test.ts b/superset-frontend/src/core/dashboard/index.test.ts new file mode 100644 index 00000000000..2476150260d --- /dev/null +++ b/superset-frontend/src/core/dashboard/index.test.ts @@ -0,0 +1,682 @@ +/** + * 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 { SupersetClient, makeApi } from '@superset-ui/core'; +import type { QueryFormData } from '@superset-ui/core'; +import { updateComponents } from 'src/dashboard/actions/dashboardLayout'; +import { dashboardInfoChanged } from 'src/dashboard/actions/dashboardInfo'; +import { applySavedFilterChanges } from 'src/dashboard/actions/nativeFilters'; +import { updateDataMask } from 'src/dataMask/actions'; +import { + setChartFormData, + triggerQuery, +} from 'src/components/Chart/chartAction'; +import { invalidateChartFormDataCache } from 'src/dashboard/util/charts/getFormDataWithExtraFilters'; +import { applyDefaultFormData } from 'src/explore/store'; +import extractUrlParams from 'src/dashboard/util/extractUrlParams'; +import { store } from 'src/views/store'; +import { navigation } from '../navigation'; +import { dashboard } from './index'; + +jest.mock('@superset-ui/core', () => ({ + ...jest.requireActual('@superset-ui/core'), + SupersetClient: { get: jest.fn() }, + makeApi: jest.fn(), +})); + +jest.mock('src/dashboard/actions/dashboardLayout', () => ({ + updateComponents: jest.fn(nextComponents => ({ + type: 'MOCK_UPDATE_COMPONENTS', + nextComponents, + })), +})); + +jest.mock('src/dashboard/actions/dashboardInfo', () => ({ + dashboardInfoChanged: jest.fn(newInfo => ({ + type: 'MOCK_DASHBOARD_INFO_CHANGED', + newInfo, + })), +})); + +jest.mock('src/dashboard/actions/nativeFilters', () => ({ + applySavedFilterChanges: jest.fn(), +})); + +jest.mock('src/dataMask/actions', () => ({ + updateDataMask: jest.fn((filterId, dataMask) => ({ + type: 'MOCK_UPDATE_DATA_MASK', + filterId, + dataMask, + })), +})); + +jest.mock('src/explore/store', () => ({ + applyDefaultFormData: jest.fn(formData => formData), +})); + +jest.mock('src/dashboard/util/extractUrlParams', () => jest.fn(() => ({}))); + +jest.mock('src/dashboard/util/charts/getFormDataWithExtraFilters', () => ({ + invalidateChartFormDataCache: jest.fn(), +})); + +jest.mock('src/views/store', () => ({ + store: { + getState: jest.fn(), + dispatch: jest.fn(), + subscribe: jest.fn(() => jest.fn()), + }, +})); + +jest.mock('../navigation', () => ({ + navigation: { getPage: jest.fn() }, +})); + +const mockGetPage = navigation.getPage as jest.Mock; +const mockGetState = store.getState as jest.Mock; +const mockDispatch = store.dispatch as jest.Mock; +const mockStoreSubscribe = store.subscribe as jest.Mock; +const mockClientGet = SupersetClient.get as jest.Mock; +const mockMakeApi = makeApi as jest.Mock; +const mockApplySavedFilterChanges = applySavedFilterChanges as jest.Mock; +const mockExtractUrlParams = extractUrlParams as jest.Mock; +const mockApplyDefaultFormData = applyDefaultFormData as jest.Mock; +const mockInvalidateChartFormDataCache = + invalidateChartFormDataCache as jest.Mock; + +// isDashboardActive() matches this against `window.location.pathname`'s +// `:idOrSlug`, so a test that activates dashboard N must also route the +// (mocked) browser to it, the way `DashboardPage`'s own URL would. +function activateDashboard( + overrides: Record<string, unknown> = {}, + pathname?: string, +) { + mockGetPage.mockReturnValue('dashboard'); + const dashboardInfo = (overrides.dashboardInfo as + | { id?: number; slug?: string } + | undefined) ?? { id: 7, css: '' }; + window.location.pathname = pathname ?? `/dashboard/${dashboardInfo.id}/`; + mockGetState.mockReturnValue({ + dashboardInfo: { id: 7, css: '' }, + dashboardLayout: { present: {} }, + dashboardState: { activeTabs: [] }, + nativeFilters: { filters: {} }, + dataMask: {}, + charts: {}, + ...overrides, + }); +} + +beforeEach(() => { + jest.clearAllMocks(); + // Reassigning window.location wholesale (rather than just .pathname) + // ensures it's a plain, freely writable object regardless of what a + // previous test left it as. + Object.defineProperty(window, 'location', { + writable: true, + value: { pathname: '/' }, + }); + mockGetPage.mockReturnValue('home'); + mockGetState.mockReturnValue({ + dashboardInfo: {}, + dashboardLayout: { present: {} }, + dashboardState: { activeTabs: [] }, + nativeFilters: { filters: {} }, + dataMask: {}, + charts: {}, + }); + mockExtractUrlParams.mockReturnValue({}); + mockApplyDefaultFormData.mockImplementation(formData => formData); + // clearAllMocks() only clears calls, not a mockImplementation override + // left over from a previous test, so restore the default here. + mockStoreSubscribe.mockImplementation(() => jest.fn()); +}); + +const existingFilter = { + id: 'NATIVE_FILTER-1', + type: 'NATIVE_FILTER', + name: 'Region', + filterType: 'filter_select', + targets: [{ datasetId: 1, column: { name: 'region' } }], + scope: { rootPath: ['ROOT_ID'], excluded: [] }, + controlValues: {}, + cascadeParentIds: [], + defaultDataMask: {}, + description: '', +}; + +test('getDashboardId returns undefined when the dashboard is not active', () => { + expect(dashboard.getDashboardId()).toBeUndefined(); +}); + +test('getDashboardId returns the active dashboard id', () => { + activateDashboard({ dashboardInfo: { id: 42, css: '' } }); + expect(dashboard.getDashboardId()).toBe(42); +}); + +test('getDashboardId is undefined mid-navigation, when the route has moved on to a new dashboard but Redux hasn’t hydrated it yet', () => { + // The browser has already routed to dashboard 8, but dashboardInfo (and + // the other Redux slices) still hold dashboard 5's data until its + // HYDRATE_DASHBOARD completes. + mockGetPage.mockReturnValue('dashboard'); + window.location.pathname = '/dashboard/8/'; + mockGetState.mockReturnValue({ + dashboardInfo: { id: 5, css: '' }, + dashboardLayout: { present: {} }, + dashboardState: { activeTabs: [] }, + nativeFilters: { filters: {} }, + dataMask: {}, + charts: {}, + }); + + expect(dashboard.getDashboardId()).toBeUndefined(); + expect(dashboard.getCss()).toBe(''); + expect(dashboard.getFilters()).toEqual([]); +}); + +test('getDashboardId is active again once dashboardInfo catches up with the routed dashboard', () => { + mockGetPage.mockReturnValue('dashboard'); + window.location.pathname = '/dashboard/8/'; + mockGetState.mockReturnValue({ + dashboardInfo: { id: 8, css: '' }, + dashboardLayout: { present: {} }, + dashboardState: { activeTabs: [] }, + nativeFilters: { filters: {} }, + dataMask: {}, + charts: {}, + }); + + expect(dashboard.getDashboardId()).toBe(8); +}); + +test('getDashboardId matches the routed dashboard by slug', () => { + mockGetPage.mockReturnValue('dashboard'); + window.location.pathname = '/dashboard/my-dashboard-slug/'; + mockGetState.mockReturnValue({ + dashboardInfo: { id: 8, slug: 'my-dashboard-slug', css: '' }, + dashboardLayout: { present: {} }, + dashboardState: { activeTabs: [] }, + nativeFilters: { filters: {} }, + dataMask: {}, + charts: {}, + }); + + expect(dashboard.getDashboardId()).toBe(8); +}); + +test('getLayout returns {} when the dashboard is not active', () => { + expect(dashboard.getLayout()).toEqual({}); +}); + +test('getLayout returns a copy of the current layout when active', () => { + const present = { 'CHART-1': { id: 'CHART-1', meta: { width: 4 } } }; + activateDashboard({ dashboardLayout: { present } }); + const layout = dashboard.getLayout(); + expect(layout).toEqual(present); + expect(layout).not.toBe(present); +}); + +test('getLayout freezes the returned map and every node, including nested meta', () => { + const present = { + 'CHART-1': { + id: 'CHART-1', + children: ['CHART-1-content'], + meta: { width: 4, chartId: 99 }, + }, + }; + activateDashboard({ dashboardLayout: { present } }); + const layout = dashboard.getLayout(); + + expect(Object.isFrozen(layout)).toBe(true); + expect(Object.isFrozen(layout['CHART-1'])).toBe(true); + expect(Object.isFrozen(layout['CHART-1'].meta)).toBe(true); + expect(Object.isFrozen(layout['CHART-1'].children)).toBe(true); + + // A mutation attempt must not leak into the next call's result, and must + // not corrupt the live Redux state it's sharing node references with. + expect(() => { + (layout['CHART-1'].meta as Record<string, unknown>).width = 999; + }).toThrow(TypeError); + expect(present['CHART-1'].meta.width).toBe(4); + expect(dashboard.getLayout()['CHART-1'].meta.width).toBe(4); +}); + +test('getActiveTabs returns [] when the dashboard is not active', () => { + expect(dashboard.getActiveTabs()).toEqual([]); +}); + +test('getActiveTabs returns a copy of the current active tab IDs when active', () => { + const activeTabs = ['TAB-1', 'TAB-1-1']; + activateDashboard({ dashboardState: { activeTabs } }); + const result = dashboard.getActiveTabs(); + expect(result).toEqual(activeTabs); + expect(result).not.toBe(activeTabs); +}); + +test('updateLayoutNode throws when the dashboard is not active', async () => { + await expect( + dashboard.updateLayoutNode('CHART-1', { width: 6 }), + ).rejects.toThrow('No dashboard is currently active'); +}); + +test('updateLayoutNode throws when the node does not exist', async () => { + activateDashboard(); + await expect( + dashboard.updateLayoutNode('CHART-missing', { width: 6 }), + ).rejects.toThrow('Layout node "CHART-missing" not found'); +}); + +test('updateLayoutNode merges meta into the node and preserves other fields', async () => { + const present = { + 'CHART-1': { + id: 'CHART-1', + type: 'CHART', + meta: { width: 4, height: 50, chartId: 99 }, + }, + }; + activateDashboard({ dashboardLayout: { present } }); + + await dashboard.updateLayoutNode('CHART-1', { width: 6 }); + + expect(updateComponents).toHaveBeenCalledWith({ + 'CHART-1': { + id: 'CHART-1', + type: 'CHART', + meta: { width: 6, height: 50, chartId: 99 }, + }, + }); + expect(mockDispatch).toHaveBeenCalledWith({ + type: 'MOCK_UPDATE_COMPONENTS', + nextComponents: { + 'CHART-1': { + id: 'CHART-1', + type: 'CHART', + meta: { width: 6, height: 50, chartId: 99 }, + }, + }, + }); +}); + +test('getCss returns "" when the dashboard is not active', () => { + expect(dashboard.getCss()).toBe(''); +}); + +test('getCss returns the current dashboard css', () => { + activateDashboard({ dashboardInfo: { id: 1, css: '.foo {}' } }); + expect(dashboard.getCss()).toBe('.foo {}'); +}); + +test('setCss throws when the dashboard is not active', async () => { + await expect(dashboard.setCss('.foo {}')).rejects.toThrow( + 'No dashboard is currently active', + ); +}); + +test('setCss dispatches dashboardInfoChanged with the new css', async () => { + activateDashboard(); + await dashboard.setCss('.foo {}'); + expect(dashboardInfoChanged).toHaveBeenCalledWith({ css: '.foo {}' }); + expect(mockDispatch).toHaveBeenCalledWith({ + type: 'MOCK_DASHBOARD_INFO_CHANGED', + newInfo: { css: '.foo {}' }, + }); +}); + +test('getFilters returns [] when the dashboard is not active', () => { + expect(dashboard.getFilters()).toEqual([]); +}); + +test('getFilters returns only native filters, with their current mask values', () => { + activateDashboard({ + nativeFilters: { + filters: { + 'NATIVE_FILTER-1': { + id: 'NATIVE_FILTER-1', + type: 'NATIVE_FILTER', + name: 'Region', + filterType: 'filter_select', + targets: [{ datasetId: 1, column: { name: 'region' } }], + }, + 'DIVIDER-1': { + id: 'DIVIDER-1', + type: 'DIVIDER', + title: 'Section', + }, + }, + }, + dataMask: { + 'NATIVE_FILTER-1': { + extraFormData: { filters: [{ col: 'region', op: 'IN', val: ['US'] }] }, + filterState: { value: ['US'] }, + }, + }, + }); + + expect(dashboard.getFilters()).toEqual([ + { + id: 'NATIVE_FILTER-1', + name: 'Region', + filterType: 'filter_select', + targets: [{ datasetId: 1, column: { name: 'region' } }], + extraFormData: { filters: [{ col: 'region', op: 'IN', val: ['US'] }] }, + filterState: { value: ['US'] }, + }, + ]); +}); + +test('updateFilters throws when the dashboard is not active', async () => { + await expect( + dashboard.updateFilters([{ filterId: 'NATIVE_FILTER-1' }]), + ).rejects.toThrow('No dashboard is currently active'); +}); + +test('updateFilters throws when an update references a filter not on this dashboard, and dispatches nothing', async () => { + activateDashboard({ + nativeFilters: { filters: { 'NATIVE_FILTER-1': existingFilter } }, + }); + + await expect( + dashboard.updateFilters([ + { filterId: 'NATIVE_FILTER-1', filterState: { value: ['US'] } }, + { filterId: 'NATIVE_FILTER-missing', filterState: { value: ['US'] } }, + ]), + ).rejects.toThrow( + 'Filter "NATIVE_FILTER-missing" not found on this dashboard', + ); + expect(updateDataMask).not.toHaveBeenCalled(); +}); + +test('updateFilters only includes the fields the caller supplied', async () => { + activateDashboard({ + nativeFilters: { filters: { 'NATIVE_FILTER-1': existingFilter } }, + }); + + await dashboard.updateFilters([ + { filterId: 'NATIVE_FILTER-1', filterState: { value: ['US'] } }, + ]); + + // A filterState-only update must not clear extraFormData by sending it as + // undefined — that would erase the filter's query-modifying payload. + expect(updateDataMask).toHaveBeenCalledWith('NATIVE_FILTER-1', { + filterState: { value: ['US'] }, + }); +}); + +test('updateFilters applies both extraFormData and filterState when both are supplied', async () => { + activateDashboard({ + nativeFilters: { filters: { 'NATIVE_FILTER-1': existingFilter } }, + }); + + const extraFormData = { filters: [{ col: 'region', op: 'IN', val: ['US'] }] }; + await dashboard.updateFilters([ + { + filterId: 'NATIVE_FILTER-1', + extraFormData, + filterState: { value: ['US'] }, + }, + ]); + + expect(updateDataMask).toHaveBeenCalledWith('NATIVE_FILTER-1', { + extraFormData, + filterState: { value: ['US'] }, + }); +}); + +test('refreshChart throws when the dashboard is not active', async () => { + await expect(dashboard.refreshChart(1)).rejects.toThrow( + 'No dashboard is currently active', + ); +}); + +test('refreshChart throws when the chart is not on the current dashboard', async () => { + activateDashboard({ charts: {} }); + await expect(dashboard.refreshChart(123)).rejects.toThrow( + 'Chart 123 is not on the current dashboard', + ); + expect(mockClientGet).not.toHaveBeenCalled(); +}); + +test("refreshChart throws when the chart's configuration can't be retrieved", async () => { + activateDashboard({ charts: { 123: { id: 123 } } }); + mockClientGet.mockResolvedValue({ json: { result: [] } }); + + await expect(dashboard.refreshChart(123)).rejects.toThrow( + "Could not load chart 123's current configuration", + ); +}); + +test('refreshChart re-fetches, merges url params, and re-queries the chart', async () => { + activateDashboard({ + charts: { 123: { id: 123, chartStatus: 'rendered' } }, + }); + mockClientGet.mockResolvedValue({ + json: { + result: [ + { + id: 123, + form_data: { + slice_id: 123, + viz_type: 'big_number', + url_params: { foo: 'bar' }, + }, + }, + ], + }, + }); + mockExtractUrlParams.mockReturnValue({ standalone: undefined, baz: 'qux' }); + + await dashboard.refreshChart(123); + + expect(mockClientGet).toHaveBeenCalledWith({ + endpoint: '/api/v1/dashboard/7/charts', + }); + const expectedFormData = { + slice_id: 123, + viz_type: 'big_number', + url_params: { foo: 'bar', standalone: undefined, baz: 'qux' }, + }; + expect(applyDefaultFormData).toHaveBeenCalledWith(expectedFormData); + expect(mockInvalidateChartFormDataCache).toHaveBeenCalledWith(123); + // setChartFormData/triggerQuery are plain, real (unmocked) action + // creators here, so calling them again produces the same comparable + // action object dispatch should have received. + expect(mockDispatch).toHaveBeenNthCalledWith( + 1, + setChartFormData(expectedFormData as unknown as QueryFormData, 123), + ); + expect(mockDispatch).toHaveBeenNthCalledWith(2, triggerQuery(true, 123)); + expect(mockDispatch).toHaveBeenCalledTimes(2); +}); + +test('refreshChart waits for the chart to reach a terminal status before resolving', async () => { + const charts: Record<number, { id: number; chartStatus?: string }> = { + 123: { id: 123, chartStatus: 'loading' }, + }; + activateDashboard({ charts }); + mockClientGet.mockResolvedValue({ + json: { + result: [ + { id: 123, form_data: { slice_id: 123, viz_type: 'big_number' } }, + ], + }, + }); + const unsubscribe = jest.fn(); + mockStoreSubscribe.mockImplementation(listener => { + // Redux never calls a listener synchronously from within subscribe() + // itself — only after some later dispatch — so defer this like a real + // store would, to also exercise the returned unsubscribe being called. + queueMicrotask(() => { + charts[123].chartStatus = 'rendered'; + listener(); + }); + return unsubscribe; + }); + + await dashboard.refreshChart(123); + + expect(mockStoreSubscribe).toHaveBeenCalled(); + expect(unsubscribe).toHaveBeenCalled(); +}); + +test('saveFilters throws when the dashboard is not active', async () => { + await expect( + dashboard.saveFilters([{ filterId: 'NATIVE_FILTER-1', name: 'EMEA' }]), + ).rejects.toThrow('No dashboard is currently active'); +}); + +test('saveFilters throws when an update references a filter not on this dashboard', async () => { + activateDashboard({ nativeFilters: { filters: {} } }); + + await expect( + dashboard.saveFilters([{ filterId: 'NATIVE_FILTER-missing', name: 'x' }]), + ).rejects.toThrow( + 'Filter "NATIVE_FILTER-missing" not found on this dashboard', + ); + expect(mockMakeApi).not.toHaveBeenCalled(); +}); + +test('saveFilters is a no-op when there are no updates or deletions', async () => { + activateDashboard({ + nativeFilters: { filters: { 'NATIVE_FILTER-1': existingFilter } }, + }); + + await dashboard.saveFilters([]); + + expect(mockMakeApi).not.toHaveBeenCalled(); + expect(mockApplySavedFilterChanges).not.toHaveBeenCalled(); +}); + +test('saveFilters merges partial updates onto the existing filter and persists them', async () => { + activateDashboard({ + nativeFilters: { filters: { 'NATIVE_FILTER-1': existingFilter } }, + }); + const savedFilter = { + ...existingFilter, + name: 'Region (EMEA)', + // The server's copy of scope fields, which the save call itself never + // sent — these must be stripped before the result reaches Redux so + // this session's own calculateScopes output isn't clobbered. + chartsInScope: [1, 2], + tabsInScope: ['TAB-1'], + }; + const mockRequest = jest.fn().mockResolvedValue({ result: [savedFilter] }); + mockMakeApi.mockReturnValue(mockRequest); + + await dashboard.saveFilters([ + { filterId: 'NATIVE_FILTER-1', name: 'Region (EMEA)' }, + ]); + + expect(mockMakeApi).toHaveBeenCalledWith({ + method: 'PUT', + endpoint: '/api/v1/dashboard/7/filters', + }); + const expectedFilterChanges = { + modified: [{ ...existingFilter, name: 'Region (EMEA)' }], + deleted: [], + reordered: [], + }; + expect(mockRequest).toHaveBeenCalledWith(expectedFilterChanges); + expect(mockApplySavedFilterChanges).toHaveBeenCalledWith( + store.dispatch, + expectedFilterChanges, + [savedFilter], + { 'NATIVE_FILTER-1': existingFilter }, + ); +}); + +test('saveFilters restores a filter’s live value after a save that only changed its name', async () => { + // A prior updateFilters() call left a session-only value live in + // dataMask, which applySavedFilterChanges would otherwise wipe back to + // the filter's default since this save includes it in `modified`. + activateDashboard({ + nativeFilters: { filters: { 'NATIVE_FILTER-1': existingFilter } }, + dataMask: { + 'NATIVE_FILTER-1': { + extraFormData: { filters: [{ col: 'region', op: 'IN', val: ['US'] }] }, + filterState: { value: ['US'] }, + }, + }, + }); + const mockRequest = jest + .fn() + .mockResolvedValue({ result: [{ ...existingFilter, name: 'EMEA' }] }); + mockMakeApi.mockReturnValue(mockRequest); + + await dashboard.saveFilters([{ filterId: 'NATIVE_FILTER-1', name: 'EMEA' }]); + + expect(updateDataMask).toHaveBeenCalledWith('NATIVE_FILTER-1', { + extraFormData: { filters: [{ col: 'region', op: 'IN', val: ['US'] }] }, + filterState: { value: ['US'] }, + }); +}); + +test('saveFilters does not restore a live value when the update explicitly sets a new default', async () => { + activateDashboard({ + nativeFilters: { filters: { 'NATIVE_FILTER-1': existingFilter } }, + dataMask: { + 'NATIVE_FILTER-1': { + extraFormData: { filters: [{ col: 'region', op: 'IN', val: ['US'] }] }, + filterState: { value: ['US'] }, + }, + }, + }); + const mockRequest = jest.fn().mockResolvedValue({ result: [existingFilter] }); + mockMakeApi.mockReturnValue(mockRequest); + + await dashboard.saveFilters([ + { filterId: 'NATIVE_FILTER-1', defaultDataMask: { filterState: {} } }, + ]); + + expect(updateDataMask).not.toHaveBeenCalled(); +}); + +test('saveFilters supports deleting filters without any modifications', async () => { + activateDashboard({ + nativeFilters: { filters: { 'NATIVE_FILTER-1': existingFilter } }, + }); + const mockRequest = jest.fn().mockResolvedValue({ result: [] }); + mockMakeApi.mockReturnValue(mockRequest); + + await dashboard.saveFilters([], ['NATIVE_FILTER-1']); + + const expectedFilterChanges = { + modified: [], + deleted: ['NATIVE_FILTER-1'], + reordered: [], + }; + expect(mockRequest).toHaveBeenCalledWith(expectedFilterChanges); + expect(mockApplySavedFilterChanges).toHaveBeenCalledWith( + store.dispatch, + expectedFilterChanges, + [], + { 'NATIVE_FILTER-1': existingFilter }, + ); +}); + +test('saveFilters propagates a failed save instead of swallowing it', async () => { + activateDashboard({ + nativeFilters: { filters: { 'NATIVE_FILTER-1': existingFilter } }, + }); + const mockRequest = jest.fn().mockRejectedValue(new Error('network error')); + mockMakeApi.mockReturnValue(mockRequest); + + await expect( + dashboard.saveFilters([{ filterId: 'NATIVE_FILTER-1', name: 'x' }]), + ).rejects.toThrow('network error'); + expect(mockApplySavedFilterChanges).not.toHaveBeenCalled(); +}); diff --git a/superset-frontend/src/core/dashboard/index.ts b/superset-frontend/src/core/dashboard/index.ts new file mode 100644 index 00000000000..fefac0dcd8b --- /dev/null +++ b/superset-frontend/src/core/dashboard/index.ts @@ -0,0 +1,303 @@ +/** + * 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 { dashboard as dashboardApi } from '@apache-superset/core'; +import { isNativeFilter, makeApi, SupersetClient } from '@superset-ui/core'; +import type { + DataMask, + Divider, + Filter, + JsonObject, + QueryFormData, +} from '@superset-ui/core'; +import { matchPath } from 'react-router-dom'; +import { updateComponents } from 'src/dashboard/actions/dashboardLayout'; +import { dashboardInfoChanged } from 'src/dashboard/actions/dashboardInfo'; +import { applySavedFilterChanges } from 'src/dashboard/actions/nativeFilters'; +import type { SaveFilterChangesType } from 'src/dashboard/components/nativeFilters/FiltersConfigModal/types'; +import { updateDataMask } from 'src/dataMask/actions'; +import { + setChartFormData, + triggerQuery, +} from 'src/components/Chart/chartAction'; +import { invalidateChartFormDataCache } from 'src/dashboard/util/charts/getFormDataWithExtraFilters'; +import { applyDefaultFormData } from 'src/explore/store'; +import extractUrlParams from 'src/dashboard/util/extractUrlParams'; +import { RoutePaths } from 'src/views/routePaths'; +import { store, RootState } from 'src/views/store'; +import { navigation } from '../navigation'; + +const getState = () => store.getState() as RootState; + +// The `:idOrSlug` the browser's current URL routes to, read directly off +// `window.location` rather than from React Router context, matching how +// `navigation`'s own page derivation works. +const getRoutedIdOrSlug = (): string | undefined => + matchPath<{ idOrSlug: string }>(window.location.pathname, { + path: RoutePaths.DASHBOARD, + exact: false, + })?.params.idOrSlug; + +// `dashboardInfo`/`dashboardLayout`/`nativeFilters`/`charts` are retained +// across an in-SPA navigation, and even once the browser has routed to a new +// dashboard, they keep the *previous* dashboard's data until that +// dashboard's HYDRATE_DASHBOARD completes. Comparing the URL's `idOrSlug` +// against `dashboardInfo`'s own id/slug — rather than trusting the page type +// alone — closes this window: they only match again once hydration has +// actually caught up, including for a same-surface dashboard-to-dashboard +// navigation. +const isDashboardActive = (): boolean => { + if (navigation.getPage() !== 'dashboard') return false; + const idOrSlug = getRoutedIdOrSlug(); + if (idOrSlug == null) return false; + const { id, slug } = getState().dashboardInfo; + return id != null && (String(id) === idOrSlug || slug === idOrSlug); +}; + +const requireDashboardId = (): number => { + const { id } = getState().dashboardInfo; + if (!isDashboardActive() || id == null) { + throw new Error('No dashboard is currently active'); + } + return id; +}; + +const getDashboardId: typeof dashboardApi.getDashboardId = () => + isDashboardActive() ? (getState().dashboardInfo.id ?? undefined) : undefined; + +// Every dashboardLayout reducer treats a node (and its `meta`/`children`/ +// `parents`) as immutable already — an update always replaces it with a new +// object rather than mutating it in place — so freezing the copy below only +// makes that existing contract explicit, without risking a reducer's own +// future in-place update on a node returned from here. +function deepFreeze<T>(value: T): T { + if (value !== null && typeof value === 'object' && !Object.isFrozen(value)) { + Object.freeze(value); + Object.values(value as Record<string, unknown>).forEach(deepFreeze); + } + return value; +} + +const getLayout: typeof dashboardApi.getLayout = () => + isDashboardActive() + ? deepFreeze({ ...getState().dashboardLayout.present }) + : {}; + +const getActiveTabs: typeof dashboardApi.getActiveTabs = () => + isDashboardActive() ? [...(getState().dashboardState.activeTabs ?? [])] : []; + +const updateLayoutNode: typeof dashboardApi.updateLayoutNode = async ( + nodeId: string, + meta: Record<string, unknown>, +) => { + requireDashboardId(); + const node = getState().dashboardLayout.present[nodeId]; + if (!node) { + throw new Error(`Layout node "${nodeId}" not found`); + } + // UPDATE_COMPONENTS replaces each keyed entry wholesale (it's not a deep + // merge), so the node's other fields must be carried through alongside + // the merged meta. + store.dispatch( + updateComponents({ + [nodeId]: { ...node, meta: { ...node.meta, ...meta } }, + }) as any, + ); +}; + +const getCss: typeof dashboardApi.getCss = () => + isDashboardActive() ? (getState().dashboardInfo.css ?? '') : ''; + +const setCss: typeof dashboardApi.setCss = async (css: string) => { + requireDashboardId(); + store.dispatch(dashboardInfoChanged({ css })); +}; + +const getFilters: typeof dashboardApi.getFilters = () => { + if (!isDashboardActive()) return []; + const { nativeFilters, dataMask } = getState(); + const filterElements = Object.values(nativeFilters.filters) as Array< + Filter | Divider + >; + return filterElements.filter(isNativeFilter).map(filter => { + const mask = dataMask[filter.id]; + return { + id: filter.id, + name: filter.name, + filterType: filter.filterType, + targets: filter.targets, + extraFormData: mask?.extraFormData, + filterState: mask?.filterState, + }; + }); +}; + +const updateFilters: typeof dashboardApi.updateFilters = async ( + updates: dashboardApi.FilterValueUpdate[], +) => { + requireDashboardId(); + const { filters: currentFilters } = getState().nativeFilters; + updates.forEach(({ filterId }) => { + if (!currentFilters[filterId]) { + throw new Error(`Filter "${filterId}" not found on this dashboard`); + } + }); + updates.forEach(({ filterId, extraFormData, filterState }) => { + const dataMask: DataMask = {}; + if (extraFormData !== undefined) { + dataMask.extraFormData = extraFormData; + } + if (filterState !== undefined) { + dataMask.filterState = filterState; + } + store.dispatch(updateDataMask(filterId, dataMask)); + }); +}; + +const saveFilters: typeof dashboardApi.saveFilters = async ( + updates: dashboardApi.FilterConfigUpdate[], + deletedFilterIds: string[] = [], +) => { + const dashboardId = requireDashboardId(); + const { filters: currentFilters } = getState().nativeFilters; + const { dataMask: currentDataMask } = getState(); + + const modified = updates.map( + ({ filterId, name, targets, defaultDataMask }) => { + const existing = currentFilters[filterId]; + if (!existing) { + throw new Error(`Filter "${filterId}" not found on this dashboard`); + } + return { + ...existing, + ...(name !== undefined && { name }), + ...(targets !== undefined && { targets }), + ...(defaultDataMask !== undefined && { defaultDataMask }), + }; + }, + ) as SaveFilterChangesType['modified']; + + if (modified.length === 0 && deletedFilterIds.length === 0) { + return; + } + + const filterChanges: SaveFilterChangesType = { + modified, + deleted: deletedFilterIds, + reordered: [], + }; + + const putFilters = makeApi<SaveFilterChangesType, { result: Filter[] }>({ + method: 'PUT', + endpoint: `/api/v1/dashboard/${dashboardId}/filters`, + }); + const response = await putFilters(filterChanges); + applySavedFilterChanges( + store.dispatch, + filterChanges, + response.result, + currentFilters, + ); + + // applySavedFilterChanges resets each modified filter's live extraFormData/ + // filterState back to its default unless the filter is required or + // defaultToFirstItem (see updateDataMaskForFilterChanges), even for an + // update that never touched the filter's value (e.g. a rename). Restore + // whatever was live immediately before this save for any filter whose + // update didn't explicitly set a new one. + updates.forEach(({ filterId, defaultDataMask }) => { + if (defaultDataMask !== undefined) return; + const liveMask = currentDataMask[filterId]; + if (!liveMask) return; + store.dispatch( + updateDataMask(filterId, { + extraFormData: liveMask.extraFormData, + filterState: liveMask.filterState, + }), + ); + }); +}; + +// TRIGGER_QUERY synchronously sets chartStatus to 'loading'; the mounted +// Chart component then re-queries and re-renders it, landing on one of +// these terminal statuses (see chartReducer.ts). +const isTerminalChartStatus = (status: string | null | undefined): boolean => + status === 'rendered' || status === 'failed' || status === 'stopped'; + +const waitForChartRefresh = (chartId: number): Promise<void> => + new Promise(resolve => { + if (isTerminalChartStatus(getState().charts[chartId]?.chartStatus)) { + resolve(); + return; + } + const unsubscribe = store.subscribe(() => { + if (isTerminalChartStatus(getState().charts[chartId]?.chartStatus)) { + unsubscribe(); + resolve(); + } + }); + }); + +const refreshChart: typeof dashboardApi.refreshChart = async ( + chartId: number, +) => { + const dashboardId = requireDashboardId(); + if (!getState().charts[chartId]) { + throw new Error(`Chart ${chartId} is not on the current dashboard`); + } + + const { json } = await SupersetClient.get({ + endpoint: `/api/v1/dashboard/${dashboardId}/charts`, + }); + const chartEntity = ( + json?.result as { id: number; form_data?: JsonObject }[] + )?.find(({ id }) => id === chartId); + if (!chartEntity?.form_data) { + throw new Error(`Could not load chart ${chartId}'s current configuration`); + } + + const formData = applyDefaultFormData({ + ...chartEntity.form_data, + url_params: { + ...(chartEntity.form_data.url_params as JsonObject), + ...extractUrlParams('regular'), + }, + } as Parameters<typeof applyDefaultFormData>[0]) as QueryFormData; + + // getFormDataWithExtraFilters's per-chart cache only invalidates on + // dataMask/nativeFilters/filters/color/customization changes, not on the + // chart's own form_data, so it must be evicted explicitly or the query + // triggered below can run with the previous configuration. + invalidateChartFormDataCache(chartId); + store.dispatch(setChartFormData(formData, chartId)); + store.dispatch(triggerQuery(true, chartId)); + await waitForChartRefresh(chartId); +}; + +export const dashboard: typeof dashboardApi = { + getDashboardId, + getLayout, + getActiveTabs, + updateLayoutNode, + getCss, + setCss, + getFilters, + updateFilters, + saveFilters, + refreshChart, +}; diff --git a/superset-frontend/src/core/explore/index.test.ts b/superset-frontend/src/core/explore/index.test.ts new file mode 100644 index 00000000000..0601f1e4a03 --- /dev/null +++ b/superset-frontend/src/core/explore/index.test.ts @@ -0,0 +1,320 @@ +/** + * 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 { setControlValue } from 'src/explore/actions/exploreActions'; +import { requestChartDataResolved } from 'src/components/Chart/chartAction'; +import { store } from 'src/views/store'; +import { navigation } from '../navigation'; +import { explore } from './index'; + +jest.mock('src/explore/actions/exploreActions', () => ({ + setControlValue: jest.fn((controlName, value, validationErrors, options) => ({ + type: 'MOCK_SET_FIELD_VALUE', + controlName, + value, + validationErrors, + programmatic: options?.programmatic ?? false, + })), +})); + +// requestChartDataResolved already resolves an async 202 job envelope into +// its final result array (see its own coverage in chartActions.test.ts), so +// this module only needs to mock that resolved boundary. +jest.mock('src/components/Chart/chartAction', () => ({ + requestChartDataResolved: jest.fn(), +})); + +jest.mock('src/views/store', () => ({ + store: { + getState: jest.fn(), + dispatch: jest.fn(), + }, +})); + +jest.mock('../navigation', () => ({ + navigation: { getPage: jest.fn() }, +})); + +const mockGetPage = navigation.getPage as jest.Mock; +const mockGetState = store.getState as jest.Mock; +const mockDispatch = store.dispatch as jest.Mock; +const mockRequestChartDataResolved = requestChartDataResolved as jest.Mock; + +// Mirrors the current controls Explore would be querying with, matching the +// pre-normalization `form_data` slice used elsewhere in these tests. +const defaultControls = { + datasource: { value: '1__table' }, + viz_type: { value: 'echarts_timeseries_bar' }, +}; + +function activateExplore(overrides: Record<string, unknown> = {}) { + mockGetPage.mockReturnValue('explore'); + mockGetState.mockReturnValue({ + explore: { + slice: { slice_id: 55 }, + form_data: { datasource: '1__table', viz_type: 'echarts_timeseries_bar' }, + controls: defaultControls, + ...overrides, + }, + dataMask: {}, + }); +} + +beforeEach(() => { + jest.clearAllMocks(); + mockGetPage.mockReturnValue('dashboard'); + mockGetState.mockReturnValue({ explore: {}, dataMask: {} }); +}); + +test('getChartId returns undefined when Explore is not the active surface', () => { + expect(explore.getChartId()).toBeUndefined(); +}); + +test('getChartId returns the loaded chart id when active', () => { + activateExplore(); + expect(explore.getChartId()).toBe(55); +}); + +test('getChartId returns undefined when active but no chart is loaded', () => { + activateExplore({ slice: undefined }); + expect(explore.getChartId()).toBeUndefined(); +}); + +test('getControlValues returns {} when Explore is not the active surface', () => { + expect(explore.getControlValues()).toEqual({}); +}); + +test('getControlValues returns a copy of the current form_data', () => { + const formData = { datasource: '1__table', viz_type: 'big_number' }; + activateExplore({ form_data: formData }); + const values = explore.getControlValues(); + expect(values).toEqual(formData); + expect(values).not.toBe(formData); +}); + +test('getControlValue returns undefined when Explore is not the active surface', () => { + expect(explore.getControlValue('viz_type')).toBeUndefined(); +}); + +test("getControlValue returns a single control's current value", () => { + activateExplore({ + form_data: { datasource: '1__table', viz_type: 'big_number' }, + }); + expect(explore.getControlValue('viz_type')).toBe('big_number'); + expect(explore.getControlValue('nonexistent')).toBeUndefined(); +}); + +test('setControlValues throws when Explore is not the active surface', async () => { + await expect( + explore.setControlValues({ echart_options: '{}' }), + ).rejects.toThrow('No chart is currently loaded in Explore'); +}); + +test('setControlValues throws when active but no chart is loaded (no datasource)', async () => { + activateExplore({ controls: {} }); + await expect( + explore.setControlValues({ echart_options: '{}' }), + ).rejects.toThrow('No chart is currently loaded in Explore'); +}); + +test('setControlValues dispatches a programmatic setControlValue per entry', async () => { + activateExplore(); + + await explore.setControlValues({ + echart_options: '{ "title": "Revenue" }', + row_limit: 100, + }); + + expect(setControlValue).toHaveBeenCalledWith( + 'echart_options', + '{ "title": "Revenue" }', + undefined, + { programmatic: true }, + ); + expect(setControlValue).toHaveBeenCalledWith('row_limit', 100, undefined, { + programmatic: true, + }); + expect(mockDispatch).toHaveBeenCalledTimes(2); +}); + +test('getQuery throws when Explore is not the active surface', async () => { + await expect(explore.getQuery()).rejects.toThrow( + 'No chart is currently loaded in Explore', + ); +}); + +test('getQuery builds form data from the current controls, not stale form_data', async () => { + activateExplore({ + // A control the reducer's form_data hasn't caught up with yet (e.g. + // filled in by a default or by mapStateToProps) should still be used. + controls: { ...defaultControls, row_limit: { value: 100 } }, + form_data: { datasource: '1__table', viz_type: 'echarts_timeseries_bar' }, + }); + mockRequestChartDataResolved.mockResolvedValue([{ query: 'SELECT 1' }]); + + const sql = await explore.getQuery(); + + expect(sql).toEqual(['SELECT 1']); + expect(mockRequestChartDataResolved).toHaveBeenCalledWith({ + formData: { + datasource: '1__table', + viz_type: 'echarts_timeseries_bar', + row_limit: 100, + }, + resultFormat: 'json', + resultType: 'query', + ownState: undefined, + }); +}); + +test('getQuery includes the chart’s current ownState (e.g. table pagination)', async () => { + activateExplore(); + mockGetState.mockReturnValue({ + explore: { + slice: { slice_id: 55 }, + controls: defaultControls, + }, + dataMask: { 55: { ownState: { currentPage: 2 } } }, + }); + mockRequestChartDataResolved.mockResolvedValue([{ query: 'SELECT 1' }]); + + await explore.getQuery(); + + expect(mockRequestChartDataResolved).toHaveBeenCalledWith( + expect.objectContaining({ ownState: { currentPage: 2 } }), + ); +}); + +test('getQuery throws when the result has an error', async () => { + activateExplore(); + mockRequestChartDataResolved.mockResolvedValue([{ error: 'boom' }]); + + await expect(explore.getQuery()).rejects.toThrow('boom'); +}); + +test('getQuery throws a fallback message when no result comes back', async () => { + activateExplore(); + mockRequestChartDataResolved.mockResolvedValue([]); + + await expect(explore.getQuery()).rejects.toThrow( + 'No query result was returned', + ); +}); + +test('getQuery returns one query string per query the chart runs', async () => { + activateExplore(); + mockRequestChartDataResolved.mockResolvedValue([ + { query: 'SELECT 1' }, + { query: 'SELECT 2' }, + ]); + + await expect(explore.getQuery()).resolves.toEqual(['SELECT 1', 'SELECT 2']); +}); + +test('getQuery throws if any of the chart’s queries errored, even if others succeeded', async () => { + activateExplore(); + mockRequestChartDataResolved.mockResolvedValue([ + { query: 'SELECT 1' }, + { error: 'boom' }, + ]); + + await expect(explore.getQuery()).rejects.toThrow('boom'); +}); + +test('getChartData throws when Explore is not the active surface', async () => { + await expect(explore.getChartData()).rejects.toThrow( + 'No chart is currently loaded in Explore', + ); +}); + +test('getChartData requests a full result and returns columns/rows', async () => { + activateExplore(); + mockRequestChartDataResolved.mockResolvedValue([ + { colnames: ['region', 'sales'], data: [{ region: 'US', sales: 100 }] }, + ]); + + const data = await explore.getChartData(); + + expect(data).toEqual([ + { + columns: ['region', 'sales'], + rows: [{ region: 'US', sales: 100 }], + }, + ]); + expect(mockRequestChartDataResolved).toHaveBeenCalledWith({ + formData: { datasource: '1__table', viz_type: 'echarts_timeseries_bar' }, + resultFormat: 'json', + resultType: 'full', + ownState: undefined, + }); +}); + +test('getChartData includes the chart’s current ownState (e.g. table pagination)', async () => { + activateExplore(); + mockGetState.mockReturnValue({ + explore: { + slice: { slice_id: 55 }, + controls: defaultControls, + }, + dataMask: { 55: { ownState: { currentPage: 2 } } }, + }); + mockRequestChartDataResolved.mockResolvedValue([{ colnames: [], data: [] }]); + + await explore.getChartData(); + + expect(mockRequestChartDataResolved).toHaveBeenCalledWith( + expect.objectContaining({ ownState: { currentPage: 2 } }), + ); +}); + +test('getChartData defaults columns and rows to [] when the result omits them', async () => { + activateExplore(); + mockRequestChartDataResolved.mockResolvedValue([{}]); + + expect(await explore.getChartData()).toEqual([{ columns: [], rows: [] }]); +}); + +test('getChartData throws when the result has an error', async () => { + activateExplore(); + mockRequestChartDataResolved.mockResolvedValue([{ error: 'boom' }]); + + await expect(explore.getChartData()).rejects.toThrow('boom'); +}); + +test('getChartData returns one result per query the chart runs', async () => { + activateExplore(); + mockRequestChartDataResolved.mockResolvedValue([ + { colnames: ['a'], data: [{ a: 1 }] }, + { colnames: ['b'], data: [{ b: 2 }] }, + ]); + + await expect(explore.getChartData()).resolves.toEqual([ + { columns: ['a'], rows: [{ a: 1 }] }, + { columns: ['b'], rows: [{ b: 2 }] }, + ]); +}); + +test('getChartData throws if any of the chart’s queries errored, even if others succeeded', async () => { + activateExplore(); + mockRequestChartDataResolved.mockResolvedValue([ + { colnames: ['a'], data: [] }, + { error: 'boom' }, + ]); + + await expect(explore.getChartData()).rejects.toThrow('boom'); +}); diff --git a/superset-frontend/src/core/explore/index.ts b/superset-frontend/src/core/explore/index.ts new file mode 100644 index 00000000000..8a8afc398da --- /dev/null +++ b/superset-frontend/src/core/explore/index.ts @@ -0,0 +1,151 @@ +/** + * 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 { explore as exploreApi } from '@apache-superset/core'; +import type { + ChartDataResponseResult, + JsonObject, + QueryFormData, +} from '@superset-ui/core'; +import { setControlValue } from 'src/explore/actions/exploreActions'; +import { getVisibleFormDataFromControls } from 'src/explore/controlUtils'; +import { requestChartDataResolved } from 'src/components/Chart/chartAction'; +import { store, RootState } from 'src/views/store'; +import { navigation } from '../navigation'; + +const getExploreState = () => (store.getState() as RootState).explore; + +// The Redux slice below is retained across an in-SPA navigation, so +// checking it alone can't tell a still-active chart from a stale one left +// over from before the user navigated to another page. +const isExploreActive = (): boolean => navigation.getPage() === 'explore'; + +const getChartId: typeof exploreApi.getChartId = () => + isExploreActive() + ? (getExploreState().slice?.slice_id ?? undefined) + : undefined; + +const getControlValues: typeof exploreApi.getControlValues = () => + isExploreActive() + ? { ...(getExploreState().form_data as Record<string, unknown>) } + : {}; + +const getControlValue: typeof exploreApi.getControlValue = (name: string) => + isExploreActive() + ? (getExploreState().form_data as Record<string, unknown>)[name] + : undefined; + +// Explore queries with the form data derived from the current `controls` +// state (see ExploreViewContainer's mapStateToProps), not the pre-normalization +// `form_data` slice, which can lag behind controls filled in by defaults or by +// mapStateToProps. Sharing getVisibleFormDataFromControls with +// mapStateToProps keeps getQuery()/getChartData() describing the same data +// as the chart on screen. +const getCurrentFormData = (): QueryFormData => { + const { controls, hiddenFormData } = getExploreState(); + return getVisibleFormDataFromControls(controls ?? {}, hiddenFormData); +}; + +// Mirrors the currently-applied server-side state (e.g. a Table chart's +// current page) that Explore keeps in `dataMask` rather than `form_data`, so +// a paginated chart's exported query/data matches what's on screen. +const getOwnState = (chartId: number | undefined): JsonObject | undefined => + chartId != null + ? (store.getState() as RootState).dataMask[chartId]?.ownState + : undefined; + +const requireFormData = (): QueryFormData => { + const formData = getCurrentFormData(); + if (!isExploreActive() || !formData?.datasource) { + throw new Error('No chart is currently loaded in Explore'); + } + return formData; +}; + +const setControlValues: typeof exploreApi.setControlValues = async ( + values: Record<string, unknown>, +) => { + requireFormData(); + Object.entries(values).forEach(([controlName, value]) => { + store.dispatch( + setControlValue(controlName, value, undefined, { programmatic: true }), + ); + }); +}; + +// A chart type like Mixed Timeseries can issue more than one query — one +// entry below per query, in the same order Explore itself runs them. +const getQueryResults = ( + results: ChartDataResponseResult[] | undefined, +): ChartDataResponseResult[] => { + if (!results || results.length === 0) { + throw new Error('No query result was returned'); + } + // This API has no way to represent a partial failure (some queries + // succeeding, others not) in its all-strings/all-rows return shape, so + // any one query erroring fails the whole call. + const erroredResult = results.find(result => result.error); + if (erroredResult) { + throw new Error(erroredResult.error ?? 'Failed to run the query'); + } + return results; +}; + +const getQuery: typeof exploreApi.getQuery = async () => { + const formData = requireFormData(); + // With GLOBAL_ASYNC_QUERIES enabled, an uncached request comes back as a + // 202 job envelope rather than the result array, so this must go through + // the same async-response handling as a normal chart query. + const results = await requestChartDataResolved({ + formData, + resultFormat: 'json', + resultType: 'query', + ownState: getOwnState(getChartId()), + }); + return getQueryResults(results as ChartDataResponseResult[] | undefined).map( + result => result.query, + ); +}; + +const getChartData: typeof exploreApi.getChartData = async () => { + const formData = requireFormData(); + // With GLOBAL_ASYNC_QUERIES enabled, an uncached request comes back as a + // 202 job envelope rather than the result array, so this must go through + // the same async-response handling as a normal chart query. + const results = await requestChartDataResolved({ + formData, + resultFormat: 'json', + resultType: 'full', + ownState: getOwnState(getChartId()), + }); + return getQueryResults(results as ChartDataResponseResult[] | undefined).map( + result => ({ + columns: result.colnames ?? [], + rows: (result.data ?? []) as Record<string, unknown>[], + }), + ); +}; + +export const explore: typeof exploreApi = { + getChartId, + getControlValues, + getControlValue, + setControlValues, + getQuery, + getChartData, +}; diff --git a/superset-frontend/src/core/index.ts b/superset-frontend/src/core/index.ts index ce5b7f4d505..02bd4b3c6a8 100644 --- a/superset-frontend/src/core/index.ts +++ b/superset-frontend/src/core/index.ts @@ -29,7 +29,9 @@ export const core: typeof coreType = { export * from './authentication'; export * from './chat'; export * from './commands'; +export * from './dashboard'; export * from './editors'; +export * from './explore'; export * from './extensions'; export * from './menus'; export * from './models'; diff --git a/superset-frontend/src/dashboard/actions/dashboardState.ts b/superset-frontend/src/dashboard/actions/dashboardState.ts index af5f7707dec..02cefc5fe1d 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.ts +++ b/superset-frontend/src/dashboard/actions/dashboardState.ts @@ -30,6 +30,7 @@ import { promiseTimeout, JsonObject, } from '@superset-ui/core'; +import type { QueryFormData, AgGridChartState } from '@superset-ui/core'; import { addChart, removeChart, @@ -63,7 +64,6 @@ import { navigateWithState, navigateTo } from 'src/utils/navigationUtils'; import type { AnyAction } from 'redux'; import type { ThunkDispatch } from 'redux-thunk'; import { ResourceStatus } from 'src/hooks/apiResources/apiResources'; -import type { AgGridChartState } from '@superset-ui/core'; import type { DashboardChartStates } from 'src/dashboard/types/chartState'; import { UPDATE_COMPONENTS_PARENTS_LIST } from './dashboardLayout'; import { @@ -895,7 +895,7 @@ export function addSliceToDashboard( id, form_data: applyDefaultFormData( form_data as Parameters<typeof applyDefaultFormData>[0], - ), + ) as QueryFormData, }; return Promise.all([ diff --git a/superset-frontend/src/dashboard/actions/nativeFilters.ts b/superset-frontend/src/dashboard/actions/nativeFilters.ts index af3d711495a..ce28b66718e 100644 --- a/superset-frontend/src/dashboard/actions/nativeFilters.ts +++ b/superset-frontend/src/dashboard/actions/nativeFilters.ts @@ -68,6 +68,37 @@ const isFilterChangesEmpty = (filterChanges: SaveFilterChangesType) => array => Array.isArray(array) && !array.length, ); +/** + * Applies a successfully-persisted native filter save to Redux: the + * config change itself, the dashboardInfo copy the filter modal reads, and + * the resulting dataMask changes (e.g. a new default value taking effect + * immediately). Shared by `setFilterConfiguration` (the Filter Bar's "Edit + * filters" save flow) and the extension API's `dashboard.saveFilters`, so + * the two save paths can't silently diverge. + */ +export const applySavedFilterChanges = ( + dispatch: Dispatch, + filterChanges: SaveFilterChangesType, + savedFilters: Filter[], + oldFilters?: Filters, +) => { + // chartsInScope/tabsInScope are derived from the live layout, and the + // response carries the persisted copy for every filter - including the + // ones this save never touched, whose copy is whatever was stored when + // the dashboard was last saved. Dropping them lets the reducers keep the + // scopes calculateScopes already computed for this session. + const cleanedFilters = savedFilters.map( + filter => omit(filter, ['chartsInScope', 'tabsInScope']) as Filter, + ); + dispatch({ + type: SET_NATIVE_FILTERS_CONFIG_COMPLETE, + filterChanges: cleanedFilters, + deletedIds: filterChanges.deleted, + }); + dispatch(nativeFiltersConfigChanged(cleanedFilters)); + dispatch(setDataMaskForFilterChangesComplete(filterChanges, oldFilters)); +}; + export const setFilterConfiguration = (filterChanges: SaveFilterChangesType) => async (dispatch: Dispatch, getState: () => RootState) => { @@ -89,12 +120,12 @@ export const setFilterConfiguration = }); try { const response = await updateFilters(filterChanges); - dispatch({ - type: SET_NATIVE_FILTERS_CONFIG_COMPLETE, - filterChanges: response.result, - }); - dispatch(nativeFiltersConfigChanged(response.result)); - dispatch(setDataMaskForFilterChangesComplete(filterChanges, oldFilters)); + applySavedFilterChanges( + dispatch, + filterChanges, + response.result, + oldFilters, + ); } catch (err) { dispatch({ type: SET_NATIVE_FILTERS_CONFIG_FAIL, diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index f6d755adf6d..6a4ac7b5997 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -139,9 +139,12 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => { const readyToRender = Boolean(dashboard && charts); const { dashboard_title, id = 0 } = dashboard || {}; - // Get CSS from dashboardInfo (unified properties location) + // Get CSS from dashboardInfo (unified properties location). Use `??`, not + // `||`, so an explicit empty-string override isn't mistaken for the + // pre-hydration unset state and doesn't fall back to the stale fetched + // dashboard's CSS. const css = - useSelector((state: RootState) => state.dashboardInfo.css) || + useSelector((state: RootState) => state.dashboardInfo.css) ?? dashboard?.css; useEffect(() => { diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts index 2a7056bebab..0917a1dfa9e 100644 --- a/superset-frontend/src/dashboard/types.ts +++ b/superset-frontend/src/dashboard/types.ts @@ -19,7 +19,6 @@ import { ChartCustomization, ChartCustomizationDivider, - ChartProps, DataMaskStateWithId, DatasourceType, ExtraFormData, @@ -64,7 +63,6 @@ export type ChartReducerInitialState = typeof chart; // Ref: https://github.com/apache/superset/blob/dcac860f3e5528ecbc39e58f045c7388adb5c3d0/superset-frontend/src/dashboard/reducers/getInitialState.js#L120 export interface ChartQueryPayload extends Partial<ChartReducerInitialState> { id: number; - form_data?: ChartProps['rawFormData']; [key: string]: unknown; } diff --git a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts index 45756b77481..2ade58f6338 100644 --- a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts +++ b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts @@ -21,7 +21,6 @@ import { DataMaskStateWithId, DataRecordFilters, DataRecordValue, - ensureIsArray, JsonObject, PartialFilters, ChartCustomization, @@ -80,6 +79,16 @@ const cachedFormdataByChart: Record< } > = {}; +// The cache above is only invalidated by changes to dataMask/nativeFilters/ +// filters/color/customization — never by the chart's own form_data — so a +// caller that knows a chart's saved configuration changed independently of +// those (e.g. after re-fetching it from the server) must evict it explicitly +// to avoid re-querying with a stale cached form data. +export function invalidateChartFormDataCache(chartId: number): void { + delete cachedFiltersByChart[chartId]; + delete cachedFormdataByChart[chartId]; +} + export interface GetFormDataWithExtraFiltersArguments { chartConfiguration: ChartConfiguration; chartCustomizationItems?: ChartCustomization[]; @@ -314,13 +323,13 @@ function processGroupByCustomizations( return datasetMatches && chartMatches; }); - const chartType = chart.form_data?.viz_type; + const chartType = chart.form_data?.viz_type ?? ''; if (isChartWithoutGroupBy(chartType) || chartType === 'chord') { return {}; } const existingColumns = buildExistingColumnsSet(chart); - const existingGroupBy = ensureIsArray(chart.form_data?.groupby); + const existingGroupBy = extractColumnNames(chart.form_data?.groupby ?? []); const xAxisColumn = chart.form_data?.x_axis; const groupByColumns: string[] = []; diff --git a/superset-frontend/src/dashboard/util/getFormDataWithExtraFilters.test.ts b/superset-frontend/src/dashboard/util/getFormDataWithExtraFilters.test.ts index 3e090134bee..e3de164b9a9 100644 --- a/superset-frontend/src/dashboard/util/getFormDataWithExtraFilters.test.ts +++ b/superset-frontend/src/dashboard/util/getFormDataWithExtraFilters.test.ts @@ -19,6 +19,7 @@ import getFormDataWithExtraFilters, { CachedFormDataWithExtraControls, GetFormDataWithExtraFiltersArguments, + invalidateChartFormDataCache, } from 'src/dashboard/util/charts/getFormDataWithExtraFilters'; import { ChartCustomizationType } from '@superset-ui/core'; import { sliceId as chartId } from 'spec/fixtures/mockChartQueries'; @@ -157,6 +158,41 @@ test('should compose extra control', () => { expect(result.stack).toEqual('Stacked'); }); +test('caches by dataMask/filters/etc, so a form_data-only change is not picked up on its own', () => { + const updatedArgs: GetFormDataWithExtraFiltersArguments = { + ...mockArgs, + chart: { + ...mockChart, + form_data: { ...mockChart.form_data, viz_type: 'big_number' }, + }, + }; + + getFormDataWithExtraFilters(mockArgs); + const staleResult = getFormDataWithExtraFilters( + updatedArgs, + ) as CachedFormDataWithExtraControls; + + expect(staleResult.viz_type).toBe('filter_select'); +}); + +test('invalidateChartFormDataCache forces the next call to pick up a form_data change', () => { + const updatedArgs: GetFormDataWithExtraFiltersArguments = { + ...mockArgs, + chart: { + ...mockChart, + form_data: { ...mockChart.form_data, viz_type: 'big_number' }, + }, + }; + + getFormDataWithExtraFilters(mockArgs); + invalidateChartFormDataCache(chartId); + const freshResult = getFormDataWithExtraFilters( + updatedArgs, + ) as CachedFormDataWithExtraControls; + + expect(freshResult.viz_type).toBe('big_number'); +}); + test('should merge extraFormData from chart customizations', () => { const customizationId = 'CHART_CUSTOMIZATION-1'; const argsWithCustomization: GetFormDataWithExtraFiltersArguments = { diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.tsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.tsx index 73d732d0cce..b17254aab36 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer/index.tsx +++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.tsx @@ -57,7 +57,6 @@ import { LocalStorageKeys, } from 'src/utils/localStorageHelpers'; import { RESERVED_CHART_URL_PARAMS, URL_PARAMS } from 'src/constants'; -import { QUERY_MODE_REQUISITES } from 'src/explore/constants'; import { areObjectsEqual } from 'src/reduxUtils'; import * as logActions from 'src/logger/actions'; import { @@ -72,7 +71,10 @@ import { mergeExtraFormData } from 'src/dashboard/components/nativeFilters/utils import { postFormData, putFormData } from 'src/explore/exploreUtils/formData'; import { datasourcesActions } from 'src/explore/actions/datasourcesActions'; import { mountExploreUrl } from 'src/explore/exploreUtils'; -import { getFormDataFromControls } from 'src/explore/controlUtils'; +import { + getFormDataFromControls, + getVisibleFormDataFromControls, +} from 'src/explore/controlUtils'; import * as exploreActions from 'src/explore/actions/exploreActions'; import * as saveModalActions from 'src/explore/actions/saveModalActions'; import { useTabId } from 'src/hooks/useTabId'; @@ -1043,13 +1045,6 @@ function ExploreViewContainer(props: ExploreViewContainerProps) { ); } -const retainQueryModeRequirements = ( - hiddenFormData: Partial<QueryFormData> | undefined, -): string[] => - Object.keys(hiddenFormData ?? {}).filter( - key => !QUERY_MODE_REQUISITES.has(key), - ); - interface SliceWithSubheader extends Slice { form_data?: QueryFormData & { subheader?: string; @@ -1083,15 +1078,10 @@ function mapStateToProps(state: ExploreRootState) { saveModal, } = state; const { controls, slice, datasource, metadata, hiddenFormData } = explore; - const hasQueryMode = !!controls?.query_mode?.value; - const fieldsToOmit = hasQueryMode - ? retainQueryModeRequirements(hiddenFormData) - : Object.keys(hiddenFormData ?? {}); - - const controlsBasedFormData = omit( - getFormDataFromControls(controls), - fieldsToOmit, - ) as QueryFormData; + const controlsBasedFormData = getVisibleFormDataFromControls( + controls, + hiddenFormData, + ); const isDeckGLChart = explore.form_data?.viz_type === 'deck_multi'; const getDeckGLFormData = (): QueryFormData => { diff --git a/superset-frontend/src/explore/controlUtils/getVisibleFormDataFromControls.ts b/superset-frontend/src/explore/controlUtils/getVisibleFormDataFromControls.ts new file mode 100644 index 00000000000..e0cd364df1a --- /dev/null +++ b/superset-frontend/src/explore/controlUtils/getVisibleFormDataFromControls.ts @@ -0,0 +1,46 @@ +/** + * 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 { omit } from 'lodash-es'; +import { QueryFormData } from '@superset-ui/core'; +import { ControlStateMapping } from '@superset-ui/chart-controls'; +import { QUERY_MODE_REQUISITES } from 'src/explore/constants'; +import { getFormDataFromControls } from './getFormDataFromControls'; + +/** + * Builds form data from the current `controls` state, hiding fields that + * `hiddenFormData` marks as inapplicable to the current viz/query mode. This + * is the shared basis for any consumer that needs to describe the same data + * as the chart currently on screen (e.g. ExploreViewContainer's + * mapStateToProps, or the Explore core API). + */ +export function getVisibleFormDataFromControls( + controls: ControlStateMapping, + hiddenFormData: Partial<QueryFormData> | undefined, +): QueryFormData { + const hasQueryMode = !!controls?.query_mode?.value; + const fieldsToOmit = hasQueryMode + ? Object.keys(hiddenFormData ?? {}).filter( + key => !QUERY_MODE_REQUISITES.has(key), + ) + : Object.keys(hiddenFormData ?? {}); + return omit( + getFormDataFromControls(controls ?? {}), + fieldsToOmit, + ) as QueryFormData; +} diff --git a/superset-frontend/src/explore/controlUtils/index.ts b/superset-frontend/src/explore/controlUtils/index.ts index b39233422b8..96fb46b24b5 100644 --- a/superset-frontend/src/explore/controlUtils/index.ts +++ b/superset-frontend/src/explore/controlUtils/index.ts @@ -20,5 +20,6 @@ export * from './getSectionsToRender'; export * from './getControlConfig'; export * from './getControlState'; export * from './getFormDataFromControls'; +export * from './getVisibleFormDataFromControls'; export * from './getControlValuesCompatibleWithDatasource'; export * from './standardizedFormData'; diff --git a/superset-frontend/src/explore/types.ts b/superset-frontend/src/explore/types.ts index c3f8614ec3e..17bf12a5ae1 100644 --- a/superset-frontend/src/explore/types.ts +++ b/superset-frontend/src/explore/types.ts @@ -59,6 +59,7 @@ export interface ChartState { chartUpdateStartTime: number; lastRendered: number; latestQueryFormData: LatestQueryFormData; + form_data?: QueryFormData; sliceFormData: QueryFormData | null; queryController: AbortController | null; queriesResponse: QueryData[] | null; diff --git a/superset-frontend/src/extensions/ExtensionsStartup.tsx b/superset-frontend/src/extensions/ExtensionsStartup.tsx index 8dc663b96dc..852f1f172aa 100644 --- a/superset-frontend/src/extensions/ExtensionsStartup.tsx +++ b/superset-frontend/src/extensions/ExtensionsStartup.tsx @@ -25,7 +25,9 @@ import { chat, core, commands, + dashboard, editors, + explore, extensions, menus, navigation, @@ -59,7 +61,9 @@ const ExtensionsStartup: React.FC<{ children?: React.ReactNode }> = ({ chat, core, commands, + dashboard, editors, + explore, extensions, menus, navigation, diff --git a/superset-frontend/src/extensions/Namespaces.ts b/superset-frontend/src/extensions/Namespaces.ts index 0e7f6a3063b..93fb166ad7d 100644 --- a/superset-frontend/src/extensions/Namespaces.ts +++ b/superset-frontend/src/extensions/Namespaces.ts @@ -31,7 +31,9 @@ import type { chat, commands, core, + dashboard, editors, + explore, extensions, menus, navigation, @@ -45,7 +47,9 @@ export interface Namespaces { core: typeof core; chat: typeof chat; commands: typeof commands; + dashboard: typeof dashboard; editors: typeof editors; + explore: typeof explore; extensions: typeof extensions; menus: typeof menus; navigation: typeof navigation;
