feat: Add Table Notices (#957)

* Alert styling on Table Detail page Signed-off-by: Marcos Iglesias <[email protected]> * Sources table notification from the config object Signed-off-by: Marcos Iglesias <[email protected]> * Adds severities to notices Signed-off-by: Marcos Iglesias <[email protected]> * Adding it to dashboard Signed-off-by: Marcos Iglesias <[email protected]> * Updating for dashboard notices as well Signed-off-by: Marcos Iglesias <[email protected]> * Update betterer results Signed-off-by: Marcos Iglesias <[email protected]> * Adding documentation Signed-off-by: Marcos Iglesias <[email protected]> * Unique table and dashboard identifications, messageHtml as key and docs and tests updates Signed-off-by: Marcos Iglesias <[email protected]>
amundsen-io · Apr 2, 2021 · e3be638 · e3be638
1 parent fe04a06
commit e3be638
Show file tree

Hide file tree

Showing 18 changed files with 332 additions and 76 deletions.
diff --git a/amundsen_application/static/.betterer.results b/amundsen_application/static/.betterer.results
@@ -507,13 +507,13 @@ exports[`eslint`] = {
     "js/components/Tags/index.tsx:3468508233": [
       [38, 4, 21, "Must use destructuring props assignment", "4236634811"]
     ],
-    "js/config/config-default.ts:442852777": [
+    "js/config/config-default.ts:1637253929": [
       [2, 0, 72, "\`../interfaces\` import should occur before import of \`./config-types\`", "1449508543"],
-      [220, 6, 21, "\'partitionKey\' is defined but never used.", "399589312"],
-      [221, 6, 23, "\'partitionValue\' is defined but never used.", "793372348"]
+      [222, 6, 21, "\'partitionKey\' is defined but never used.", "399589312"],
+      [223, 6, 23, "\'partitionValue\' is defined but never used.", "793372348"]
     ],
-    "js/config/config-utils.ts:108982451": [
-      [7, 0, 45, "\`../interfaces\` import should occur before import of \`./config-types\`", "3885176344"]
+    "js/config/config-utils.ts:157517129": [
+      [11, 0, 45, "\`../interfaces\` import should occur before import of \`./config-types\`", "3885176344"]
     ],
     "js/ducks/announcements/api/v0.ts:908063915": [
       [30, 13, 64, "Expected the Promise rejection reason to be an Error.", "3665078104"]
@@ -769,39 +769,25 @@ exports[`eslint`] = {
       [89, 25, 5, "\'props\' is assigned a value but never used.", "187023499"],
       [100, 27, 5, "\'props\' is assigned a value but never used.", "187023499"]
     ],
-    "js/pages/DashboardPage/index.spec.tsx:3246337270": [
+    "js/pages/DashboardPage/index.spec.tsx:3159805396": [
       [22, 0, 63, "\`../../fixtures/mockRouter\` import should occur before import of \`./ChartList\`", "1389010998"],
       [24, 0, 41, "\`./constants\` import should occur before import of \`.\`", "1965203596"],
-      [59, 56, 10, "Prop spreading is forbidden", "480399587"],
-      [139, 14, 7, "\'wrapper\' is already declared in the upper scope.", "990908086"],
-      [168, 16, 7, "\'wrapper\' is already declared in the upper scope.", "990908086"],
-      [184, 14, 7, "\'wrapper\' is already declared in the upper scope.", "990908086"],
-      [202, 14, 7, "\'wrapper\' is already declared in the upper scope.", "990908086"],
-      [242, 14, 7, "\'wrapper\' is already declared in the upper scope.", "990908086"]
-    ],
-    "js/pages/DashboardPage/index.tsx:277488038": [
-      [82, 20, 16, "Must use destructuring props assignment", "1899951550"],
-      [88, 47, 19, "Must use destructuring props assignment", "1779396464"],
-      [89, 20, 16, "Must use destructuring props assignment", "1899951550"],
-      [91, 4, 23, "Must use destructuring props assignment", "3651858367"],
-      [96, 20, 16, "Must use destructuring props assignment", "1899951550"],
-      [98, 8, 14, "Must use destructuring state assignment", "1770565882"],
-      [99, 49, 19, "Must use destructuring props assignment", "1779396464"],
-      [100, 6, 13, "Do not use setState in componentDidUpdate", "57229240"],
-      [101, 6, 23, "Must use destructuring props assignment", "3651858367"],
-      [106, 23, 20, "\'searchDashboardGroup\' is already declared in the upper scope.", "2919356688"],
-      [127, 20, 20, "Must use destructuring props assignment", "3424894889"],
-      [133, 24, 20, "Must use destructuring props assignment", "3424894889"],
-      [136, 8, 20, "Must use destructuring props assignment", "3424894889"],
-      [138, 36, 20, "Must use destructuring props assignment", "3424894889"],
-      [140, 26, 20, "Must use destructuring props assignment", "3424894889"],
-      [147, 19, 20, "Must use destructuring props assignment", "3424894889"],
-      [148, 19, 20, "Must use destructuring props assignment", "3424894889"],
-      [152, 25, 20, "Must use destructuring props assignment", "3424894889"],
-      [168, 8, 21, "Must use destructuring props assignment", "3005585300"],
-      [172, 10, 7, "A form label must be associated with a control.", "2729454337"],
-      [295, 28, 20, "Must use destructuring props assignment", "3424894889"],
-      [336, 31, 14, "Must use destructuring state assignment", "1770565882"]
+      [60, 56, 10, "Prop spreading is forbidden", "480399587"],
+      [140, 14, 7, "\'wrapper\' is already declared in the upper scope.", "990908086"],
+      [169, 16, 7, "\'wrapper\' is already declared in the upper scope.", "990908086"],
+      [185, 14, 7, "\'wrapper\' is already declared in the upper scope.", "990908086"],
+      [203, 14, 7, "\'wrapper\' is already declared in the upper scope.", "990908086"],
+      [243, 14, 7, "\'wrapper\' is already declared in the upper scope.", "990908086"]
+    ],
+    "js/pages/DashboardPage/index.tsx:3355322503": [
+      [88, 20, 16, "Must use destructuring props assignment", "1899951550"],
+      [94, 22, 12, "\'getDashboard\' is already declared in the upper scope.", "2023796375"],
+      [103, 22, 12, "\'getDashboard\' is already declared in the upper scope.", "2023796375"],
+      [106, 8, 14, "Must use destructuring state assignment", "1770565882"],
+      [108, 6, 13, "Do not use setState in componentDidUpdate", "57229240"],
+      [114, 23, 20, "\'searchDashboardGroup\' is already declared in the upper scope.", "2919356688"],
+      [182, 10, 7, "A form label must be associated with a control.", "2729454337"],
+      [352, 31, 14, "Must use destructuring state assignment", "1770565882"]
     ],
     "js/pages/HomePage/index.spec.tsx:4057001982": [
       [26, 48, 10, "Prop spreading is forbidden", "480399587"],
@@ -1049,12 +1035,11 @@ exports[`eslint`] = {
     "js/pages/TableDetailPage/index.spec.tsx:163729336": [
       [57, 6, 25, "Use object destructuring.", "1230260048"]
     ],
-    "js/pages/TableDetailPage/index.tsx:1290852100": [
-      [126, 2, 20, "key should be placed after componentDidUpdate", "3916788587"],
-      [136, 22, 12, "\'getTableData\' is already declared in the upper scope.", "3938384029"],
-      [145, 22, 12, "\'getTableData\' is already declared in the upper scope.", "3938384029"],
-      [183, 4, 23, "Must use destructuring props assignment", "2314094642"],
-      [220, 6, 28, "\'openRequestDescriptionDialog\' is already declared in the upper scope.", "4102713454"]
+    "js/pages/TableDetailPage/index.tsx:2815038016": [
+      [129, 2, 20, "key should be placed after componentDidUpdate", "3916788587"],
+      [139, 22, 12, "\'getTableData\' is already declared in the upper scope.", "3938384029"],
+      [148, 22, 12, "\'getTableData\' is already declared in the upper scope.", "3938384029"],
+      [224, 6, 28, "\'openRequestDescriptionDialog\' is already declared in the upper scope.", "4102713454"]
     ],
     "js/utils/textUtils.ts:2545492889": [
       [19, 6, 46, "Unexpected lexical declaration in case block.", "156477898"]

diff --git a/amundsen_application/static/js/components/Alert/index.spec.tsx b/amundsen_application/static/js/components/Alert/index.spec.tsx
@@ -4,6 +4,8 @@
 import * as React from 'react';
 import { mount } from 'enzyme';
 
+import { NoticeSeverity } from 'config/config-types';
+
 import Alert, { AlertProps } from '.';
 
 const setup = (propOverrides?: Partial<AlertProps>) => {
@@ -96,6 +98,40 @@ describe('Alert', () => {
         expect(actual).toEqual(expected);
       });
     });
+
+    describe('when passing a severity', () => {
+      it('should render the alert icon by default', () => {
+        const { wrapper } = setup();
+        const expected = 1;
+        const actual = wrapper.find('.alert-triangle-svg-icon').length;
+
+        expect(actual).toEqual(expected);
+      });
+
+      it('should render the info icon when info severity', () => {
+        const { wrapper } = setup({ severity: NoticeSeverity.INFO });
+        const expected = 1;
+        const actual = wrapper.find('.info-svg-icon').length;
+
+        expect(actual).toEqual(expected);
+      });
+
+      it('should render the alert icon when alert severity', () => {
+        const { wrapper } = setup({ severity: NoticeSeverity.ALERT });
+        const expected = 1;
+        const actual = wrapper.find('.alert-triangle-svg-icon').length;
+
+        expect(actual).toEqual(expected);
+      });
+
+      it('should render the alert icon when warning severity', () => {
+        const { wrapper } = setup({ severity: NoticeSeverity.WARNING });
+        const expected = 1;
+        const actual = wrapper.find('.alert-triangle-svg-icon').length;
+
+        expect(actual).toEqual(expected);
+      });
+    });
   });
 
   describe('lifetime', () => {

diff --git a/amundsen_application/static/js/components/Alert/index.tsx b/amundsen_application/static/js/components/Alert/index.tsx
@@ -2,16 +2,23 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import * as React from 'react';
+import SanitizedHTML from 'react-sanitized-html';
 
 import { IconSizes } from 'interfaces';
-import { AlertIcon } from '../SVGIcons';
+import { NoticeSeverity } from 'config/config-types';
+import { AlertIcon, InformationIcon } from '../SVGIcons';
 
 import './styles.scss';
 
-const STROKE_COLOR = '#b8072c'; // $red70
+const SEVERITY_TO_COLOR_MAP = {
+  [NoticeSeverity.INFO]: '#3a97d3', // cyan50
+  [NoticeSeverity.WARNING]: '#ffb146', // $amber50
+  [NoticeSeverity.ALERT]: '#b8072c', // $red70
+};
 
 export interface AlertProps {
   message: string | React.ReactNode;
+  severity?: NoticeSeverity;
   actionLink?: React.ReactNode;
   actionText?: string;
   actionHref?: string;
@@ -20,6 +27,7 @@ export interface AlertProps {
 
 const Alert: React.FC<AlertProps> = ({
   message,
+  severity = NoticeSeverity.WARNING,
   onAction,
   actionText,
   actionHref,
@@ -51,10 +59,31 @@ const Alert: React.FC<AlertProps> = ({
     action = <span className="alert-action">{actionLink}</span>;
   }
 
+  let iconComponent: React.ReactNode = null;
+  if (severity === NoticeSeverity.INFO) {
+    iconComponent = (
+      <InformationIcon
+        fill={SEVERITY_TO_COLOR_MAP[severity]}
+        size={IconSizes.REGULAR}
+      />
+    );
+  } else {
+    iconComponent = (
+      <AlertIcon
+        stroke={SEVERITY_TO_COLOR_MAP[severity]}
+        size={IconSizes.SMALL}
+      />
+    );
+  }
+
+  // If we receive a string, we want to sanitize any html inside
+  const formattedMessage =
+    typeof message === 'string' ? <SanitizedHTML html={message} /> : message;
+
   return (
     <div className="alert">
-      <AlertIcon stroke={STROKE_COLOR} size={IconSizes.SMALL} />
-      <p className="alert-message">{message}</p>
+      {iconComponent}
+      <p className="alert-message">{formattedMessage}</p>
       {action}
     </div>
   );

diff --git a/amundsen_application/static/js/components/Alert/styles.scss b/amundsen_application/static/js/components/Alert/styles.scss
@@ -22,7 +22,8 @@ $alert-message-line-height: 24px;
     display: inline;
   }
 
-  .alert-triangle-svg-icon {
+  .alert-triangle-svg-icon,
+  .info-svg-icon {
     flex-shrink: 0;
     align-self: center;
     margin-right: $spacer-1;

diff --git a/amundsen_application/static/js/components/SVGIcons/InformationIcon.tsx b/amundsen_application/static/js/components/SVGIcons/InformationIcon.tsx
@@ -16,7 +16,12 @@ export const InformationIcon: React.FC<IconProps> = ({
   const id = `info_icon_${uuidv4()}`;
 
   return (
-    <svg width={size} height={size} viewBox="0 0 24 24">
+    <svg
+      width={size}
+      height={size}
+      viewBox="0 0 24 24"
+      className="info-svg-icon"
+    >
       <title>Info</title>
       <defs>
         <path

diff --git a/amundsen_application/static/js/config/config-default.ts b/amundsen_application/static/js/config/config-default.ts
@@ -98,6 +98,7 @@ const configDefault: AppConfig = {
           type: FilterType.INPUT_SELECT,
         },
       ],
+      notices: {},
     },
     [ResourceType.table]: {
       displayName: 'Datasets',
@@ -193,6 +194,7 @@ const configDefault: AppConfig = {
           iconPath: '/static/images/github.png',
         },
       },
+      notices: {},
     },
     [ResourceType.user]: {
       displayName: 'People',

diff --git a/amundsen_application/static/js/config/config-types.ts b/amundsen_application/static/js/config/config-types.ts
@@ -140,13 +140,28 @@ type SortCriteriaConfig = {
   [key: string]: SortCriteria;
 };
 
+export enum NoticeSeverity {
+  INFO = 'info',
+  WARNING = 'warning',
+  ALERT = 'alert',
+}
+export interface NoticeType {
+  severity: NoticeSeverity;
+  messageHtml: string;
+}
 /**
  * Stats configuration options
  */
 type StatsConfig = {
   uniqueValueTypeName: string;
 };
 
+/**
+ * A list of notices where the key is the 'schema.name' of the table and the value
+ * a Notice
+ */
+type NoticesConfigType = Record<string, NoticeType>;
+
 /**
  * Base interface for all possible ResourceConfig objects
  *
@@ -157,6 +172,7 @@ interface BaseResourceConfig {
   displayName: string;
   filterCategories?: FilterConfig;
   supportedSources?: SourcesConfig;
+  notices?: NoticesConfigType;
 }
 
 interface TableResourceConfig extends BaseResourceConfig {

diff --git a/amundsen_application/static/js/config/config-utils.ts b/amundsen_application/static/js/config/config-utils.ts
@@ -1,10 +1,14 @@
 import AppConfig from 'config/config';
 import { BadgeStyle, BadgeStyleConfig } from 'config/config-types';
-import { TableMetadata } from 'interfaces/TableMetadata';
 import { convertText, CaseType } from 'utils/textUtils';
 
-import { AnalyticsConfig, FilterConfig, LinkConfig } from './config-types';
-
+import { TableMetadata } from 'interfaces/TableMetadata';
+import {
+  AnalyticsConfig,
+  FilterConfig,
+  LinkConfig,
+  NoticeType,
+} from './config-types';
 import { ResourceType } from '../interfaces';
 
 export const DEFAULT_DATABASE_ICON_CLASS = 'icon-database icon-color';
@@ -60,6 +64,22 @@ export function getSourceIconClass(
   return config.supportedSources[sourceId].iconClass;
 }
 
+/**
+ * Returns notices for the given resource name if present
+ */
+export function getResourceNotices(
+  resourceType: ResourceType,
+  resourceName: string
+): NoticeType | false {
+  const { notices } = AppConfig.resourceConfig[resourceType];
+
+  if (notices && notices[resourceName]) {
+    return notices[resourceName];
+  }
+
+  return false;
+}
+
 /**
  * Returns the displayName for the given resourceType
  */