Merge pull request #5751 from keithjgrant/5502-inventory-group-refresh

Inventory details refresh

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul

awx/ui_next/src/components/CodeMirrorInput/VariablesDetail.jsx

@@ -1,4 +1,4 @@
-import React, { useState } from 'react';
+import React, { useState, useEffect } from 'react';
 import { string, number } from 'prop-types';
 import { Split, SplitItem, TextListItemVariants } from '@patternfly/react-core';
 import { DetailName, DetailValue } from '@components/DetailList';
@@ -7,11 +7,30 @@ import CodeMirrorInput from './CodeMirrorInput';
 import YamlJsonToggle from './YamlJsonToggle';
 import { JSON_MODE, YAML_MODE } from './constants';
 
+function getValueAsMode(value, mode) {
+  if (!value) {
+    if (mode === JSON_MODE) {
+      return '{}';
+    }
+    return '---';
+  }
+  const modeMatches = isJson(value) === (mode === JSON_MODE);
+  if (modeMatches) {
+    return value;
+  }
+  return mode === YAML_MODE ? jsonToYaml(value) : yamlToJson(value);
+}
+
 function VariablesDetail({ value, label, rows }) {
   const [mode, setMode] = useState(isJson(value) ? JSON_MODE : YAML_MODE);
-  const [currentValue, setCurrentValue] = useState(value);
+  const [currentValue, setCurrentValue] = useState(value || '---');
   const [error, setError] = useState(null);
 
+  useEffect(() => {
+    setCurrentValue(getValueAsMode(value, mode));
+    /* eslint-disable-next-line react-hooks/exhaustive-deps */
+  }, [value]);
+
   return (
     <>
       <DetailName
@@ -35,11 +54,7 @@ function VariablesDetail({ value, label, rows }) {
               mode={mode}
               onChange={newMode => {
                 try {
-                  const newVal =
+                  setCurrentValue(getValueAsMode(currentValue, newMode));
-                    newMode === YAML_MODE
-                      ? jsonToYaml(currentValue)
-                      : yamlToJson(currentValue);
-                  setCurrentValue(newVal);
                   setMode(newMode);
                 } catch (err) {
                   setError(err);
@@ -56,7 +71,7 @@ function VariablesDetail({ value, label, rows }) {
       >
         <CodeMirrorInput
           mode={mode}
-          value={currentValue || '---'} // When github issue https://github.com/ansible/awx/issues/5502 gets resolved this line of code should be revisited and refactored if possible.
+          value={currentValue}
           readOnly
           rows={rows}
           css="margin-top: 10px"

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,42 @@ 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}');
+  });
+
+  test('should default yaml value to "---"', () => {
+    const wrapper = shallow(<VariablesDetail value="" label="Variables" />);
+    const input = wrapper.find('Styled(CodeMirrorInput)');
+    expect(input.prop('value')).toEqual('---');
+  });
+
+  test('should default empty json to "{}"', () => {
+    const wrapper = mount(<VariablesDetail value="" label="Variables" />);
+    act(() => {
+      wrapper.find('YamlJsonToggle').invoke('onChange')('javascript');
+    });
+    wrapper.setProps({ value: '' });
+    const input = wrapper.find('Styled(CodeMirrorInput)');
+    expect(input.prop('value')).toEqual('{}');
+  });
 });

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);

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 () => {

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 = () => {