From 38da552a57b9be999fb36b2cf7ae1c9231a010eb Mon Sep 17 00:00:00 2001 From: Erik Ritter Date: Wed, 19 Aug 2020 20:45:33 -0700 Subject: [PATCH] feat: add certification to metrics (#10630) --- superset-frontend/images/icons/certified.svg | 19 ++++++ .../src/CRUD/CollectionTable.tsx | 9 ++- superset-frontend/src/CRUD/Field.jsx | 17 ++++-- .../components/CertifiedIconWithTooltip.tsx | 49 +++++++++++++++ .../src/components/Icon/index.tsx | 3 + .../src/datasource/DatasourceEditor.jsx | 60 +++++++++++++++---- .../src/datasource/DatasourceModal.tsx | 30 +++++++++- superset/connectors/sqla/models.py | 29 +++++++++ superset/connectors/sqla/views.py | 10 ++++ 9 files changed, 204 insertions(+), 22 deletions(-) create mode 100644 superset-frontend/images/icons/certified.svg create mode 100644 superset-frontend/src/components/CertifiedIconWithTooltip.tsx diff --git a/superset-frontend/images/icons/certified.svg b/superset-frontend/images/icons/certified.svg new file mode 100644 index 0000000000..567ccf84ac --- /dev/null +++ b/superset-frontend/images/icons/certified.svg @@ -0,0 +1,19 @@ + + diff --git a/superset-frontend/src/CRUD/CollectionTable.tsx b/superset-frontend/src/CRUD/CollectionTable.tsx index f8632bbdb4..dfa89b0dc8 100644 --- a/superset-frontend/src/CRUD/CollectionTable.tsx +++ b/superset-frontend/src/CRUD/CollectionTable.tsx @@ -33,7 +33,12 @@ interface CRUDCollectionProps { expandFieldset: ReactNode; extraButtons: ReactNode; itemGenerator?: () => any; - itemRenderers?: any; + itemRenderers?: (( + val: unknown, + onChange: () => void, + label: string, + record: any, + ) => ReactNode)[]; onChange?: (arg0: any) => void; tableColumns: Array; } @@ -183,7 +188,7 @@ export default class CRUDCollection extends React.PureComponent< const renderer = this.props.itemRenderers && this.props.itemRenderers[col]; const val = record[col]; const onChange = this.onCellChange.bind(this, record.id, col); - return renderer ? renderer(val, onChange, this.getLabel(col)) : val; + return renderer ? renderer(val, onChange, this.getLabel(col), record) : val; } renderItem(record: any) { const { diff --git a/superset-frontend/src/CRUD/Field.jsx b/superset-frontend/src/CRUD/Field.jsx index bfd528bede..7b249b439a 100644 --- a/superset-frontend/src/CRUD/Field.jsx +++ b/superset-frontend/src/CRUD/Field.jsx @@ -32,7 +32,7 @@ import './crud.less'; const propTypes = { value: PropTypes.any.isRequired, label: PropTypes.string.isRequired, - descr: PropTypes.node, + description: PropTypes.node, fieldKey: PropTypes.string.isRequired, control: PropTypes.node.isRequired, onChange: PropTypes.func, @@ -54,7 +54,14 @@ export default class Field extends React.PureComponent { this.props.onChange(this.props.fieldKey, newValue); } render() { - const { compact, value, label, control, descr, fieldKey } = this.props; + const { + compact, + value, + label, + control, + description, + fieldKey, + } = this.props; const hookedControl = React.cloneElement(control, { value, onChange: this.onChange, @@ -63,12 +70,12 @@ export default class Field extends React.PureComponent { {label || fieldKey} - {compact && descr && ( + {compact && description && ( - {descr} + {description} } > @@ -78,7 +85,7 @@ export default class Field extends React.PureComponent { {hookedControl} - {!compact && descr && {descr}} + {!compact && description && {description}} ); } diff --git a/superset-frontend/src/components/CertifiedIconWithTooltip.tsx b/superset-frontend/src/components/CertifiedIconWithTooltip.tsx new file mode 100644 index 0000000000..fcd2fe30f1 --- /dev/null +++ b/superset-frontend/src/components/CertifiedIconWithTooltip.tsx @@ -0,0 +1,49 @@ +/** + * 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 { t } from '@superset-ui/translation'; +import { supersetTheme } from '@superset-ui/style'; +import Icon from 'src/components/Icon'; +import TooltipWrapper from 'src/components/TooltipWrapper'; + +interface CertifiedIconWithTooltipProps { + certifiedBy?: string; + details?: string; +} + +function CertifiedIconWithTooltip({ + certifiedBy, + details, +}: CertifiedIconWithTooltipProps) { + return ( + + {certifiedBy &&
{t('Certified by %s', certifiedBy)}
} +
{details}
+ + } + > + + + ); +} + +export default CertifiedIconWithTooltip; diff --git a/superset-frontend/src/components/Icon/index.tsx b/superset-frontend/src/components/Icon/index.tsx index c325c8133f..f798f1c987 100644 --- a/superset-frontend/src/components/Icon/index.tsx +++ b/superset-frontend/src/components/Icon/index.tsx @@ -19,6 +19,7 @@ import React, { SVGProps } from 'react'; import { ReactComponent as CancelXIcon } from 'images/icons/cancel-x.svg'; import { ReactComponent as CardViewIcon } from 'images/icons/card-view.svg'; +import { ReactComponent as CertifiedIcon } from 'images/icons/certified.svg'; import { ReactComponent as CheckboxHalfIcon } from 'images/icons/checkbox-half.svg'; import { ReactComponent as CheckboxOffIcon } from 'images/icons/checkbox-off.svg'; import { ReactComponent as CheckboxOnIcon } from 'images/icons/checkbox-on.svg'; @@ -46,6 +47,7 @@ import { ReactComponent as WarningIcon } from 'images/icons/warning.svg'; type IconName = | 'cancel-x' | 'card-view' + | 'certified' | 'check' | 'checkbox-half' | 'checkbox-off' @@ -88,6 +90,7 @@ export const iconsRegistry: Record< 'list-view': ListViewIcon, 'sort-asc': SortAscIcon, 'sort-desc': SortDescIcon, + certified: CertifiedIcon, check: CheckIcon, close: CloseIcon, compass: CompassIcon, diff --git a/superset-frontend/src/datasource/DatasourceEditor.jsx b/superset-frontend/src/datasource/DatasourceEditor.jsx index dd20ca5532..9f3d110e93 100644 --- a/superset-frontend/src/datasource/DatasourceEditor.jsx +++ b/superset-frontend/src/datasource/DatasourceEditor.jsx @@ -28,6 +28,7 @@ import Label from 'src/components/Label'; import Button from 'src/components/Button'; import Loading from 'src/components/Loading'; import TableSelector from 'src/components/TableSelector'; +import CertifiedIconWithTooltip from 'src/components/CertifiedIconWithTooltip'; import getClientErrorObject from '../utils/getClientErrorObject'; import CheckboxControl from '../explore/components/controls/CheckboxControl'; @@ -59,6 +60,15 @@ const DatasourceContainer = styled.div` } `; +const FlexRowContainer = styled.div` + align-items: center; + display: flex; + + > svg { + margin-right: ${({ theme }) => theme.gridUnit}px; + } +`; + const checkboxGenerator = (d, onChange) => ( ); @@ -130,7 +140,7 @@ function ColumnCollectionTable({ {t('The pattern of timestamp format. For strings use ')} @@ -460,7 +470,7 @@ export class DatasourceEditor extends React.PureComponent { handleError={this.props.addDangerToast} /> } - descr={t( + description={t( 'The pointer to a physical table. Keep in mind that the chart is ' + 'associated to this Superset logical table, and this logical table points ' + 'the physical table referenced here.', @@ -477,7 +487,7 @@ export class DatasourceEditor extends React.PureComponent { } @@ -485,14 +495,14 @@ export class DatasourceEditor extends React.PureComponent { } /> {this.state.isSqla && ( {t('The JSON metric or post aggregation definition.')} } control={ @@ -561,7 +571,7 @@ export class DatasourceEditor extends React.PureComponent { } @@ -575,7 +585,7 @@ export class DatasourceEditor extends React.PureComponent { } @@ -642,7 +652,7 @@ export class DatasourceEditor extends React.PureComponent { }} expandFieldset={ -
+
} /> + } + /> + + } + />
} @@ -678,8 +704,16 @@ export class DatasourceEditor extends React.PureComponent { expression: '', })} itemRenderers={{ - metric_name: (v, onChange) => ( - + metric_name: (v, onChange, _, record) => ( + + {record.is_certified && ( + + )} + + ), verbose_name: (v, onChange) => ( diff --git a/superset-frontend/src/datasource/DatasourceModal.tsx b/superset-frontend/src/datasource/DatasourceModal.tsx index 19ea9d8267..eb33a32ad9 100644 --- a/superset-frontend/src/datasource/DatasourceModal.tsx +++ b/superset-frontend/src/datasource/DatasourceModal.tsx @@ -36,6 +36,18 @@ interface DatasourceModalProps { show: boolean; } +function buildMetricExtraJsonObject(metric: Record) { + 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 DatasourceModal: FunctionComponent = ({ addSuccessToast, datasource, @@ -48,11 +60,19 @@ const DatasourceModal: FunctionComponent = ({ const dialog = useRef(null); const onConfirmSave = () => { + // Pull out extra fields into the extra object + SupersetClient.post({ endpoint: '/datasource/save/', postPayload: { data: { ...currentDatasource, + metrics: currentDatasource?.metrics?.map( + (metric: Record) => ({ + ...metric, + extra: buildMetricExtraJsonObject(metric), + }), + ), type: currentDatasource.type || currentDatasource.datasource_type, }, }, @@ -75,8 +95,14 @@ const DatasourceModal: FunctionComponent = ({ ); }; - const onDatasourceChange = (data: object, err: Array) => { - setCurrentDatasource(data); + const onDatasourceChange = (data: Record, err: Array) => { + setCurrentDatasource({ + ...data, + metrics: data?.metrics.map((metric: Record) => ({ + ...metric, + is_certified: metric?.certified_by || metric?.certification_details, + })), + }); setErrors(err); }; diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index cf19d64aec..e4065aeeea 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +import json import logging from collections import OrderedDict from dataclasses import dataclass, field @@ -351,6 +352,7 @@ class SqlMetric(Model, BaseMetric): foreign_keys=[table_id], ) expression = Column(Text, nullable=False) + extra = Column(Text) export_fields = [ "metric_name", @@ -360,6 +362,7 @@ class SqlMetric(Model, BaseMetric): "expression", "description", "d3format", + "extra", "warning_text", ] update_from_object_fields = list( @@ -399,6 +402,32 @@ class SqlMetric(Model, BaseMetric): return import_datasource.import_simple_obj(db.session, i_metric, lookup_obj) + def get_extra_dict(self) -> Dict[str, Any]: + try: + return json.loads(self.extra) + except (TypeError, json.JSONDecodeError): + return {} + + @property + def is_certified(self) -> bool: + return bool(self.get_extra_dict().get("certification")) + + @property + def certified_by(self) -> Optional[str]: + return self.get_extra_dict().get("certification", {}).get("certified_by") + + @property + def certification_details(self) -> Optional[str]: + return self.get_extra_dict().get("certification", {}).get("details") + + @property + def data(self) -> Dict[str, Any]: + attrs = ("is_certified", "certified_by", "certification_details") + attr_dict = {s: getattr(self, s) for s in attrs} + + attr_dict.update(super().data) + return attr_dict + sqlatable_user = Table( "sqlatable_user", diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index ae2a7c41bf..107e10abaf 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -189,6 +189,7 @@ class SqlMetricInlineView( # pylint: disable=too-many-ancestors "expression", "table", "d3format", + "extra", "warning_text", ] description_columns = { @@ -205,6 +206,14 @@ class SqlMetricInlineView( # pylint: disable=too-many-ancestors "formats", True, ), + "extra": utils.markdown( + "Extra data to specify metric metadata. Currently supports " + 'certification data of the format: `{ "certification": "certified_by": ' + '"Taylor Swift", "details": "This metric is the source of truth." ' + "} }`. This should be modified from the edit datasource model in " + "Explore to ensure correct formatting.", + True, + ), } add_columns = edit_columns page_size = 500 @@ -216,6 +225,7 @@ class SqlMetricInlineView( # pylint: disable=too-many-ancestors "expression": _("SQL Expression"), "table": _("Table"), "d3format": _("D3 Format"), + "extra": _("Extra"), "warning_text": _("Warning Message"), }