Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Pass dashboard context to explore through local storage #20743

Merged
merged 16 commits into from
Jul 25, 2022
1 change: 1 addition & 0 deletions superset-frontend/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions superset-frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@
"exports-loader": "^0.7.0",
"fetch-mock": "^7.7.3",
"fork-ts-checker-webpack-plugin": "^6.3.3",
"history": "^4.10.1",
"ignore-styles": "^5.0.1",
"imports-loader": "^3.0.0",
"jest": "^26.6.3",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ export type Filters = {
[filterId: string]: Filter | Divider;
};

export type PartialFilters = {
[filterId: string]: Partial<Filters[keyof Filters]>;
};

export type NativeFiltersState = {
filters: Filters;
filterSets: FilterSets;
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/spec/fixtures/mockNativeFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ export const singleNativeFiltersState = {
inverseSelection: false,
allowsMultipleValues: false,
isRequired: false,
chartsInScope: [230],
},
},
};
Expand Down
23 changes: 12 additions & 11 deletions superset-frontend/src/components/EditableTitle/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
import React, { MouseEvent, useEffect, useState, useRef } from 'react';
import React, { useEffect, useState, useRef } from 'react';
import { Link } from 'react-router-dom';
import cx from 'classnames';
import { css, styled, t } from '@superset-ui/core';
import { css, styled, SupersetTheme, t } from '@superset-ui/core';
import { Tooltip } from 'src/components/Tooltip';
import CertifiedBadge from '../CertifiedBadge';

Expand All @@ -37,7 +38,7 @@ export interface EditableTitleProps {
placeholder?: string;
certifiedBy?: string;
certificationDetails?: string;
onClickTitle?: (event: MouseEvent) => void;
url?: string;
}

const StyledCertifiedBadge = styled(CertifiedBadge)`
Expand All @@ -58,7 +59,7 @@ export default function EditableTitle({
placeholder = '',
certifiedBy,
certificationDetails,
onClickTitle,
url,
// rest is related to title tooltip
...rest
}: EditableTitleProps) {
Expand Down Expand Up @@ -218,20 +219,20 @@ export default function EditableTitle({
}
if (!canEdit) {
// don't actually want an input in this case
titleComponent = onClickTitle ? (
<span
role="button"
onClick={onClickTitle}
tabIndex={0}
titleComponent = url ? (
<Link
to={url}
data-test="editable-title-input"
css={css`
css={(theme: SupersetTheme) => css`
color: ${theme.colors.grayscale.dark1};
text-decoration: none;
:hover {
text-decoration: underline;
}
`}
>
{value}
</span>
</Link>
) : (
<span data-test="editable-title-input">{value}</span>
);
Expand Down
6 changes: 5 additions & 1 deletion superset-frontend/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export const URL_PARAMS = {
},
sliceId: {
name: 'slice_id',
type: 'string',
type: 'number',
},
datasourceId: {
name: 'datasource_id',
Expand Down Expand Up @@ -103,6 +103,10 @@ export const URL_PARAMS = {
name: 'save_action',
type: 'string',
},
dashboardPageId: {
name: 'dashboard_page_id',
type: 'string',
},
} as const;

export const RESERVED_CHART_URL_PARAMS: string[] = [
Expand Down
27 changes: 27 additions & 0 deletions superset-frontend/src/dashboard/actions/hydrate.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ import { FeatureFlag, isFeatureEnabled } from '../../featureFlags';
import extractUrlParams from '../util/extractUrlParams';
import getNativeFilterConfig from '../util/filterboxMigrationHelper';
import { updateColorSchema } from './dashboardInfo';
import { getChartIdsInFilterScope } from '../util/getChartIdsInFilterScope';
import updateComponentParentsList from '../util/updateComponentParentsList';

export const HYDRATE_DASHBOARD = 'HYDRATE_DASHBOARD';

Expand Down Expand Up @@ -256,6 +258,19 @@ export const hydrateDashboard =
layout[layoutId].meta.sliceName = slice.slice_name;
}
});

// make sure that parents tree is built
if (
Object.values(layout).some(
element => element.id !== DASHBOARD_ROOT_ID && !element.parents,
)
) {
updateComponentParentsList({
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some sample dashboards don't have parents in their layout metadata, which caused e2e tests to fail

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we fix the sample dashboards metadata instead of introducing this logic? We can do it in a follow-up if needed.

currentComponent: layout[DASHBOARD_ROOT_ID],
layout,
});
}

buildActiveFilters({
dashboardFilters,
components: layout,
Expand Down Expand Up @@ -333,9 +348,21 @@ export const hydrateDashboard =
rootPath: [DASHBOARD_ROOT_ID],
excluded: [chartId], // By default it doesn't affects itself
},
chartsInScope: Array.from(sliceIds),
},
};
}
if (
behaviors.includes(Behavior.INTERACTIVE_CHART) &&
!metadata.chart_configuration[chartId].crossFilters?.chartsInScope
) {
metadata.chart_configuration[chartId].crossFilters.chartsInScope =
Copy link
Member Author

@kgabryje kgabryje Jul 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optimization for cross filters (works the same as the one implemented for native filters a few months ago) to speed up finding charts in scope. Also, thanks to that, we don't need to save dashboard layout in local storage

getChartIdsInFilterScope(
metadata.chart_configuration[chartId].crossFilters.scope,
charts,
dashboardLayout.present,
);
}
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import Button from 'src/components/Button';
import { AntdForm } from 'src/components';
import { setChartConfiguration } from 'src/dashboard/actions/dashboardInfo';
import { ChartConfiguration } from 'src/dashboard/reducers/types';
import { ChartsState, Layout, RootState } from 'src/dashboard/types';
import { getChartIdsInFilterScope } from 'src/dashboard/util/getChartIdsInFilterScope';
import CrossFilterScopingForm from './CrossFilterScopingForm';
import { CrossFilterScopingFormType } from './types';
import { StyledForm } from '../nativeFilters/FiltersConfigModal/FiltersConfigModal';
Expand All @@ -44,14 +46,24 @@ const CrossFilterScopingModal: FC<CrossFilterScopingModalProps> = ({
const chartConfig = useSelector<any, ChartConfiguration>(
({ dashboardInfo }) => dashboardInfo?.metadata?.chart_configuration,
);
const charts = useSelector<RootState, ChartsState>(state => state.charts);
const layout = useSelector<RootState, Layout>(
state => state.dashboardLayout.present,
);
const scope = chartConfig?.[chartId]?.crossFilters?.scope;
const handleSave = () => {
const chartsInScope = getChartIdsInFilterScope(
form.getFieldValue('scope'),
charts,
layout,
);

dispatch(
setChartConfiguration({
...chartConfig,
[chartId]: {
id: chartId,
crossFilters: { scope: form.getFieldValue('scope') },
crossFilters: { scope: form.getFieldValue('scope'), chartsInScope },
},
}),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import datasources from 'spec/fixtures/mockDatasource';
import {
extraFormData,
NATIVE_FILTER_ID,
layoutForSingleNativeFilter,
singleNativeFiltersState,
dataMaskWith1Filter,
} from 'spec/fixtures/mockNativeFilters';
Expand Down Expand Up @@ -157,7 +156,7 @@ describe('Dashboard', () => {
...getAllActiveFilters({
dataMask: dataMaskWith1Filter,
nativeFilters: singleNativeFiltersState.filters,
layout: layoutForSingleNativeFilter,
allSliceIds: [227, 229, 230],
}),
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
} from '@superset-ui/core';
import { NO_TIME_RANGE, TIME_FILTER_MAP } from 'src/explore/constants';
import { getChartIdsInFilterBoxScope } from 'src/dashboard/util/activeDashboardFilters';
import { CHART_TYPE } from 'src/dashboard/util/componentTypes';
import { ChartConfiguration } from 'src/dashboard/reducers/types';
import { Layout } from 'src/dashboard/types';
import { areObjectsEqual } from 'src/reduxUtils';
Expand Down Expand Up @@ -293,18 +292,9 @@ export const selectNativeIndicatorsForChart = (
let crossFilterIndicators: any = [];
if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) {
const dashboardLayoutValues = Object.values(dashboardLayout);
const chartLayoutItem = dashboardLayoutValues.find(
layoutItem => layoutItem?.meta?.chartId === chartId,
);
crossFilterIndicators = Object.values(chartConfiguration)
.filter(
chartConfig =>
!chartConfig.crossFilters.scope.excluded.includes(chartId) &&
chartConfig.crossFilters.scope.rootPath.some(
elementId =>
chartLayoutItem?.type === CHART_TYPE &&
chartLayoutItem?.parents?.includes(elementId),
),
.filter(chartConfig =>
chartConfig.crossFilters.chartsInScope.includes(chartId),
)
.map(chartConfig => {
const filterState = dataMask[chartConfig.id]?.filterState;
Expand Down
Loading