From e816f73ecfaa244d14c5c14cdcf60405df56da2d Mon Sep 17 00:00:00 2001
From: Marliana Lara <mlara@redhat.com>
Date: Tue, 10 Mar 2020 12:19:36 -0400
Subject: [PATCH 1/2] Show content error when the top-level inventory and host
 inventory do not match

---
 .../Inventory/InventoryHost/InventoryHost.jsx | 124 +++++++++---------
 .../InventoryHost/InventoryHost.test.jsx      |   9 ++
 2 files changed, 71 insertions(+), 62 deletions(-)

diff --git a/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.jsx b/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.jsx
index 89121e8cd5..ae4dc53bc1 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.jsx
@@ -23,6 +23,11 @@ import JobList from '@components/JobList';
 import InventoryHostDetail from '../InventoryHostDetail';
 import InventoryHostEdit from '../InventoryHostEdit';
 
+const checkHostInventory = (host, inventory) =>
+  host &&
+  inventory &&
+  host.summary_fields?.inventory?.id === parseInt(inventory.id, 10);
+
 function InventoryHost({ i18n, setBreadcrumb, inventory }) {
   const location = useLocation();
   const match = useRouteMatch('/inventories/inventory/:id/hosts/:hostId');
@@ -40,7 +45,7 @@ function InventoryHost({ i18n, setBreadcrumb, inventory }) {
       return {
         host: data,
       };
-    }, [match.params.hostId]), // eslint-disable-line react-hooks/exhaustive-deps
+    }, [match.params.hostId]),
     {
       host: null,
     }
@@ -48,7 +53,7 @@ function InventoryHost({ i18n, setBreadcrumb, inventory }) {
 
   useEffect(() => {
     fetchHost();
-  }, [fetchHost]);
+  }, [fetchHost, location.pathname]);
 
   useEffect(() => {
     if (inventory && host) {
@@ -89,24 +94,7 @@ function InventoryHost({ i18n, setBreadcrumb, inventory }) {
     },
   ];
 
-  let cardHeader = (
-    <TabbedCardHeader>
-      <RoutedTabs tabsArray={tabsArray} />
-      <CardActions>
-        <CardCloseButton linkTo={hostListUrl} />
-      </CardActions>
-    </TabbedCardHeader>
-  );
-
-  if (location.pathname.endsWith('edit')) {
-    cardHeader = null;
-  }
-
-  if (isLoading) {
-    return <ContentLoading />;
-  }
-
-  if (!isLoading && contentError) {
+  if (contentError) {
     return (
       <Card>
         <ContentError error={contentError}>
@@ -123,50 +111,62 @@ function InventoryHost({ i18n, setBreadcrumb, inventory }) {
     );
   }
 
+  const isInventoryValid = checkHostInventory(host, inventory);
+  if (!isLoading && !isInventoryValid) {
+    return (
+      <ContentError isNotFound>
+        <Link to={hostListUrl}>{i18n._(t`View all Inventory Hosts.`)}</Link>
+      </ContentError>
+    );
+  }
+
   return (
     <>
-      {cardHeader}
-      <Switch>
-        <Redirect
-          from="/inventories/inventory/:id/hosts/:hostId"
-          to="/inventories/inventory/:id/hosts/:hostId/details"
-          exact
-        />
-        {host &&
-          inventory && [
-            <Route
-              key="details"
-              path="/inventories/inventory/:id/hosts/:hostId/details"
-            >
-              <InventoryHostDetail host={host} />
-            </Route>,
-            <Route
-              key="edit"
-              path="/inventories/inventory/:id/hosts/:hostId/edit"
-            >
-              <InventoryHostEdit host={host} inventory={inventory} />
-            </Route>,
-            <Route
-              key="completed-jobs"
-              path="/inventories/inventory/:id/hosts/:hostId/completed_jobs"
-            >
-              <JobList defaultParams={{ job__hosts: host.id }} />
-            </Route>,
-          ]}
-        <Route
-          key="not-found"
-          path="*"
-          render={() =>
-            !isLoading && (
-              <ContentError isNotFound>
-                <Link to={`${match.url}/details`}>
-                  {i18n._(`View Inventory Host Details`)}
-                </Link>
-              </ContentError>
-            )
-          }
-        />
-      </Switch>
+      {['edit'].some(name => location.pathname.includes(name)) ? null : (
+        <TabbedCardHeader>
+          <RoutedTabs tabsArray={tabsArray} />
+          <CardActions>
+            <CardCloseButton linkTo={hostListUrl} />
+          </CardActions>
+        </TabbedCardHeader>
+      )}
+
+      {isLoading && <ContentLoading />}
+
+      {!isLoading && isInventoryValid && (
+        <Switch>
+          <Redirect
+            from="/inventories/inventory/:id/hosts/:hostId"
+            to="/inventories/inventory/:id/hosts/:hostId/details"
+            exact
+          />
+          <Route
+            key="details"
+            path="/inventories/inventory/:id/hosts/:hostId/details"
+          >
+            <InventoryHostDetail host={host} />
+          </Route>
+          <Route
+            key="edit"
+            path="/inventories/inventory/:id/hosts/:hostId/edit"
+          >
+            <InventoryHostEdit host={host} inventory={inventory} />
+          </Route>
+          <Route
+            key="completed-jobs"
+            path="/inventories/inventory/:id/hosts/:hostId/completed_jobs"
+          >
+            <JobList defaultParams={{ job__hosts: host.id }} />
+          </Route>
+          <Route key="not-found" path="*">
+            <ContentError isNotFound>
+              <Link to={`${match.url}/details`}>
+                {i18n._(`View Inventory Host Details`)}
+              </Link>
+            </ContentError>
+          </Route>
+        </Switch>
+      )}
     </>
   );
 }
diff --git a/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.test.jsx
index bea26df827..b844668eca 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.test.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.test.jsx
@@ -76,4 +76,13 @@ describe('<InventoryHost />', () => {
     });
     await waitForElement(wrapper, 'ContentError', el => el.length === 1);
   });
+
+  test('should show content error when inventory id does not match host inventory', async () => {
+    await act(async () => {
+      wrapper = mountWithContexts(
+        <InventoryHost inventory={{ id: 99 }} setBreadcrumb={() => {}} />
+      );
+    });
+    await waitForElement(wrapper, 'ContentError', el => el.length === 1);
+  });
 });

From 2cb5046ec6c3d34503a63c6f772a61d18d4e18d0 Mon Sep 17 00:00:00 2001
From: Marliana Lara <mlara@redhat.com>
Date: Tue, 10 Mar 2020 15:33:39 -0400
Subject: [PATCH 2/2] Throw an error when host inventory doesn't match parent
 inventory

---
 awx/ui_next/src/api/models/Inventories.js     | 17 +++++++++++
 .../Inventory/InventoryHost/InventoryHost.jsx | 28 ++++++-------------
 .../InventoryHost/InventoryHost.test.jsx      |  6 ++--
 3 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/awx/ui_next/src/api/models/Inventories.js b/awx/ui_next/src/api/models/Inventories.js
index 858e7a390a..becba96649 100644
--- a/awx/ui_next/src/api/models/Inventories.js
+++ b/awx/ui_next/src/api/models/Inventories.js
@@ -8,6 +8,7 @@ class Inventories extends InstanceGroupsMixin(Base) {
 
     this.readAccessList = this.readAccessList.bind(this);
     this.readHosts = this.readHosts.bind(this);
+    this.readHostDetail = this.readHostDetail.bind(this);
     this.readGroups = this.readGroups.bind(this);
     this.readGroupsOptions = this.readGroupsOptions.bind(this);
     this.promoteGroup = this.promoteGroup.bind(this);
@@ -27,6 +28,22 @@ class Inventories extends InstanceGroupsMixin(Base) {
     return this.http.get(`${this.baseUrl}${id}/hosts/`, { params });
   }
 
+  async readHostDetail(inventoryId, hostId) {
+    const {
+      data: { results },
+    } = await this.http.get(
+      `${this.baseUrl}${inventoryId}/hosts/?id=${hostId}`
+    );
+
+    if (Array.isArray(results) && results.length) {
+      return results[0];
+    }
+
+    throw new Error(
+      `How did you get here? Host not found for Inventory ID: ${inventoryId}`
+    );
+  }
+
   readGroups(id, params) {
     return this.http.get(`${this.baseUrl}${id}/groups/`, { params });
   }
diff --git a/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.jsx b/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.jsx
index ae4dc53bc1..3c07032096 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.jsx
@@ -11,7 +11,7 @@ import {
 } from 'react-router-dom';
 import useRequest from '@util/useRequest';
 
-import { HostsAPI } from '@api';
+import { InventoriesAPI } from '@api';
 import { Card, CardActions } from '@patternfly/react-core';
 import { CaretLeftIcon } from '@patternfly/react-icons';
 import { TabbedCardHeader } from '@components/Card';
@@ -23,11 +23,6 @@ import JobList from '@components/JobList';
 import InventoryHostDetail from '../InventoryHostDetail';
 import InventoryHostEdit from '../InventoryHostEdit';
 
-const checkHostInventory = (host, inventory) =>
-  host &&
-  inventory &&
-  host.summary_fields?.inventory?.id === parseInt(inventory.id, 10);
-
 function InventoryHost({ i18n, setBreadcrumb, inventory }) {
   const location = useLocation();
   const match = useRouteMatch('/inventories/inventory/:id/hosts/:hostId');
@@ -40,12 +35,14 @@ function InventoryHost({ i18n, setBreadcrumb, inventory }) {
     request: fetchHost,
   } = useRequest(
     useCallback(async () => {
-      const { data } = await HostsAPI.readDetail(match.params.hostId);
-
+      const response = await InventoriesAPI.readHostDetail(
+        inventory.id,
+        match.params.hostId
+      );
       return {
-        host: data,
+        host: response,
       };
-    }, [match.params.hostId]),
+    }, [inventory.id, match.params.hostId]),
     {
       host: null,
     }
@@ -111,15 +108,6 @@ function InventoryHost({ i18n, setBreadcrumb, inventory }) {
     );
   }
 
-  const isInventoryValid = checkHostInventory(host, inventory);
-  if (!isLoading && !isInventoryValid) {
-    return (
-      <ContentError isNotFound>
-        <Link to={hostListUrl}>{i18n._(t`View all Inventory Hosts.`)}</Link>
-      </ContentError>
-    );
-  }
-
   return (
     <>
       {['edit'].some(name => location.pathname.includes(name)) ? null : (
@@ -133,7 +121,7 @@ function InventoryHost({ i18n, setBreadcrumb, inventory }) {
 
       {isLoading && <ContentLoading />}
 
-      {!isLoading && isInventoryValid && (
+      {!isLoading && host && (
         <Switch>
           <Redirect
             from="/inventories/inventory/:id/hosts/:hostId"
diff --git a/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.test.jsx b/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.test.jsx
index b844668eca..0d73f4d04a 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.test.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryHost/InventoryHost.test.jsx
@@ -1,7 +1,7 @@
 import React from 'react';
 import { act } from 'react-dom/test-utils';
 import { createMemoryHistory } from 'history';
-import { HostsAPI } from '@api';
+import { InventoriesAPI } from '@api';
 import { mountWithContexts, waitForElement } from '@testUtils/enzymeHelpers';
 import mockHost from '../shared/data.host.json';
 import InventoryHost from './InventoryHost';
@@ -15,7 +15,7 @@ jest.mock('react-router-dom', () => ({
   }),
 }));
 
-HostsAPI.readDetail.mockResolvedValue({
+InventoriesAPI.readHostDetail.mockResolvedValue({
   data: { ...mockHost },
 });
 
@@ -54,7 +54,7 @@ describe('<InventoryHost />', () => {
   });
 
   test('should show content error when api throws error on initial render', async () => {
-    HostsAPI.readDetail.mockRejectedValueOnce(new Error());
+    InventoriesAPI.readHostDetail.mockRejectedValueOnce(new Error());
     await act(async () => {
       wrapper = mountWithContexts(
         <InventoryHost inventory={mockInventory} setBreadcrumb={() => {}} />