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..e855a74751 100644 --- a/frontend/src/concepts/connectionTypes/ConnectionTypeForm.tsx +++ b/frontend/src/concepts/connectionTypes/ConnectionTypeForm.tsx @@ -30,6 +30,7 @@ type Props = Pick< connectionValues?: { [key: string]: ConnectionTypeValueType; }; + disableTypeSelection?: boolean; }; const ConnectionTypeForm: React.FC = ({ @@ -42,6 +43,7 @@ const ConnectionTypeForm: React.FC = ({ connectionValues, onChange, onValidate, + disableTypeSelection, }) => { const options: TypeaheadSelectOption[] = React.useMemo(() => { if (isPreview && connectionType?.metadata.annotations?.['openshift.io/display-name']) { @@ -73,7 +75,7 @@ const ConnectionTypeForm: React.FC = ({ 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' @@ -90,7 +92,7 @@ const ConnectionTypeForm: React.FC = ({ )} {(isPreview || connectionType?.metadata.name) && ( - + = ({ k8sName: { value: '', state: { - immutable: true, + immutable: false, invalidCharacters: false, invalidLength: false, maxLength: 0, @@ -111,7 +113,7 @@ const ConnectionTypeForm: React.FC = ({ }, } } - onDataChange={setConnectionNameDesc} + onDataChange={setConnectionNameDesc ?? (() => undefined)} // onDataChange needs to be truthy to show resource name /> = ({ [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) => ( @@ -74,6 +91,7 @@ const ConnectionTypeFormFields: React.FC = ({ {renderDataFields(fieldGroup.fields)} ), )} + {unmatchedValues.length > 0 && renderDataFields(unmatchedValues)} ); }; diff --git a/frontend/src/concepts/connectionTypes/utils.ts b/frontend/src/concepts/connectionTypes/utils.ts index db76161530..05c9e1e01c 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,36 @@ 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 { + response[key] = JSON.parse(decodedString); + } catch { + response[key] = decodedString; + } + } else { + response[key] = decodedString; + } + } + + return response; +}; 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 && ( { refreshConnections(); } }} - onSubmit={(connection: Connection) => createSecret(connection)} + onSubmit={(connection: Connection) => + manageConnectionModal.isEdit ? replaceSecret(connection) : createSecret(connection) + } + isEdit={manageConnectionModal.isEdit} /> )} 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 = ({ connections, connectionTypes, refreshConnections, + setManageConnectionModal, }) => { const [deleteConnection, setDeleteConnection] = React.useState(); @@ -30,7 +32,7 @@ const ConnectionsTable: React.FC = ({ 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..88fbc0012f 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; + isEdit?: boolean; }; export const ManageConnectionModal: React.FC = ({ @@ -27,9 +33,11 @@ export const ManageConnectionModal: React.FC = ({ project, onClose, onSubmit, + isEdit = false, }) => { const [error, setError] = React.useState(); 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 = ({ 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 = ({ return ( { onClose(); @@ -109,7 +130,7 @@ export const ManageConnectionModal: React.FC = ({ variant="medium" footer={ { setIsSaving(true); @@ -139,25 +160,50 @@ export const ManageConnectionModal: React.FC = ({ }); }} error={error} - isSubmitDisabled={!isFormValid} + isSubmitDisabled={!isFormValid || !isModified} isSubmitLoading={isSaving} alertTitle="" /> } > + {isEdit && ( + + Connection changes are not applied to dependent resources until those resources are + restarted, redeployed, or otherwise regenerated. + + )} { + 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} /> ); 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', () => { 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..749774f4b6 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(); @@ -330,3 +331,142 @@ 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( + , + ); + + 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 non matching values as short text', async () => { + render( + , + ); + + 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!'); + }); +});