Do not render setting field if config is empty

2026-01-17 12:41:19 -03:30 · 2020-11-03 14:34:11 -05:00 · 2020-11-03 14:34:11 -05:00 · d57fee7b63
commit d57fee7b63
parent 79930347f9
7 changed files with 77 additions and 58 deletions
--- a/awx/ui_next/src/screens/Setting/ActivityStream/ActivityStreamEdit/ActivityStreamEdit.test.jsx
+++ b/awx/ui_next/src/screens/Setting/ActivityStream/ActivityStreamEdit/ActivityStreamEdit.test.jsx
@ -68,12 +68,24 @@ describe('<ActivityStreamEdit />', () => {

  test('should successfully send request to api on form submission', async () => {
    expect(SettingsAPI.updateAll).toHaveBeenCalledTimes(0);
+    expect(
+      wrapper.find('Switch#ACTIVITY_STREAM_ENABLED').prop('isChecked')
+    ).toEqual(false);
+
+    await act(async () => {
+      wrapper.find('Switch#ACTIVITY_STREAM_ENABLED').invoke('onChange')(true);
+    });
+    wrapper.update();
+    expect(
+      wrapper.find('Switch#ACTIVITY_STREAM_ENABLED').prop('isChecked')
+    ).toEqual(true);
+
    await act(async () => {
      wrapper.find('Form').invoke('onSubmit')();
    });
    expect(SettingsAPI.updateAll).toHaveBeenCalledTimes(1);
    expect(SettingsAPI.updateAll).toHaveBeenCalledWith({
-      ACTIVITY_STREAM_ENABLED: false,
+      ACTIVITY_STREAM_ENABLED: true,
      ACTIVITY_STREAM_ENABLED_FOR_INVENTORY_SYNC: true,
    });
  });
--- a/awx/ui_next/src/screens/Setting/AzureAD/AzureADEdit/AzureADEdit.jsx
+++ b/awx/ui_next/src/screens/Setting/AzureAD/AzureADEdit/AzureADEdit.jsx
@ -14,6 +14,7 @@ import {
  InputField,
  ObjectField,
 } from '../../shared/SharedFields';
+import { formatJson } from '../../shared/settingUtils';
 import useModal from '../../../../util/useModal';
 import useRequest from '../../../../util/useRequest';
 import { SettingsAPI } from '../../../../api';
@ -57,10 +58,10 @@ function AzureADEdit() {
  const handleSubmit = async form => {
    await submitForm({
      ...form,
-      SOCIAL_AUTH_AZUREAD_OAUTH2_TEAM_MAP: JSON.parse(
+      SOCIAL_AUTH_AZUREAD_OAUTH2_TEAM_MAP: formatJson(
        form.SOCIAL_AUTH_AZUREAD_OAUTH2_TEAM_MAP
      ),
-      SOCIAL_AUTH_AZUREAD_OAUTH2_ORGANIZATION_MAP: JSON.parse(
+      SOCIAL_AUTH_AZUREAD_OAUTH2_ORGANIZATION_MAP: formatJson(
        form.SOCIAL_AUTH_AZUREAD_OAUTH2_ORGANIZATION_MAP
      ),
    });
--- a/awx/ui_next/src/screens/Setting/Logging/LoggingEdit/LoggingEdit.jsx
+++ b/awx/ui_next/src/screens/Setting/Logging/LoggingEdit/LoggingEdit.jsx
@ -22,6 +22,7 @@ import {
 } from '../../shared';
 import useModal from '../../../../util/useModal';
 import useRequest, { useDismissableError } from '../../../../util/useRequest';
+import { formatJson } from '../../shared/settingUtils';
 import { SettingsAPI } from '../../../../api';

 function LoggingEdit({ i18n }) {
@ -39,6 +40,9 @@ function LoggingEdit({ i18n }) {
      const { data } = await SettingsAPI.readCategory('logging');
      const mergedData = {};
      Object.keys(data).forEach(key => {
+        if (!options[key]) {
+          return;
+        }
        mergedData[key] = options[key];
        mergedData[key].value = data[key];
      });
@ -65,7 +69,7 @@ function LoggingEdit({ i18n }) {
  const handleSubmit = async form => {
    await submitForm({
      ...form,
-      LOG_AGGREGATOR_LOGGERS: JSON.parse(form.LOG_AGGREGATOR_LOGGERS),
+      LOG_AGGREGATOR_LOGGERS: formatJson(form.LOG_AGGREGATOR_LOGGERS),
      LOG_AGGREGATOR_HOST: form.LOG_AGGREGATOR_HOST || null,
      LOG_AGGREGATOR_TYPE: form.LOG_AGGREGATOR_TYPE || null,
    });
@ -127,10 +131,7 @@ function LoggingEdit({ i18n }) {
      {isLoading && <ContentLoading />}
      {!isLoading && error && <ContentError error={error} />}
      {!isLoading && logging && (
-        <Formik
-          initialValues={{ ...initialValues(logging) }}
-          onSubmit={handleSubmit}
-        >
+        <Formik initialValues={initialValues(logging)} onSubmit={handleSubmit}>
          {formik => {
            return (
              <Form autoComplete="off" onSubmit={formik.handleSubmit}>
--- a/awx/ui_next/src/screens/Setting/MiscSystem/MiscSystemEdit/MiscSystemEdit.jsx
+++ b/awx/ui_next/src/screens/Setting/MiscSystem/MiscSystemEdit/MiscSystemEdit.jsx
@ -21,7 +21,7 @@ import {
 import useModal from '../../../../util/useModal';
 import useRequest from '../../../../util/useRequest';
 import { SettingsAPI } from '../../../../api';
-import { pluck } from '../../shared/settingUtils';
+import { pluck, formatJson } from '../../shared/settingUtils';

 function MiscSystemEdit({ i18n }) {
  const history = useHistory();
@ -95,6 +95,9 @@ function MiscSystemEdit({ i18n }) {

      const mergedData = {};
      Object.keys(systemData).forEach(key => {
+        if (!systemOptions[key]) {
+          return;
+        }
        mergedData[key] = systemOptions[key];
        mergedData[key].value = systemData[key];
      });
@ -127,8 +130,8 @@ function MiscSystemEdit({ i18n }) {
    } = form;
    await submitForm({
      ...formData,
-      CUSTOM_VENV_PATHS: JSON.parse(formData.CUSTOM_VENV_PATHS),
-      REMOTE_HOST_HEADERS: JSON.parse(formData.REMOTE_HOST_HEADERS),
+      CUSTOM_VENV_PATHS: formatJson(formData.CUSTOM_VENV_PATHS),
+      REMOTE_HOST_HEADERS: formatJson(formData.REMOTE_HOST_HEADERS),
      OAUTH2_PROVIDER: {
        ACCESS_TOKEN_EXPIRE_SECONDS,
        REFRESH_TOKEN_EXPIRE_SECONDS,
@ -181,10 +184,7 @@ function MiscSystemEdit({ i18n }) {
      {isLoading && <ContentLoading />}
      {!isLoading && error && <ContentError error={error} />}
      {!isLoading && system && (
-        <Formik
-          initialValues={{ ...initialValues(system) }}
-          onSubmit={handleSubmit}
-        >
+        <Formik initialValues={initialValues(system)} onSubmit={handleSubmit}>
          {formik => {
            return (
              <Form autoComplete="off" onSubmit={formik.handleSubmit}>
--- a/awx/ui_next/src/screens/Setting/shared/RevertButton.jsx
+++ b/awx/ui_next/src/screens/Setting/shared/RevertButton.jsx
@ -45,6 +45,7 @@ function RevertButton({ i18n, id, defaultValue, isDisabled = false }) {
      <ButtonWrapper>
        <Button
          aria-label={isRevertable ? i18n._(t`Revert`) : i18n._(t`Undo`)}
+          data-cy={`${id}-revert`}
          isInline
          isSmall
          onClick={handleConfirm}
--- a/awx/ui_next/src/screens/Setting/shared/SharedFields.jsx
+++ b/awx/ui_next/src/screens/Setting/shared/SharedFields.jsx
@ -44,39 +44,36 @@ const SettingGroup = withI18n()(
    label,
    popoverContent,
    validated,
-  }) => {
-    return (
-      <FormGroup
-        fieldId={fieldId}
-        helperTextInvalid={helperTextInvalid}
-        isRequired={isRequired}
-        label={label}
-        validated={validated}
-        labelIcon={
-          <>
-            <Popover
-              content={popoverContent}
-              ariaLabel={`${i18n._(t`More information for`)} ${label}`}
-            />
-            <RevertButton
-              id={fieldId}
-              defaultValue={defaultValue}
-              isDisabled={isDisabled}
-            />
-          </>
-        }
-      >
-        {children}
-      </FormGroup>
-    );
-  }
+  }) => (
+    <FormGroup
+      fieldId={fieldId}
+      helperTextInvalid={helperTextInvalid}
+      isRequired={isRequired}
+      label={label}
+      validated={validated}
+      labelIcon={
+        <>
+          <Popover
+            content={popoverContent}
+            ariaLabel={`${i18n._(t`More information for`)} ${label}`}
+          />
+          <RevertButton
+            id={fieldId}
+            defaultValue={defaultValue}
+            isDisabled={isDisabled}
+          />
+        </>
+      }
+    >
+      {children}
+    </FormGroup>
+  )
 );

 const BooleanField = withI18n()(
  ({ i18n, ariaLabel = '', name, config, disabled = false }) => {
    const [field, meta, helpers] = useField(name);
-
-    return (
+    return config ? (
      <SettingGroup
        defaultValue={config.default ?? false}
        fieldId={name}
@ -95,7 +92,7 @@ const BooleanField = withI18n()(
          aria-label={ariaLabel || config.label}
        />
      </SettingGroup>
-    );
+    ) : null;
  }
 );
 BooleanField.propTypes = {
@ -110,7 +107,7 @@ const ChoiceField = withI18n()(({ i18n, name, config, isRequired = false }) => {
  const [field, meta] = useField({ name, validate });
  const isValid = !meta.error || !meta.touched;

-  return (
+  return config ? (
    <SettingGroup
      defaultValue={config.default ?? ''}
      fieldId={name}
@ -132,7 +129,7 @@ const ChoiceField = withI18n()(({ i18n, name, config, isRequired = false }) => {
        ]}
      />
    </SettingGroup>
-  );
+  ) : null;
 });
 ChoiceField.propTypes = {
  name: string.isRequired,
@ -146,7 +143,7 @@ const EncryptedField = withI18n()(
    const [, meta] = useField({ name, validate });
    const isValid = !(meta.touched && meta.error);

-    return (
+    return config ? (
      <SettingGroup
        defaultValue={config.default ?? ''}
        fieldId={name}
@ -166,7 +163,7 @@ const EncryptedField = withI18n()(
          />
        </InputGroup>
      </SettingGroup>
-    );
+    ) : null;
  }
 );
 EncryptedField.propTypes = {
@ -177,10 +174,8 @@ EncryptedField.propTypes = {

 const InputField = withI18n()(
  ({ i18n, name, config, type = 'text', isRequired = false }) => {
-    const {
-      min_value = Number.MIN_SAFE_INTEGER,
-      max_value = Number.MAX_SAFE_INTEGER,
-    } = config;
+    const min_value = config?.min_value ?? Number.MIN_SAFE_INTEGER;
+    const max_value = config?.max_value ?? Number.MAX_SAFE_INTEGER;
    const validators = [
      ...(isRequired ? [required(null, i18n)] : []),
      ...(type === 'url' ? [url(i18n)] : []),
@ -191,7 +186,7 @@ const InputField = withI18n()(
    const [field, meta] = useField({ name, validate: combine(validators) });
    const isValid = !(meta.touched && meta.error);

-    return (
+    return config ? (
      <SettingGroup
        defaultValue={config.default || ''}
        fieldId={name}
@ -213,7 +208,7 @@ const InputField = withI18n()(
          }}
        />
      </SettingGroup>
-    );
+    ) : null;
  }
 );
 InputField.propTypes = {
@ -228,12 +223,12 @@ const ObjectField = withI18n()(({ i18n, name, config, isRequired = false }) => {
  const [field, meta, helpers] = useField({ name, validate });
  const isValid = !(meta.touched && meta.error);

-  const emptyDefault = config.type === 'list' ? '[]' : '{}';
-  const defaultRevertValue = config.default
+  const emptyDefault = config?.type === 'list' ? '[]' : '{}';
+  const defaultRevertValue = config?.default
    ? JSON.stringify(config.default, null, 2)
    : emptyDefault;

-  return (
+  return config ? (
    <FormFullWidthLayout>
      <SettingGroup
        defaultValue={defaultRevertValue}
@ -254,7 +249,7 @@ const ObjectField = withI18n()(({ i18n, name, config, isRequired = false }) => {
        />
      </SettingGroup>
    </FormFullWidthLayout>
-  );
+  ) : null;
 });
 ObjectField.propTypes = {
  name: string.isRequired,
--- a/awx/ui_next/src/screens/Setting/shared/settingUtils.js
+++ b/awx/ui_next/src/screens/Setting/shared/settingUtils.js
@ -1,3 +1,5 @@
+import { isJsonString } from '../../../util/yaml';
+
 export function sortNestedDetails(obj = {}) {
  const nestedTypes = ['nested object', 'list', 'boolean'];
  const notNested = Object.entries(obj).filter(
@ -18,3 +20,10 @@ export function sortNestedDetails(obj = {}) {
 export function pluck(sourceObject, ...keys) {
  return Object.assign({}, ...keys.map(key => ({ [key]: sourceObject[key] })));
 }
+
+export function formatJson(jsonString) {
+  if (!jsonString) {
+    return null;
+  }
+  return isJsonString(jsonString) ? JSON.parse(jsonString) : jsonString;
+}