fix: OwnerEditor readOnly support (#545)

* Udpdate UI; readOnly support

* Unit test

* Code cleanup

* Code cleanup

* Code cleanup

* Address review comments

* Lint

amundsen_application/static/js/components/TableDetail/OwnerEditor/constants.ts

@@ -2,4 +2,9 @@ export const DEFAULT_ERROR_TEXT =
   'There was a problem with the request, please reload the page.';
 export const USERID_LABEL = 'email address';
 export const ADD_ITEM = 'Add';
+export const ADD_OWNER = 'Add Owner';
 export const DELETE_ITEM = 'Delete Item';
+export const NO_OWNER_TEXT = 'No owner';
+export const OWNED_BY = 'Owned By';
+export const CANCEL_TEXT = 'Cancel';
+export const SAVE_TEXT = 'Save';

amundsen_application/static/js/components/TableDetail/OwnerEditor/index.spec.tsx

+import * as React from 'react';
+
+import { mount } from 'enzyme';
+
+import globalState from 'fixtures/globalState';
+import AvatarLabel from 'components/common/AvatarLabel';
+
+import {
+  OwnerEditor,
+  OwnerEditorProps,
+  mapStateToProps,
+  mapDispatchToProps,
+} from '.';
+import * as Constants from './constants';
+
+describe('OwnerEditor', () => {
+  const setup = (propOverrides?: Partial<OwnerEditorProps>) => {
+    const props: OwnerEditorProps = {
+      errorText: null,
+      isLoading: false,
+      itemProps: {},
+      isEditing: null,
+      setEditMode: jest.fn(),
+      onUpdateList: jest.fn(),
+      readOnly: null,
+      ...propOverrides,
+    };
+    const wrapper = mount<OwnerEditor>(<OwnerEditor {...props} />);
+    return { props, wrapper };
+  };
+
+  describe('render', () => {
+    describe('when no owners', () => {
+      it('renders text if readOnly', () => {
+        const { wrapper } = setup({ readOnly: true });
+        expect(wrapper.find(AvatarLabel).text()).toContain(
+          Constants.NO_OWNER_TEXT
+        );
+      });
+
+      it('renders add button if not readOnly', () => {
+        const { wrapper } = setup();
+        expect(wrapper.find('.add-item-button').text()).toContain(
+          Constants.ADD_OWNER
+        );
+      });
+    });
+
+    it('renders owners if they exist', () => {
+      const { wrapper } = setup({
+        itemProps: { owner1: {}, owner2: {}, owner3: {} },
+      });
+      expect(wrapper.find(AvatarLabel).length).toBe(3);
+    });
+
+    describe('editing modal', () => {
+      it('renders when not readOnly', () => {
+        const { wrapper } = setup();
+        expect(wrapper.find('.owner-editor-modal').exists()).toBe(true);
+      });
+
+      it('does not render when readOnly', () => {
+        const { wrapper } = setup({ readOnly: true });
+        expect(wrapper.find('.owner-editor-modal').exists()).toBe(false);
+      });
+    });
+  });
+});

amundsen_application/static/js/components/TableDetail/OwnerEditor/index.tsx

@@ -44,7 +44,7 @@ export interface StateFromProps {
   itemProps: { [id: string]: OwnerAvatarLabelProps };
 }
 
-type OwnerEditorProps = ComponentProps &
+export type OwnerEditorProps = ComponentProps &
   DispatchFromProps &
   StateFromProps &
   EditableSectionChildProps;
@@ -52,7 +52,6 @@ type OwnerEditorProps = ComponentProps &
 interface OwnerEditorState {
   errorText: string | null;
   itemProps: { [id: string]: OwnerAvatarLabelProps };
-  readOnly: boolean;
   tempItemProps: { [id: string]: AvatarLabelProps };
 }
 
@@ -75,7 +74,6 @@ export class OwnerEditor extends React.Component<
     this.state = {
       errorText: props.errorText,
       itemProps: props.itemProps,
-      readOnly: props.readOnly,
       tempItemProps: props.itemProps,
     };
 
@@ -120,7 +118,6 @@ export class OwnerEditor extends React.Component<
     const onFailureCallback = () => {
       this.setState({
         errorText: Constants.DEFAULT_ERROR_TEXT,
-        readOnly: true,
       });
       this.props.setEditMode(false);
     };
@@ -208,7 +205,7 @@ export class OwnerEditor extends React.Component<
   };
 
   render() {
-    let content;
+    const hasItems = Object.keys(this.state.itemProps).length > 0;
 
     if (this.state.errorText) {
       return (
@@ -218,88 +215,91 @@ export class OwnerEditor extends React.Component<
       );
     }
 
-    if (this.state.itemProps.size === 0) {
-      content = <label className="status-message">No entries exist</label>;
-    } else {
-      content = (
-        <ul className="component-list">
-          {Object.keys(this.state.itemProps).map((key) => {
-            const owner = this.state.itemProps[key];
-            const avatarLabel = React.createElement(AvatarLabel, owner);
-
-            let listItem;
-            if (owner.link === undefined) {
-              listItem = avatarLabel;
-            } else if (owner.isExternal) {
-              listItem = (
-                <a
-                  href={owner.link}
-                  target="_blank"
-                  id={`table-owners:${key}`}
-                  onClick={logClick}
-                  rel="noreferrer"
-                >
-                  {avatarLabel}
-                </a>
-              );
-            } else {
-              listItem = (
-                <Link
-                  to={owner.link}
-                  id={`table-owners:${key}`}
-                  onClick={logClick}
-                >
-                  {avatarLabel}
-                </Link>
-              );
-            }
-
-            return <li key={`list-item:${key}`}>{listItem}</li>;
-          })}
-        </ul>
-      );
-    }
+    const ownerList = hasItems ? (
+      <ul className="component-list">
+        {Object.keys(this.state.itemProps).map((key) => {
+          const owner = this.state.itemProps[key];
+          const avatarLabel = React.createElement(AvatarLabel, owner);
+
+          let listItem;
+          if (owner.link === undefined) {
+            listItem = avatarLabel;
+          } else if (owner.isExternal) {
+            listItem = (
+              <a
+                href={owner.link}
+                target="_blank"
+                id={`table-owners:${key}`}
+                onClick={logClick}
+                rel="noopener noreferrer"
+              >
+                {avatarLabel}
+              </a>
+            );
+          } else {
+            listItem = (
+              <Link
+                to={owner.link}
+                id={`table-owners:${key}`}
+                onClick={logClick}
+              >
+                {avatarLabel}
+              </Link>
+            );
+          }
+          return <li key={`list-item:${key}`}>{listItem}</li>;
+        })}
+      </ul>
+    ) : null;
 
     return (
       <div className="owner-editor-component">
-        {content}
-        {!this.state.readOnly &&
-          Object.keys(this.state.itemProps).length === 0 && (
-            <button
-              className="btn btn-flat-icon add-item-button"
-              onClick={this.handleShow}
-            >
-              <img className="icon icon-plus-circle" alt="" />
-              <span>Add Owner</span>
-            </button>
-          )}
-
-        <Modal
-          className="owner-editor-modal"
-          show={this.props.isEditing}
-          onHide={this.cancelEdit}
-        >
-          <Modal.Header className="text-center" closeButton={false}>
-            <Modal.Title>Owned By</Modal.Title>
-          </Modal.Header>
-          {this.renderModalBody()}
-          <Modal.Footer>
-            <button
-              type="button"
-              className="btn btn-default"
-              onClick={this.cancelEdit}
-            >
-              Cancel
-            </button>
-            <button
-              type="button"
-              className="btn btn-primary"
-              onClick={this.saveEdit}
-            >
-              Save
-            </button>
-          </Modal.Footer>
-        </Modal>
+        {ownerList}
+        {this.props.readOnly && !hasItems && (
+          <AvatarLabel
+            avatarClass="gray-avatar"
+            labelClass="text-placeholder"
+            label={Constants.NO_OWNER_TEXT}
+          />
+        )}
+        {!this.props.readOnly && !hasItems && (
+          <button
+            type="button"
+            className="btn btn-flat-icon add-item-button"
+            onClick={this.handleShow}
+          >
+            <img className="icon icon-plus-circle" alt="" />
+            <span>{Constants.ADD_OWNER}</span>
+          </button>
+        )}
+        {!this.props.readOnly && (
+          <Modal
+            className="owner-editor-modal"
+            show={this.props.isEditing}
+            onHide={this.cancelEdit}
+          >
+            <Modal.Header className="text-center" closeButton={false}>
+              <Modal.Title>{Constants.OWNED_BY}</Modal.Title>
+            </Modal.Header>
+            {this.renderModalBody()}
+            <Modal.Footer>
+              <button
+                type="button"
+                className="btn btn-default"
+                onClick={this.cancelEdit}
+              >
+                {Constants.CANCEL_TEXT}
+              </button>
+              <button
+                type="button"
+                className="btn btn-primary"
+                onClick={this.saveEdit}
+              >
+                {Constants.SAVE_TEXT}
+              </button>
+            </Modal.Footer>
+          </Modal>
+        )}
       </div>
     );
   }

amundsen_application/static/js/components/common/EditableSection/index.tsx

@@ -24,6 +24,7 @@ interface EditableSectionState {
 export interface EditableSectionChildProps {
   isEditing?: boolean;
   setEditMode?: (isEditing: boolean) => void;
+  readOnly?: boolean;
 }
 
 export class EditableSection extends React.Component<
@@ -109,19 +110,18 @@ export class EditableSection extends React.Component<
   };
 
   render() {
-    const { title, readOnly = false } = this.props;
-    const childrenWithProps = !readOnly
-      ? React.Children.map(this.props.children, (child) => {
-          if (!React.isValidElement(child)) {
-            return child;
-          }
-
-          return React.cloneElement(child, {
-            isEditing: this.state.isEditing,
-            setEditMode: this.setEditMode,
-          });
-        })
-      : this.props.children;
+    const { children, title, readOnly = false } = this.props;
+    const childrenWithProps = React.Children.map(children, (child) => {
+      if (!React.isValidElement(child)) {
+        return child;
+      }
+
+      return React.cloneElement(child, {
+        readOnly,
+        isEditing: this.state.isEditing,
+        setEditMode: this.setEditMode,
+      });
+    });
 
     return (
       <section className="editable-section">