Update Dependencies based on Security Issues (#354)

* Update dependencies to resolve security issues * Fixes `getDerivedStateFromProps` usage * Refactor TagsList behavior, fix tests
amundsen-io · Dec 18, 2019 · 2ac2dc3 · 2ac2dc3
1 parent dbb33e1
commit 2ac2dc3
Show file tree

Hide file tree

Showing 18 changed files with 9,872 additions and 9,527 deletions.
diff --git a/amundsen_application/static/jest.config.js b/amundsen_application/static/jest.config.js
@@ -34,7 +34,6 @@ module.exports = {
   transform: {
     '^.+\\.tsx?$': 'ts-jest',
     '^.+\\.js$': 'babel-jest',
-    '^.+\\.(css|scss)$': '<rootDir>/node_modules/jest-css-modules',
   },
   testRegex: '(/tests/.*|(\\.|/)(test|spec))\\.(j|t)sx?$',
   moduleDirectories: ['node_modules', 'js'],
@@ -45,6 +44,9 @@ module.exports = {
     'jsx',
     'json',
   ],
+  moduleNameMapper: {
+    '^.+\\.(css|scss)$': '<rootDir>/node_modules/jest-css-modules',
+  },
   globals: {
     'ts-jest': {
       diagnostics: false,

diff --git a/amundsen_application/static/js/components/AnnouncementPage/tests/index.spec.tsx b/amundsen_application/static/js/components/AnnouncementPage/tests/index.spec.tsx
@@ -2,7 +2,7 @@ import * as React from 'react';
 import * as DocumentTitle from 'react-document-title';
 import SanitizedHTML from 'react-sanitized-html';
 
-import { mount, shallow } from 'enzyme';
+import { shallow } from 'enzyme';
 
 import { AnnouncementPage, AnnouncementPageProps, mapDispatchToProps, mapStateToProps } from '../';
 
@@ -26,7 +26,7 @@ describe('AnnouncementPage', () => {
             html_content: '<div>Just kidding</div>',
           }],
         };
-        subject = mount(<AnnouncementPage {...props} />);
+        subject = shallow(<AnnouncementPage {...props} />);
     });
 
     describe('componentDidMount', () => {

diff --git a/amundsen_application/static/js/components/TableDetail/OwnerEditor/index.tsx b/amundsen_application/static/js/components/TableDetail/OwnerEditor/index.tsx
@@ -25,7 +25,6 @@ export interface DispatchFromProps {
 
 export interface ComponentProps {
   errorText?: string | null;
-  readOnly: boolean;
 }
 
 interface OwnerAvatarLabelProps extends AvatarLabelProps {
@@ -42,7 +41,6 @@ type OwnerEditorProps = ComponentProps & DispatchFromProps & StateFromProps & Ed
 
 interface OwnerEditorState {
   errorText: string | null;
-  isLoading: boolean;
   itemProps: { [id: string]: OwnerAvatarLabelProps };
   readOnly: boolean;
   tempItemProps: { [id: string]: AvatarLabelProps };
@@ -56,20 +54,13 @@ export class OwnerEditor extends React.Component<OwnerEditorProps, OwnerEditorSt
     isLoading: false,
     itemProps: {},
     onUpdateList: () => undefined,
-    readOnly: true,
   };
 
-  static getDerivedStateFromProps(nextProps) {
-    const { isLoading, itemProps, readOnly } = nextProps;
-    return { isLoading, itemProps, readOnly, tempItemProps: itemProps };
-  }
-
   constructor(props) {
     super(props);
 
     this.state = {
       errorText: props.errorText,
-      isLoading: props.isLoading,
       itemProps: props.itemProps,
       readOnly: props.readOnly,
       tempItemProps: props.itemProps,
@@ -78,6 +69,13 @@ export class OwnerEditor extends React.Component<OwnerEditorProps, OwnerEditorSt
     this.inputRef = React.createRef();
   }
 
+  componentDidUpdate(prevProps) {
+    // TODO - itemProps is a new object and this check needs to be fixed
+    if (prevProps.itemProps !== this.props.itemProps) {
+      this.setState({ itemProps: this.props.itemProps, tempItemProps: this.props.itemProps });
+    }
+  }
+
   handleShow = () => {
     this.props.setEditMode(true)
   };
@@ -140,7 +138,7 @@ export class OwnerEditor extends React.Component<OwnerEditorProps, OwnerEditorSt
       return null;
     }
 
-    if (this.state.isLoading) {
+    if (this.props.isLoading) {
       return (
         <Modal.Body>
           <LoadingSpinner/>

diff --git a/amundsen_application/static/js/components/TableDetail/index.tsx b/amundsen_application/static/js/components/TableDetail/index.tsx
@@ -177,7 +177,7 @@ class TableDetail extends React.Component<TableDetailProps & RouteComponentProps
                   </EditableSection>
 
                   <EditableSection title="Owners">
-                    <OwnerEditor readOnly={false}/>
+                    <OwnerEditor />
                   </EditableSection>
 
                 </section>

diff --git a/amundsen_application/static/js/components/common/SearchBar/index.tsx b/amundsen_application/static/js/components/common/SearchBar/index.tsx
@@ -69,22 +69,23 @@ export class SearchBar extends React.Component<SearchBarProps, SearchBarState> {
     };
   }
 
-  static getDerivedStateFromProps(props, state) {
-    const { searchTerm } = props;
-    return { searchTerm };
-  }
-
   clearSearchTerm = () : void => {
     this.setState({ showTypeAhead: false, searchTerm: '' });
   };
 
   componentDidMount = () => {
     document.addEventListener('mousedown', this.updateTypeAhead, false);
-  }
+  };
 
   componentWillUnmount = () => {
     document.removeEventListener('mousedown', this.updateTypeAhead, false);
-  }
+  };
+
+  componentDidUpdate = (prevProps: SearchBarProps) => {
+    if (this.props.searchTerm !== prevProps.searchTerm) {
+      this.setState({ searchTerm: this.props.searchTerm });
+    }
+  };
 
   handleValueChange = (event: React.SyntheticEvent<HTMLInputElement>) : void => {
     const searchTerm = (event.target as HTMLInputElement).value.toLowerCase();
@@ -107,7 +108,7 @@ export class SearchBar extends React.Component<SearchBarProps, SearchBarState> {
 
   hideTypeAhead = () : void => {
     this.setState({ showTypeAhead: false });
-  }
+  };
 
   isFormValid = (searchTerm: string) : boolean => {
     if (searchTerm.length === 0) {

diff --git a/amundsen_application/static/js/components/common/SearchBar/tests/index.spec.tsx b/amundsen_application/static/js/components/common/SearchBar/tests/index.spec.tsx
@@ -50,20 +50,6 @@ describe('SearchBar', () => {
     });
   });
 
-  describe('getDerivedStateFromProps', () => {
-    it('sets searchTerm on state from props', () => {
-      const { props, wrapper } = setup();
-      const prevState = wrapper.state();
-      props.searchTerm = 'newTerm';
-      // @ts-ignore: Why does this work in other tests but complain here
-      wrapper.setProps(props);
-      expect(wrapper.state()).toMatchObject({
-        ...prevState,
-        searchTerm: 'newTerm',
-      });
-    });
-  });
-
   describe('clearSearchTerm', () => {
     it('sets the searchTerm to an empty string', () => {
       setStateSpy.mockClear();
@@ -90,6 +76,20 @@ describe('SearchBar', () => {
     });
   });
 
+  describe('componentDidUpdate', () => {
+    it('sets the searchTerm state when props update', () => {
+      const { props, wrapper } = setup();
+      const prevState = wrapper.state();
+      props.searchTerm = 'newTerm';
+      // @ts-ignore: Why does this work in other tests but complain here
+      wrapper.setProps(props);
+      expect(wrapper.state()).toMatchObject({
+        ...prevState,
+        searchTerm: 'newTerm',
+      });
+    });
+  });
+
   describe('handleValueChange', () => {
     let shouldShowTypeAheadSpy;
 

diff --git a/amundsen_application/static/js/components/common/TagsList/index.tsx b/amundsen_application/static/js/components/common/TagsList/index.tsx
@@ -4,58 +4,30 @@ import { bindActionCreators } from 'redux';
 
 import './styles.scss';
 
-import AppConfig from 'config/config';
 import LoadingSpinner from 'components/common/LoadingSpinner';
 import TagInfo from 'components/Tags/TagInfo';
 import { Tag } from 'interfaces';
 
 import { GlobalState } from 'ducks/rootReducer';
 import { getAllTags } from 'ducks/allTags/reducer';
 import { GetAllTagsRequest } from 'ducks/allTags/types';
+import { getCuratedTags, showAllTags } from 'config/config-utils';
 
 export interface StateFromProps {
-  allTags: Tag[];
+  curatedTags: Tag[];
+  otherTags: Tag[];
   isLoading: boolean;
 }
 
 export interface DispatchFromProps {
   getAllTags: () => GetAllTagsRequest;
 }
 
-interface TagsListState {
-  curatedTags: Tag[];
-  otherTags: Tag[];
-}
-
 export type TagsListProps = StateFromProps & DispatchFromProps;
 
-export class TagsList extends React.Component<TagsListProps, TagsListState> {
+export class TagsList extends React.Component<TagsListProps> {
   constructor(props) {
     super(props);
-
-    this.state = {
-      curatedTags: [],
-      otherTags: [],
-    };
-  }
-
-  static getDerivedStateFromProps(nextProps, prevState) {
-    const { allTags, isLoading } = nextProps;
-    if (isLoading) {
-      return {
-        ...prevState,
-        isLoading,
-      };
-    }
-
-    const curatedTagsList = AppConfig.browse.curatedTags;
-    const curatedTags = allTags.filter((tag) => curatedTagsList.indexOf(tag.tag_name) !== -1);
-
-    let otherTags = [];
-    if (AppConfig.browse.showAllTags) {
-      otherTags = allTags.filter((tag) => curatedTagsList.indexOf(tag.tag_name) === -1);
-    }
-    return { curatedTags, otherTags, isLoading };
   }
 
   componentDidMount() {
@@ -68,28 +40,34 @@ export class TagsList extends React.Component<TagsListProps, TagsListState> {
   }
 
   render() {
-    let innerContent;
     if (this.props.isLoading) {
-      innerContent = <LoadingSpinner/>;
-    } else {
-      innerContent = (
-        <div id="browse-body" className="browse-body">
-          {this.generateTagInfo(this.state.curatedTags)}
-          {
-            this.state.curatedTags.length > 0 && this.state.otherTags.length > 0 &&
-              <hr />
-          }
-          {this.generateTagInfo(this.state.otherTags)}
-        </div>
-      );
+      return <LoadingSpinner/>;
     }
-    return (innerContent);
+    return (
+      <div id="tags-list" className="tags-list">
+        { this.generateTagInfo(this.props.curatedTags) }
+        {
+          showAllTags() && this.props.curatedTags.length > 0 && this.props.otherTags.length > 0 &&
+            <hr />
+        }
+        {
+          showAllTags() && this.props.otherTags.length > 0 &&
+            this.generateTagInfo(this.props.otherTags)
+        }
+      </div>
+    );
   }
 }
 
 export const mapStateToProps = (state: GlobalState) => {
+  const curatedTagsList = getCuratedTags();
+  const allTags = state.allTags.allTags;
+  const curatedTags = allTags.filter((tag) => curatedTagsList.indexOf(tag.tag_name) !== -1);
+  const otherTags = allTags.filter((tag) => curatedTagsList.indexOf(tag.tag_name) === -1);
+
   return {
-    allTags: state.allTags.allTags,
+    curatedTags,
+    otherTags,
     isLoading: state.allTags.isLoading,
   };
 };

diff --git a/amundsen_application/static/js/components/common/TagsList/styles.scss b/amundsen_application/static/js/components/common/TagsList/styles.scss
@@ -4,6 +4,6 @@ hr.header-hr {
   border: 2px solid $brand-color-4;
 }
 
-.browse-body {
+.tags-list {
   margin: 0 -4px;
 }