Refactor code around TableDetail page loading (#374)

* Refactor code around TableDetail page loading
* Add page load logging params to inline search results

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

@@ -45,49 +45,65 @@ export interface DispatchFromProps {
   getTableData: (key: string, searchIndex?: string, source?: string, ) => GetTableDataRequest;
 }
 
-type TableDetailProps = StateFromProps & DispatchFromProps;
+interface MatchProps {
+  cluster: string;
+  database: string;
+  schema: string;
+  table: string;
+}
+
+type TableDetailProps = StateFromProps & DispatchFromProps & RouteComponentProps<MatchProps>;
 
 class TableDetail extends React.Component<TableDetailProps & RouteComponentProps<any>> {
-  private cluster: string;
-  private database: string;
-  private displayName: string;
   private key: string;
-  private schema: string;
-  private tableName: string;
   private didComponentMount: boolean = false;
 
   constructor(props) {
     super(props);
+  }
 
-    const { match } = props;
-    const params = match.params;
-    this.cluster = params ? params.cluster : '';
-    this.database = params ? params.db : '';
-    this.schema = params ? params.schema : '';
-    this.tableName = params ? params.table : '';
-    this.displayName = params ? `${this.schema}.${this.tableName}` : '';
+  componentDidMount() {
+    const { index, source } = this.getLoggingParams();
 
-    /*
-    This 'key' is the `table_uri` format described in metadataservice. Because it contains the '/' character,
-    we can't pass it as a single URL parameter without encodeURIComponent which makes ugly URLs.
-    DO NOT CHANGE
-    */
-    this.key = params ? `${this.database}://${this.cluster}.${this.schema}/${this.tableName}` : '';
+    this.key = this.getTableKey();
+    this.props.getTableData(this.key, index, source);
+    this.didComponentMount = true;
+  }
+
+  componentDidUpdate(prevProps) {
+    const newKey = this.getTableKey();
 
+    if (this.key !== newKey) {
+      const { index, source } = this.getLoggingParams();
+      this.props.getTableData(this.key, index, source);
+      this.key = newKey;
+    }
   }
 
-  componentDidMount() {
-    // TODO - Move into utility function
+  getLoggingParams() {
     const params = qs.parse(this.props.location.search);
-    const searchIndex = params['index'];
+    const index = params['index'];
     const source = params['source'];
     /* update the url stored in the browser history to remove params used for logging purposes */
-    if (searchIndex !== undefined || source !== undefined) {
+    if (index !== undefined || source !== undefined) {
       window.history.replaceState({}, '', `${window.location.origin}${window.location.pathname}`);
     }
+    return { index, source };
+  }
 
-    this.props.getTableData(this.key, searchIndex, source);
-    this.didComponentMount = true;
+  getDisplayName() {
+    const params = this.props.match.params;
+    return `${params.schema}.${params.table}`;
+  }
+
+  getTableKey() {
+    /*
+    This 'key' is the `table_uri` format described in metadataservice. Because it contains the '/' character,
+    we can't pass it as a single URL parameter without encodeURIComponent which makes ugly URLs.
+    DO NOT CHANGE
+    */
+    const params = this.props.match.params;
+    return `${params.database}://${params.cluster}.${params.schema}/${params.table}`;
   }
 
   render() {
@@ -116,7 +132,7 @@ class TableDetail extends React.Component<TableDetailProps & RouteComponentProps
             </div>
             <div className="header-section header-title">
               <h3 className="header-title-text truncated">
-                  { this.displayName }
+                  { this.getDisplayName() }
               </h3>
               <BookmarkIcon bookmarkKey={ this.props.tableData.key }/>
               <div className="body-2">
@@ -141,7 +157,7 @@ class TableDetail extends React.Component<TableDetailProps & RouteComponentProps
               <SourceLink tableSource={ data.source }/>
             </div>
             <div className="header-section header-buttons">
-              <DataPreviewButton modalTitle={ this.displayName }/>
+              <DataPreviewButton modalTitle={ this.getDisplayName() }/>
               <ExploreButton tableData={ data }/>
             </div>
           </header>
@@ -192,7 +208,7 @@ class TableDetail extends React.Component<TableDetailProps & RouteComponentProps
     }
 
     return (
-      <DocumentTitle title={ `${this.displayName} - Amundsen Table Details` }>
+      <DocumentTitle title={ `${ this.getDisplayName() } - Amundsen Table Details` }>
         { innerContent }
       </DocumentTitle>
     );

amundsen_application/static/js/components/common/SearchBar/InlineSearchResults/index.tsx

@@ -77,9 +77,9 @@ export class InlineSearchResults extends React.Component<InlineSearchResultsProp
 
   getSuggestedResultsForResource = (resourceType: ResourceType): SuggestedResult[] => {
     const results = this.getResultsForResource(resourceType);
-    return results.map((result) => {
+    return results.map((result, index) => {
       return {
-        href: this.getSuggestedResultHref(resourceType, result),
+        href: this.getSuggestedResultHref(resourceType, result, index),
         iconClass: this.getSuggestedResultIconClass(resourceType, result),
         subtitle: this.getSuggestedResultSubTitle(resourceType, result),
         title: this.getSuggestedResultTitle(resourceType, result),
@@ -88,14 +88,15 @@ export class InlineSearchResults extends React.Component<InlineSearchResultsProp
     });
   };
 
-  getSuggestedResultHref = (resourceType: ResourceType, result: Resource): string => {
+  getSuggestedResultHref = (resourceType: ResourceType, result: Resource, index: number): string => {
+    const logParams = `source=inline_search&index=${index}`;
     switch (resourceType) {
       case ResourceType.table:
         const table = result as TableResource;
-        return `/table_detail/${table.cluster}/${table.database}/${table.schema_name}/${table.name}`;
+        return `/table_detail/${table.cluster}/${table.database}/${table.schema_name}/${table.name}?${logParams}`;
       case ResourceType.user:
         const user = result as UserResource;
-        return `/user/${user.user_id}`;
+        return `/user/${user.user_id}?${logParams}`;
       default:
         return '';
     }

amundsen_application/static/js/components/common/SearchBar/InlineSearchResults/tests/index.spec.tsx

@@ -152,7 +152,7 @@ describe('InlineSearchResults', () => {
       const givenResource = ResourceType.user;
       const output = wrapper.instance().getSuggestedResultsForResource(givenResource);
       output.forEach((result, index) => {
-        expect(getSuggestedResultHrefSpy).toHaveBeenCalledWith(givenResource, mockResourceResults[index]);
+        expect(getSuggestedResultHrefSpy).toHaveBeenCalledWith(givenResource, mockResourceResults[index], index);
         expect(getSuggestedResultIconClassSpy).toHaveBeenCalledWith(givenResource, mockResourceResults[index]);
         expect(getSuggestedResultSubTitleSpy).toHaveBeenCalledWith(givenResource, mockResourceResults[index]);
         expect(getSuggestedResultTitleSpy).toHaveBeenCalledWith(givenResource, mockResourceResults[index]);
@@ -181,15 +181,17 @@ describe('InlineSearchResults', () => {
       wrapper = setupResult.wrapper;
     });
     it('returns the correct href for ResourceType.table', () => {
-      const givenTable = props.tables.results[0];
+      const index = 0;
+      const givenTable = props.tables.results[index];
       const { cluster, database, schema_name, name } = givenTable;
-      const output = wrapper.instance().getSuggestedResultHref(ResourceType.table, givenTable);
-      expect(output).toEqual(`/table_detail/${cluster}/${database}/${schema_name}/${name}`);
+      const output = wrapper.instance().getSuggestedResultHref(ResourceType.table, givenTable, index);
+      expect(output).toEqual(`/table_detail/${cluster}/${database}/${schema_name}/${name}?source=inline_search&index=${index}`);
     });
     it('returns the correct href for ResourceType.user', () => {
-      const givenUser = props.users.results[0];
-      const output = wrapper.instance().getSuggestedResultHref(ResourceType.user, givenUser);
-      expect(output).toEqual(`/user/${givenUser.user_id}`);
+      const index = 0;
+      const givenUser = props.users.results[index];
+      const output = wrapper.instance().getSuggestedResultHref(ResourceType.user, givenUser, index);
+      expect(output).toEqual(`/user/${givenUser.user_id}?source=inline_search&index=${index}`);
     });
     it('returns empty string as the default', () => {
       const output = wrapper.instance().getSuggestedResultHref('unsupported');

amundsen_application/static/js/index.tsx

@@ -38,7 +38,7 @@ ReactDOM.render(
           <Preloader/>
           <Route component={NavBar} />
           <Switch>
-            <Route path="/table_detail/:cluster/:db/:schema/:table" component={TableDetail} />
+            <Route path="/table_detail/:cluster/:database/:schema/:table" component={TableDetail} />
             <Route path="/announcements" component={AnnouncementPage} />
             <Route path="/browse" component={BrowsePage} />
             <Route path="/search" component={SearchPage} />