From 4ad8a3d50bf43260bc81ea375f8863722cbddd49 Mon Sep 17 00:00:00 2001 From: emilys314 Date: Wed, 9 Oct 2024 10:18:16 -0400 Subject: [PATCH 1/4] notebook connections --- frontend/src/components/MultiSelection.tsx | 24 +- .../table/TableRowTitleDescription.tsx | 3 + .../projects/notebook/useNotebooksStates.ts | 16 +- .../detail/connections/ConnectionsTable.tsx | 16 +- .../connections/ConnectionsTableRow.tsx | 73 +++--- .../screens/spawner/SpawnerFooter.tsx | 24 +- .../projects/screens/spawner/SpawnerPage.tsx | 49 +++- .../connections/ConnectionsFormSection.tsx | 237 ++++++++++++++++++ .../connections/DetachConnectionModal.tsx | 55 ++++ .../connections/DuplicateEnvVarsWarning.tsx | 70 ++++++ .../connections/SelectConnectionsModal.tsx | 114 +++++++++ .../__tests__/ConnectionsFormSection.spec.tsx | 135 ++++++++++ .../screens/spawner/connections/utils.ts | 43 ++++ .../pages/projects/screens/spawner/const.ts | 1 + .../useNotebookEnvVariables.tsx | 9 +- .../pages/projects/screens/spawner/types.ts | 1 + 16 files changed, 800 insertions(+), 70 deletions(-) create mode 100644 frontend/src/pages/projects/screens/spawner/connections/ConnectionsFormSection.tsx create mode 100644 frontend/src/pages/projects/screens/spawner/connections/DetachConnectionModal.tsx create mode 100644 frontend/src/pages/projects/screens/spawner/connections/DuplicateEnvVarsWarning.tsx create mode 100644 frontend/src/pages/projects/screens/spawner/connections/SelectConnectionsModal.tsx create mode 100644 frontend/src/pages/projects/screens/spawner/connections/__tests__/ConnectionsFormSection.spec.tsx create mode 100644 frontend/src/pages/projects/screens/spawner/connections/utils.ts diff --git a/frontend/src/components/MultiSelection.tsx b/frontend/src/components/MultiSelection.tsx index 1ab72eeadb..160d6c8159 100644 --- a/frontend/src/components/MultiSelection.tsx +++ b/frontend/src/components/MultiSelection.tsx @@ -15,10 +15,11 @@ import { HelperTextItem, SelectGroup, Divider, + SelectOptionProps, } from '@patternfly/react-core'; import { TimesIcon } from '@patternfly/react-icons/dist/esm/icons/times-icon'; -export type SelectionOptions = { +export type SelectionOptions = Omit & { id: number | string; name: string; selected?: boolean; @@ -49,9 +50,12 @@ type MultiSelectionProps = { isCreateOptionOnTop?: boolean; /** Message to display to create a new option */ createOptionMessage?: string | ((newValue: string) => string); + filterFunction?: (filterText: string, options: SelectionOptions[]) => SelectionOptions[]; }; const defaultCreateOptionMessage = (newValue: string) => `Create "${newValue}"`; +const defaultFilterFunction = (filterText: string, options: SelectionOptions[]) => + options.filter((o) => !filterText || o.name.toLowerCase().includes(filterText.toLowerCase())); export const MultiSelection: React.FC = ({ value = [], @@ -69,6 +73,7 @@ export const MultiSelection: React.FC = ({ isCreatable = false, isCreateOptionOnTop = false, createOptionMessage = defaultCreateOptionMessage, + filterFunction = defaultFilterFunction, }) => { const [isOpen, setIsOpen] = React.useState(false); const [inputValue, setInputValue] = React.useState(''); @@ -80,16 +85,14 @@ export const MultiSelection: React.FC = ({ let counter = 0; return groupedValues .map((g) => { - const values = g.values.filter( - (v) => !inputValue || v.name.toLowerCase().includes(inputValue.toLowerCase()), - ); + const values = filterFunction(inputValue, g.values); return { ...g, values: values.map((v) => ({ ...v, index: counter++ })), }; }) .filter((g) => g.values.length); - }, [inputValue, groupedValues]); + }, [filterFunction, groupedValues, inputValue]); const setOpen = (open: boolean) => { setIsOpen(open); @@ -104,10 +107,11 @@ export const MultiSelection: React.FC = ({ const selectOptions = React.useMemo( () => - value - .filter((v) => !inputValue || v.name.toLowerCase().includes(inputValue.toLowerCase())) - .map((v, index) => ({ ...v, index: groupOptions.length + index })), - [groupOptions, inputValue, value], + filterFunction(inputValue, value).map((v, index) => ({ + ...v, + index: groupOptions.length + index, + })), + [filterFunction, groupOptions, inputValue, value], ); const allValues = React.useMemo(() => { @@ -340,6 +344,7 @@ export const MultiSelection: React.FC = ({ value={option.id} ref={null} isSelected={option.selected} + description={option.description} > {option.name} @@ -363,6 +368,7 @@ export const MultiSelection: React.FC = ({ value={option.id} ref={null} isSelected={option.selected} + description={option.description} > {option.name} diff --git a/frontend/src/components/table/TableRowTitleDescription.tsx b/frontend/src/components/table/TableRowTitleDescription.tsx index a80076bef1..75fdf11ba3 100644 --- a/frontend/src/components/table/TableRowTitleDescription.tsx +++ b/frontend/src/components/table/TableRowTitleDescription.tsx @@ -7,6 +7,7 @@ import TruncatedText from '~/components/TruncatedText'; type TableRowTitleDescriptionProps = { title: React.ReactNode; boldTitle?: boolean; + titleIcon?: React.ReactNode; resource?: K8sResourceCommon; subtitle?: React.ReactNode; description?: React.ReactNode; @@ -19,6 +20,7 @@ type TableRowTitleDescriptionProps = { const TableRowTitleDescription: React.FC = ({ title, boldTitle = true, + titleIcon, description, resource, subtitle, @@ -56,6 +58,7 @@ const TableRowTitleDescription: React.FC = ({ ) : ( title )} + {titleIcon} {subtitle} {descriptionNode} diff --git a/frontend/src/pages/projects/notebook/useNotebooksStates.ts b/frontend/src/pages/projects/notebook/useNotebooksStates.ts index 00ab80044c..d8d5ad998a 100644 --- a/frontend/src/pages/projects/notebook/useNotebooksStates.ts +++ b/frontend/src/pages/projects/notebook/useNotebooksStates.ts @@ -1,5 +1,5 @@ import * as React from 'react'; -import useFetchState, { FetchState } from '~/utilities/useFetchState'; +import useFetchState, { FetchState, NotReadyError } from '~/utilities/useFetchState'; import { NotebookKind } from '~/k8sTypes'; import { POLL_INTERVAL } from '~/utilities/const'; import { getNotebooksStates } from '~/pages/projects/notebook/useProjectNotebookStates'; @@ -8,11 +8,17 @@ import { NotebookState } from './types'; export const useNotebooksStates = ( notebooks: NotebookKind[], namespace: string, + checkStatus = true, ): FetchState => { - const fetchNotebooksStatus = React.useCallback( - () => getNotebooksStates(notebooks, namespace), - [namespace, notebooks], - ); + const fetchNotebooksStatus = React.useCallback(() => { + if (!namespace) { + return Promise.reject(new NotReadyError('No namespace')); + } + if (!checkStatus) { + return Promise.reject(new NotReadyError('Not running')); + } + return getNotebooksStates(notebooks, namespace); + }, [namespace, notebooks, checkStatus]); return useFetchState(fetchNotebooksStatus, [], { refreshRate: POLL_INTERVAL, diff --git a/frontend/src/pages/projects/screens/detail/connections/ConnectionsTable.tsx b/frontend/src/pages/projects/screens/detail/connections/ConnectionsTable.tsx index c669b591b2..87516c77a3 100644 --- a/frontend/src/pages/projects/screens/detail/connections/ConnectionsTable.tsx +++ b/frontend/src/pages/projects/screens/detail/connections/ConnectionsTable.tsx @@ -34,8 +34,20 @@ const ConnectionsTable: React.FC = ({ key={connection.metadata.name} obj={connection} connectionTypes={connectionTypes} - onEditConnection={() => setManageConnectionModal(connection)} - onDeleteConnection={() => setDeleteConnection(connection)} + kebabActions={[ + { + title: 'Edit', + onClick: () => { + setManageConnectionModal(connection); + }, + }, + { + title: 'Delete', + onClick: () => { + setDeleteConnection(connection); + }, + }, + ]} /> )} isStriped diff --git a/frontend/src/pages/projects/screens/detail/connections/ConnectionsTableRow.tsx b/frontend/src/pages/projects/screens/detail/connections/ConnectionsTableRow.tsx index 95d5d9749d..477e8b12d3 100644 --- a/frontend/src/pages/projects/screens/detail/connections/ConnectionsTableRow.tsx +++ b/frontend/src/pages/projects/screens/detail/connections/ConnectionsTableRow.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; -import { ActionsColumn, Td, Tr } from '@patternfly/react-table'; -import { LabelGroup, Truncate } from '@patternfly/react-core'; +import { ActionsColumn, IAction, Td, Tr } from '@patternfly/react-table'; +import { Icon, LabelGroup, Truncate } from '@patternfly/react-core'; +import { ExclamationTriangleIcon } from '@patternfly/react-icons'; import { Connection, ConnectionTypeConfigMapObj } from '~/concepts/connectionTypes/types'; import { TableRowTitleDescription } from '~/components/table'; import { getDescriptionFromK8sResource, getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; @@ -11,15 +12,19 @@ import ConnectedResources from '~/pages/projects/screens/detail/connections/Conn type ConnectionsTableRowProps = { obj: Connection; connectionTypes?: ConnectionTypeConfigMapObj[]; - onEditConnection: (pvc: Connection) => void; - onDeleteConnection: (dataConnection: Connection) => void; + kebabActions: IAction[]; + showCompatibilityCell?: boolean; + showConnectedResourcesCell?: boolean; + showWarningIcon?: boolean; }; const ConnectionsTableRow: React.FC = ({ obj, connectionTypes, - onEditConnection, - onDeleteConnection, + kebabActions, + showCompatibilityCell = true, + showConnectedResourcesCell = true, + showWarningIcon = false, }) => { const connectionTypeDisplayName = React.useMemo(() => { const matchingType = connectionTypes?.find( @@ -45,6 +50,13 @@ const ConnectionsTableRow: React.FC = ({ } boldTitle={false} + titleIcon={ + showWarningIcon ? ( + + + + ) : undefined + } resource={obj} description={getDescriptionFromK8sResource(obj)} truncateDescriptionLines={2} @@ -52,37 +64,26 @@ const ConnectionsTableRow: React.FC = ({ /> {connectionTypeDisplayName} - - {compatibleTypes.length ? ( - - {compatibleTypes.map((compatibleType) => ( - - ))} - - ) : ( - '-' - )} - - - - + {showCompatibilityCell && ( + + {compatibleTypes.length ? ( + + {compatibleTypes.map((compatibleType) => ( + + ))} + + ) : ( + '-' + )} + + )} + {showConnectedResourcesCell && ( + + + + )} - { - onEditConnection(obj); - }, - }, - { - title: 'Delete', - onClick: () => { - onDeleteConnection(obj); - }, - }, - ]} - /> + ); diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx index c693e944d1..bba20feb87 100644 --- a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx +++ b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx @@ -20,6 +20,7 @@ import { useUser } from '~/redux/selectors'; import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext'; import { useAppContext } from '~/app/AppContext'; import { ProjectSectionID } from '~/pages/projects/screens/detail/types'; +import { Connection } from '~/concepts/connectionTypes/types'; import { fireFormTrackingEvent } from '~/concepts/analyticsTracking/segmentIOUtils'; import { FormTrackingEventProperties, @@ -33,12 +34,15 @@ import { } from './service'; import { checkRequiredFieldsForNotebookStart } from './spawnerUtils'; import { getNotebookDataConnection } from './dataConnection/useNotebookDataConnection'; +import { setConnectionsOnEnvFrom } from './connections/utils'; type SpawnerFooterProps = { startNotebookData: StartNotebookData; storageData: StorageData; envVariables: EnvVariable[]; dataConnection: DataConnectionData; + isConnectionTypesEnabled: boolean; + connections: Connection[]; canEnablePipelines: boolean; }; @@ -47,6 +51,8 @@ const SpawnerFooter: React.FC = ({ storageData, envVariables, dataConnection, + isConnectionTypesEnabled, + connections, canEnablePipelines, }) => { const [error, setError] = React.useState(); @@ -60,6 +66,7 @@ const SpawnerFooter: React.FC = ({ const { notebooks: { data }, dataConnections: { data: existingDataConnections }, + connections: { data: projectConnections }, refreshAllProjectData, } = React.useContext(ProjectDetailsContext); const { notebookName } = useParams(); @@ -141,14 +148,22 @@ const SpawnerFooter: React.FC = ({ dryRun, ); - const envFrom = await updateConfigMapsAndSecretsForNotebook( + let envFrom = await updateConfigMapsAndSecretsForNotebook( projectName, editNotebook, envVariables, dataConnection, existingNotebookDataConnection, dryRun, - ); + ).catch(handleError); + + if (isConnectionTypesEnabled && envFrom) { + envFrom = setConnectionsOnEnvFrom(connections, envFrom, projectConnections); + } + + if (!pvcDetails || !envFrom) { + return; + } const annotations = { ...editNotebook.metadata.annotations }; if (envFrom.length > 0) { @@ -194,13 +209,16 @@ const SpawnerFooter: React.FC = ({ ? [dataConnection.existing] : []; const pvcDetails = await createPvcDataForNotebook(projectName, storageData, dryRun); - const envFrom = await createConfigMapsAndSecretsForNotebook( + let envFrom = await createConfigMapsAndSecretsForNotebook( projectName, [...envVariables, ...newDataConnection], dryRun, ); const { volumes, volumeMounts } = pvcDetails; + if (isConnectionTypesEnabled) { + envFrom = setConnectionsOnEnvFrom(connections, envFrom, projectConnections); + } const newStartData: StartNotebookData = { ...startNotebookData, volumes, diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx index 9fc14141c0..f78c79282a 100644 --- a/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx +++ b/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx @@ -31,6 +31,8 @@ import K8sNameDescriptionField, { useK8sNameDescriptionFieldData, } from '~/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField'; import { LimitNameResourceType } from '~/concepts/k8s/K8sNameDescriptionField/utils'; +import useConnectionTypesEnabled from '~/concepts/connectionTypes/useConnectionTypesEnabled'; +import { Connection } from '~/concepts/connectionTypes/types'; import { SpawnerPageSectionID } from './types'; import { ScrollableSelectorID, SpawnerPageSectionTitles } from './const'; import SpawnerFooter from './SpawnerFooter'; @@ -46,13 +48,19 @@ import { useNotebookDataConnection } from './dataConnection/useNotebookDataConne import { useNotebookSizeState } from './useNotebookSizeState'; import useDefaultStorageClass from './storage/useDefaultStorageClass'; import usePreferredStorageClass from './storage/usePreferredStorageClass'; +import { ConnectionsFormSection } from './connections/ConnectionsFormSection'; +import { getConnectionsFromNotebook } from './connections/utils'; type SpawnerPageProps = { existingNotebook?: NotebookKind; }; const SpawnerPage: React.FC = ({ existingNotebook }) => { - const { currentProject, dataConnections } = React.useContext(ProjectDetailsContext); + const { + currentProject, + dataConnections, + connections: { data: projectConnections, refresh: refreshProjectConnections }, + } = React.useContext(ProjectDetailsContext); const displayName = getDisplayNameFromK8sResource(currentProject); const k8sNameDescriptionData = useK8sNameDescriptionFieldData({ @@ -88,6 +96,13 @@ const SpawnerPage: React.FC = ({ existingNotebook }) => { existingNotebook, ); + const isConnectionTypesEnabled = useConnectionTypesEnabled(); + const [notebookConnections, setNotebookConnections] = React.useState( + isConnectionTypesEnabled && existingNotebook + ? getConnectionsFromNotebook(existingNotebook, projectConnections) + : [], + ); + const [selectedAcceleratorProfile, setSelectedAcceleratorProfile] = useGenericObjectState({ profile: undefined, @@ -218,16 +233,28 @@ const SpawnerPage: React.FC = ({ existingNotebook }) => { /> - - - + ) : ( + + + + )} @@ -257,6 +284,8 @@ const SpawnerPage: React.FC = ({ existingNotebook }) => { storageData={storageData} envVariables={envVariables} dataConnection={dataConnectionData} + isConnectionTypesEnabled={isConnectionTypesEnabled} + connections={notebookConnections} canEnablePipelines={canEnablePipelines} /> )} diff --git a/frontend/src/pages/projects/screens/spawner/connections/ConnectionsFormSection.tsx b/frontend/src/pages/projects/screens/spawner/connections/ConnectionsFormSection.tsx new file mode 100644 index 0000000000..d9170b2737 --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/connections/ConnectionsFormSection.tsx @@ -0,0 +1,237 @@ +import React from 'react'; +import { + Bullseye, + Button, + EmptyState, + EmptyStateBody, + EmptyStateHeader, + EmptyStateIcon, + FormSection, + Tooltip, +} from '@patternfly/react-core'; +import { PlusCircleIcon } from '@patternfly/react-icons'; +import { SortableData, Table } from '~/components/table'; +import { createSecret, replaceSecret } from '~/api'; +import { NotebookKind, ProjectKind } from '~/k8sTypes'; +import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; +import { Connection } from '~/concepts/connectionTypes/types'; +import { useWatchConnectionTypes } from '~/utilities/useWatchConnectionTypes'; +import { useNotebooksStates } from '~/pages/projects/notebook/useNotebooksStates'; +import { SpawnerPageSectionTitles } from '~/pages/projects/screens/spawner/const'; +import { SpawnerPageSectionID } from '~/pages/projects/screens/spawner/types'; +import { ManageConnectionModal } from '~/pages/projects/screens/detail/connections/ManageConnectionsModal'; +import ConnectionsTableRow from '~/pages/projects/screens/detail/connections/ConnectionsTableRow'; +import { SelectConnectionsModal } from './SelectConnectionsModal'; +import { connectionEnvVarConflicts, DuplicateEnvVarWarning } from './DuplicateEnvVarsWarning'; +import { DetachConnectionModal } from './DetachConnectionModal'; + +const columns: SortableData[] = [ + { + field: 'name', + label: 'Name', + sortable: (a, b) => + getDisplayNameFromK8sResource(a).localeCompare(getDisplayNameFromK8sResource(b)), + }, + { + field: 'type', + label: 'Type', + sortable: (a, b) => + a.metadata.annotations['opendatahub.io/connection-type'].localeCompare( + b.metadata.annotations['opendatahub.io/connection-type'], + ), + }, + { + field: 'kebab', + label: '', + sortable: false, + }, +]; + +type Props = { + project: ProjectKind; + projectConnections: Connection[]; + refreshProjectConnections: () => void; + notebook?: NotebookKind; + notebookDisplayName: string; + selectedConnections: Connection[]; + setSelectedConnections: (connections: Connection[]) => void; +}; + +export const ConnectionsFormSection: React.FC = ({ + project, + projectConnections, + refreshProjectConnections, + notebook, + notebookDisplayName, + selectedConnections, + setSelectedConnections, +}) => { + const [connectionTypes] = useWatchConnectionTypes(); + + const [initialNumberConnections] = React.useState(selectedConnections.length); + const notebookArray = React.useMemo(() => (notebook ? [notebook] : []), [notebook]); + const [notebookStates] = useNotebooksStates( + notebookArray, + notebook?.metadata.namespace || '', + initialNumberConnections > 0, + ); + const isRunning = React.useMemo( + () => + !!notebookStates.find((n) => n.notebook.metadata.name === notebook?.metadata.name)?.isRunning, + [notebookStates, notebook], + ); + + const unselectedConnections = React.useMemo( + () => + projectConnections.filter( + (pc) => !selectedConnections.find((sc) => pc.metadata.name === sc.metadata.name), + ), + [projectConnections, selectedConnections], + ); + + const [showAttachConnectionsModal, setShowAttachConnectionsModal] = React.useState(false); + const [detachConnectionModal, setDetachConnectionModal] = React.useState(); + const [manageConnectionModal, setManageConnectionModal] = React.useState<{ + connection?: Connection; + isEdit?: boolean; + }>(); + + const envVarConflicts = React.useMemo( + () => connectionEnvVarConflicts(selectedConnections), + [selectedConnections], + ); + + return ( + + {SpawnerPageSectionTitles[SpawnerPageSectionID.CONNECTIONS]}{' '} + + + {' '} + + + } + id={SpawnerPageSectionID.CONNECTIONS} + aria-label={SpawnerPageSectionTitles[SpawnerPageSectionID.CONNECTIONS]} + > + {envVarConflicts.length > 0 && } + {selectedConnections.length > 0 ? ( + ( + { + setManageConnectionModal({ connection, isEdit: true }); + }, + }, + { + title: 'Detach', + onClick: () => { + setDetachConnectionModal(connection); + }, + }, + ]} + showCompatibilityCell={false} + showConnectedResourcesCell={false} + showWarningIcon={ + !!envVarConflicts.find( + (conflict) => + conflict.firstConnection === getDisplayNameFromK8sResource(connection) || + conflict.secondConnection === getDisplayNameFromK8sResource(connection), + ) + } + /> + )} + isStriped + /> + ) : ( + + + } + titleText="No connections" + headingLevel="h2" + /> + + Connections enable you to store and retrieve information that typically should not be + stored in code. For example, you can store details (including credentials) for object + storage, databases, and more. You can then attach the connections to artifacts in your + project, such as workbenches and model servers. + + + + )} + {showAttachConnectionsModal && ( + { + setSelectedConnections([...selectedConnections, ...connections]); + setShowAttachConnectionsModal(false); + }} + onClose={() => setShowAttachConnectionsModal(false)} + /> + )} + {detachConnectionModal && ( + { + setSelectedConnections( + selectedConnections.filter( + (c) => c.metadata.name !== detachConnectionModal.metadata.name, + ), + ); + setDetachConnectionModal(undefined); + }} + onClose={() => setDetachConnectionModal(undefined)} + /> + )} + {manageConnectionModal && ( + { + setManageConnectionModal(undefined); + if (refresh) { + refreshProjectConnections(); + } + }} + onSubmit={(connection: Connection) => { + if (manageConnectionModal.isEdit) { + return replaceSecret(connection); + } + setSelectedConnections([...selectedConnections, connection]); + return createSecret(connection); + }} + isEdit={manageConnectionModal.isEdit} + /> + )} + + ); +}; diff --git a/frontend/src/pages/projects/screens/spawner/connections/DetachConnectionModal.tsx b/frontend/src/pages/projects/screens/spawner/connections/DetachConnectionModal.tsx new file mode 100644 index 0000000000..36270d015f --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/connections/DetachConnectionModal.tsx @@ -0,0 +1,55 @@ +import React from 'react'; +import { Button, Modal } from '@patternfly/react-core'; +import { Connection } from '~/concepts/connectionTypes/types'; +import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; + +type Props = { + connection: Connection; + isRunning?: boolean; + notebookDisplayName: string; + onDetach: () => void; + onClose: () => void; +}; + +export const DetachConnectionModal: React.FC = ({ + connection, + isRunning, + notebookDisplayName, + onDetach, + onClose, +}) => ( + { + onDetach(); + }} + > + Detach + , + , + ]} + > + {isRunning ? ( + <> + The {getDisplayNameFromK8sResource(connection)} connection will be detached from the + workbench. To avoid losing your work, save any recent data in the current workbench,{' '} + {notebookDisplayName}. + + ) : ( + <> + The {getDisplayNameFromK8sResource(connection)} connection will be detached from the{' '} + {notebookDisplayName} workbench. + + )} + +); diff --git a/frontend/src/pages/projects/screens/spawner/connections/DuplicateEnvVarsWarning.tsx b/frontend/src/pages/projects/screens/spawner/connections/DuplicateEnvVarsWarning.tsx new file mode 100644 index 0000000000..c6fd78776b --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/connections/DuplicateEnvVarsWarning.tsx @@ -0,0 +1,70 @@ +import React from 'react'; +import { Alert, ExpandableSection, List, ListItem } from '@patternfly/react-core'; +import { Connection } from '~/concepts/connectionTypes/types'; +import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; + +type Conflict = { + firstConnection: string; + secondConnection: string; + vars: string[]; +}; + +export const connectionEnvVarConflicts = (connections: Connection[]): Conflict[] => { + const conflicts: Conflict[] = []; + for (const first of connections) { + for (const second of connections) { + if (first.metadata.name === second.metadata.name) { + break; + } + if (!first.data || !second.data) { + continue; + } + const envVars = [...Object.keys(first.data), ...Object.keys(second.data)]; + const duplicates = envVars.filter((e, i) => envVars.indexOf(e) !== i); + if (duplicates.length > 0) { + conflicts.push({ + firstConnection: getDisplayNameFromK8sResource(first), + secondConnection: getDisplayNameFromK8sResource(second), + vars: duplicates, + }); + } + } + } + return conflicts; +}; + +type Props = { + envVarConflicts: Conflict[]; +}; + +export const DuplicateEnvVarWarning: React.FC = ({ envVarConflicts }) => { + const [showConflicts, setShowConflicts] = React.useState(false); + + return ( + + Two or more connections contain conflicting environment variables. When environment variables + conflict, only one of the values is used in the workbench. + setShowConflicts((prev) => !prev)} + isExpanded={showConflicts} + isIndented + > + {envVarConflicts.map((conflict, conflictIndex) => ( +
+ {conflict.firstConnection} and {conflict.secondConnection} contain the + following conflicting variables: + + {conflict.vars.map((value, valueIndex) => ( + + {value} + + ))} + +
+
+ ))} + + + ); +}; diff --git a/frontend/src/pages/projects/screens/spawner/connections/SelectConnectionsModal.tsx b/frontend/src/pages/projects/screens/spawner/connections/SelectConnectionsModal.tsx new file mode 100644 index 0000000000..38222e38df --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/connections/SelectConnectionsModal.tsx @@ -0,0 +1,114 @@ +import React from 'react'; +import { Button, Flex, FlexItem, Form, FormGroup, Modal, Truncate } from '@patternfly/react-core'; +import { MultiSelection, SelectionOptions } from '~/components/MultiSelection'; +import { Connection, ConnectionTypeConfigMapObj } from '~/concepts/connectionTypes/types'; +import { getDescriptionFromK8sResource, getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; +import { connectionEnvVarConflicts, DuplicateEnvVarWarning } from './DuplicateEnvVarsWarning'; + +type Props = { + connectionTypes: ConnectionTypeConfigMapObj[]; + connectionsToList: Connection[]; + onSave: (connections: Connection[]) => void; + onClose: () => void; +}; + +export const SelectConnectionsModal: React.FC = ({ + connectionTypes, + connectionsToList, + onSave, + onClose, +}) => { + const [selectionOptions, setSelectionOptions] = React.useState(() => + connectionsToList.map((c) => { + const category = connectionTypes + .find( + (type) => c.metadata.annotations['opendatahub.io/connection-type'] === type.metadata.name, + ) + ?.data?.category?.join(' '); + + return { + id: c.metadata.name, + name: getDisplayNameFromK8sResource(c), + selected: connectionsToList.length === 1 || false, + description: ( + + {getDescriptionFromK8sResource(c) && ( + + + + )} + {category && ( + + + + )} + + ), + data: `${getDescriptionFromK8sResource(c)}`, + }; + }), + ); + + const selectedConnections = React.useMemo(() => { + const connectionsFromSelections: Connection[] = []; + for (const s of selectionOptions) { + const selected = connectionsToList.find( + (c) => c.metadata.name === s.id && s.selected === true, + ); + if (selected) { + connectionsFromSelections.push(selected); + } + } + return connectionsFromSelections; + }, [connectionsToList, selectionOptions]); + + const envVarConflicts = React.useMemo( + () => connectionEnvVarConflicts(selectedConnections), + [selectedConnections], + ); + + return ( + selection.selected === false)} + onClick={() => { + onSave(selectedConnections); + }} + > + Attach + , + , + ]} + > +
+ {envVarConflicts.length > 0 && } + + + options.filter( + (o) => + !filterText || + o.name.toLowerCase().includes(filterText.toLowerCase()) || + o.data?.toLowerCase().includes(filterText.toLowerCase()), + ) + } + /> + + + + ); +}; diff --git a/frontend/src/pages/projects/screens/spawner/connections/__tests__/ConnectionsFormSection.spec.tsx b/frontend/src/pages/projects/screens/spawner/connections/__tests__/ConnectionsFormSection.spec.tsx new file mode 100644 index 0000000000..3ecb026759 --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/connections/__tests__/ConnectionsFormSection.spec.tsx @@ -0,0 +1,135 @@ +import React, { act } from 'react'; +import '@testing-library/jest-dom'; +import { render, within } from '@testing-library/react'; +import { mockProjectK8sResource } from '~/__mocks__'; +import { ConnectionsFormSection } from '~/pages/projects/screens/spawner/connections/ConnectionsFormSection'; +import { mockConnection } from '~/__mocks__/mockConnection'; + +jest.mock('~/pages/projects/notebook/useNotebooksStates', () => ({ + useNotebooksStates: jest.fn().mockReturnValue([[]]), +})); +jest.mock('~/utilities/useWatchConnectionTypes', () => ({ + useWatchConnectionTypes: jest.fn().mockReturnValue([[]]), +})); + +describe('ConnectionsFormSection', () => { + it('should render empty section', () => { + const result = render( + undefined} + notebookDisplayName="" + selectedConnections={[]} + setSelectedConnections={() => undefined} + />, + ); + + expect(result.getByRole('button', { name: 'Attach existing connections' })).toBeTruthy(); + expect(result.getByRole('button', { name: 'Create connection' })).toBeTruthy(); + expect(result.getByRole('heading', { name: 'No connections' })).toBeTruthy(); + }); + + it('should list existing connections', () => { + const result = render( + undefined} + notebookDisplayName="" + selectedConnections={[ + mockConnection({ name: 's3-connection-1', displayName: 's3 connection 1' }), + mockConnection({ name: 's3-connection-2', displayName: 's3 connection 2' }), + ]} + setSelectedConnections={() => undefined} + />, + ); + + expect(result.getByRole('columnheader', { name: 'Name' })).toBeTruthy(); + expect(result.getByRole('columnheader', { name: 'Type' })).toBeTruthy(); + expect(result.getByRole('cell', { name: 's3 connection 1' })).toBeTruthy(); + expect(result.getByRole('cell', { name: 's3 connection 2' })).toBeTruthy(); + expect(result.getAllByRole('cell', { name: 's3' }).length).toEqual(2); + }); + + it('should show env conflicts', async () => { + const result = render( + undefined} + notebookDisplayName="" + selectedConnections={[ + mockConnection({ + name: 's3-connection-1', + displayName: 's3 connection 1', + data: { ENV1: '' }, + }), + mockConnection({ + name: 's3-connection-2', + displayName: 's3 connection 2', + data: { ENV1: '' }, + }), + ]} + setSelectedConnections={() => undefined} + />, + ); + + expect( + result.getByRole('heading', { name: 'Warning alert: Connections conflict' }), + ).toBeTruthy(); + + await act(async () => result.getByRole('button', { name: 'Show conflicts' }).click()); + expect(result.getByTestId('envvar-conflict-0')).toHaveTextContent( + 's3 connection 2 and s3 connection 1 contain the following conflicting variables:ENV1', + ); + }); + + it('should attach existing connection', async () => { + const setSelectedConnectionsMock = jest.fn(); + const result = render( + undefined} + notebookDisplayName="" + selectedConnections={[ + mockConnection({ name: 's3-connection-1', displayName: 's3 connection 1' }), + ]} + setSelectedConnections={setSelectedConnectionsMock} + />, + ); + + act(() => result.getByRole('button', { name: 'Attach existing connections' }).click()); + const attachModal = result.getByRole('dialog', { name: 'Attach existing connections' }); + expect(attachModal).toBeTruthy(); + expect(within(attachModal).getByRole('button', { name: 'Attach' })).toBeDisabled(); + expect(within(attachModal).getByRole('button', { name: 'Cancel' })).toBeEnabled(); + expect(within(attachModal).getByRole('combobox', { name: 'Type to filter' })).toHaveValue(''); + + await act(async () => result.getByRole('button', { name: 'Connections' }).click()); + expect(within(attachModal).queryByRole('option', { name: 's3 connection 1' })).toBeFalsy(); // don't show attached connections + expect(within(attachModal).getByRole('option', { name: 's3 connection 2' })).toBeTruthy(); + expect(within(attachModal).getByRole('option', { name: 's3 connection 3' })).toBeTruthy(); + + await act(async () => result.getByRole('option', { name: 's3 connection 3' }).click()); + expect( + within(attachModal).getByRole('group', { name: 'Current selections' }), + ).toHaveTextContent('s3 connection 3'); + + await act(async () => result.getByRole('button', { name: 'Attach' }).click()); + expect(result.queryByRole('dialog', { name: 'Attach existing connections' })).toBeFalsy(); + expect(setSelectedConnectionsMock).toBeCalledWith([ + mockConnection({ name: 's3-connection-1', displayName: 's3 connection 1' }), + mockConnection({ name: 's3-connection-3', displayName: 's3 connection 3' }), + ]); + }); +}); diff --git a/frontend/src/pages/projects/screens/spawner/connections/utils.ts b/frontend/src/pages/projects/screens/spawner/connections/utils.ts new file mode 100644 index 0000000000..b81f21a39b --- /dev/null +++ b/frontend/src/pages/projects/screens/spawner/connections/utils.ts @@ -0,0 +1,43 @@ +import { Connection } from '~/concepts/connectionTypes/types'; +import { NotebookKind } from '~/k8sTypes'; +import { EnvironmentFromVariable } from '~/pages/projects/types'; + +export const getConnectionsFromNotebook = ( + notebook: NotebookKind, + projectConnections: Connection[], +): Connection[] => { + const connectionNames = []; + for (const env of notebook.spec.template.spec.containers[0].envFrom ?? []) { + if (env.secretRef?.name) { + connectionNames.push(env.secretRef.name); + } + } + + const attachedConnections: Connection[] = []; + for (const name of connectionNames) { + const found = projectConnections.find((c) => c.metadata.name === name); + if (found) { + attachedConnections.push(found); + } + } + return attachedConnections; +}; + +const isNameInConnections = (name: string | undefined, connections: Connection[]): boolean => + !!name && !!connections.find((c) => name === c.metadata.name); + +export const setConnectionsOnEnvFrom = ( + notebookConnections: Connection[], + envFrom: EnvironmentFromVariable[], + projectConnections: Connection[], +): EnvironmentFromVariable[] => { + const newEnvFrom = envFrom.filter( + (env) => !isNameInConnections(env.secretRef?.name, projectConnections), + ); + newEnvFrom.push( + ...notebookConnections.map((c) => ({ + secretRef: { name: c.metadata.name }, + })), + ); + return newEnvFrom; +}; diff --git a/frontend/src/pages/projects/screens/spawner/const.ts b/frontend/src/pages/projects/screens/spawner/const.ts index 9b2b1754c1..4407ba4934 100644 --- a/frontend/src/pages/projects/screens/spawner/const.ts +++ b/frontend/src/pages/projects/screens/spawner/const.ts @@ -8,6 +8,7 @@ export const SpawnerPageSectionTitles: SpawnerPageSectionTitlesType = { [SpawnerPageSectionID.ENVIRONMENT_VARIABLES]: 'Environment variables', [SpawnerPageSectionID.CLUSTER_STORAGE]: 'Cluster storage', [SpawnerPageSectionID.DATA_CONNECTIONS]: 'Data connections', + [SpawnerPageSectionID.CONNECTIONS]: 'Connections', }; export const ScrollableSelectorID = 'workbench-spawner-page'; diff --git a/frontend/src/pages/projects/screens/spawner/environmentVariables/useNotebookEnvVariables.tsx b/frontend/src/pages/projects/screens/spawner/environmentVariables/useNotebookEnvVariables.tsx index 73a3cf7fca..25ef5c66d6 100644 --- a/frontend/src/pages/projects/screens/spawner/environmentVariables/useNotebookEnvVariables.tsx +++ b/frontend/src/pages/projects/screens/spawner/environmentVariables/useNotebookEnvVariables.tsx @@ -1,13 +1,15 @@ import * as React from 'react'; -import { DATA_CONNECTION_PREFIX, getConfigMap, getSecret } from '~/api'; +import { getConfigMap, getSecret } from '~/api'; import { ConfigMapKind, NotebookKind, SecretKind } from '~/k8sTypes'; import { EnvVarResourceType } from '~/types'; +import { isConnection } from '~/concepts/connectionTypes/types'; import { ConfigMapCategory, EnvironmentVariableType, EnvVariable, SecretCategory, } from '~/pages/projects/types'; +import { isSecretKind } from './utils'; export const fetchNotebookEnvVariables = (notebook: NotebookKind): Promise => { const envFromList = notebook.spec.template.spec.containers[0].envFrom || []; @@ -40,10 +42,7 @@ export const fetchNotebookEnvVariables = (notebook: NotebookKind): Promise ({ key, value: data[key] })) : [], }, }; - } else if ( - resource.kind === EnvVarResourceType.Secret && - !resource.metadata.name.startsWith(DATA_CONNECTION_PREFIX) - ) { + } else if (isSecretKind(resource) && !isConnection(resource)) { envVar = { type: EnvironmentVariableType.SECRET, existingName: resource.metadata.name, diff --git a/frontend/src/pages/projects/screens/spawner/types.ts b/frontend/src/pages/projects/screens/spawner/types.ts index 1fc4d61f7d..b781b39c23 100644 --- a/frontend/src/pages/projects/screens/spawner/types.ts +++ b/frontend/src/pages/projects/screens/spawner/types.ts @@ -7,6 +7,7 @@ export enum SpawnerPageSectionID { ENVIRONMENT_VARIABLES = 'environment-variables', CLUSTER_STORAGE = 'cluster-storage', DATA_CONNECTIONS = 'data-connections', + CONNECTIONS = 'connections', } export type SpawnerPageSectionTitlesType = { From 9bf71af5914f71d22b6eb4b958c12bced09009bd Mon Sep 17 00:00:00 2001 From: emilys314 Date: Mon, 14 Oct 2024 10:04:22 -0400 Subject: [PATCH 2/4] Add cypress test --- .../cypress/cypress/pages/workbench.ts | 30 ++++++++ .../tests/mocked/projects/workbench.cy.ts | 70 +++++++++++++++++++ .../screens/spawner/SpawnerFooter.tsx | 14 ++-- .../connections/ConnectionsFormSection.tsx | 4 +- .../connections/SelectConnectionsModal.tsx | 1 + 5 files changed, 109 insertions(+), 10 deletions(-) diff --git a/frontend/src/__tests__/cypress/cypress/pages/workbench.ts b/frontend/src/__tests__/cypress/cypress/pages/workbench.ts index e369f0fdd6..d26a380023 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/workbench.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/workbench.ts @@ -174,6 +174,22 @@ class NotebookRow extends TableRow { } } +class AttachConnectionModal extends Modal { + constructor() { + super('Attach existing connections'); + } + + selectConnectionOption(name: string) { + this.find().findByRole('button', { name: 'Connections' }).click(); + this.find().findByRole('option', { name }).click(); + this.find().findByRole('button', { name: 'Connections' }).click(); + } + + findAttachButton() { + return this.find().findByTestId('attach-button'); + } +} + class CreateSpawnerPage { k8sNameDescription = new K8sNameDescriptionField('workbench'); @@ -309,6 +325,19 @@ class CreateSpawnerPage { findContainerSizeInput(name: string) { return cy.findByTestId('container-size-group').contains(name); } + + findAttachConnectionButton() { + return cy.findByTestId('attach-existing-connection-button'); + } + + findConnectionsTable() { + return cy.findByTestId('connections-table'); + } + + findConnectionsTableRow(name: string, type: string) { + this.findConnectionsTable().find(`[data-label=Name]`).contains(name); + this.findConnectionsTable().find(`[data-label=Type]`).contains(type); + } } class EditSpawnerPage extends CreateSpawnerPage { @@ -373,3 +402,4 @@ export const notebookConfirmModal = new NotebookConfirmModal(); export const editSpawnerPage = new EditSpawnerPage(); export const storageModal = new StorageModal(); export const notFoundSpawnerPage = new NotFoundSpawnerPage(); +export const attachConnectionModal = new AttachConnectionModal(); diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/workbench.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/workbench.cy.ts index fef7f93dff..5d81eab73e 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/workbench.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/workbench.cy.ts @@ -13,6 +13,7 @@ import { mockImageStreamK8sResource } from '~/__mocks__/mockImageStreamK8sResour import { mockPVCK8sResource } from '~/__mocks__/mockPVCK8sResource'; import { mockPodK8sResource } from '~/__mocks__/mockPodK8sResource'; import { + attachConnectionModal, createSpawnerPage, editSpawnerPage, notFoundSpawnerPage, @@ -37,6 +38,7 @@ import { import { mock200Status } from '~/__mocks__/mockK8sStatus'; import type { NotebookSize } from '~/types'; import { mockAcceleratorProfile } from '~/__mocks__/mockAcceleratorProfile'; +import { mockConnectionTypeConfigMap } from '~/__mocks__/mockConnectionType'; const configYamlPath = '../../__mocks__/mock-upload-configmap.yaml'; @@ -407,6 +409,74 @@ describe('Workbench page', () => { verifyRelativeURL('/projects/test-project?section=workbenches'); }); + it('Create workbench with connection', () => { + initIntercepts({ isEmpty: true }); + cy.interceptOdh('GET /api/config', mockDashboardConfig({ disableConnectionTypes: false })); + cy.interceptOdh('GET /api/connection-types', [mockConnectionTypeConfigMap({})]); + cy.interceptK8sList( + { model: SecretModel, ns: 'test-project' }, + mockK8sResourceList([ + mockSecretK8sResource({ name: 'test1', displayName: 'test1' }), + mockSecretK8sResource({ name: 'test2', displayName: 'test2' }), + ]), + ); + + workbenchPage.visit('test-project'); + workbenchPage.findCreateButton().click(); + createSpawnerPage.findSubmitButton().should('be.disabled'); + verifyRelativeURL('/projects/test-project/spawner'); + createSpawnerPage.k8sNameDescription.findDisplayNameInput().fill('1234'); + createSpawnerPage.findNotebookImage('test-9').click(); + + createSpawnerPage.findAttachConnectionButton().click(); + attachConnectionModal.shouldBeOpen(); + attachConnectionModal.findAttachButton().should('be.disabled'); + attachConnectionModal.selectConnectionOption('test1'); + attachConnectionModal.findAttachButton().should('be.enabled'); + attachConnectionModal.selectConnectionOption('test2'); + attachConnectionModal.findAttachButton().click(); + + createSpawnerPage.findConnectionsTableRow('test1', 's3'); + createSpawnerPage.findConnectionsTableRow('test2', 's3'); + + createSpawnerPage.findSubmitButton().click(); + cy.wait('@createWorkbench').then((interception) => { + expect(interception.request.body).to.containSubset({ + metadata: { + annotations: { + 'openshift.io/display-name': '1234', + }, + name: 'wb-1234', + namespace: 'test-project', + }, + spec: { + template: { + spec: { + affinity: {}, + containers: [ + { + envFrom: [ + { + secretRef: { + name: 'test1', + }, + }, + { + secretRef: { + name: 'test2', + }, + }, + ], + }, + ], + }, + }, + }, + }); + }); + verifyRelativeURL('/projects/test-project?section=workbenches'); + }); + it('list workbench and table sorting', () => { initIntercepts({ notebookSizes: [ diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx index bba20feb87..54ed3f874f 100644 --- a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx +++ b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx @@ -41,8 +41,8 @@ type SpawnerFooterProps = { storageData: StorageData; envVariables: EnvVariable[]; dataConnection: DataConnectionData; - isConnectionTypesEnabled: boolean; - connections: Connection[]; + isConnectionTypesEnabled?: boolean; + connections?: Connection[]; canEnablePipelines: boolean; }; @@ -52,7 +52,7 @@ const SpawnerFooter: React.FC = ({ envVariables, dataConnection, isConnectionTypesEnabled, - connections, + connections = [], canEnablePipelines, }) => { const [error, setError] = React.useState(); @@ -155,16 +155,12 @@ const SpawnerFooter: React.FC = ({ dataConnection, existingNotebookDataConnection, dryRun, - ).catch(handleError); + ); - if (isConnectionTypesEnabled && envFrom) { + if (isConnectionTypesEnabled) { envFrom = setConnectionsOnEnvFrom(connections, envFrom, projectConnections); } - if (!pvcDetails || !envFrom) { - return; - } - const annotations = { ...editNotebook.metadata.annotations }; if (envFrom.length > 0) { annotations['notebooks.opendatahub.io/notebook-restart'] = 'true'; diff --git a/frontend/src/pages/projects/screens/spawner/connections/ConnectionsFormSection.tsx b/frontend/src/pages/projects/screens/spawner/connections/ConnectionsFormSection.tsx index d9170b2737..ae22584397 100644 --- a/frontend/src/pages/projects/screens/spawner/connections/ConnectionsFormSection.tsx +++ b/frontend/src/pages/projects/screens/spawner/connections/ConnectionsFormSection.tsx @@ -111,6 +111,7 @@ export const ConnectionsFormSection: React.FC = ({ trigger={unselectedConnections.length === 0 ? 'mouseenter focus' : 'manual'} > {' '}
( = ({ onClose={onClose} actions={[ - {' '} - - + {unselectedConnections.length === 0 && ( + + )} + + + + + } id={SpawnerPageSectionID.CONNECTIONS} aria-label={SpawnerPageSectionTitles[SpawnerPageSectionID.CONNECTIONS]} diff --git a/frontend/src/pages/projects/screens/spawner/connections/SelectConnectionsModal.tsx b/frontend/src/pages/projects/screens/spawner/connections/SelectConnectionsModal.tsx index d3789122e7..10d3075373 100644 --- a/frontend/src/pages/projects/screens/spawner/connections/SelectConnectionsModal.tsx +++ b/frontend/src/pages/projects/screens/spawner/connections/SelectConnectionsModal.tsx @@ -24,12 +24,12 @@ export const SelectConnectionsModal: React.FC = ({ .find( (type) => c.metadata.annotations['opendatahub.io/connection-type'] === type.metadata.name, ) - ?.data?.category?.join(' '); + ?.data?.category?.join(', '); return { id: c.metadata.name, name: getDisplayNameFromK8sResource(c), - selected: connectionsToList.length === 1 || false, + selected: connectionsToList.length === 1, description: ( {getDescriptionFromK8sResource(c) && ( @@ -44,7 +44,7 @@ export const SelectConnectionsModal: React.FC = ({ )} ), - data: `${getDescriptionFromK8sResource(c)}`, + data: getDescriptionFromK8sResource(c), }; }), ); From d5e9295b44c53ef1c7656fa2282efa1e63bf5df3 Mon Sep 17 00:00:00 2001 From: emilys314 Date: Wed, 16 Oct 2024 09:23:05 -0400 Subject: [PATCH 4/4] updates part 2 --- .../src/concepts/connectionTypes/utils.ts | 14 ++++++++++++++ .../connections/ConnectionsTableRow.tsx | 15 +++++---------- .../connections/ConnectionsFormSection.tsx | 19 ++++++++++++------- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/frontend/src/concepts/connectionTypes/utils.ts b/frontend/src/concepts/connectionTypes/utils.ts index 617208a4ab..87ec4dce53 100644 --- a/frontend/src/concepts/connectionTypes/utils.ts +++ b/frontend/src/concepts/connectionTypes/utils.ts @@ -201,3 +201,17 @@ export const parseConnectionSecretValues = ( return response; }; + +export const getConnectionTypeDisplayName = ( + connection: Connection, + connectionTypes: ConnectionTypeConfigMapObj[], +): string => { + const matchingType = connectionTypes.find( + (type) => + type.metadata.name === connection.metadata.annotations['opendatahub.io/connection-type'], + ); + return ( + matchingType?.metadata.annotations?.['openshift.io/display-name'] || + connection.metadata.annotations['opendatahub.io/connection-type'] + ); +}; diff --git a/frontend/src/pages/projects/screens/detail/connections/ConnectionsTableRow.tsx b/frontend/src/pages/projects/screens/detail/connections/ConnectionsTableRow.tsx index 477e8b12d3..87ff7a7eb4 100644 --- a/frontend/src/pages/projects/screens/detail/connections/ConnectionsTableRow.tsx +++ b/frontend/src/pages/projects/screens/detail/connections/ConnectionsTableRow.tsx @@ -5,7 +5,7 @@ import { ExclamationTriangleIcon } from '@patternfly/react-icons'; import { Connection, ConnectionTypeConfigMapObj } from '~/concepts/connectionTypes/types'; import { TableRowTitleDescription } from '~/components/table'; import { getDescriptionFromK8sResource, getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; -import { getCompatibleTypes } from '~/concepts/connectionTypes/utils'; +import { getCompatibleTypes, getConnectionTypeDisplayName } from '~/concepts/connectionTypes/utils'; import CompatibilityLabel from '~/concepts/connectionTypes/CompatibilityLabel'; import ConnectedResources from '~/pages/projects/screens/detail/connections/ConnectedResources'; @@ -26,15 +26,10 @@ const ConnectionsTableRow: React.FC = ({ showConnectedResourcesCell = true, showWarningIcon = false, }) => { - const connectionTypeDisplayName = React.useMemo(() => { - const matchingType = connectionTypes?.find( - (type) => type.metadata.name === obj.metadata.annotations['opendatahub.io/connection-type'], - ); - return ( - matchingType?.metadata.annotations?.['openshift.io/display-name'] || - obj.metadata.annotations['opendatahub.io/connection-type'] - ); - }, [obj, connectionTypes]); + const connectionTypeDisplayName = React.useMemo( + () => getConnectionTypeDisplayName(obj, connectionTypes ?? []), + [obj, connectionTypes], + ); const compatibleTypes = obj.data ? getCompatibleTypes( diff --git a/frontend/src/pages/projects/screens/spawner/connections/ConnectionsFormSection.tsx b/frontend/src/pages/projects/screens/spawner/connections/ConnectionsFormSection.tsx index 0321ed4335..74b4243c9d 100644 --- a/frontend/src/pages/projects/screens/spawner/connections/ConnectionsFormSection.tsx +++ b/frontend/src/pages/projects/screens/spawner/connections/ConnectionsFormSection.tsx @@ -16,7 +16,8 @@ import { SortableData, Table } from '~/components/table'; import { createSecret, replaceSecret } from '~/api'; import { NotebookKind, ProjectKind } from '~/k8sTypes'; import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; -import { Connection } from '~/concepts/connectionTypes/types'; +import { Connection, ConnectionTypeConfigMapObj } from '~/concepts/connectionTypes/types'; +import { getConnectionTypeDisplayName } from '~/concepts/connectionTypes/utils'; import { useWatchConnectionTypes } from '~/utilities/useWatchConnectionTypes'; import { useNotebooksStates } from '~/pages/projects/notebook/useNotebooksStates'; import { SpawnerPageSectionTitles } from '~/pages/projects/screens/spawner/const'; @@ -27,7 +28,7 @@ import { SelectConnectionsModal } from './SelectConnectionsModal'; import { connectionEnvVarConflicts, DuplicateEnvVarWarning } from './DuplicateEnvVarsWarning'; import { DetachConnectionModal } from './DetachConnectionModal'; -const columns: SortableData[] = [ +const getColumns = (connectionTypes: ConnectionTypeConfigMapObj[]): SortableData[] => [ { field: 'name', label: 'Name', @@ -38,8 +39,8 @@ const columns: SortableData[] = [ field: 'type', label: 'Type', sortable: (a, b) => - a.metadata.annotations['opendatahub.io/connection-type'].localeCompare( - b.metadata.annotations['opendatahub.io/connection-type'], + getConnectionTypeDisplayName(a, connectionTypes).localeCompare( + getConnectionTypeDisplayName(b, connectionTypes), ), }, { @@ -70,12 +71,14 @@ export const ConnectionsFormSection: React.FC = ({ }) => { const [connectionTypes] = useWatchConnectionTypes(); - const [initialNumberConnections] = React.useState(selectedConnections.length); + const columns = React.useMemo(() => getColumns(connectionTypes), [connectionTypes]); + + const initialNumberConnections = React.useRef(selectedConnections.length); const notebookArray = React.useMemo(() => (notebook ? [notebook] : []), [notebook]); const [notebookStates] = useNotebooksStates( notebookArray, notebook?.metadata.namespace || '', - initialNumberConnections > 0, + initialNumberConnections.current > 0, ); const isRunning = React.useMemo( () => @@ -113,7 +116,9 @@ export const ConnectionsFormSection: React.FC = ({