feat: add warning metadata to tables and metrics (#13606)

2026-04-07 10:31:50 +00:00 · 2021-03-17 21:27:34 -07:00
parent db57f90a34
commit 64785c20bd
13 changed files with 529 additions and 415 deletions
--- a/superset-frontend/package-lock.json
+++ b/superset-frontend/package-lock.json
--- a/superset-frontend/package.json
+++ b/superset-frontend/package.json
@@ -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",
--- a/superset-frontend/spec/javascripts/explore/components/DatasourcePanel_spec.jsx
+++ b/superset-frontend/spec/javascripts/explore/components/DatasourcePanel_spec.jsx
@@ -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', () => {
@@ -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();
+  });
 });
--- a/superset-frontend/src/components/CertifiedIconWithTooltip.tsx
+++ b/superset-frontend/src/components/CertifiedIconWithTooltip.tsx
@@ -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>
        </>
      }
--- a/superset-frontend/src/components/TableSelector/index.tsx
+++ b/superset-frontend/src/components/TableSelector/index.tsx
@@ -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};
@@ -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>
    );
--- a/superset-frontend/src/components/WarningIconWithTooltip.tsx
+++ b/superset-frontend/src/components/WarningIconWithTooltip.tsx
@@ -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;
--- a/superset-frontend/src/dashboard/components/gridComponents/Markdown.jsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Markdown.jsx
@@ -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';
@@ -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);
@@ -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
@@ -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',
-          }),
-        ]}
      />
    );
  }
--- a/superset-frontend/src/datasource/DatasourceEditor.jsx
+++ b/superset-frontend/src/datasource/DatasourceEditor.jsx
@@ -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';
@@ -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
@@ -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"
@@ -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>
        }
@@ -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>
          ),
--- a/superset-frontend/src/datasource/DatasourceModal.tsx
+++ b/superset-frontend/src/datasource/DatasourceModal.tsx
@@ -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> = ({
--- a/superset-frontend/src/explore/components/controls/DatasourceControl.jsx
+++ b/superset-frontend/src/explore/components/controls/DatasourceControl.jsx
@@ -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,
@@ -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">
@@ -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']}
--- a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx
+++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx
@@ -47,6 +47,7 @@ import FacePile from 'src/components/FacePile';
 import CertifiedIconWithTooltip from 'src/components/CertifiedIconWithTooltip';
 import ImportModelsModal from 'src/components/ImportModal/index';
 import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
+import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';
 import AddDatasetModal from './AddDatasetModal';

 const PAGE_SIZE = 25;
@@ -235,16 +236,21 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
          const titleLink = <a href={exploreURL}>{datasetTitle}</a>;
          try {
            const parsedExtra = JSON.parse(extra);
-            return parsedExtra?.certification ? (
+            return (
              <FlexRowContainer>
-                <CertifiedIconWithTooltip
-                  certifiedBy={parsedExtra.certification.certified_by}
-                  details={parsedExtra.certification.details}
-                />
+                {parsedExtra?.certification && (
+                  <CertifiedIconWithTooltip
+                    certifiedBy={parsedExtra.certification.certified_by}
+                    details={parsedExtra.certification.details}
+                  />
+                )}
+                {parsedExtra?.warning_markdown && (
+                  <WarningIconWithTooltip
+                    warningMarkdown={parsedExtra.warning_markdown}
+                  />
+                )}
                {titleLink}
              </FlexRowContainer>
-            ) : (
-              titleLink
            );
          } catch {
            return titleLink;
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -415,9 +415,18 @@ class SqlMetric(Model, BaseMetric):
    def certification_details(self) -> Optional[str]:
        return self.get_extra_dict().get("certification", {}).get("details")

+    @property
+    def warning_markdown(self) -> Optional[str]:
+        return self.get_extra_dict().get("warning_markdown")
+
    @property
    def data(self) -> Dict[str, Any]:
-        attrs = ("is_certified", "certified_by", "certification_details")
+        attrs = (
+            "is_certified",
+            "certified_by",
+            "certification_details",
+            "warning_markdown",
+        )
        attr_dict = {s: getattr(self, s) for s in attrs}

        attr_dict.update(super().data)
@@ -485,7 +494,7 @@ class SqlaTable(  # pylint: disable=too-many-public-methods,too-many-instance-at
        "fetch_values_predicate",
        "extra",
    ]
-    update_from_object_fields = [f for f in export_fields if not f == "database_id"]
+    update_from_object_fields = [f for f in export_fields if f != "database_id"]
    export_parent = "database"
    export_children = ["metrics", "columns"]

@@ -715,6 +724,7 @@ class SqlaTable(  # pylint: disable=too-many-public-methods,too-many-instance-at
                if config.get("DATASET_HEALTH_CHECK")
                else None
            )
+            data_["extra"] = self.extra
        return data_

    @property
--- a/superset/connectors/sqla/views.py
+++ b/superset/connectors/sqla/views.py
@@ -235,10 +235,10 @@ class SqlMetricInlineView(  # pylint: disable=too-many-ancestors
        ),
        "extra": utils.markdown(
            "Extra data to specify metric metadata. Currently supports "
-            'certification data of the format: `{ "certification": "certified_by": '
+            'metadata of the format: `{ "certification": { "certified_by": '
            '"Data Platform Team", "details": "This metric is the source of truth." '
-            "} }`. This should be modified from the edit datasource model in "
-            "Explore to ensure correct formatting.",
+            '}, "warning_markdown": "This is a warning." }`. This should be modified '
+            "from the edit datasource model in Explore to ensure correct formatting.",
            True,
        ),
    }
@@ -463,9 +463,9 @@ class TableModelView(  # pylint: disable=too-many-ancestors
        ),
        "extra": utils.markdown(
            "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." }`.',
            True,
        ),
    }