Skip to content
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
19 changes: 19 additions & 0 deletions superset-frontend/images/icons/certified.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 7 additions & 2 deletions superset-frontend/src/CRUD/CollectionTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<any>;
}
Expand Down Expand Up @@ -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 {
Expand Down
17 changes: 12 additions & 5 deletions superset-frontend/src/CRUD/Field.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import './crud.less';
const propTypes = {
value: PropTypes.any.isRequired,
label: PropTypes.string.isRequired,
descr: PropTypes.node,
Copy link
Member Author

Choose a reason for hiding this comment

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

why make this shorter and less clear? Turns out because this file is JSX, some usages of this component set the description prop, which doesn't even work!

I've cleaned it all up to use description instead

Copy link
Member

Choose a reason for hiding this comment

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

+1

description: PropTypes.node,
fieldKey: PropTypes.string.isRequired,
control: PropTypes.node.isRequired,
onChange: PropTypes.func,
Expand All @@ -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,
Expand All @@ -63,12 +70,12 @@ export default class Field extends React.PureComponent {
<FormGroup controlId={fieldKey}>
<FormLabel className="m-r-5">
{label || fieldKey}
{compact && descr && (
{compact && description && (
<OverlayTrigger
placement="right"
overlay={
<Tooltip id="field-descr" bsSize="lg">
{descr}
{description}
</Tooltip>
}
>
Expand All @@ -78,7 +85,7 @@ export default class Field extends React.PureComponent {
</FormLabel>
{hookedControl}
<FormControl.Feedback />
{!compact && descr && <HelpBlock>{descr}</HelpBlock>}
{!compact && description && <HelpBlock>{description}</HelpBlock>}
</FormGroup>
);
}
Expand Down
49 changes: 49 additions & 0 deletions superset-frontend/src/components/CertifiedIconWithTooltip.tsx
Original file line number Diff line number Diff line change
@@ -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 {
Copy link
Member

Choose a reason for hiding this comment

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

Naming/API design nit: is there a variant of CertifiedIcon which doesn't have a tooltip? If no, then WithTooltip is probably not needed; if yes, you can also just make tooltip as an optional prop and add the TooltipWrapper conditionally.

Copy link
Member Author

Choose a reason for hiding this comment

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

In the Icon file, we do:
import { ReactComponent as CertifiedIcon } from 'images/icons/certified.svg';
so I think there's a general understanding of an "Icon" component only including the icon and nothing else.

certifiedBy?: string;
details?: string;
}

function CertifiedIconWithTooltip({
certifiedBy,
details,
}: CertifiedIconWithTooltipProps) {
return (
<TooltipWrapper
label="certified-details"
tooltip={
<>
{certifiedBy && <div>{t('Certified by %s', certifiedBy)}</div>}
<div>{details}</div>
</>
}
>
<Icon color={supersetTheme.colors.primary.base} name="certified" />
</TooltipWrapper>
);
}

export default CertifiedIconWithTooltip;
3 changes: 3 additions & 0 deletions superset-frontend/src/components/Icon/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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,
Expand Down
60 changes: 47 additions & 13 deletions superset-frontend/src/datasource/DatasourceEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) => (
<CheckboxControl value={d} onChange={onChange} />
);
Expand Down Expand Up @@ -130,7 +140,7 @@ function ColumnCollectionTable({
<Field
fieldKey="python_date_format"
label={t('Datetime Format')}
descr={
description={
/* Note the fragmented translations may not work. */
<div>
{t('The pattern of timestamp format. For strings use ')}
Expand Down Expand Up @@ -419,7 +429,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.',
Expand All @@ -436,22 +446,22 @@ export class DatasourceEditor extends React.PureComponent {
<Field
fieldKey="default_endpoint"
label={t('Default URL')}
descr={t(
description={t(
'Default URL to redirect to when accessing from the datasource list page',
)}
control={<TextControl />}
/>
<Field
fieldKey="filter_select_enabled"
label={t('Autocomplete filters')}
descr={t('Whether to populate autocomplete filters options')}
description={t('Whether to populate autocomplete filters options')}
control={<CheckboxControl />}
/>
{this.state.isSqla && (
<Field
fieldKey="fetch_values_predicate"
label={t('Autocomplete Query Predicate')}
descr={t(
description={t(
'When using "Autocomplete filters", this can be used to improve performance ' +
'of the query fetching the values. Use this option to apply a ' +
'predicate (WHERE clause) to the query selecting the distinct ' +
Expand All @@ -464,7 +474,7 @@ export class DatasourceEditor extends React.PureComponent {
<Field
fieldKey="owners"
label={t('Owners')}
descr={t('Owners of the datasource')}
description={t('Owners of the datasource')}
control={
<SelectAsyncControl
dataEndpoint="/users/api/read"
Expand Down Expand Up @@ -495,7 +505,7 @@ export class DatasourceEditor extends React.PureComponent {
<Field
fieldKey="sql"
label={t('SQL')}
descr={t(
description={t(
'When specifying SQL, the datasource acts as a view. ' +
'Superset will use this statement as a subquery while grouping and filtering ' +
'on the generated parent queries.',
Expand All @@ -509,7 +519,7 @@ export class DatasourceEditor extends React.PureComponent {
<Field
fieldKey="json"
label={t('JSON')}
descr={
description={
<div>{t('The JSON metric or post aggregation definition.')}</div>
}
control={
Expand All @@ -520,7 +530,7 @@ export class DatasourceEditor extends React.PureComponent {
<Field
fieldKey="cache_timeout"
label={t('Cache Timeout')}
descr={t(
description={t(
'The duration of time in seconds before the cache is invalidated',
)}
control={<TextControl />}
Expand All @@ -534,7 +544,7 @@ export class DatasourceEditor extends React.PureComponent {
<Field
fieldKey="template_params"
label={t('Template parameters')}
descr={t(
description={t(
'A set of parameters that become available in the query using Jinja templating syntax',
)}
control={<TextControl />}
Expand Down Expand Up @@ -601,7 +611,7 @@ export class DatasourceEditor extends React.PureComponent {
}}
expandFieldset={
<FormContainer>
<Fieldset>
<Fieldset compact>
<Field
fieldKey="verbose_name"
label={t('Label')}
Expand All @@ -625,6 +635,22 @@ export class DatasourceEditor extends React.PureComponent {
)}
control={<TextControl placeholder={t('Warning Message')} />}
/>
<Field
label={t('Certified By')}
fieldKey="certified_by"
description={t(
'Person or group that has certified this metric',
)}
control={<TextControl placeholder={t('Certified By')} />}
/>
<Field
label={t('Certification Details')}
fieldKey="certification_details"
description={t('Details of the certification')}
control={
<TextControl placeholder={t('Certification Details')} />
}
/>
</Fieldset>
</FormContainer>
}
Expand All @@ -637,8 +663,16 @@ export class DatasourceEditor extends React.PureComponent {
expression: '',
})}
itemRenderers={{
metric_name: (v, onChange) => (
<EditableTitle canEdit title={v} onSaveTitle={onChange} />
metric_name: (v, onChange, _, record) => (
<FlexRowContainer>
{record.is_certified && (
<CertifiedIconWithTooltip
certifiedBy={record.certified_by}
details={record.certification_details}
/>
)}
<EditableTitle canEdit title={v} onSaveTitle={onChange} />
</FlexRowContainer>
),
verbose_name: (v, onChange) => (
<EditableTitle canEdit title={v} onSaveTitle={onChange} />
Expand Down
30 changes: 28 additions & 2 deletions superset-frontend/src/datasource/DatasourceModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,18 @@ interface DatasourceModalProps {
show: boolean;
}

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 DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
addSuccessToast,
datasource,
Expand All @@ -48,11 +60,19 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
const dialog = useRef<any>(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<string, unknown>) => ({
...metric,
extra: buildMetricExtraJsonObject(metric),
}),
),
type: currentDatasource.type || currentDatasource.datasource_type,
},
},
Expand All @@ -75,8 +95,14 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
);
};

const onDatasourceChange = (data: object, err: Array<any>) => {
setCurrentDatasource(data);
const onDatasourceChange = (data: Record<string, any>, err: Array<any>) => {
setCurrentDatasource({
...data,
metrics: data?.metrics.map((metric: Record<string, unknown>) => ({
...metric,
is_certified: metric?.certified_by || metric?.certification_details,
})),
});
setErrors(err);
};

Expand Down
Loading