From 6ca046e84181da3eab4f6358211cf1da277d77ae Mon Sep 17 00:00:00 2001
From: Emily Samoylov <93456589+emilys314@users.noreply.github.com>
Date: Thu, 26 Sep 2024 10:32:45 -0400
Subject: [PATCH] Edit for existing project connection instances (#3250)

* Allow edit of existing project connections

* Allow interactive preview of k8s resource name

* Hide 'connections' from 'data connections' + show disabled type selection in edit

* Connection modal typeahead enhancements
---
 .../tests/mocked/projects/connections.cy.ts   |  32 +++
 .../connectionTypes/ConnectionTypeForm.tsx    |  94 ++++++--
 .../fields/ConnectionTypeFormFields.tsx       |  18 ++
 .../src/concepts/connectionTypes/utils.ts     |  45 +++-
 .../K8sNameDescriptionField.tsx               |   2 +-
 .../ResourceNameField.tsx                     |   4 +-
 .../detail/connections/ConnectionsList.tsx    |  11 +-
 .../detail/connections/ConnectionsTable.tsx   |   4 +-
 .../connections/ManageConnectionsModal.tsx    |  74 +++++--
 .../__tests__/ConnectionsTable.spec.tsx       |   2 +
 .../__tests__/ManageConnectionsModal.spec.tsx | 206 +++++++++++++++++-
 .../screens/detail/data-connections/utils.ts  |   3 +-
 12 files changed, 443 insertions(+), 52 deletions(-)

diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/connections.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/connections.cy.ts
index 1ae3425001..c30e11b09e 100644
--- a/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/connections.cy.ts
+++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/connections.cy.ts
@@ -119,4 +119,36 @@ describe('Connections', () => {
 
     cy.wait('@createConnection');
   });
+
+  it('Edit a connection', () => {
+    initIntercepts();
+    cy.interceptOdh('GET /api/connection-types', [
+      mockConnectionTypeConfigMap({
+        name: 'postgres',
+        fields: [
+          {
+            name: 'field A',
+            type: ConnectionTypeFieldType.ShortText,
+            envVar: 'field_env',
+            properties: {},
+          },
+        ],
+      }),
+    ]);
+    cy.interceptK8s(
+      'PUT',
+      SecretModel,
+      mockSecretK8sResource({
+        name: 'test2',
+      }),
+    ).as('editConnection');
+
+    projectDetails.visitSection('test-project', 'connections');
+
+    connectionsPage.getConnectionRow('test2').findKebabAction('Edit').click();
+    cy.findByTestId(['field_env']).fill('new data');
+    cy.findByTestId('modal-submit-button').click();
+
+    cy.wait('@editConnection');
+  });
 });
diff --git a/frontend/src/concepts/connectionTypes/ConnectionTypeForm.tsx b/frontend/src/concepts/connectionTypes/ConnectionTypeForm.tsx
index 775d8a62e1..939fa9ce70 100644
--- a/frontend/src/concepts/connectionTypes/ConnectionTypeForm.tsx
+++ b/frontend/src/concepts/connectionTypes/ConnectionTypeForm.tsx
@@ -1,11 +1,21 @@
 import * as React from 'react';
-import { Form, FormGroup, FormSection, MenuToggleStatus, Title } from '@patternfly/react-core';
+import {
+  Flex,
+  FlexItem,
+  Form,
+  FormGroup,
+  FormSection,
+  MenuToggleStatus,
+  Title,
+  Truncate,
+} from '@patternfly/react-core';
 import ConnectionTypeFormFields from '~/concepts/connectionTypes/fields/ConnectionTypeFormFields';
 import {
   ConnectionTypeConfigMapObj,
   ConnectionTypeValueType,
 } from '~/concepts/connectionTypes/types';
 import {
+  getDescriptionFromK8sResource,
   getDisplayNameFromK8sResource,
   getResourceNameFromK8sResource,
 } from '~/concepts/k8s/utils';
@@ -17,6 +27,48 @@ import {
 } from '~/concepts/k8s/K8sNameDescriptionField/types';
 import { ConnectionTypeDetailsHelperText } from './ConnectionTypeDetailsHelperText';
 
+const getConnectionTypeSelectOptions = (
+  isPreview: boolean,
+  selectedConnectionType?: ConnectionTypeConfigMapObj,
+  connectionTypes?: ConnectionTypeConfigMapObj[],
+): TypeaheadSelectOption[] => {
+  if (isPreview && selectedConnectionType?.metadata.annotations?.['openshift.io/display-name']) {
+    return [
+      {
+        value: '',
+        content: selectedConnectionType.metadata.annotations['openshift.io/display-name'],
+        isSelected: true,
+      },
+    ];
+  }
+  if (!isPreview && connectionTypes) {
+    return connectionTypes.map((t) => ({
+      value: getResourceNameFromK8sResource(t),
+      content: getDisplayNameFromK8sResource(t),
+      description: (
+        <Flex direction={{ default: 'column' }} rowGap={{ default: 'rowGapNone' }}>
+          {getDescriptionFromK8sResource(t) && (
+            <FlexItem>
+              <Truncate content={getDescriptionFromK8sResource(t)} />
+            </FlexItem>
+          )}
+          {t.data?.category?.length && (
+            <FlexItem>
+              <Truncate content={`Category: ${t.data.category.join(', ')}`} />
+            </FlexItem>
+          )}
+        </Flex>
+      ),
+      data: `${getDescriptionFromK8sResource(t)} ${t.data?.category?.join(' ')}`,
+      isSelected:
+        !!selectedConnectionType &&
+        getResourceNameFromK8sResource(t) ===
+          getResourceNameFromK8sResource(selectedConnectionType),
+    }));
+  }
+  return [];
+};
+
 type Props = Pick<
   React.ComponentProps<typeof ConnectionTypeFormFields>,
   'onChange' | 'onValidate'
@@ -30,6 +82,7 @@ type Props = Pick<
   connectionValues?: {
     [key: string]: ConnectionTypeValueType;
   };
+  disableTypeSelection?: boolean;
 };
 
 const ConnectionTypeForm: React.FC<Props> = ({
@@ -42,26 +95,12 @@ const ConnectionTypeForm: React.FC<Props> = ({
   connectionValues,
   onChange,
   onValidate,
+  disableTypeSelection,
 }) => {
-  const options: TypeaheadSelectOption[] = React.useMemo(() => {
-    if (isPreview && connectionType?.metadata.annotations?.['openshift.io/display-name']) {
-      return [
-        {
-          value: '',
-          content: connectionType.metadata.annotations['openshift.io/display-name'],
-          isSelected: true,
-        },
-      ];
-    }
-    if (!isPreview && connectionTypes) {
-      return connectionTypes.map((t) => ({
-        value: getResourceNameFromK8sResource(t),
-        content: getDisplayNameFromK8sResource(t),
-        isSelected: t.metadata.name === connectionType?.metadata.name,
-      }));
-    }
-    return [];
-  }, [isPreview, connectionType?.metadata, connectionTypes]);
+  const options: TypeaheadSelectOption[] = React.useMemo(
+    () => getConnectionTypeSelectOptions(isPreview, connectionType, connectionTypes),
+    [isPreview, connectionType, connectionTypes],
+  );
 
   return (
     <Form>
@@ -73,7 +112,7 @@ const ConnectionTypeForm: React.FC<Props> = ({
           onSelect={(_, selection) =>
             setConnectionType?.(connectionTypes?.find((c) => c.metadata.name === selection))
           }
-          isDisabled={isPreview || connectionTypes?.length === 1}
+          isDisabled={isPreview || disableTypeSelection}
           placeholder={
             isPreview && !connectionType?.metadata.annotations?.['openshift.io/display-name']
               ? 'Unspecified'
@@ -84,13 +123,22 @@ const ConnectionTypeForm: React.FC<Props> = ({
               ? { status: MenuToggleStatus.danger }
               : undefined
           }
+          isScrollable
+          popperProps={{ maxWidth: 'trigger' }}
+          filterFunction={(filterValue: string, filterOptions: TypeaheadSelectOption[]) =>
+            filterOptions.filter(
+              (o) =>
+                String(o.content).toLowerCase().includes(filterValue.toLowerCase()) ||
+                String(o.data).toLowerCase().includes(filterValue.toLowerCase()),
+            )
+          }
         />
         {connectionType && (
           <ConnectionTypeDetailsHelperText connectionType={connectionType} isPreview={isPreview} />
         )}
       </FormGroup>
       {(isPreview || connectionType?.metadata.name) && (
-        <FormSection title="Connection details" style={{ marginTop: 0 }}>
+        <FormSection title="Connection details">
           <K8sNameDescriptionField
             dataTestId="connection-name-desc"
             nameLabel="Connection name"
@@ -102,7 +150,7 @@ const ConnectionTypeForm: React.FC<Props> = ({
                 k8sName: {
                   value: '',
                   state: {
-                    immutable: true,
+                    immutable: false,
                     invalidCharacters: false,
                     invalidLength: false,
                     maxLength: 0,
diff --git a/frontend/src/concepts/connectionTypes/fields/ConnectionTypeFormFields.tsx b/frontend/src/concepts/connectionTypes/fields/ConnectionTypeFormFields.tsx
index 9a843c8ddc..9a62c84b63 100644
--- a/frontend/src/concepts/connectionTypes/fields/ConnectionTypeFormFields.tsx
+++ b/frontend/src/concepts/connectionTypes/fields/ConnectionTypeFormFields.tsx
@@ -7,6 +7,7 @@ import {
   ConnectionTypeField,
   ConnectionTypeFieldType,
   ConnectionTypeValueType,
+  isConnectionTypeDataField,
   SectionField,
 } from '~/concepts/connectionTypes/types';
 
@@ -47,6 +48,22 @@ const ConnectionTypeFormFields: React.FC<Props> = ({
     [fields],
   );
 
+  const unmatchedValues: ConnectionTypeDataField[] = React.useMemo(() => {
+    const unmatched: ConnectionTypeDataField[] = [];
+    for (const key in connectionValues) {
+      const matching = fields?.find((f) => isConnectionTypeDataField(f) && f.envVar === key);
+      if (!matching) {
+        unmatched.push({
+          type: ConnectionTypeFieldType.ShortText,
+          envVar: key,
+          name: key,
+          properties: {},
+        });
+      }
+    }
+    return unmatched;
+  }, [connectionValues, fields]);
+
   const renderDataFields = (dataFields: ConnectionTypeDataField[]) =>
     dataFields.map((field, i) => (
       <DataFormFieldGroup key={i} field={field}>
@@ -74,6 +91,7 @@ const ConnectionTypeFormFields: React.FC<Props> = ({
           <React.Fragment key={i}>{renderDataFields(fieldGroup.fields)}</React.Fragment>
         ),
       )}
+      {unmatchedValues.length > 0 && renderDataFields(unmatchedValues)}
     </>
   );
 };
diff --git a/frontend/src/concepts/connectionTypes/utils.ts b/frontend/src/concepts/connectionTypes/utils.ts
index db76161530..617208a4ab 100644
--- a/frontend/src/concepts/connectionTypes/utils.ts
+++ b/frontend/src/concepts/connectionTypes/utils.ts
@@ -137,7 +137,12 @@ export const assembleConnectionSecret = (
   },
 ): Connection => {
   const connectionValuesAsStrings = Object.fromEntries(
-    Object.entries(values).map(([key, value]) => [key, String(value)]),
+    Object.entries(values).map(([key, value]) => {
+      if (Array.isArray(value)) {
+        return [key, JSON.stringify(value)]; // multi select
+      }
+      return [key, String(value)];
+    }),
   );
   return {
     apiVersion: 'v1',
@@ -158,3 +163,41 @@ export const assembleConnectionSecret = (
     stringData: connectionValuesAsStrings,
   };
 };
+
+export const parseConnectionSecretValues = (
+  connection: Connection,
+  connectionType?: ConnectionTypeConfigMapObj,
+): { [key: string]: ConnectionTypeValueType } => {
+  const response: { [key: string]: ConnectionTypeValueType } = {};
+
+  for (const [key, value] of Object.entries(connection.data ?? {})) {
+    const decodedString = window.atob(value);
+    const matchingField = connectionType?.data?.fields?.find(
+      (f) => isConnectionTypeDataField(f) && f.envVar === key,
+    );
+
+    if (matchingField?.type === ConnectionTypeFieldType.Boolean) {
+      response[key] = decodedString === 'true';
+    } else if (matchingField?.type === ConnectionTypeFieldType.Numeric) {
+      response[key] = Number(decodedString);
+    } else if (
+      matchingField?.type === ConnectionTypeFieldType.Dropdown &&
+      matchingField.properties.variant === 'multi'
+    ) {
+      try {
+        const parsed = JSON.parse(decodedString);
+        if (Array.isArray(parsed)) {
+          response[key] = parsed.map((v) => String(v));
+        } else {
+          response[key] = [decodedString];
+        }
+      } catch {
+        response[key] = [decodedString];
+      }
+    } else {
+      response[key] = decodedString;
+    }
+  }
+
+  return response;
+};
diff --git a/frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx b/frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx
index 4c73b20537..889753e37b 100644
--- a/frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx
+++ b/frontend/src/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField.tsx
@@ -72,7 +72,7 @@ const K8sNameDescriptionField: React.FC<K8sNameDescriptionFieldProps> = ({
           value={name}
           onChange={(event, value) => onDataChange?.('name', value)}
         />
-        {!showK8sField && !!onDataChange && !k8sName.state.immutable && (
+        {!showK8sField && !k8sName.state.immutable && (
           <FormHelperText>
             {k8sName.value && (
               <HelperText>
diff --git a/frontend/src/concepts/k8s/K8sNameDescriptionField/ResourceNameField.tsx b/frontend/src/concepts/k8s/K8sNameDescriptionField/ResourceNameField.tsx
index 1c3375dd7d..b254a82e11 100644
--- a/frontend/src/concepts/k8s/K8sNameDescriptionField/ResourceNameField.tsx
+++ b/frontend/src/concepts/k8s/K8sNameDescriptionField/ResourceNameField.tsx
@@ -34,7 +34,7 @@ const ResourceNameField: React.FC<ResourceNameFieldProps> = ({
     return <FormGroup {...formGroupProps}>{k8sName.value}</FormGroup>;
   }
 
-  if (!allowEdit || !onDataChange) {
+  if (!allowEdit) {
     return null;
   }
 
@@ -52,7 +52,7 @@ const ResourceNameField: React.FC<ResourceNameFieldProps> = ({
         name={`${dataTestId}-resourceName`}
         isRequired
         value={k8sName.value}
-        onChange={(event, value) => onDataChange('k8sName', value)}
+        onChange={(event, value) => onDataChange?.('k8sName', value)}
         validated={validated}
       />
       <HelperText>
diff --git a/frontend/src/pages/projects/screens/detail/connections/ConnectionsList.tsx b/frontend/src/pages/projects/screens/detail/connections/ConnectionsList.tsx
index bec75b6558..646f1e75df 100644
--- a/frontend/src/pages/projects/screens/detail/connections/ConnectionsList.tsx
+++ b/frontend/src/pages/projects/screens/detail/connections/ConnectionsList.tsx
@@ -10,7 +10,7 @@ import DashboardPopupIconButton from '~/concepts/dashboard/DashboardPopupIconBut
 import { ProjectObjectType, typedEmptyImage } from '~/concepts/design/utils';
 import { Connection } from '~/concepts/connectionTypes/types';
 import { useWatchConnectionTypes } from '~/utilities/useWatchConnectionTypes';
-import { createSecret } from '~/api';
+import { createSecret, replaceSecret } from '~/api';
 import ConnectionsTable from './ConnectionsTable';
 import { ManageConnectionModal } from './ManageConnectionsModal';
 
@@ -26,6 +26,7 @@ const ConnectionsList: React.FC = () => {
 
   const [manageConnectionModal, setManageConnectionModal] = React.useState<{
     connection?: Connection;
+    isEdit?: boolean;
   }>();
 
   return (
@@ -75,6 +76,9 @@ const ConnectionsList: React.FC = () => {
         connections={connections}
         connectionTypes={connectionTypes}
         refreshConnections={refreshConnections}
+        setManageConnectionModal={(modalConnection?: Connection) =>
+          setManageConnectionModal({ connection: modalConnection, isEdit: true })
+        }
       />
       {manageConnectionModal && (
         <ManageConnectionModal
@@ -87,7 +91,10 @@ const ConnectionsList: React.FC = () => {
               refreshConnections();
             }
           }}
-          onSubmit={(connection: Connection) => createSecret(connection)}
+          onSubmit={(connection: Connection) =>
+            manageConnectionModal.isEdit ? replaceSecret(connection) : createSecret(connection)
+          }
+          isEdit={manageConnectionModal.isEdit}
         />
       )}
     </DetailsSection>
diff --git a/frontend/src/pages/projects/screens/detail/connections/ConnectionsTable.tsx b/frontend/src/pages/projects/screens/detail/connections/ConnectionsTable.tsx
index 0fc56fec91..6b5fd84178 100644
--- a/frontend/src/pages/projects/screens/detail/connections/ConnectionsTable.tsx
+++ b/frontend/src/pages/projects/screens/detail/connections/ConnectionsTable.tsx
@@ -10,12 +10,14 @@ type ConnectionsTableProps = {
   connections: Connection[];
   connectionTypes?: ConnectionTypeConfigMapObj[];
   refreshConnections: () => void;
+  setManageConnectionModal: (connection: Connection) => void;
 };
 
 const ConnectionsTable: React.FC<ConnectionsTableProps> = ({
   connections,
   connectionTypes,
   refreshConnections,
+  setManageConnectionModal,
 }) => {
   const [deleteConnection, setDeleteConnection] = React.useState<Connection>();
 
@@ -30,7 +32,7 @@ const ConnectionsTable: React.FC<ConnectionsTableProps> = ({
             key={connection.metadata.name}
             obj={connection}
             connectionTypes={connectionTypes}
-            onEditConnection={() => undefined}
+            onEditConnection={() => setManageConnectionModal(connection)}
             onDeleteConnection={() => setDeleteConnection(connection)}
           />
         )}
diff --git a/frontend/src/pages/projects/screens/detail/connections/ManageConnectionsModal.tsx b/frontend/src/pages/projects/screens/detail/connections/ManageConnectionsModal.tsx
index ffb316faf1..6888a7096c 100644
--- a/frontend/src/pages/projects/screens/detail/connections/ManageConnectionsModal.tsx
+++ b/frontend/src/pages/projects/screens/detail/connections/ManageConnectionsModal.tsx
@@ -1,5 +1,5 @@
 import React from 'react';
-import { Modal } from '@patternfly/react-core';
+import { Alert, Modal } from '@patternfly/react-core';
 import DashboardModalFooter from '~/concepts/dashboard/DashboardModalFooter';
 import ConnectionTypeForm from '~/concepts/connectionTypes/ConnectionTypeForm';
 import {
@@ -11,7 +11,12 @@ import {
 } from '~/concepts/connectionTypes/types';
 import { ProjectKind, SecretKind } from '~/k8sTypes';
 import { useK8sNameDescriptionFieldData } from '~/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField';
-import { assembleConnectionSecret, getDefaultValues } from '~/concepts/connectionTypes/utils';
+import {
+  assembleConnectionSecret,
+  getDefaultValues,
+  parseConnectionSecretValues,
+} from '~/concepts/connectionTypes/utils';
+import { K8sNameDescriptionFieldData } from '~/concepts/k8s/K8sNameDescriptionField/types';
 
 type Props = {
   connection?: Connection;
@@ -19,6 +24,7 @@ type Props = {
   project: ProjectKind;
   onClose: (submitted?: boolean) => void;
   onSubmit: (connection: Connection) => Promise<SecretKind>;
+  isEdit?: boolean;
 };
 
 export const ManageConnectionModal: React.FC<Props> = ({
@@ -27,9 +33,11 @@ export const ManageConnectionModal: React.FC<Props> = ({
   project,
   onClose,
   onSubmit,
+  isEdit = false,
 }) => {
   const [error, setError] = React.useState<Error>();
   const [isSaving, setIsSaving] = React.useState(false);
+  const [isModified, setIsModified] = React.useState(false);
 
   const enabledConnectionTypes = React.useMemo(
     () =>
@@ -39,13 +47,26 @@ export const ManageConnectionModal: React.FC<Props> = ({
 
   const [selectedConnectionType, setSelectedConnectionType] = React.useState<
     ConnectionTypeConfigMapObj | undefined
-  >(enabledConnectionTypes.length === 1 ? enabledConnectionTypes[0] : undefined);
-  const { data: nameDescData, onDataChange: setNameDescData } = useK8sNameDescriptionFieldData();
+  >(() => {
+    if (isEdit && connection) {
+      return connectionTypes.find(
+        (t) =>
+          t.metadata.name === connection.metadata.annotations['opendatahub.io/connection-type'],
+      );
+    }
+    if (enabledConnectionTypes.length === 1) {
+      return enabledConnectionTypes[0];
+    }
+    return undefined;
+  });
+  const { data: nameDescData, onDataChange: setNameDescData } = useK8sNameDescriptionFieldData({
+    initialData: connection,
+  });
   const [connectionValues, setConnectionValues] = React.useState<{
     [key: string]: ConnectionTypeValueType;
   }>(() => {
     if (connection?.data) {
-      return connection.data;
+      return parseConnectionSecretValues(connection, selectedConnectionType);
     }
     if (enabledConnectionTypes.length === 1) {
       return getDefaultValues(enabledConnectionTypes[0]);
@@ -101,7 +122,7 @@ export const ManageConnectionModal: React.FC<Props> = ({
 
   return (
     <Modal
-      title="Add Connection"
+      title={isEdit ? 'Edit connection' : 'Add connection'}
       isOpen
       onClose={() => {
         onClose();
@@ -109,7 +130,7 @@ export const ManageConnectionModal: React.FC<Props> = ({
       variant="medium"
       footer={
         <DashboardModalFooter
-          submitLabel="Create"
+          submitLabel={isEdit ? 'Save' : 'Create'}
           onCancel={onClose}
           onSubmit={() => {
             setIsSaving(true);
@@ -139,25 +160,50 @@ export const ManageConnectionModal: React.FC<Props> = ({
               });
           }}
           error={error}
-          isSubmitDisabled={!isFormValid}
+          isSubmitDisabled={!isFormValid || !isModified}
           isSubmitLoading={isSaving}
           alertTitle=""
         />
       }
     >
+      {isEdit && (
+        <Alert
+          style={{ marginBottom: 32 }}
+          variant="warning"
+          isInline
+          title="Dependent resources require further action"
+        >
+          Connection changes are not applied to dependent resources until those resources are
+          restarted, redeployed, or otherwise regenerated.
+        </Alert>
+      )}
       <ConnectionTypeForm
-        connectionTypes={enabledConnectionTypes}
+        connectionTypes={isEdit ? connectionTypes : enabledConnectionTypes}
         connectionType={selectedConnectionType}
-        setConnectionType={changeSelectionType}
+        setConnectionType={(obj?: ConnectionTypeConfigMapObj) => {
+          if (!isModified) {
+            setIsModified(true);
+          }
+          changeSelectionType(obj);
+        }}
         connectionNameDesc={nameDescData}
-        setConnectionNameDesc={setNameDescData}
+        setConnectionNameDesc={(key: keyof K8sNameDescriptionFieldData, value: string) => {
+          if (!isModified) {
+            setIsModified(true);
+          }
+          setNameDescData(key, value);
+        }}
         connectionValues={connectionValues}
-        onChange={(field, value) =>
-          setConnectionValues((prev) => ({ ...prev, [field.envVar]: value }))
-        }
+        onChange={(field, value) => {
+          if (!isModified) {
+            setIsModified(true);
+          }
+          setConnectionValues((prev) => ({ ...prev, [field.envVar]: value }));
+        }}
         onValidate={(field, isValid) =>
           setValidations((prev) => ({ ...prev, [field.envVar]: isValid }))
         }
+        disableTypeSelection={isEdit || enabledConnectionTypes.length === 1}
       />
     </Modal>
   );
diff --git a/frontend/src/pages/projects/screens/detail/connections/__tests__/ConnectionsTable.spec.tsx b/frontend/src/pages/projects/screens/detail/connections/__tests__/ConnectionsTable.spec.tsx
index 7cd8b359c5..736de605af 100644
--- a/frontend/src/pages/projects/screens/detail/connections/__tests__/ConnectionsTable.spec.tsx
+++ b/frontend/src/pages/projects/screens/detail/connections/__tests__/ConnectionsTable.spec.tsx
@@ -10,6 +10,7 @@ describe('ConnectionsTable', () => {
       <ConnectionsTable
         connections={[mockConnection({ displayName: 'connection1', description: 'desc1' })]}
         refreshConnections={() => undefined}
+        setManageConnectionModal={() => undefined}
       />,
     );
 
@@ -27,6 +28,7 @@ describe('ConnectionsTable', () => {
           mockConnectionTypeConfigMapObj({ name: 's3', displayName: 'S3 Buckets' }),
         ]}
         refreshConnections={() => undefined}
+        setManageConnectionModal={() => undefined}
       />,
     );
 
diff --git a/frontend/src/pages/projects/screens/detail/connections/__tests__/ManageConnectionsModal.spec.tsx b/frontend/src/pages/projects/screens/detail/connections/__tests__/ManageConnectionsModal.spec.tsx
index f9be0ef09b..3c02ac2e4f 100644
--- a/frontend/src/pages/projects/screens/detail/connections/__tests__/ManageConnectionsModal.spec.tsx
+++ b/frontend/src/pages/projects/screens/detail/connections/__tests__/ManageConnectionsModal.spec.tsx
@@ -4,6 +4,7 @@ import { fireEvent, render, screen } from '@testing-library/react';
 import { ManageConnectionModal } from '~/pages/projects/screens/detail/connections/ManageConnectionsModal';
 import { mockConnectionTypeConfigMapObj } from '~/__mocks__/mockConnectionType';
 import { mockProjectK8sResource } from '~/__mocks__';
+import { mockConnection } from '~/__mocks__/mockConnection';
 
 describe('Add connection modal', () => {
   const onCloseMock = jest.fn();
@@ -31,7 +32,7 @@ describe('Add connection modal', () => {
       />,
     );
 
-    expect(screen.getByRole('dialog', { name: 'Add Connection' })).toBeTruthy();
+    expect(screen.getByRole('dialog', { name: 'Add connection' })).toBeTruthy();
     expect(screen.getByRole('combobox')).toHaveValue('the only type');
     expect(screen.getByRole('textbox', { name: 'Connection name' })).toBeVisible();
     expect(screen.getByRole('textbox', { name: 'Connection description' })).toBeVisible();
@@ -67,6 +68,18 @@ describe('Add connection modal', () => {
               },
             ],
           }),
+          mockConnectionTypeConfigMapObj({
+            name: 'type three disabled',
+            enabled: false,
+            fields: [
+              {
+                type: 'short-text',
+                name: 'Short text 2',
+                envVar: 'env2',
+                properties: {},
+              },
+            ],
+          }),
         ]}
       />,
     );
@@ -74,11 +87,12 @@ describe('Add connection modal', () => {
     await act(async () => {
       screen.getByRole('button', { name: 'Typeahead menu toggle' }).click();
     });
-    expect(screen.getByRole('option', { name: 'type one' })).toBeTruthy();
-    expect(screen.getByRole('option', { name: 'type two' })).toBeTruthy();
+    expect(screen.getByRole('option', { name: /type one/ })).toBeTruthy();
+    expect(screen.getByRole('option', { name: /type two/ })).toBeTruthy();
+    expect(screen.queryByRole('option', { name: /type three disabled/ })).toBeFalsy();
 
     await act(async () => {
-      screen.getByRole('option', { name: 'type one' }).click();
+      screen.getByRole('option', { name: /type one/ }).click();
     });
     expect(screen.getByRole('combobox')).toHaveValue('type one');
     expect(screen.getByRole('textbox', { name: 'Connection name' })).toBeVisible();
@@ -280,7 +294,7 @@ describe('Add connection modal', () => {
       screen.getByRole('button', { name: 'Typeahead menu toggle' }).click();
     });
     await act(async () => {
-      screen.getByRole('option', { name: 'type one' }).click();
+      screen.getByRole('option', { name: /type one/ }).click();
     });
     await act(async () => {
       fireEvent.change(screen.getByRole('textbox', { name: 'Connection name' }), {
@@ -305,7 +319,7 @@ describe('Add connection modal', () => {
       screen.getByRole('button', { name: 'Typeahead menu toggle' }).click();
     });
     await act(async () => {
-      screen.getByRole('option', { name: 'type two' }).click();
+      screen.getByRole('option', { name: /type two/ }).click();
     });
     expect(screen.getByRole('textbox', { name: 'Connection name' })).toHaveValue(
       'connection one name',
@@ -319,7 +333,7 @@ describe('Add connection modal', () => {
       screen.getByRole('button', { name: 'Typeahead menu toggle' }).click();
     });
     await act(async () => {
-      screen.getByRole('option', { name: 'type one' }).click();
+      screen.getByRole('option', { name: /type one/ }).click();
     });
     expect(screen.getByRole('textbox', { name: 'Connection name' })).toHaveValue(
       'connection one name',
@@ -330,3 +344,181 @@ describe('Add connection modal', () => {
     expect(screen.getByRole('textbox', { name: 'Short text 1' })).toHaveValue('one field');
   });
 });
+
+describe('Edit connection modal', () => {
+  const onCloseMock = jest.fn();
+  const onSubmitMock = jest.fn().mockResolvedValue(() => undefined);
+
+  it('should load existing connection', async () => {
+    render(
+      <ManageConnectionModal
+        isEdit
+        project={mockProjectK8sResource({})}
+        onClose={onCloseMock}
+        onSubmit={onSubmitMock}
+        connection={mockConnection({
+          name: 's3-connection',
+          description: 's3 desc',
+          connectionType: 's3',
+          data: {
+            env1: window.btoa('saved data'),
+            env2: window.btoa('3'),
+            env3: window.btoa('true'),
+            env4: window.btoa('a'),
+            env5: window.btoa('["a","b"]'),
+          },
+        })}
+        connectionTypes={[
+          mockConnectionTypeConfigMapObj({
+            name: 's3',
+            fields: [
+              {
+                type: 'short-text',
+                name: 'short text 1',
+                envVar: 'env1',
+                properties: {},
+              },
+              {
+                type: 'numeric',
+                name: 'numeric 2',
+                envVar: 'env2',
+                properties: {},
+              },
+              {
+                type: 'boolean',
+                name: 'boolean 3',
+                envVar: 'env3',
+                properties: {},
+              },
+              {
+                type: 'dropdown',
+                name: 'dropdown 4',
+                envVar: 'env4',
+                properties: {
+                  items: [
+                    { label: 'a', value: 'a' },
+                    { label: 'b', value: 'b' },
+                  ],
+                },
+              },
+              {
+                type: 'dropdown',
+                name: 'dropdown 5 multi',
+                envVar: 'env5',
+                properties: {
+                  variant: 'multi',
+                  items: [
+                    { label: 'a', value: 'a' },
+                    { label: 'b', value: 'b' },
+                  ],
+                },
+              },
+            ],
+          }),
+          mockConnectionTypeConfigMapObj({
+            name: 'postgres',
+            fields: [
+              {
+                type: 'short-text',
+                name: 'Short text',
+                envVar: 'env1',
+                properties: {},
+              },
+            ],
+          }),
+        ]}
+      />,
+    );
+
+    expect(screen.getByRole('dialog', { name: 'Edit connection' })).toBeTruthy();
+    expect(screen.getByRole('combobox')).toHaveValue('s3');
+    expect(screen.getByRole('textbox', { name: 'Connection name' })).toHaveValue('s3-connection');
+    expect(screen.getByRole('textbox', { name: 'Connection description' })).toHaveValue('s3 desc');
+    expect(screen.getByRole('textbox', { name: 'short text 1' })).toHaveValue('saved data');
+    expect(screen.getByRole('spinbutton', { name: 'Input' })).toHaveValue(3);
+    expect(screen.getByRole('checkbox', { name: 'boolean 3' })).toBeChecked();
+    expect(screen.getByRole('button', { name: 'dropdown 4' })).toHaveTextContent('a');
+    expect(screen.getByRole('button', { name: 'dropdown 5 multi' })).toHaveTextContent(
+      'Select dropdown 5 multi 2 selected',
+    );
+    expect(screen.getByRole('button', { name: 'Save' })).toBeTruthy();
+  });
+
+  it('should list disabled connection type for existing instance', () => {
+    render(
+      <ManageConnectionModal
+        isEdit
+        project={mockProjectK8sResource({})}
+        onClose={onCloseMock}
+        onSubmit={onSubmitMock}
+        connection={mockConnection({
+          name: 's3-connection',
+          description: 's3 desc',
+          connectionType: 's3',
+          data: { env1: window.btoa('saved data') },
+        })}
+        connectionTypes={[
+          mockConnectionTypeConfigMapObj({
+            name: 's3',
+            enabled: false,
+            fields: [
+              {
+                type: 'short-text',
+                name: 'short text 1',
+                envVar: 'env1',
+                properties: {},
+              },
+            ],
+          }),
+          mockConnectionTypeConfigMapObj({
+            name: 'postgres',
+          }),
+        ]}
+      />,
+    );
+
+    expect(screen.getByRole('combobox')).toHaveValue('s3');
+    expect(screen.getByRole('textbox', { name: 'Connection name' })).toHaveValue('s3-connection');
+    expect(screen.getByRole('textbox', { name: 'Connection description' })).toHaveValue('s3 desc');
+    expect(screen.getByRole('textbox', { name: 'short text 1' })).toHaveValue('saved data');
+  });
+
+  it('should list non matching values as short text', async () => {
+    render(
+      <ManageConnectionModal
+        isEdit
+        project={mockProjectK8sResource({})}
+        onClose={onCloseMock}
+        onSubmit={onSubmitMock}
+        connection={mockConnection({
+          name: 's3-connection',
+          description: 's3 desc',
+          connectionType: 's3',
+          data: {
+            UNMATCHED_1: window.btoa('unmatched1!'),
+            env1: window.btoa('saved data'),
+            UNMATCHED_2: window.btoa('unmatched2!'),
+          },
+        })}
+        connectionTypes={[
+          mockConnectionTypeConfigMapObj({
+            name: 's3',
+            fields: [
+              {
+                type: 'short-text',
+                name: 'Short text',
+                envVar: 'env1',
+                properties: {},
+              },
+            ],
+          }),
+        ]}
+      />,
+    );
+
+    expect(screen.getByRole('combobox')).toHaveValue('s3');
+    expect(screen.getByRole('textbox', { name: 'Short text' })).toHaveValue('saved data');
+    expect(screen.getByRole('textbox', { name: 'UNMATCHED_1' })).toHaveValue('unmatched1!');
+    expect(screen.getByRole('textbox', { name: 'UNMATCHED_2' })).toHaveValue('unmatched2!');
+  });
+});
diff --git a/frontend/src/pages/projects/screens/detail/data-connections/utils.ts b/frontend/src/pages/projects/screens/detail/data-connections/utils.ts
index 0b5ed85bce..24d8d9b1b9 100644
--- a/frontend/src/pages/projects/screens/detail/data-connections/utils.ts
+++ b/frontend/src/pages/projects/screens/detail/data-connections/utils.ts
@@ -13,7 +13,8 @@ import { getDescriptionFromK8sResource, getDisplayNameFromK8sResource } from '~/
 import { DATA_CONNECTION_TYPES } from './connectionRenderers';
 
 export const isSecretAWSSecretKind = (secret: SecretKind): secret is AWSSecretKind =>
-  !!secret.metadata.labels?.[KnownLabels.DATA_CONNECTION_AWS];
+  !!secret.metadata.labels?.[KnownLabels.DATA_CONNECTION_AWS] &&
+  secret.metadata.annotations?.['opendatahub.io/connection-type'] === 's3';
 
 export const isDataConnectionAWS = (
   dataConnection: DataConnection,