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
apache · Dec 21, 2021 · 8057582 · 8057582
1 parent 70228ad
commit 8057582
Show file tree

Hide file tree

Showing 27 changed files with 543 additions and 68 deletions.
diff --git a/superset-frontend/src/components/ListView/types.ts b/superset-frontend/src/components/ListView/types.ts
@@ -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',
 }
diff --git a/superset-frontend/src/components/ListViewCard/index.tsx b/superset-frontend/src/components/ListViewCard/index.tsx
@@ -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>}

diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx
@@ -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,
@@ -493,6 +494,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}

diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx
@@ -90,6 +90,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: '',
@@ -122,6 +124,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',
@@ -156,15 +160,18 @@ 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();
   expect(screen.getByRole('button', { name: 'Cancel' })).toBeInTheDocument();
   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).toBeCalledWith(
@@ -192,15 +199,18 @@ 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();
   expect(screen.getByRole('button', { name: 'Cancel' })).toBeInTheDocument();
   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).toBeCalledWith(
@@ -219,10 +229,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);
 });
 
@@ -321,3 +331,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('');
+});
diff --git a/superset-frontend/src/explore/components/ExploreChartHeader.jsx b/superset-frontend/src/explore/components/ExploreChartHeader.jsx
@@ -36,16 +36,17 @@ import {
 } from 'src/reports/actions/reports';
 import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
 import HeaderReportActionsDropdown from 'src/components/ReportModal/HeaderReportActionsDropdown';
-import { chartPropShape } from '../../dashboard/util/propShapes';
+import { chartPropShape } from 'src/dashboard/util/propShapes';
+import EditableTitle from 'src/components/EditableTitle';
+import AlteredSliceTag from 'src/components/AlteredSliceTag';
+import FaveStar from 'src/components/FaveStar';
+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';
-import EditableTitle from '../../components/EditableTitle';
-import AlteredSliceTag from '../../components/AlteredSliceTag';
-import FaveStar from '../../components/FaveStar';
-import Timer from '../../components/Timer';
-import CachedLabel from '../../components/CachedLabel';
-import PropertiesModal from './PropertiesModal';
-import { sliceUpdated } from '../actions/exploreActions';
 
 const CHART_STATUS_MAP = {
   failed: 'danger',
@@ -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={!this.props.slice || this.props.can_overwrite}

diff --git a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx
@@ -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('');
+});