From 49907e337a52cc2b56507da423a513e108277a76 Mon Sep 17 00:00:00 2001
From: Keith Grant <keithjgrant@redhat.com>
Date: Thu, 23 Jan 2020 11:02:29 -0800
Subject: [PATCH] prevent inventory updates after unmount

---
 .../CodeMirrorInput/VariablesDetail.test.jsx  | 20 ++++++++++++++++++-
 .../src/screens/Inventory/Inventory.jsx       |  1 +
 .../InventoryDetail/InventoryDetail.jsx       | 14 +++++++++++--
 .../Inventory/InventoryEdit/InventoryEdit.jsx | 14 +++++++++++--
 4 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/awx/ui_next/src/components/CodeMirrorInput/VariablesDetail.test.jsx b/awx/ui_next/src/components/CodeMirrorInput/VariablesDetail.test.jsx
index ea13a366c9..b99fda9f16 100644
--- a/awx/ui_next/src/components/CodeMirrorInput/VariablesDetail.test.jsx
+++ b/awx/ui_next/src/components/CodeMirrorInput/VariablesDetail.test.jsx
@@ -1,5 +1,6 @@
 import React from 'react';
-import { shallow } from 'enzyme';
+import { act } from 'react-dom/test-utils';
+import { shallow, mount } from 'enzyme';
 import VariablesDetail from './VariablesDetail';
 
 jest.mock('@api');
@@ -40,9 +41,26 @@ describe('<VariablesDetail>', () => {
     expect(input2.prop('mode')).toEqual('yaml');
     expect(input2.prop('value')).toEqual('foo: bar\n');
   });
+
   test('should render label and value= --- when there are no values', () => {
     const wrapper = shallow(<VariablesDetail value="" label="Variables" />);
     expect(wrapper.find('Styled(CodeMirrorInput)').length).toBe(1);
     expect(wrapper.find('div.pf-c-form__label').text()).toBe('Variables');
   });
+
+  test('should update value if prop changes', () => {
+    const wrapper = mount(
+      <VariablesDetail value="---foo: bar" label="Variables" />
+    );
+    act(() => {
+      wrapper.find('YamlJsonToggle').invoke('onChange')('javascript');
+    });
+    wrapper.setProps({
+      value: '---bar: baz',
+    });
+    wrapper.update();
+    const input = wrapper.find('Styled(CodeMirrorInput)');
+    expect(input.prop('mode')).toEqual('javascript');
+    expect(input.prop('value')).toEqual('{\n  "bar": "baz"\n}');
+  });
 });
diff --git a/awx/ui_next/src/screens/Inventory/Inventory.jsx b/awx/ui_next/src/screens/Inventory/Inventory.jsx
index 6d2ccedea6..66d4872d95 100644
--- a/awx/ui_next/src/screens/Inventory/Inventory.jsx
+++ b/awx/ui_next/src/screens/Inventory/Inventory.jsx
@@ -37,6 +37,7 @@ function Inventory({ i18n, setBreadcrumb }) {
   useEffect(() => {
     async function fetchData() {
       try {
+        setHasContentLoading(true);
         const { data } = await InventoriesAPI.readDetail(match.params.id);
         setBreadcrumb(data);
         setInventory(data);
diff --git a/awx/ui_next/src/screens/Inventory/InventoryDetail/InventoryDetail.jsx b/awx/ui_next/src/screens/Inventory/InventoryDetail/InventoryDetail.jsx
index d3bc8fe5fa..3788dc130f 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryDetail/InventoryDetail.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryDetail/InventoryDetail.jsx
@@ -1,4 +1,4 @@
-import React, { useState, useEffect } from 'react';
+import React, { useState, useEffect, useRef } from 'react';
 import { Link, useHistory } from 'react-router-dom';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
@@ -18,19 +18,29 @@ function InventoryDetail({ inventory, i18n }) {
   const [hasContentLoading, setHasContentLoading] = useState(true);
   const [contentError, setContentError] = useState(null);
   const history = useHistory();
+  const isMounted = useRef(null);
 
   useEffect(() => {
+    isMounted.current = true;
     (async () => {
       setHasContentLoading(true);
       try {
         const { data } = await InventoriesAPI.readInstanceGroups(inventory.id);
+        if (!isMounted.current) {
+          return;
+        }
         setInstanceGroups(data.results);
       } catch (err) {
         setContentError(err);
       } finally {
-        setHasContentLoading(false);
+        if (isMounted.current) {
+          setHasContentLoading(false);
+        }
       }
     })();
+    return () => {
+      isMounted.current = false;
+    };
   }, [inventory.id]);
 
   const deleteInventory = async () => {
diff --git a/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.jsx b/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.jsx
index 0dfa05ae2e..f0b82ae943 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.jsx
@@ -1,4 +1,4 @@
-import React, { useState, useEffect } from 'react';
+import React, { useState, useEffect, useRef } from 'react';
 import { useHistory } from 'react-router-dom';
 import { object } from 'prop-types';
 
@@ -15,8 +15,10 @@ function InventoryEdit({ inventory }) {
   const [contentLoading, setContentLoading] = useState(true);
   const [credentialTypeId, setCredentialTypeId] = useState(null);
   const history = useHistory();
+  const isMounted = useRef(null);
 
   useEffect(() => {
+    isMounted.current = true;
     const loadData = async () => {
       try {
         const [
@@ -32,15 +34,23 @@ function InventoryEdit({ inventory }) {
             kind: 'insights',
           }),
         ]);
+        if (!isMounted.current) {
+          return;
+        }
         setInstanceGroups(loadedInstanceGroups);
         setCredentialTypeId(loadedCredentialTypeId[0].id);
       } catch (err) {
         setError(err);
       } finally {
-        setContentLoading(false);
+        if (isMounted.current) {
+          setContentLoading(false);
+        }
       }
     };
     loadData();
+    return () => {
+      isMounted.current = false;
+    };
   }, [inventory.id, contentLoading, inventory, credentialTypeId]);
 
   const handleCancel = () => {