refactor: Refactors connector out of ColumnLIstItems (#628)

* Refactors connector out of ColumnLIstItems
Signed-off-by: Marcos Iglesias <miglesiasvalle@lyft.com>

* Clean up initial usage stuff
Signed-off-by: Marcos Iglesias <miglesiasvalle@lyft.com>

* Updating betterer results
Signed-off-by: Marcos Iglesias <miglesiasvalle@lyft.com>

amundsen_application/static/.betterer.results

@@ -319,11 +319,11 @@ exports[`strict null compilation`] = {
     "js/pages/SearchPage/index.tsx:4000484656": [
       [174, 11, 12, "Property \'filterSections\' is missing in type \'{}\' but required in type \'Readonly<Pick<StateFromProps, \\"filterSections\\">>\'.", "250899467"]
     ],
-    "js/pages/TableDetailPage/ColumnList/index.tsx:355148301": [
-      [22, 6, 7, "Object is possibly \'undefined\'.", "3718923584"],
-      [27, 21, 7, "Object is possibly \'undefined\'.", "3718923584"],
-      [33, 6, 8, "Type \'string | undefined\' is not assignable to type \'string\'.\\n  Type \'undefined\' is not assignable to type \'string\'.", "1427606500"],
-      [34, 6, 7, "Type \'string | undefined\' is not assignable to type \'string\'.\\n  Type \'undefined\' is not assignable to type \'string\'.", "3817619378"]
+    "js/pages/TableDetailPage/ColumnList/index.tsx:3163605539": [
+      [31, 6, 7, "Object is possibly \'undefined\'.", "3718923584"],
+      [36, 21, 7, "Object is possibly \'undefined\'.", "3718923584"],
+      [43, 6, 8, "Type \'string | undefined\' is not assignable to type \'string\'.\\n  Type \'undefined\' is not assignable to type \'string\'.", "1427606500"],
+      [44, 6, 7, "Type \'string | undefined\' is not assignable to type \'string\'.\\n  Type \'undefined\' is not assignable to type \'string\'.", "3817619378"]
     ],
     "js/pages/TableDetailPage/ColumnStats/index.spec.tsx:1228258528": [
       [90, 39, 4, "Argument of type \'null\' is not assignable to parameter of type \'number\'.", "2087897566"]
@@ -347,22 +347,22 @@ exports[`strict null compilation`] = {
     "js/pages/TableDetailPage/WatermarkLabel/index.tsx:1354016727": [
       [80, 34, 3, "Argument of type \'string | null\' is not assignable to parameter of type \'string\'.\\n  Type \'null\' is not assignable to type \'string\'.", "193412913"]
     ],
-    "js/pages/TableDetailPage/index.spec.tsx:2169788066": [
+    "js/pages/TableDetailPage/index.spec.tsx:3148704474": [
       [32, 4, 8, "Argument of type \'Partial<Location<{} | null | undefined>> | undefined\' is not assignable to parameter of type \'Partial<Location<{} | null | undefined>>\'.\\n  Type \'undefined\' is not assignable to type \'Partial<Location<{} | null | undefined>>\'.", "2700611480"]
     ],
-    "js/pages/TableDetailPage/index.tsx:1890000211": [
-      [152, 10, 13, "Type \'null\' is not assignable to type \'((newValue: string, onSuccess?: (() => any) | undefined, onFailure?: (() => any) | undefined) => void) | undefined\'.", "67794331"],
-      [185, 11, 26, "Type \'{ itemsPerPage: number; source: string; }\' is missing the following properties from type \'Readonly<Pick<TableDashboardResourceListProps, \\"source\\" | \\"isLoading\\" | \\"dashboards\\" | \\"itemsPerPage\\" | \\"errorText\\"> & OwnProps>\': isLoading, dashboards, errorText", "2224258167"],
-      [265, 16, 7, "Type \'string | null\' is not assignable to type \'string | undefined\'.\\n  Type \'null\' is not assignable to type \'string | undefined\'.", "3817619378"],
-      [307, 20, 35, "Argument of type \'ProgrammaticDescription[] | undefined\' is not assignable to parameter of type \'ProgrammaticDescription[]\'.\\n  Type \'undefined\' is not assignable to type \'ProgrammaticDescription[]\'.", "4249007202"],
-      [321, 20, 36, "Argument of type \'ProgrammaticDescription[] | undefined\' is not assignable to parameter of type \'ProgrammaticDescription[]\'.\\n  Type \'undefined\' is not assignable to type \'ProgrammaticDescription[]\'.", "2770872537"],
-      [326, 16, 36, "Argument of type \'ProgrammaticDescription[] | undefined\' is not assignable to parameter of type \'ProgrammaticDescription[]\'.\\n  Type \'undefined\' is not assignable to type \'ProgrammaticDescription[]\'.", "2776557981"]
+    "js/pages/TableDetailPage/index.tsx:1188316820": [
+      [160, 10, 13, "Type \'null\' is not assignable to type \'((newValue: string, onSuccess?: (() => any) | undefined, onFailure?: (() => any) | undefined) => void) | undefined\'.", "67794331"],
+      [199, 11, 26, "Type \'{ itemsPerPage: number; source: string; }\' is missing the following properties from type \'Readonly<Pick<TableDashboardResourceListProps, \\"source\\" | \\"isLoading\\" | \\"dashboards\\" | \\"itemsPerPage\\" | \\"errorText\\"> & OwnProps>\': isLoading, dashboards, errorText", "2224258167"],
+      [279, 16, 7, "Type \'string | null\' is not assignable to type \'string | undefined\'.\\n  Type \'null\' is not assignable to type \'string | undefined\'.", "3817619378"],
+      [321, 20, 35, "Argument of type \'ProgrammaticDescription[] | undefined\' is not assignable to parameter of type \'ProgrammaticDescription[]\'.\\n  Type \'undefined\' is not assignable to type \'ProgrammaticDescription[]\'.", "4249007202"],
+      [335, 20, 36, "Argument of type \'ProgrammaticDescription[] | undefined\' is not assignable to parameter of type \'ProgrammaticDescription[]\'.\\n  Type \'undefined\' is not assignable to type \'ProgrammaticDescription[]\'.", "2770872537"],
+      [340, 16, 36, "Argument of type \'ProgrammaticDescription[] | undefined\' is not assignable to parameter of type \'ProgrammaticDescription[]\'.\\n  Type \'undefined\' is not assignable to type \'ProgrammaticDescription[]\'.", "2776557981"]
     ],
     "js/utils/navigationUtils.ts:1127210474": [
       [19, 50, 21, "Type \'undefined\' cannot be used as an index type.", "602535635"]
     ],
-    "webpack.common.ts:4170625548": [
-      [32, 24, 20, "No overload matches this call.\\n  Overload 1 of 2, \'(...items: ConcatArray<never>[]): never[]\', gave the following error.\\n    Argument of type \'string\' is not assignable to parameter of type \'ConcatArray<never>\'.\\n  Overload 2 of 2, \'(...items: ConcatArray<never>[]): never[]\', gave the following error.\\n    Argument of type \'string\' is not assignable to parameter of type \'ConcatArray<never>\'.", "806093104"]
+    "webpack.common.ts:998025681": [
+      [33, 24, 20, "No overload matches this call.\\n  Overload 1 of 2, \'(...items: ConcatArray<never>[]): never[]\', gave the following error.\\n    Argument of type \'string\' is not assignable to parameter of type \'ConcatArray<never>\'.\\n  Overload 2 of 2, \'(...items: ConcatArray<never>[]): never[]\', gave the following error.\\n    Argument of type \'string\' is not assignable to parameter of type \'ConcatArray<never>\'.", "806093104"]
     ]
   }`
 };

amundsen_application/static/js/pages/TableDetailPage/ColumnList/index.tsx

@@ -2,13 +2,21 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import * as React from 'react';
-import { TableColumn } from 'interfaces';
+
+import { OpenRequestAction } from 'ducks/notification/types';
+
+import { TableColumn, RequestMetadataType } from 'interfaces';
+
 import ColumnListItem from '../ColumnListItem';
 
 import './styles.scss';
 
 interface ColumnListProps {
   columns?: TableColumn[];
+  openRequestDescriptionDialog: (
+    requestMetadataType: RequestMetadataType,
+    columnName: string
+  ) => OpenRequestAction;
   database: string;
   editText?: string;
   editUrl?: string;
@@ -19,6 +27,7 @@ const ColumnList: React.FC<ColumnListProps> = ({
   database,
   editText,
   editUrl,
+  openRequestDescriptionDialog,
 }: ColumnListProps) => {
   if (columns.length < 1) {
     return <div />;
@@ -27,6 +36,7 @@ const ColumnList: React.FC<ColumnListProps> = ({
 
   const columnList = columns.map((entry, index) => (
     <ColumnListItem
+      openRequestDescriptionDialog={openRequestDescriptionDialog}
       key={`column:${index}`}
       data={entry}
       database={database}

amundsen_application/static/js/pages/TableDetailPage/ColumnListItem/index.spec.tsx

@@ -10,15 +10,14 @@ import * as UtilMethods from 'ducks/utilMethods';
 import { RequestMetadataType } from 'interfaces/Notifications';
 import ColumnStats from '../ColumnStats';
 import ColumnDescEditableText from '../ColumnDescEditableText';
-import { ColumnListItem, ColumnListItemProps, mapDispatchToProps } from '.';
+import { ColumnListItem, ColumnListItemProps } from '.';
 
 import ColumnType from './ColumnType';
 
 const logClickSpy = jest.spyOn(UtilMethods, 'logClick');
 logClickSpy.mockImplementation(() => null);
 
-describe('ColumnListItem', () => {
-  const setup = (propOverrides?: Partial<ColumnListItemProps>) => {
+const setup = (propOverrides?: Partial<ColumnListItemProps>) => {
   const props = {
     data: {
       name: 'test_column_name',
@@ -44,8 +43,9 @@ describe('ColumnListItem', () => {
 
   const wrapper = shallow<ColumnListItem>(<ColumnListItem {...props} />);
   return { wrapper, props };
-  };
+};
 
+describe('ColumnListItem', () => {
   const { wrapper, props } = setup();
   const instance = wrapper.instance();
   const setStateSpy = jest.spyOn(instance, 'setState');
@@ -254,6 +254,7 @@ describe('ColumnListItem', () => {
         });
       });
     });
+
     describe('when not expanded', () => {
       let newWrapper;
       beforeAll(() => {
@@ -275,16 +276,3 @@ describe('ColumnListItem', () => {
     });
   });
 });
-
-describe('mapDispatchToProps', () => {
-  let dispatch;
-  let result;
-  beforeAll(() => {
-    dispatch = jest.fn(() => Promise.resolve());
-    result = mapDispatchToProps(dispatch);
-  });
-
-  it('sets openRequestDescriptionDialog on the props', () => {
-    expect(result.openRequestDescriptionDialog).toBeInstanceOf(Function);
-  });
-});

amundsen_application/static/js/pages/TableDetailPage/ColumnListItem/index.tsx

@@ -3,33 +3,29 @@
 
 import * as React from 'react';
 import { Dropdown, MenuItem } from 'react-bootstrap';
-import { bindActionCreators } from 'redux';
-import { connect } from 'react-redux';
 
 import { notificationsEnabled, getMaxLength } from 'config/config-utils';
-import { openRequestDescriptionDialog } from 'ducks/notification/reducer';
-import { OpenRequestAction } from 'ducks/notification/types';
 import { logClick } from 'ducks/utilMethods';
-import { RequestMetadataType, TableColumn } from 'interfaces';
 
-import './styles.scss';
+import { TableColumn, RequestMetadataType } from 'interfaces';
+import { OpenRequestAction } from 'ducks/notification/types';
+
 import EditableSection from 'components/common/EditableSection';
 import ColumnStats from '../ColumnStats';
 import ColumnDescEditableText from '../ColumnDescEditableText';
 import ColumnType from './ColumnType';
 
+import './styles.scss';
+
 const MORE_BUTTON_TEXT = 'More options';
 const EDITABLE_SECTION_TITLE = 'Description';
 
-interface DispatchFromProps {
+export interface ColumnListItemProps {
+  data: TableColumn;
   openRequestDescriptionDialog: (
     requestMetadataType: RequestMetadataType,
     columnName: string
   ) => OpenRequestAction;
-}
-
-interface OwnProps {
-  data: TableColumn;
   database: string;
   index: number;
   editText: string;
@@ -40,8 +36,6 @@ interface ColumnListItemState {
   isExpanded: boolean;
 }
 
-export type ColumnListItemProps = DispatchFromProps & OwnProps;
-
 export class ColumnListItem extends React.Component<
   ColumnListItemProps,
   ColumnListItemState
@@ -93,6 +87,7 @@ export class ColumnListItem extends React.Component<
 
   render() {
     const { data, database } = this.props;
+
     return (
       <li className="list-group-item clickable" onClick={this.toggleExpand}>
         <div className="column-list-item">
@@ -116,7 +111,6 @@ export class ColumnListItem extends React.Component<
                 type={data.col_type}
               />
             </div>
-            <div className="badges">{/* Placeholder */}</div>
             <div className="actions">
               {
                 // TODO - Make this dropdown into a separate component
@@ -169,11 +163,4 @@ export class ColumnListItem extends React.Component<
   }
 }
 
-export const mapDispatchToProps = (dispatch: any) => {
-  return bindActionCreators({ openRequestDescriptionDialog }, dispatch);
-};
-
-export default connect<{}, DispatchFromProps, OwnProps>(
-  null,
-  mapDispatchToProps
-)(ColumnListItem);
+export default ColumnListItem;

amundsen_application/static/js/pages/TableDetailPage/ColumnListItem/styles.scss

@@ -3,35 +3,36 @@
 
 @import 'variables';
 
+$column-header-height: 38px;
+$column-details-cell-width: 50%;
+$resource-type-cell-width: 100px;
+
 .list-group-item .column-list-item {
-  padding: 16px 24px;
+  padding: $spacer-2 $spacer-3;
 
   .column-header {
     display: flex;
-    height: 38px;
+    height: $column-header-height;
 
     .column-details {
-      flex-basis: 50%;
+      flex-basis: $column-details-cell-width;
       flex-grow: 1;
-      margin-right: 16px;
+      margin-right: $spacer-2;
     }
 
     .resource-type,
     .badges,
+    .usage,
     .actions {
       align-items: center;
       display: flex;
     }
 
     .resource-type {
-      flex-basis: 100px;
+      flex-basis: $resource-type-cell-width;
       overflow: hidden;
     }
 
-    .badges {
-      // placeholder
-    }
-
     .actions {
       .column-dropdown {
         &.open {
@@ -72,7 +73,7 @@
   }
 
   .expanded-content {
-    margin-top: -16px;
+    margin-top: -$spacer-2;
   }
 
   .stop-propagation {

amundsen_application/static/js/pages/TableDetailPage/RequestMetadataForm/index.spec.tsx

@@ -272,6 +272,7 @@ describe('RequestMetadataForm', () => {
           });
           wrapper = setupResult.wrapper;
         });
+
         it('renders checked column description checkbox', () => {
           element = wrapper.find('#request-type-form-group');
           const label = element.find('label').at(2);
@@ -283,7 +284,7 @@ describe('RequestMetadataForm', () => {
           element = wrapper.find('#additional-comments-form-group');
           const textArea = element.find('textarea');
 
-          expect(textArea.text()).toEqual(
+          expect(textArea.props().defaultValue).toEqual(
             `${COLUMN_REQUESTED_COMMENT_PREFIX}Test`
           );
           expect(textArea.props().required).toBe(true);

amundsen_application/static/js/pages/TableDetailPage/RequestMetadataForm/index.tsx

@@ -91,20 +91,35 @@ export class RequestMetadataForm extends React.Component<
     );
   };
 
-  submitNotification = (event) => {
-    event.preventDefault();
+  extractDataFromForm = () => {
     const form = document.getElementById('RequestForm') as HTMLFormElement;
     const formData = new FormData(form);
     const recipientString = formData.get('recipients') as string;
-    const recipients = recipientString.split(
+
+    return {
+      comment: formData.get('comment') as string,
+      fieldsRequested: formData.get('column-description') === 'on',
+      descriptionRequested: formData.get('table-description') === 'on',
+      sender: formData.get('sender') as string,
+      recipients: recipientString.split(
         Constants.RECIPIENT_LIST_DELIMETER.trim()
-    );
-    const sender = formData.get('sender') as string;
-    const descriptionRequested = formData.get('table-description') === 'on';
-    const fieldsRequested = formData.get('column-description') === 'on';
-    const comment = formData.get('comment') as string;
-    const { cluster, database, schema, name } = this.props.tableMetadata;
-    this.props.submitNotification(
+      ),
+    };
+  };
+
+  submitNotification = (event) => {
+    event.preventDefault();
+    const {
+      comment,
+      fieldsRequested,
+      descriptionRequested,
+      sender,
+      recipients,
+    } = this.extractDataFromForm();
+    const { submitNotification, tableMetadata } = this.props;
+    const { cluster, database, schema, name } = tableMetadata;
+
+    submitNotification(
       recipients,
       sender,
       NotificationType.METADATA_REQUESTED,
@@ -124,7 +139,6 @@ export class RequestMetadataForm extends React.Component<
       requestIsOpen,
       requestMetadataType,
       sendState,
-      tableMetadata,
       tableOwners,
       userEmail,
     } = this.props;
@@ -215,9 +229,8 @@ export class RequestMetadataForm extends React.Component<
               required={colDescriptionNeeded}
               rows={8}
               maxLength={2000}
-            >
-              {defaultComment}
-            </textarea>
+              defaultValue={defaultComment}
+            />
           </div>
           <button
             id="submit-request-button"

amundsen_application/static/js/pages/TableDetailPage/index.spec.tsx

@@ -39,6 +39,7 @@ const setup = (
     statusCode: 200,
     tableData: tableMetadata,
     getTableData: jest.fn(),
+    openRequestDescriptionDialog: jest.fn(),
     ...routerProps,
     ...propOverrides,
   };

amundsen_application/static/js/pages/TableDetailPage/index.tsx

@@ -9,7 +9,9 @@ import { RouteComponentProps } from 'react-router';
 
 import { GlobalState } from 'ducks/rootReducer';
 import { getTableData } from 'ducks/tableMetadata/reducer';
+import { openRequestDescriptionDialog } from 'ducks/notification/reducer';
 import { GetTableDataRequest } from 'ducks/tableMetadata/types';
+import { OpenRequestAction } from 'ducks/notification/types';
 
 import {
   getDescriptionSourceDisplayName,
@@ -27,15 +29,18 @@ import TabsComponent, { TabInfo } from 'components/common/TabsComponent';
 import TagInput from 'components/common/Tags/TagInput';
 import EditableText from 'components/common/EditableText';
 import LoadingSpinner from 'components/common/LoadingSpinner';
+import EditableSection from 'components/common/EditableSection';
+
+import { formatDateTimeShort } from 'utils/dateUtils';
+import { getLoggingParams } from 'utils/logUtils';
 
 import {
   ProgrammaticDescription,
   ResourceType,
   TableMetadata,
+  RequestMetadataType,
 } from 'interfaces';
-import EditableSection from 'components/common/EditableSection';
-import { formatDateTimeShort } from 'utils/dateUtils';
-import { getLoggingParams } from 'utils/logUtils';
+
 import ColumnList from './ColumnList';
 import DataPreviewButton from './DataPreviewButton';
 import ExploreButton from './ExploreButton';
@@ -50,7 +55,6 @@ import TableIssues from './TableIssues';
 import WatermarkLabel from './WatermarkLabel';
 import WriterLink from './WriterLink';
 import TableReportsDropdown from './ResourceReportsDropdown';
-
 import RequestDescriptionText from './RequestDescriptionText';
 import RequestMetadataForm from './RequestMetadataForm';
 
@@ -75,6 +79,10 @@ export interface DispatchFromProps {
     searchIndex?: string,
     source?: string
   ) => GetTableDataRequest;
+  openRequestDescriptionDialog: (
+    requestMetadataType: RequestMetadataType,
+    columnName: string
+  ) => OpenRequestAction;
 }
 
 export interface MatchProps {
@@ -158,12 +166,18 @@ export class TableDetail extends React.Component<
 
   renderTabs(editText, editUrl) {
     const tabInfo: TabInfo[] = [];
-    const { isLoadingDashboards, numRelatedDashboards, tableData } = this.props;
+    const {
+      isLoadingDashboards,
+      numRelatedDashboards,
+      tableData,
+      openRequestDescriptionDialog,
+    } = this.props;
 
     // Default Column content
     tabInfo.push({
       content: (
         <ColumnList
+          openRequestDescriptionDialog={openRequestDescriptionDialog}
           columns={tableData.columns}
           database={tableData.database}
           editText={editText}
@@ -360,7 +374,10 @@ export const mapStateToProps = (state: GlobalState) => {
 };
 
 export const mapDispatchToProps = (dispatch: any) => {
-  return bindActionCreators({ getTableData }, dispatch);
+  return bindActionCreators(
+    { getTableData, openRequestDescriptionDialog },
+    dispatch
+  );
 };
 
 export default connect<StateFromProps, DispatchFromProps>(

amundsen_application/static/package.json

@@ -29,7 +29,7 @@
     "build-storybook": "cross-env TS_NODE_PROJECT='tsconfig.webpack.json' build-storybook",
     "betterer": "betterer",
     "betterer:update": "betterer --update",
-    "check": "npm run eslint:errors && npm run stylelint && npm run tsc && npn run betterer"
+    "check": "npm run eslint:errors && npm run stylelint && npm run tsc && npm run betterer"
   },
   "author": "",
   "license": "Apache-2.0",