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: add warning metadata to tables and metrics #13606

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
669 changes: 342 additions & 327 deletions superset-frontend/package-lock.json

Large diffs are not rendered by default.

54 changes: 27 additions & 27 deletions superset-frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,34 +65,34 @@
"@babel/runtime-corejs3": "^7.12.5",
"@data-ui/sparkline": "^0.0.84",
"@emotion/core": "^10.0.35",
"@superset-ui/chart-controls": "^0.17.19",
"@superset-ui/core": "^0.17.18",
"@superset-ui/legacy-plugin-chart-calendar": "^0.17.19",
"@superset-ui/legacy-plugin-chart-chord": "^0.17.19",
"@superset-ui/legacy-plugin-chart-country-map": "^0.17.19",
"@superset-ui/legacy-plugin-chart-event-flow": "^0.17.19",
"@superset-ui/legacy-plugin-chart-force-directed": "^0.17.19",
"@superset-ui/legacy-plugin-chart-heatmap": "^0.17.19",
"@superset-ui/legacy-plugin-chart-histogram": "^0.17.19",
"@superset-ui/legacy-plugin-chart-horizon": "^0.17.19",
"@superset-ui/legacy-plugin-chart-map-box": "^0.17.19",
"@superset-ui/legacy-plugin-chart-paired-t-test": "^0.17.19",
"@superset-ui/legacy-plugin-chart-parallel-coordinates": "^0.17.19",
"@superset-ui/legacy-plugin-chart-partition": "^0.17.19",
"@superset-ui/legacy-plugin-chart-pivot-table": "^0.17.19",
"@superset-ui/legacy-plugin-chart-rose": "^0.17.19",
"@superset-ui/legacy-plugin-chart-sankey": "^0.17.19",
"@superset-ui/legacy-plugin-chart-sankey-loop": "^0.17.19",
"@superset-ui/legacy-plugin-chart-sunburst": "^0.17.19",
"@superset-ui/legacy-plugin-chart-treemap": "^0.17.19",
"@superset-ui/legacy-plugin-chart-world-map": "^0.17.19",
"@superset-ui/legacy-preset-chart-big-number": "^0.17.19",
"@superset-ui/chart-controls": "^0.17.21",
"@superset-ui/core": "^0.17.21",
"@superset-ui/legacy-plugin-chart-calendar": "^0.17.21",
"@superset-ui/legacy-plugin-chart-chord": "^0.17.21",
"@superset-ui/legacy-plugin-chart-country-map": "^0.17.21",
"@superset-ui/legacy-plugin-chart-event-flow": "^0.17.21",
"@superset-ui/legacy-plugin-chart-force-directed": "^0.17.21",
"@superset-ui/legacy-plugin-chart-heatmap": "^0.17.21",
"@superset-ui/legacy-plugin-chart-histogram": "^0.17.21",
"@superset-ui/legacy-plugin-chart-horizon": "^0.17.21",
"@superset-ui/legacy-plugin-chart-map-box": "^0.17.21",
"@superset-ui/legacy-plugin-chart-paired-t-test": "^0.17.21",
"@superset-ui/legacy-plugin-chart-parallel-coordinates": "^0.17.21",
"@superset-ui/legacy-plugin-chart-partition": "^0.17.21",
"@superset-ui/legacy-plugin-chart-pivot-table": "^0.17.21",
"@superset-ui/legacy-plugin-chart-rose": "^0.17.21",
"@superset-ui/legacy-plugin-chart-sankey": "^0.17.21",
"@superset-ui/legacy-plugin-chart-sankey-loop": "^0.17.21",
"@superset-ui/legacy-plugin-chart-sunburst": "^0.17.21",
"@superset-ui/legacy-plugin-chart-treemap": "^0.17.21",
"@superset-ui/legacy-plugin-chart-world-map": "^0.17.21",
"@superset-ui/legacy-preset-chart-big-number": "^0.17.21",
"@superset-ui/legacy-preset-chart-deckgl": "^0.4.6",
"@superset-ui/legacy-preset-chart-nvd3": "^0.17.19",
"@superset-ui/plugin-chart-echarts": "^0.17.19",
"@superset-ui/plugin-chart-table": "^0.17.20",
"@superset-ui/plugin-chart-word-cloud": "^0.17.19",
"@superset-ui/preset-chart-xy": "^0.17.19",
"@superset-ui/legacy-preset-chart-nvd3": "^0.17.21",
"@superset-ui/plugin-chart-echarts": "^0.17.21",
"@superset-ui/plugin-chart-table": "^0.17.21",
"@superset-ui/plugin-chart-word-cloud": "^0.17.21",
"@superset-ui/preset-chart-xy": "^0.17.21",
"@vx/responsive": "^0.0.195",
"abortcontroller-polyfill": "^1.1.9",
"antd": "^4.9.4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ describe('datasourcepanel', () => {
expect(screen.getByText('birth_names')).toBeTruthy();
expect(screen.getByText('Columns')).toBeTruthy();
expect(screen.getByText('Metrics')).toBeTruthy();
expect(screen.queryByTestId('warning')).not.toBeInTheDocument();
});

it('should render search results', () => {
Expand Down Expand Up @@ -103,4 +104,24 @@ describe('datasourcepanel', () => {
expect(c[0].value).toBe('metric_end_certified');
}, 201);
});

it('should render a warning', () => {
const deprecatedDatasource = {
...datasource,
extra: JSON.stringify({ warning_markdown: 'This is a warning.' }),
};
render(
setup({
...props,
datasource: deprecatedDatasource,
controls: {
datasource: {
...props.controls.datasource,
datasource: deprecatedDatasource,
},
},
}),
);
expect(screen.getByTestId('alert-solid')).toBeTruthy();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ function CertifiedIconWithTooltip({
id="certified-details-tooltip"
title={
<>
{certifiedBy && <div>{t('Certified by %s', certifiedBy)}</div>}
{certifiedBy && (
<div>
<strong>{t('Certified by %s', certifiedBy)}</strong>
</div>
)}
<div>{details}</div>
</>
}
Expand Down
7 changes: 7 additions & 0 deletions superset-frontend/src/components/TableSelector/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import FormLabel from 'src/components/FormLabel';
import DatabaseSelector from 'src/components/DatabaseSelector';
import RefreshLabel from 'src/components/RefreshLabel';
import CertifiedIconWithTooltip from 'src/components/CertifiedIconWithTooltip';
import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';

const FieldTitle = styled.p`
color: ${({ theme }) => theme.colors.secondary.light2};
Expand Down Expand Up @@ -266,6 +267,12 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
size={20}
/>
)}
{option.extra?.warning_markdown && (
<WarningIconWithTooltip
warningMarkdown={option.extra.warning_markdown}
size={20}
/>
)}
{option.label}
</TableLabel>
);
Expand Down
48 changes: 48 additions & 0 deletions superset-frontend/src/components/WarningIconWithTooltip.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* 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 React from 'react';
import { supersetTheme, SafeMarkdown } from '@superset-ui/core';
import Icon from 'src/components/Icon';
import { Tooltip } from 'src/common/components/Tooltip';

interface WarningIconWithTooltipProps {
warningMarkdown: string;
size?: number;
}

function WarningIconWithTooltip({
warningMarkdown,
size = 24,
}: WarningIconWithTooltipProps) {
return (
<Tooltip
id="warning-tooltip"
title={<SafeMarkdown source={warningMarkdown} />}
>
<Icon
color={supersetTheme.colors.alert.base}
height={size}
width={size}
name="alert-solid"
/>
</Tooltip>
);
}

export default WarningIconWithTooltip;
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,11 @@
*/
import React from 'react';
import PropTypes from 'prop-types';
import ReactMarkdown from 'react-markdown';
import htmlParser from 'react-markdown/plugins/html-parser';

import cx from 'classnames';
import { t } from '@superset-ui/core';
import { t, SafeMarkdown } from '@superset-ui/core';
import { Logger, LOG_ACTIONS_RENDER_CHART } from 'src/logger/LogUtils';
import { MarkdownEditor } from 'src/components/AsyncAceEditor';
import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';

import DeleteComponentButton from 'src/dashboard/components/DeleteComponentButton';
import DragDroppable from 'src/dashboard/components/dnd/DragDroppable';
Expand Down Expand Up @@ -80,14 +77,6 @@ Click here to edit [markdown](https://bit.ly/1dQOfRK)`;

const MARKDOWN_ERROR_MESSAGE = t('This markdown component has an error.');

function isSafeMarkup(node) {
if (node.type === 'html') {
return /href="(javascript|vbscript|file):.*"/gim.test(node.value) === false;
}

return true;
}

class Markdown extends React.PureComponent {
constructor(props) {
super(props);
Expand Down Expand Up @@ -257,7 +246,7 @@ class Markdown extends React.PureComponent {
showGutter={false}
editorProps={{ $blockScrolling: true }}
value={
// thisl allows "select all => delete" to give an empty editor
// this allows "select all => delete" to give an empty editor
typeof this.state.markdownSource === 'string'
? this.state.markdownSource
: MARKDOWN_PLACE_HOLDER
Expand All @@ -271,21 +260,14 @@ class Markdown extends React.PureComponent {

renderPreviewMode() {
const { hasError } = this.state;

return (
<ReactMarkdown
<SafeMarkdown
source={
hasError
? MARKDOWN_ERROR_MESSAGE
: this.state.markdownSource || MARKDOWN_PLACE_HOLDER
}
escapeHtml={isFeatureEnabled(FeatureFlag.ESCAPE_MARKDOWN_HTML)}
skipHtml={!isFeatureEnabled(FeatureFlag.DISPLAY_MARKDOWN_HTML)}
allowNode={isSafeMarkup}
astPlugins={[
htmlParser({
isValidNode: node => node.type !== 'script',
}),
]}
/>
);
}
Expand Down
35 changes: 20 additions & 15 deletions superset-frontend/src/datasource/DatasourceEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { styled, SupersetClient, t, supersetTheme } from '@superset-ui/core';
import Button from 'src/components/Button';
import Tabs from 'src/common/components/Tabs';
import CertifiedIconWithTooltip from 'src/components/CertifiedIconWithTooltip';
import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';
import DatabaseSelector from 'src/components/DatabaseSelector';
import Icon from 'src/components/Icon';
import Label from 'src/components/Label';
Expand Down Expand Up @@ -564,9 +565,9 @@ class DatasourceEditor extends React.PureComponent {
label={t('Extra')}
description={t(
'Extra data to specify table metadata. Currently supports ' +
'certification data of the format: `{ "certification": { "certified_by": ' +
'metadata of the format: `{ "certification": { "certified_by": ' +
'"Data Platform Team", "details": "This table is the source of truth." ' +
'} }`.',
'}, "warning_markdown": "This is a warning." }`.',
)}
control={
<TextAreaControl
Expand Down Expand Up @@ -881,19 +882,6 @@ class DatasourceEditor extends React.PureComponent {
<TextControl controlId="d3format" placeholder="%y/%m/%d" />
}
/>
<Field
label={t('Warning message')}
fieldKey="warning_text"
description={t(
'Warning message to display in the metric selector',
)}
control={
<TextControl
controlId="warning_text"
placeholder={t('Warning message')}
/>
}
/>
<Field
label={t('Certified by')}
fieldKey="certified_by"
Expand All @@ -918,6 +906,18 @@ class DatasourceEditor extends React.PureComponent {
/>
}
/>
<Field
label={t('Warning')}
fieldKey="warning_markdown"
description={t('Optional warning about use of this metric')}
control={
<TextAreaControl
controlId="warning_markdown"
language="markdown"
offerEditInModal={false}
/>
}
/>
</Fieldset>
</FormContainer>
}
Expand All @@ -938,6 +938,11 @@ class DatasourceEditor extends React.PureComponent {
details={record.certification_details}
/>
)}
{record.warning_markdown && (
<WarningIconWithTooltip
warningMarkdown={record.warning_markdown}
/>
)}
<EditableTitle canEdit title={v} onSaveTitle={onChange} />
</FlexRowContainer>
),
Expand Down
20 changes: 11 additions & 9 deletions superset-frontend/src/datasource/DatasourceModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,17 @@ interface DatasourceModalProps {
}

function buildMetricExtraJsonObject(metric: Record<string, unknown>) {
if (metric?.certified_by || metric?.certification_details) {
return JSON.stringify({
certification: {
certified_by: metric?.certified_by ?? null,
details: metric?.certification_details ?? null,
},
});
}
return null;
const certification =
metric?.certified_by || metric?.certification_details
? {
certified_by: metric?.certified_by,
details: metric?.certification_details,
}
: undefined;
return JSON.stringify({
certification,
warning_markdown: metric?.warning_markdown,
});
}

const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import DatasourceModal from 'src/datasource/DatasourceModal';
import { postForm } from 'src/explore/exploreUtils';
import Button from 'src/components/Button';
import ErrorAlert from 'src/components/ErrorMessage/ErrorAlert';
import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';

const propTypes = {
actions: PropTypes.object.isRequired,
Expand Down Expand Up @@ -178,6 +179,13 @@ class DatasourceControl extends React.PureComponent {

const { health_check_message: healthCheckMessage } = datasource;

let extra = {};
if (datasource?.extra) {
try {
extra = JSON.parse(datasource?.extra);
} catch {} // eslint-disable-line no-empty
}

return (
<Styles className="DatasourceControl">
<div className="data-container">
Expand All @@ -200,6 +208,12 @@ class DatasourceControl extends React.PureComponent {
/>
</Tooltip>
)}
{extra?.warning_markdown && ( // eslint-disable-line camelcase
<WarningIconWithTooltip
warningMarkdown={extra.warning_markdown} // eslint-disable-line camelcase
size={30}
/>
)}
<Dropdown
overlay={datasourceMenu}
trigger={['click']}
Expand Down
Loading