From bb42283ba9e8ddb7a54fce4e542d340ca43a5263 Mon Sep 17 00:00:00 2001
From: Christian Vogt <cvogt@redhat.com>
Date: Thu, 7 Nov 2024 10:47:21 -0500
Subject: [PATCH] disable required toggle when field has default value and is
 read only

---
 .../manage/ConnectionTypeDataFieldModal.tsx   |  8 ++++++-
 .../manage/DataFieldPropertiesForm.tsx        |  8 ++++++-
 .../ManageConnectionTypeFieldsTableRow.tsx    |  7 +++++-
 .../ConnectionTypeDataFieldModal.spec.tsx     | 24 +++++++++++++++++++
 ...anageConnectionTypeFieldsTableRow.spec.tsx | 16 +++++++++++++
 5 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/frontend/src/pages/connectionTypes/manage/ConnectionTypeDataFieldModal.tsx b/frontend/src/pages/connectionTypes/manage/ConnectionTypeDataFieldModal.tsx
index 9d08bfd5c4..160485f27a 100644
--- a/frontend/src/pages/connectionTypes/manage/ConnectionTypeDataFieldModal.tsx
+++ b/frontend/src/pages/connectionTypes/manage/ConnectionTypeDataFieldModal.tsx
@@ -233,7 +233,12 @@ export const ConnectionTypeDataFieldModal: React.FC<Props> = ({
         </FormGroup>
         <DataFieldPropertiesForm
           field={newField}
-          onChange={(value) => setData('properties', value)}
+          onChange={(value) => {
+            setData('properties', value);
+            if (value.defaultReadOnly && newField.required) {
+              setData('required', false);
+            }
+          }}
           onValidate={setPropertiesValid}
         />
         <FormGroup fieldId="isRequired">
@@ -241,6 +246,7 @@ export const ConnectionTypeDataFieldModal: React.FC<Props> = ({
             id="isRequired"
             data-testid="field-required-checkbox"
             label="Field is required"
+            isDisabled={properties.defaultReadOnly}
             isChecked={required || false}
             onChange={(_ev, checked) => {
               setData('required', checked);
diff --git a/frontend/src/pages/connectionTypes/manage/DataFieldPropertiesForm.tsx b/frontend/src/pages/connectionTypes/manage/DataFieldPropertiesForm.tsx
index c0993b5a48..d5d658c9fd 100644
--- a/frontend/src/pages/connectionTypes/manage/DataFieldPropertiesForm.tsx
+++ b/frontend/src/pages/connectionTypes/manage/DataFieldPropertiesForm.tsx
@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { Checkbox, FormGroup } from '@patternfly/react-core';
+import { Checkbox, FormGroup, HelperText, HelperTextItem } from '@patternfly/react-core';
 import { ConnectionTypeDataField } from '~/concepts/connectionTypes/types';
 import ConnectionTypeDataFormField from '~/concepts/connectionTypes/fields/ConnectionTypeDataFormField';
 import DataFieldAdvancedPropertiesForm from '~/pages/connectionTypes/manage/advanced/DataFieldAdvancedPropertiesForm';
@@ -45,6 +45,12 @@ const DataFieldPropertiesForm = <T extends ConnectionTypeDataField>({
           value={field.properties.defaultValue}
           data-testid="field-default-value"
         />
+        <HelperText>
+          <HelperTextItem>
+            Do not enter sensitive information. Default values are visible to users with access to
+            the connection type.
+          </HelperTextItem>
+        </HelperText>
         <Checkbox
           id="defaultReadOnly"
           label="Default value is read-only"
diff --git a/frontend/src/pages/connectionTypes/manage/ManageConnectionTypeFieldsTableRow.tsx b/frontend/src/pages/connectionTypes/manage/ManageConnectionTypeFieldsTableRow.tsx
index 9a8bd26a93..c1e8aaea67 100644
--- a/frontend/src/pages/connectionTypes/manage/ManageConnectionTypeFieldsTableRow.tsx
+++ b/frontend/src/pages/connectionTypes/manage/ManageConnectionTypeFieldsTableRow.tsx
@@ -174,7 +174,12 @@ const ManageConnectionTypeFieldsTableRow: React.FC<Props> = ({
           aria-label="toggle field required"
           isChecked={row.required || false}
           data-testid="field-required"
-          onChange={() => onChange({ ...row, required: !row.required })}
+          isDisabled={row.properties.defaultReadOnly}
+          onChange={
+            !row.properties.defaultReadOnly
+              ? () => onChange({ ...row, required: !row.required })
+              : undefined
+          }
         />
       </Td>
       <Td isActionCell>
diff --git a/frontend/src/pages/connectionTypes/manage/__tests__/ConnectionTypeDataFieldModal.spec.tsx b/frontend/src/pages/connectionTypes/manage/__tests__/ConnectionTypeDataFieldModal.spec.tsx
index bb5c436dd7..f33dcebed3 100644
--- a/frontend/src/pages/connectionTypes/manage/__tests__/ConnectionTypeDataFieldModal.spec.tsx
+++ b/frontend/src/pages/connectionTypes/manage/__tests__/ConnectionTypeDataFieldModal.spec.tsx
@@ -286,4 +286,28 @@ describe('ConnectionTypeDataFieldModal', () => {
 
     expect(submitButton).not.toBeDisabled();
   });
+
+  it('should unset required if default value is read-only', () => {
+    const field: ShortTextField = {
+      type: 'short-text',
+      name: 'test',
+      envVar: 'test_envvar',
+      required: true,
+      properties: {
+        defaultValue: 'default value',
+        defaultReadOnly: false,
+      },
+    };
+    render(
+      <ConnectionTypeDataFieldModal onClose={onClose} onSubmit={onSubmit} isEdit field={field} />,
+    );
+    const requiredCheckbox = screen.getByTestId('field-required-checkbox');
+    expect(requiredCheckbox).toBeChecked();
+    const readOnlyCheckbox = screen.getByTestId('field-default-value-readonly-checkbox');
+    expect(readOnlyCheckbox).not.toBeChecked();
+    act(() => {
+      fireEvent.click(requiredCheckbox);
+    });
+    expect(requiredCheckbox).not.toBeChecked();
+  });
 });
diff --git a/frontend/src/pages/connectionTypes/manage/__tests__/ManageConnectionTypeFieldsTableRow.spec.tsx b/frontend/src/pages/connectionTypes/manage/__tests__/ManageConnectionTypeFieldsTableRow.spec.tsx
index 4f95e0ed12..eac3880943 100644
--- a/frontend/src/pages/connectionTypes/manage/__tests__/ManageConnectionTypeFieldsTableRow.spec.tsx
+++ b/frontend/src/pages/connectionTypes/manage/__tests__/ManageConnectionTypeFieldsTableRow.spec.tsx
@@ -66,4 +66,20 @@ describe('ManageConnectionTypeFieldsTableRow', () => {
     expect(hasValidationIssueMock).toHaveBeenCalledTimes(1);
     expect(hasValidationIssueMock).toHaveBeenCalledWith(['fields', 0, 'envVar'], 'envVar_conflict');
   });
+
+  it('should not be able to set required if default value is read-only', () => {
+    const field: ShortTextField = {
+      type: 'short-text',
+      name: 'test',
+      envVar: 'test_envvar',
+      properties: {
+        defaultValue: 'default value',
+        defaultReadOnly: true,
+      },
+    };
+    render(renderRow({ row: field, fields: [field] }));
+    const requiredSwitch = screen.getByTestId('field-required');
+    expect(requiredSwitch).not.toBeChecked();
+    expect(requiredSwitch).toBeDisabled();
+  });
 });