feat: Certify Charts and Dashboards (#17335)

* Certify charts

* Format

* Certify dashboards

* Format

* Refactor card certification

* Clear details when certified by empty

* Show certification in detail page

* Add RTL tests

* Test charts api

* Enhance integration tests

* Lint

* Fix dashboards count

* Format

* Handle empty value

* Handle empty slice

* Downgrade migration

* Indent

* Use alter

* Fix revision

* Fix revision
This commit is contained in:
Geido 2021-11-24 13:42:52 +02:00 committed by GitHub
parent 9576478a5d
commit 83e49fc9ee
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
28 changed files with 589 additions and 60 deletions

View File

@ -111,4 +111,6 @@ export enum FilterOperator {
between = 'between',
dashboardIsFav = 'dashboard_is_favorite',
chartIsFav = 'chart_is_favorite',
chartIsCertified = 'chart_is_certified',
dashboardIsCertified = 'dashboard_is_certified',
}

View File

@ -21,6 +21,7 @@ import { styled, useTheme } from '@superset-ui/core';
import { AntdCard, Skeleton, ThinSkeleton } from 'src/common/components';
import { Tooltip } from 'src/components/Tooltip';
import ImageLoader, { BackgroundPosition } from './ImageLoader';
import CertifiedIcon from '../CertifiedIcon';
const ActionsWrapper = styled.div`
width: 64px;
@ -161,6 +162,8 @@ interface CardProps {
rows?: number | string;
avatar?: React.ReactElement | null;
cover?: React.ReactNode | null;
certifiedBy?: string;
certificationDetails?: string;
}
function ListViewCard({
@ -178,6 +181,8 @@ function ListViewCard({
loading,
imgPosition = 'top',
cover,
certifiedBy,
certificationDetails,
}: CardProps) {
const Link = url && linkComponent ? linkComponent : AnchorLink;
const theme = useTheme();
@ -249,7 +254,17 @@ function ListViewCard({
<TitleContainer>
<Tooltip title={title}>
<TitleLink>
<Link to={url!}>{title}</Link>
<Link to={url!}>
{certifiedBy && (
<>
<CertifiedIcon
certifiedBy={certifiedBy}
details={certificationDetails}
/>{' '}
</>
)}
{title}
</Link>
</TitleLink>
</Tooltip>
{titleRight && <TitleRight>{titleRight}</TitleRight>}

View File

@ -22,6 +22,7 @@ import React from 'react';
import PropTypes from 'prop-types';
import { styled, t } from '@superset-ui/core';
import ButtonGroup from 'src/components/ButtonGroup';
import CertifiedIcon from 'src/components/CertifiedIcon';
import {
LOG_ACTIONS_PERIODIC_RENDER_DASHBOARD,
@ -498,6 +499,14 @@ class Header extends React.PureComponent {
data-test-id={`${dashboardInfo.id}`}
>
<div className="dashboard-component-header header-large">
{dashboardInfo.certified_by && (
<>
<CertifiedIcon
certifiedBy={dashboardInfo.certified_by}
details={dashboardInfo.certification_details}
/>{' '}
</>
)}
<EditableTitle
title={dashboardTitle}
canEdit={userCanEdit && editMode}
@ -616,6 +625,8 @@ class Header extends React.PureComponent {
dashboardInfoChanged({
slug: updates.slug,
metadata: JSON.parse(updates.jsonMetadata),
certified_by: updates.certifiedBy,
certification_details: updates.certificationDetails,
});
setColorSchemeAndUnsavedChanges(updates.colorScheme);
dashboardTitleChanged(updates.title);

View File

@ -88,6 +88,8 @@ fetchMock.get(
fetchMock.get('http://localhost/api/v1/dashboard/26', {
body: {
result: {
certified_by: 'John Doe',
certification_details: 'Sample certification',
changed_by: null,
changed_by_name: '',
changed_by_url: '',
@ -121,6 +123,8 @@ fetchMock.get('http://localhost/api/v1/dashboard/26', {
});
const createProps = () => ({
certified_by: 'John Doe',
certification_details: 'Sample certification',
dashboardId: 26,
show: true,
colorScheme: 'supersetColors',
@ -155,7 +159,10 @@ test('should render - FeatureFlag disabled', async () => {
expect(screen.getByRole('heading', { name: 'Access' })).toBeInTheDocument();
expect(screen.getByRole('heading', { name: 'Colors' })).toBeInTheDocument();
expect(screen.getByRole('heading', { name: 'Advanced' })).toBeInTheDocument();
expect(screen.getAllByRole('heading')).toHaveLength(4);
expect(
screen.getByRole('heading', { name: 'Certification' }),
).toBeInTheDocument();
expect(screen.getAllByRole('heading')).toHaveLength(5);
expect(screen.getByRole('button', { name: 'Close' })).toBeInTheDocument();
expect(screen.getByRole('button', { name: 'Advanced' })).toBeInTheDocument();
@ -163,7 +170,7 @@ test('should render - FeatureFlag disabled', async () => {
expect(screen.getByRole('button', { name: 'Save' })).toBeInTheDocument();
expect(screen.getAllByRole('button')).toHaveLength(4);
expect(screen.getAllByRole('textbox')).toHaveLength(2);
expect(screen.getAllByRole('textbox')).toHaveLength(4);
expect(screen.getByRole('combobox')).toBeInTheDocument();
expect(spyColorSchemeControlWrapper).toBeCalledTimes(4);
@ -192,7 +199,10 @@ test('should render - FeatureFlag enabled', async () => {
).toBeInTheDocument();
expect(screen.getByRole('heading', { name: 'Access' })).toBeInTheDocument();
expect(screen.getByRole('heading', { name: 'Advanced' })).toBeInTheDocument();
expect(screen.getAllByRole('heading')).toHaveLength(3);
expect(
screen.getByRole('heading', { name: 'Certification' }),
).toBeInTheDocument();
expect(screen.getAllByRole('heading')).toHaveLength(4);
expect(screen.getByRole('button', { name: 'Close' })).toBeInTheDocument();
expect(screen.getByRole('button', { name: 'Advanced' })).toBeInTheDocument();
@ -200,7 +210,7 @@ test('should render - FeatureFlag enabled', async () => {
expect(screen.getByRole('button', { name: 'Save' })).toBeInTheDocument();
expect(screen.getAllByRole('button')).toHaveLength(4);
expect(screen.getAllByRole('textbox')).toHaveLength(2);
expect(screen.getAllByRole('textbox')).toHaveLength(4);
expect(screen.getAllByRole('combobox')).toHaveLength(2);
expect(spyColorSchemeControlWrapper).toBeCalledTimes(4);
@ -220,10 +230,10 @@ test('should open advance', async () => {
await screen.findByTestId('dashboard-edit-properties-form'),
).toBeInTheDocument();
expect(screen.getAllByRole('textbox')).toHaveLength(2);
expect(screen.getAllByRole('textbox')).toHaveLength(4);
expect(screen.getAllByRole('combobox')).toHaveLength(2);
userEvent.click(screen.getByRole('button', { name: 'Advanced' }));
expect(screen.getAllByRole('textbox')).toHaveLength(3);
expect(screen.getAllByRole('textbox')).toHaveLength(5);
expect(screen.getAllByRole('combobox')).toHaveLength(2);
});
@ -319,3 +329,18 @@ test('submitting with onlyApply:true', async () => {
expect(props.onSubmit).toBeCalledTimes(1);
});
});
test('Empty "Certified by" should clear "Certification details"', async () => {
const props = createProps();
const noCertifiedByProps = {
...props,
certified_by: '',
};
render(<PropertiesModal {...noCertifiedByProps} />, {
useRedux: true,
});
expect(
screen.getByRole('textbox', { name: 'Certification details' }),
).toHaveValue('');
});

View File

@ -121,6 +121,8 @@ class PropertiesModal extends React.PureComponent {
roles: [],
json_metadata: '',
colorScheme: props.colorScheme,
certified_by: '',
certification_details: '',
},
isDashboardLoaded: false,
isAdvancedOpen: false,
@ -221,6 +223,8 @@ class PropertiesModal extends React.PureComponent {
? jsonStringify(jsonMetadataObj)
: '',
colorScheme: jsonMetadataObj.color_scheme,
certified_by: dashboard.certified_by || '',
certification_details: dashboard.certification_details || '',
},
}));
const initialSelectedOwners = dashboard.owners.map(owner => ({
@ -260,6 +264,8 @@ class PropertiesModal extends React.PureComponent {
slug,
dashboard_title: dashboardTitle,
colorScheme,
certified_by: certifiedBy,
certification_details: certificationDetails,
owners: ownersValue,
roles: rolesValue,
},
@ -294,6 +300,8 @@ class PropertiesModal extends React.PureComponent {
jsonMetadata,
ownerIds: owners,
colorScheme: currentColorScheme,
certifiedBy,
certificationDetails,
...moreProps,
});
this.props.onHide();
@ -306,6 +314,9 @@ class PropertiesModal extends React.PureComponent {
slug: slug || null,
json_metadata: jsonMetadata || null,
owners,
certified_by: certifiedBy || null,
certification_details:
certifiedBy && certificationDetails ? certificationDetails : null,
...morePutProps,
}),
}).then(({ json: { result } }) => {
@ -321,6 +332,8 @@ class PropertiesModal extends React.PureComponent {
jsonMetadata: result.json_metadata,
ownerIds: result.owners,
colorScheme: currentColorScheme,
certifiedBy: result.certified_by,
certificationDetails: result.certification_details,
...moreResultProps,
});
this.props.onHide();
@ -515,6 +528,45 @@ class PropertiesModal extends React.PureComponent {
{isFeatureEnabled(FeatureFlag.DASHBOARD_RBAC)
? this.getRowsWithRoles()
: this.getRowsWithoutRoles()}
<Row>
<Col xs={24} md={24}>
<h3>{t('Certification')}</h3>
</Col>
</Row>
<Row gutter={16}>
<Col xs={24} md={12}>
<FormItem label={t('Certified by')}>
<Input
aria-label={t('Certified by')}
name="certified_by"
type="text"
value={values.certified_by}
onChange={this.onChange}
disabled={!isDashboardLoaded}
/>
<p className="help-block">
{t('Person or group that has certified this dashboard.')}
</p>
</FormItem>
</Col>
<Col xs={24} md={12}>
<FormItem label={t('Certification details')}>
<Input
aria-label={t('Certification details')}
name="certification_details"
type="text"
value={values.certification_details || ''}
onChange={this.onChange}
disabled={!isDashboardLoaded}
/>
<p className="help-block">
{t(
'Any additional detail to show in the certification tooltip.',
)}
</p>
</FormItem>
</Col>
</Row>
<Row>
<Col xs={24} md={24}>
<h3 style={{ marginTop: '1em' }}>

View File

@ -44,6 +44,7 @@ import Timer from 'src/components/Timer';
import CachedLabel from 'src/components/CachedLabel';
import PropertiesModal from 'src/explore/components/PropertiesModal';
import { sliceUpdated } from 'src/explore/actions/exploreActions';
import CertifiedIcon from 'src/components/CertifiedIcon';
import ExploreActionButtons from '../ExploreActionButtons';
import RowCountLabel from '../RowCountLabel';
@ -165,7 +166,10 @@ export class ExploreChartHeader extends React.PureComponent {
}
getSliceName() {
return this.props.sliceName || t('%s - untitled', this.props.table_name);
const { sliceName, table_name: tableName } = this.props;
const title = sliceName || t('%s - untitled', tableName);
return title;
}
postChartFormData() {
@ -241,7 +245,7 @@ export class ExploreChartHeader extends React.PureComponent {
}
render() {
const { user, form_data: formData } = this.props;
const { user, form_data: formData, slice } = this.props;
const {
chartStatus,
chartUpdateEndTime,
@ -257,6 +261,14 @@ export class ExploreChartHeader extends React.PureComponent {
return (
<StyledHeader id="slice-header" className="panel-title-large">
<div className="title-panel">
{slice?.certified_by && (
<>
<CertifiedIcon
certifiedBy={slice.certified_by}
details={slice.certification_details}
/>{' '}
</>
)}
<EditableTitle
title={this.getSliceName()}
canEdit={

View File

@ -27,6 +27,8 @@ import PropertiesModal from '.';
const createProps = () => ({
slice: {
cache_timeout: null,
certified_by: 'John Doe',
certification_details: 'Sample certification',
changed_on: '2021-03-19T16:30:56.750230',
changed_on_humanized: '7 days ago',
datasource: 'FCC 2018 Survey',
@ -87,6 +89,8 @@ fetchMock.get('http://localhost/api/v1/chart/318', {
},
result: {
cache_timeout: null,
certified_by: 'John Doe',
certification_details: 'Sample certification',
dashboards: [
{
dashboard_title: 'FCC New Coder Survey 2018',
@ -145,6 +149,8 @@ fetchMock.put('http://localhost/api/v1/chart/318', {
id: 318,
result: {
cache_timeout: null,
certified_by: 'John Doe',
certification_details: 'Sample certification',
description: null,
owners: [],
slice_name: 'Age distribution of respondents',
@ -211,7 +217,7 @@ test('Should render all elements inside modal', async () => {
const props = createProps();
render(<PropertiesModal {...props} />);
await waitFor(() => {
expect(screen.getAllByRole('textbox')).toHaveLength(3);
expect(screen.getAllByRole('textbox')).toHaveLength(5);
expect(screen.getByRole('combobox')).toBeInTheDocument();
expect(
screen.getByRole('heading', { name: 'Basic information' }),
@ -226,6 +232,12 @@ test('Should render all elements inside modal', async () => {
expect(screen.getByRole('heading', { name: 'Access' })).toBeVisible();
expect(screen.getByText('Owners')).toBeVisible();
expect(
screen.getByRole('heading', { name: 'Configuration' }),
).toBeVisible();
expect(screen.getByText('Certified by')).toBeVisible();
expect(screen.getByText('Certification details')).toBeVisible();
});
});
@ -275,3 +287,19 @@ test('"Save" button should call only "onSave"', async () => {
expect(props.onHide).toBeCalledTimes(1);
});
});
test('Empty "Certified by" should clear "Certification details"', async () => {
const props = createProps();
const noCertifiedByProps = {
...props,
slice: {
...props.slice,
certified_by: '',
},
};
render(<PropertiesModal {...noCertifiedByProps} />);
expect(
screen.getByRole('textbox', { name: 'Certification details' }),
).toHaveValue('');
});

View File

@ -18,14 +18,13 @@
*/
import React, { useMemo, useState, useCallback, useEffect } from 'react';
import Modal from 'src/components/Modal';
import { Row, Col, Input, TextArea } from 'src/common/components';
import { Form, Row, Col, Input, TextArea } from 'src/common/components';
import Button from 'src/components/Button';
import { Select } from 'src/components';
import { SelectValue } from 'antd/lib/select';
import rison from 'rison';
import { t, SupersetClient } from '@superset-ui/core';
import { t, SupersetClient, styled } from '@superset-ui/core';
import Chart, { Slice } from 'src/types/Chart';
import { Form, FormItem } from 'src/components/Form';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';
type PropertiesModalProps = {
@ -37,6 +36,16 @@ type PropertiesModalProps = {
existingOwners?: SelectValue;
};
const FormItem = Form.Item;
const StyledFormItem = styled(Form.Item)`
margin-bottom: 0;
`;
const StyledHelpBlock = styled.span`
margin-bottom: 0;
`;
export default function PropertiesModal({
slice,
onHide,
@ -44,14 +53,11 @@ export default function PropertiesModal({
show,
}: PropertiesModalProps) {
const [submitting, setSubmitting] = useState(false);
const [selectedOwners, setSelectedOwners] = useState<SelectValue | null>(
null,
);
const [form] = Form.useForm();
// values of form inputs
const [name, setName] = useState(slice.slice_name || '');
const [description, setDescription] = useState(slice.description || '');
const [cacheTimeout, setCacheTimeout] = useState(
slice.cache_timeout != null ? slice.cache_timeout : '',
const [selectedOwners, setSelectedOwners] = useState<SelectValue | null>(
null,
);
function showError({ error, statusText, message }: any) {
@ -110,14 +116,26 @@ export default function PropertiesModal({
[],
);
const onSubmit = async (event: React.FormEvent) => {
event.stopPropagation();
event.preventDefault();
const onSubmit = async (values: {
certified_by?: string;
certification_details?: string;
description?: string;
cache_timeout?: number;
}) => {
setSubmitting(true);
const {
certified_by: certifiedBy,
certification_details: certificationDetails,
description,
cache_timeout: cacheTimeout,
} = values;
const payload: { [key: string]: any } = {
slice_name: name || null,
description: description || null,
cache_timeout: cacheTimeout || null,
certified_by: certifiedBy || null,
certification_details:
certifiedBy && certificationDetails ? certificationDetails : null,
};
if (selectedOwners) {
payload.owners = (
@ -177,11 +195,10 @@ export default function PropertiesModal({
</Button>
<Button
data-test="properties-modal-save-button"
htmlType="button"
htmlType="submit"
buttonSize="small"
buttonStyle="primary"
// @ts-ignore
onClick={onSubmit}
onClick={form.submit}
disabled={submitting || !name}
cta
>
@ -192,7 +209,21 @@ export default function PropertiesModal({
responsive
wrapProps={{ 'data-test': 'properties-edit-modal' }}
>
<Form onFinish={onSubmit} layout="vertical">
<Form
form={form}
onFinish={onSubmit}
layout="vertical"
initialValues={{
name: slice.slice_name || '',
description: slice.description || '',
cache_timeout: slice.cache_timeout != null ? slice.cache_timeout : '',
certified_by: slice.certified_by || '',
certification_details:
slice.certified_by && slice.certification_details
? slice.certification_details
: '',
}}
>
<Row gutter={16}>
<Col xs={24} md={12}>
<h3>{t('Basic information')}</h3>
@ -208,40 +239,50 @@ export default function PropertiesModal({
}
/>
</FormItem>
<FormItem label={t('Description')}>
<TextArea
rows={3}
name="description"
value={description}
onChange={(event: React.ChangeEvent<HTMLTextAreaElement>) =>
setDescription(event.target.value ?? '')
}
style={{ maxWidth: '100%' }}
/>
<p className="help-block">
<FormItem>
<StyledFormItem label={t('Description')} name="description">
<TextArea rows={3} style={{ maxWidth: '100%' }} />
</StyledFormItem>
<StyledHelpBlock className="help-block">
{t(
'The description can be displayed as widget headers in the dashboard view. Supports markdown.',
)}
</p>
</StyledHelpBlock>
</FormItem>
<h3>{t('Certification')}</h3>
<FormItem>
<StyledFormItem label={t('Certified by')} name="certified_by">
<Input />
</StyledFormItem>
<StyledHelpBlock className="help-block">
{t('Person or group that has certified this chart.')}
</StyledHelpBlock>
</FormItem>
<FormItem>
<StyledFormItem
label={t('Certification details')}
name="certification_details"
>
<Input />
</StyledFormItem>
<StyledHelpBlock className="help-block">
{t(
'Any additional detail to show in the certification tooltip.',
)}
</StyledHelpBlock>
</FormItem>
</Col>
<Col xs={24} md={12}>
<h3>{t('Configuration')}</h3>
<FormItem label={t('Cache timeout')}>
<Input
name="cacheTimeout"
type="text"
value={cacheTimeout}
onChange={(event: React.ChangeEvent<HTMLInputElement>) => {
const targetValue = event.target.value ?? '';
setCacheTimeout(targetValue.replace(/[^0-9]/, ''));
}}
/>
<p className="help-block">
<FormItem>
<StyledFormItem label={t('Cache timeout')} name="cacheTimeout">
<Input />
</StyledFormItem>
<StyledHelpBlock className="help-block">
{t(
"Duration (in seconds) of the caching timeout for this chart. Note this defaults to the dataset's timeout if undefined.",
)}
</p>
</StyledHelpBlock>
</FormItem>
<h3 style={{ marginTop: '1em' }}>{t('Access')}</h3>
<FormItem label={ownersLabel}>
@ -255,11 +296,11 @@ export default function PropertiesModal({
disabled={!selectedOwners}
allowClear
/>
<p className="help-block">
<StyledHelpBlock className="help-block">
{t(
'A list of users who can alter the chart. Searchable by name or username.',
)}
</p>
</StyledHelpBlock>
</FormItem>
</Col>
</Row>

View File

@ -33,6 +33,8 @@ export interface Chart {
changed_on: string;
changed_on_delta_humanized?: string;
changed_on_utc?: string;
certified_by?: string;
certification_details?: string;
description: string | null;
cache_timeout: number | null;
thumbnail_url?: string;
@ -49,6 +51,8 @@ export type Slice = {
slice_name: string;
description: string | null;
cache_timeout: number | null;
certified_by?: string;
certification_details?: string;
form_data?: QueryFormData;
query_context?: object;
};

View File

@ -142,6 +142,8 @@ export default function ChartCard({
<ListViewCard
loading={loading}
title={chart.slice_name}
certifiedBy={chart.certified_by}
certificationDetails={chart.certification_details}
cover={
!isFeatureEnabled(FeatureFlag.THUMBNAILS) || !showThumbnails ? (
<></>

View File

@ -58,6 +58,7 @@ import { Tooltip } from 'src/components/Tooltip';
import Icons from 'src/components/Icons';
import { nativeFilterGate } from 'src/dashboard/components/nativeFilters/utils';
import setupPlugins from 'src/setup/setupPlugins';
import CertifiedIcon from 'src/components/CertifiedIcon';
import ChartCard from './ChartCard';
const PAGE_SIZE = 25;
@ -237,10 +238,23 @@ function ChartList(props: ChartListProps) {
{
Cell: ({
row: {
original: { url, slice_name: sliceName },
original: {
url,
slice_name: sliceName,
certified_by: certifiedBy,
certification_details: certificationDetails,
},
},
}: any) => (
<a href={url} data-test={`${sliceName}-list-chart-title`}>
{certifiedBy && (
<>
<CertifiedIcon
certifiedBy={certifiedBy}
details={certificationDetails}
/>{' '}
</>
)}
{sliceName}
</a>
),
@ -512,6 +526,18 @@ function ChartList(props: ChartListProps) {
paginate: true,
},
...(props.user.userId ? [favoritesFilter] : []),
{
Header: t('Certified'),
id: 'id',
urlDisplay: 'certified',
input: 'select',
operator: FilterOperator.chartIsCertified,
unfilteredLabel: t('Any'),
selects: [
{ label: t('Yes'), value: true },
{ label: t('No'), value: false },
],
},
{
Header: t('Search'),
id: 'slice_name',

View File

@ -147,6 +147,8 @@ function DashboardCard({
<ListViewCard
loading={dashboard.loading || false}
title={dashboard.dashboard_title}
certifiedBy={dashboard.certified_by}
certificationDetails={dashboard.certification_details}
titleRight={
<Label>{dashboard.published ? t('published') : t('draft')}</Label>
}

View File

@ -49,6 +49,7 @@ import ImportModelsModal from 'src/components/ImportModal/index';
import OmniContainer from 'src/components/OmniContainer';
import Dashboard from 'src/dashboard/containers/Dashboard';
import CertifiedIcon from 'src/components/CertifiedIcon';
import DashboardCard from './DashboardCard';
import { DashboardStatus } from './types';
@ -173,6 +174,8 @@ function DashboardList(props: DashboardListProps) {
json_metadata = '',
changed_on_delta_humanized,
url = '',
certified_by = '',
certification_details = '',
} = json.result;
return {
...dashboard,
@ -184,6 +187,8 @@ function DashboardList(props: DashboardListProps) {
json_metadata,
changed_on_delta_humanized,
url,
certified_by,
certification_details,
};
}
return dashboard;
@ -250,9 +255,26 @@ function DashboardList(props: DashboardListProps) {
{
Cell: ({
row: {
original: { url, dashboard_title: dashboardTitle },
original: {
url,
dashboard_title: dashboardTitle,
certified_by: certifiedBy,
certification_details: certificationDetails,
},
},
}: any) => <Link to={url}>{dashboardTitle}</Link>,
}: any) => (
<Link to={url}>
{certifiedBy && (
<>
<CertifiedIcon
certifiedBy={certifiedBy}
details={certificationDetails}
/>{' '}
</>
)}
{dashboardTitle}
</Link>
),
Header: t('Title'),
accessor: 'dashboard_title',
},
@ -478,6 +500,18 @@ function DashboardList(props: DashboardListProps) {
],
},
...(props.user.userId ? [favoritesFilter] : []),
{
Header: t('Certified'),
id: 'id',
urlDisplay: 'certified',
input: 'select',
operator: FilterOperator.dashboardIsCertified,
unfilteredLabel: t('Any'),
selects: [
{ label: t('Yes'), value: true },
{ label: t('No'), value: false },
],
},
{
Header: t('Search'),
id: 'dashboard_title',

View File

@ -564,6 +564,8 @@ export const useChartEditModal = (
slice_name: chart.slice_name,
description: chart.description,
cache_timeout: chart.cache_timeout,
certified_by: chart.certified_by,
certification_details: chart.certification_details,
});
}

View File

@ -48,6 +48,8 @@ export interface DashboardTableProps {
}
export interface Dashboard {
certified_by?: string;
certification_details?: string;
changed_by_name: string;
changed_by_url: string;
changed_on_delta_humanized?: string;

View File

@ -47,7 +47,12 @@ from superset.charts.commands.export import ExportChartsCommand
from superset.charts.commands.importers.dispatcher import ImportChartsCommand
from superset.charts.commands.update import UpdateChartCommand
from superset.charts.dao import ChartDAO
from superset.charts.filters import ChartAllTextFilter, ChartFavoriteFilter, ChartFilter
from superset.charts.filters import (
ChartAllTextFilter,
ChartCertifiedFilter,
ChartFavoriteFilter,
ChartFilter,
)
from superset.charts.schemas import (
CHART_SCHEMAS,
ChartPostSchema,
@ -104,6 +109,8 @@ class ChartRestApi(BaseSupersetModelRestApi):
method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
show_columns = [
"cache_timeout",
"certified_by",
"certification_details",
"dashboards.dashboard_title",
"dashboards.id",
"dashboards.json_metadata",
@ -119,6 +126,8 @@ class ChartRestApi(BaseSupersetModelRestApi):
]
show_select_columns = show_columns + ["table.id"]
list_columns = [
"certified_by",
"certification_details",
"cache_timeout",
"changed_by.first_name",
"changed_by.last_name",
@ -185,7 +194,7 @@ class ChartRestApi(BaseSupersetModelRestApi):
base_order = ("changed_on", "desc")
base_filters = [["id", ChartFilter, lambda: []]]
search_filters = {
"id": [ChartFavoriteFilter],
"id": [ChartFavoriteFilter, ChartCertifiedFilter],
"slice_name": [ChartAllTextFilter],
}

View File

@ -17,7 +17,7 @@
from typing import Any
from flask_babel import lazy_gettext as _
from sqlalchemy import or_
from sqlalchemy import and_, or_
from sqlalchemy.orm.query import Query
from superset import security_manager
@ -55,6 +55,22 @@ class ChartFavoriteFilter(BaseFavoriteFilter): # pylint: disable=too-few-public
model = Slice
class ChartCertifiedFilter(BaseFilter): # pylint: disable=too-few-public-methods
"""
Custom filter for the GET list that filters all certified charts
"""
name = _("Is certified")
arg_name = "chart_is_certified"
def apply(self, query: Query, value: Any) -> Query:
if value is True:
return query.filter(and_(Slice.certified_by.isnot(None)))
if value is False:
return query.filter(and_(Slice.certified_by.is_(None)))
return query
class ChartFilter(BaseFilter): # pylint: disable=too-few-public-methods
def apply(self, query: Query, value: Any) -> Query:
if security_manager.can_access_all_datasources():

View File

@ -118,6 +118,8 @@ form_data_description = (
)
description_markeddown_description = "Sanitized HTML version of the chart description."
owners_name_description = "Name of an owner of the chart."
certified_by_description = "Person or group that has certified this chart"
certification_details_description = "Details of the certification"
#
# OpenAPI method specification overrides
@ -161,6 +163,8 @@ class ChartEntityResponseSchema(Schema):
)
form_data = fields.Dict(description=form_data_description)
slice_url = fields.String(description=slice_url_description)
certified_by = fields.String(description=certified_by_description)
certification_details = fields.String(description=certification_details_description)
class ChartPostSchema(Schema):
@ -202,6 +206,10 @@ class ChartPostSchema(Schema):
description=datasource_name_description, allow_none=True
)
dashboards = fields.List(fields.Integer(description=dashboards_description))
certified_by = fields.String(description=certified_by_description, allow_none=True)
certification_details = fields.String(
description=certification_details_description, allow_none=True
)
class ChartPutSchema(Schema):
@ -239,6 +247,10 @@ class ChartPutSchema(Schema):
allow_none=True,
)
dashboards = fields.List(fields.Integer(description=dashboards_description))
certified_by = fields.String(description=certified_by_description, allow_none=True)
certification_details = fields.String(
description=certification_details_description, allow_none=True
)
class ChartGetDatasourceObjectDataResponseSchema(Schema):

View File

@ -54,6 +54,7 @@ from superset.dashboards.commands.update import UpdateDashboardCommand
from superset.dashboards.dao import DashboardDAO
from superset.dashboards.filters import (
DashboardAccessFilter,
DashboardCertifiedFilter,
DashboardFavoriteFilter,
DashboardTitleOrSlugFilter,
FilterRelatedRoles,
@ -122,6 +123,8 @@ class DashboardRestApi(BaseSupersetModelRestApi):
"position_json",
"json_metadata",
"thumbnail_url",
"certified_by",
"certification_details",
"changed_by.first_name",
"changed_by.last_name",
"changed_by.username",
@ -151,6 +154,8 @@ class DashboardRestApi(BaseSupersetModelRestApi):
]
add_columns = [
"certified_by",
"certification_details",
"dashboard_title",
"slug",
"owners",
@ -174,7 +179,7 @@ class DashboardRestApi(BaseSupersetModelRestApi):
)
search_filters = {
"dashboard_title": [DashboardTitleOrSlugFilter],
"id": [DashboardFavoriteFilter],
"id": [DashboardFavoriteFilter, DashboardCertifiedFilter],
}
base_order = ("changed_on", "desc")

View File

@ -158,3 +158,19 @@ class FilterRelatedRoles(BaseFilter): # pylint: disable=too-few-public-methods
if value:
return query.filter(role_model.name.ilike(f"%{value}%"),)
return query
class DashboardCertifiedFilter(BaseFilter): # pylint: disable=too-few-public-methods
"""
Custom filter for the GET list that filters all certified dashboards
"""
name = _("Is certified")
arg_name = "dashboard_is_certified"
def apply(self, query: Query, value: Any) -> Query:
if value is True:
return query.filter(and_(Dashboard.certified_by.isnot(None),))
if value is False:
return query.filter(and_(Dashboard.certified_by.is_(None),))
return query

View File

@ -64,6 +64,8 @@ published_description = (
charts_description = (
"The names of the dashboard's charts. Names are used for legacy reasons."
)
certified_by_description = "Person or group that has certified this dashboard"
certification_details_description = "Details of the certification"
openapi_spec_methods_override = {
"get": {"get": {"description": "Get a dashboard detail information."}},
@ -151,6 +153,8 @@ class DashboardGetResponseSchema(Schema):
css = fields.String(description=css_description)
json_metadata = fields.String(description=json_metadata_description)
position_json = fields.String(description=position_json_description)
certified_by = fields.String(description=certified_by_description)
certification_details = fields.String(description=certification_details_description)
changed_by_name = fields.String()
changed_by_url = fields.String()
changed_by = fields.Nested(UserSchema)
@ -237,6 +241,10 @@ class DashboardPostSchema(BaseDashboardSchema):
description=json_metadata_description, validate=validate_json_metadata,
)
published = fields.Boolean(description=published_description)
certified_by = fields.String(description=certified_by_description, allow_none=True)
certification_details = fields.String(
description=certification_details_description, allow_none=True
)
class DashboardPutSchema(BaseDashboardSchema):
@ -262,6 +270,10 @@ class DashboardPutSchema(BaseDashboardSchema):
validate=validate_json_metadata,
)
published = fields.Boolean(description=published_description, allow_none=True)
certified_by = fields.String(description=certified_by_description, allow_none=True)
certification_details = fields.String(
description=certification_details_description, allow_none=True
)
class ChartFavStarResponseResult(Schema):

View File

@ -0,0 +1,44 @@
# 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.
"""add_certifications_columns_to_dashboard
Revision ID: aea15018d53b
Revises: f9847149153d
Create Date: 2021-11-05 11:11:55.496618
"""
# revision identifiers, used by Alembic.
import sqlalchemy as sa
from alembic import op
revision = "aea15018d53b"
down_revision = "f9847149153d"
def upgrade():
with op.batch_alter_table("dashboards") as batch_op:
batch_op.add_column(sa.Column("certified_by", sa.Text(), nullable=True))
batch_op.add_column(
sa.Column("certification_details", sa.Text(), nullable=True)
)
def downgrade():
with op.batch_alter_table("dashboards") as batch_op:
batch_op.drop_column("certified_by")
batch_op.drop_column("certification_details")

View File

@ -0,0 +1,44 @@
# 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.
"""add_certifications_columns_to_slice
Revision ID: f9847149153d
Revises: 0ca9e5f1dacd
Create Date: 2021-11-03 14:07:09.905194
"""
# revision identifiers, used by Alembic.
import sqlalchemy as sa
from alembic import op
revision = "f9847149153d"
down_revision = "0ca9e5f1dacd"
def upgrade():
with op.batch_alter_table("slices") as batch_op:
batch_op.add_column(sa.Column("certified_by", sa.Text(), nullable=True))
batch_op.add_column(
sa.Column("certification_details", sa.Text(), nullable=True)
)
def downgrade():
with op.batch_alter_table("slices") as batch_op:
batch_op.drop_column("certified_by")
batch_op.drop_column("certification_details")

View File

@ -142,6 +142,8 @@ class Dashboard(Model, AuditMixinNullable, ImportExportMixin):
position_json = Column(utils.MediumText())
description = Column(Text)
css = Column(Text)
certified_by = Column(Text)
certification_details = Column(Text)
json_metadata = Column(Text)
slug = Column(String(255), unique=True)
slices = relationship(Slice, secondary=dashboard_slices, backref="dashboards")
@ -265,6 +267,8 @@ class Dashboard(Model, AuditMixinNullable, ImportExportMixin):
return {
"id": self.id,
"metadata": self.params_dict,
"certified_by": self.certified_by,
"certification_details": self.certification_details,
"css": self.css,
"dashboard_title": self.dashboard_title,
"published": self.published,

View File

@ -81,6 +81,8 @@ class Slice( # pylint: disable=too-many-public-methods
# when the database row was last written
last_saved_at = Column(DateTime, nullable=True)
last_saved_by_fk = Column(Integer, ForeignKey("ab_user.id"), nullable=True)
certified_by = Column(Text)
certification_details = Column(Text)
last_saved_by = relationship(
security_manager.user_model, foreign_keys=[last_saved_by_fk]
)
@ -211,6 +213,8 @@ class Slice( # pylint: disable=too-many-public-methods
"slice_id": self.id,
"slice_name": self.slice_name,
"slice_url": self.slice_url,
"certified_by": self.certified_by,
"certification_details": self.certification_details,
}
@property

View File

@ -80,7 +80,7 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
charts = []
admin = self.get_user("admin")
for cx in range(CHARTS_FIXTURE_COUNT - 1):
charts.append(self.insert_chart(f"name{cx}", [admin.id], 1))
charts.append(self.insert_chart(f"name{cx}", [admin.id], 1,))
fav_charts = []
for cx in range(round(CHARTS_FIXTURE_COUNT / 2)):
fav_star = FavStar(
@ -98,6 +98,29 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
db.session.delete(fav_chart)
db.session.commit()
@pytest.fixture()
def create_certified_charts(self):
with self.create_app().app_context():
certified_charts = []
admin = self.get_user("admin")
for cx in range(CHARTS_FIXTURE_COUNT):
certified_charts.append(
self.insert_chart(
f"certified{cx}",
[admin.id],
1,
certified_by="John Doe",
certification_details="Sample certification",
)
)
yield certified_charts
# rollback changes
for chart in certified_charts:
db.session.delete(chart)
db.session.commit()
@pytest.fixture()
def create_chart_with_report(self):
with self.create_app().app_context():
@ -420,6 +443,8 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
"datasource_id": 1,
"datasource_type": "table",
"dashboards": dashboards_ids,
"certified_by": "John Doe",
"certification_details": "Sample certification",
}
self.login(username="admin")
uri = f"api/v1/chart/"
@ -535,6 +560,8 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
"datasource_id": birth_names_table_id,
"datasource_type": "table",
"dashboards": [dash_id],
"certified_by": "Mario Rossi",
"certification_details": "Edited certification",
}
self.login(username="admin")
uri = f"api/v1/chart/{chart_id}"
@ -553,6 +580,8 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
self.assertEqual(model.datasource_id, birth_names_table_id)
self.assertEqual(model.datasource_type, "table")
self.assertEqual(model.datasource_name, full_table_name)
self.assertEqual(model.certified_by, "Mario Rossi")
self.assertEqual(model.certification_details, "Edited certification")
self.assertIn(model.id, [slice.id for slice in related_dashboard.slices])
db.session.delete(model)
db.session.commit()
@ -703,6 +732,8 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
self.assertEqual(rv.status_code, 200)
expected_result = {
"cache_timeout": None,
"certified_by": None,
"certification_details": None,
"dashboards": [],
"description": None,
"owners": [
@ -897,6 +928,36 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(data["count"], 8)
@pytest.mark.usefixtures("create_certified_charts")
def test_gets_certified_charts_filter(self):
arguments = {
"filters": [{"col": "id", "opr": "chart_is_certified", "value": True,}],
"keys": ["none"],
"columns": ["slice_name"],
}
self.login(username="admin")
uri = f"api/v1/chart/?q={prison.dumps(arguments)}"
rv = self.get_assert_metric(uri, "get_list")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(data["count"], CHARTS_FIXTURE_COUNT)
@pytest.mark.usefixtures("create_charts")
def test_gets_not_certified_charts_filter(self):
arguments = {
"filters": [{"col": "id", "opr": "chart_is_certified", "value": False,}],
"keys": ["none"],
"columns": ["slice_name"],
}
self.login(username="admin")
uri = f"api/v1/chart/?q={prison.dumps(arguments)}"
rv = self.get_assert_metric(uri, "get_list")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(data["count"], 17)
@pytest.mark.usefixtures("load_energy_charts")
def test_user_gets_none_filtered_energy_slices(self):
# test filtering on datasource_name

View File

@ -86,6 +86,8 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi
css: str = "",
json_metadata: str = "",
published: bool = False,
certified_by: Optional[str] = None,
certification_details: Optional[str] = None,
) -> Dashboard:
obj_owners = list()
obj_roles = list()
@ -107,6 +109,8 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi
slices=slices,
published=published,
created_by=created_by,
certified_by=certified_by,
certification_details=certification_details,
)
db.session.add(dashboard)
db.session.commit()
@ -125,6 +129,8 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi
f"slug{cx}",
[admin.id],
slices=charts if cx < half_dash_count else [],
certified_by="John Doe",
certification_details="Sample certification",
)
if cx < half_dash_count:
chart = self.insert_chart(f"slice{cx}", [admin.id], 1, params="{}")
@ -315,6 +321,8 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi
rv = self.get_assert_metric(uri, "get")
self.assertEqual(rv.status_code, 200)
expected_result = {
"certified_by": None,
"certification_details": None,
"changed_by": None,
"changed_by_name": "",
"changed_by_url": "",
@ -611,6 +619,38 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi
expected_model.dashboard_title == data["result"][i]["dashboard_title"]
)
@pytest.mark.usefixtures("create_dashboards")
def test_gets_certified_dashboards_filter(self):
arguments = {
"filters": [{"col": "id", "opr": "dashboard_is_certified", "value": True,}],
"keys": ["none"],
"columns": ["dashboard_title"],
}
self.login(username="admin")
uri = f"api/v1/dashboard/?q={prison.dumps(arguments)}"
rv = self.get_assert_metric(uri, "get_list")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(data["count"], DASHBOARDS_FIXTURE_COUNT)
@pytest.mark.usefixtures("create_dashboards")
def test_gets_not_certified_dashboards_filter(self):
arguments = {
"filters": [
{"col": "id", "opr": "dashboard_is_certified", "value": False,}
],
"keys": ["none"],
"columns": ["dashboard_title"],
}
self.login(username="admin")
uri = f"api/v1/dashboard/?q={prison.dumps(arguments)}"
rv = self.get_assert_metric(uri, "get_list")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(data["count"], 6)
def create_dashboard_import(self):
buf = BytesIO()
with ZipFile(buf, "w") as bundle:

View File

@ -36,6 +36,8 @@ class InsertChartMixin:
viz_type: Optional[str] = None,
params: Optional[str] = None,
cache_timeout: Optional[int] = None,
certified_by: Optional[str] = None,
certification_details: Optional[str] = None,
) -> Slice:
obj_owners = list()
for owner in owners:
@ -46,6 +48,8 @@ class InsertChartMixin:
)
slice = Slice(
cache_timeout=cache_timeout,
certified_by=certified_by,
certification_details=certification_details,
created_by=created_by,
datasource_id=datasource.id,
datasource_name=datasource.name,