diff --git a/awx/ui_next/src/components/CodeMirrorInput/VariablesDetail.jsx b/awx/ui_next/src/components/CodeMirrorInput/VariablesDetail.jsx index 45b02ef216..c2e3fe2b2c 100644 --- a/awx/ui_next/src/components/CodeMirrorInput/VariablesDetail.jsx +++ b/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 ( <> { try { - const newVal = - newMode === YAML_MODE - ? jsonToYaml(currentValue) - : yamlToJson(currentValue); - setCurrentValue(newVal); + setCurrentValue(getValueAsMode(currentValue, newMode)); setMode(newMode); } catch (err) { setError(err); @@ -56,7 +71,7 @@ function VariablesDetail({ value, label, rows }) { > ', () => { 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(); 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( + + ); + 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(); + const input = wrapper.find('Styled(CodeMirrorInput)'); + expect(input.prop('value')).toEqual('---'); + }); + + test('should default empty json to "{}"', () => { + const wrapper = mount(); + act(() => { + wrapper.find('YamlJsonToggle').invoke('onChange')('javascript'); + }); + wrapper.setProps({ value: '' }); + const input = wrapper.find('Styled(CodeMirrorInput)'); + expect(input.prop('value')).toEqual('{}'); + }); }); diff --git a/awx/ui_next/src/screens/Inventory/Inventory.jsx b/awx/ui_next/src/screens/Inventory/Inventory.jsx index 6e8f55c60b..3a445e0cf6 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 = () => {