Skip to content

Commit

Permalink
Duplicate connection page UI (#3059)
Browse files Browse the repository at this point in the history
* Add duplicate connection type page

* Add connection type preview to page

* Remove table column headings when empty

* Reduce vertical padding and hide preview button when open

* Update useConnectionType to use useFetchState hook instead

* Fix file and folder path names

* Update table

* Updates to page and table

* Updates

* Add error state to footer if create fails

* Add username to creation and util type changes
  • Loading branch information
emilys314 authored Aug 13, 2024
1 parent 9211649 commit 12b1bb9
Show file tree
Hide file tree
Showing 17 changed files with 686 additions and 4 deletions.
73 changes: 73 additions & 0 deletions frontend/src/__tests__/cypress/cypress/pages/connectionTypes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { TableRow } from './components/table';

class CreateConnectionTypeTableRow extends TableRow {
findSectionHeading() {
return this.find().findByTestId('section-heading');
}

findName() {
return this.find().findByTestId('field-name');
}

findType() {
return this.find().findByTestId('field-type');
}

findDefault() {
return this.find().findByTestId('field-default');
}

findEnvVar() {
return this.find().findByTestId('field-env');
}

findRequired() {
return this.find().findByTestId('field-required');
}
}

class CreateConnectionTypePage {
visitCreatePage() {
cy.visitWithLogin('/connectionTypes/create');
cy.findAllByText('Create connection type').should('exist');
}

visitDuplicatePage(name = 'existing') {
cy.visitWithLogin(`/connectionTypes/duplicate/${name}`);
cy.findAllByText('Create connection type').should('exist');
}

findConnectionTypeName() {
return cy.findByTestId('connection-type-name');
}

findConnectionTypeDesc() {
return cy.findByTestId('connection-type-description');
}

findConnectionTypeEnableCheckbox() {
return cy.findByTestId('connection-type-enable');
}

findConnectionTypePreviewToggle() {
return cy.findByTestId('preview-drawer-toggle-button');
}

findFieldsTable() {
return cy.findByTestId('connection-type-fields-table');
}

findAllFieldsTableRows() {
return this.findFieldsTable().findAllByTestId('row');
}

getFieldsTableRow(index: number) {
return new CreateConnectionTypeTableRow(() => this.findAllFieldsTableRows().eq(index));
}

findSubmitButton() {
return cy.findByTestId('submit-button');
}
}

export const createConnectionTypePage = new CreateConnectionTypePage();
19 changes: 19 additions & 0 deletions frontend/src/__tests__/cypress/cypress/support/commands/odh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import type {
import type { GrpcResponse } from '~/__mocks__/mlmd/utils';
import type { BuildMockPipelinveVersionsType } from '~/__mocks__';
import type { ArtifactStorage } from '~/concepts/pipelines/types';
import type { ConnectionTypeConfigMap } from '~/concepts/connectionTypes/types';

type SuccessErrorResponse = {
success: boolean;
Expand Down Expand Up @@ -590,6 +591,24 @@ declare global {
path: { namespace: string };
},
response: OdhResponse<number>,
) => Cypress.Chainable<null>) &
((
type: 'GET /api/connection-types',
response: ConnectionTypeConfigMap[],
) => Cypress.Chainable<null>) &
((
type: 'PATCH /api/connection-types/:name',
options: {
path: { name: string };
},
response: { success: boolean; error: string },
) => Cypress.Chainable<null>) &
((
type: 'GET /api/connection-types/:name',
options: {
path: { name: string };
},
response: ConnectionTypeConfigMap,
) => Cypress.Chainable<null>);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import {
mockConnectionTypeConfigMap,
mockConnectionTypeConfigMapObj,
} from '~/__mocks__/mockConnectionType';
import { createConnectionTypePage } from '~/__tests__/cypress/cypress/pages/connectionTypes';
import { asClusterAdminUser } from '~/__tests__/cypress/cypress/utils/mockUsers';

describe('create', () => {
it('Display base page', () => {
asClusterAdminUser();
createConnectionTypePage.visitCreatePage();

createConnectionTypePage.findConnectionTypeName().should('exist');
createConnectionTypePage.findConnectionTypeDesc().should('exist');
createConnectionTypePage.findConnectionTypeEnableCheckbox().should('exist');
createConnectionTypePage.findConnectionTypePreviewToggle().should('exist');
});

it('Allows create button with valid name', () => {
asClusterAdminUser();
createConnectionTypePage.visitCreatePage();

createConnectionTypePage.findConnectionTypeName().should('have.value', '');
createConnectionTypePage.findSubmitButton().should('be.disabled');

createConnectionTypePage.findConnectionTypeName().type('hello');
createConnectionTypePage.findSubmitButton().should('be.enabled');
});
});

describe('duplicate', () => {
const existing = mockConnectionTypeConfigMapObj({ name: 'existing' });

beforeEach(() => {
asClusterAdminUser();
cy.interceptOdh(
'GET /api/connection-types/:name',
{ path: { name: 'existing' } },
mockConnectionTypeConfigMap({ name: 'existing' }),
);
});

it('Prefill details from existing connection', () => {
createConnectionTypePage.visitDuplicatePage('existing');

createConnectionTypePage
.findConnectionTypeName()
.should(
'have.value',
`Duplicate of ${existing.metadata.annotations['openshift.io/display-name']}`,
);
createConnectionTypePage
.findConnectionTypeDesc()
.should('have.value', existing.metadata.annotations['openshift.io/description']);
createConnectionTypePage.findConnectionTypeEnableCheckbox().should('be.checked');
});

it('Prefill fields table from existing connection', () => {
createConnectionTypePage.visitDuplicatePage('existing');

createConnectionTypePage
.findAllFieldsTableRows()
.should('have.length', existing.data?.fields?.length);

// Row 0 - Section
const row0 = createConnectionTypePage.getFieldsTableRow(0);
row0.findName().should('contain.text', 'Short text');
row0.findSectionHeading().should('exist');

// Row 1 - Short text field
const row1 = createConnectionTypePage.getFieldsTableRow(1);
row1.findName().should('contain.text', 'Short text 1');
row1.findType().should('have.text', 'Short text');
row1.findDefault().should('have.text', '-');
row1.findRequired().not('be.checked');

// Row 2 - Short text field
const row2 = createConnectionTypePage.getFieldsTableRow(2);
row2.findName().should('contain.text', 'Short text 2');
row2.findType().should('have.text', 'Short text');
row2.findDefault().should('have.text', 'This is the default value');
row2.findRequired().should('be.checked');
});
});
2 changes: 2 additions & 0 deletions frontend/src/app/AppRoutes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { useCheckJupyterEnabled } from '~/utilities/notebookControllerUtils';
import { SupportedArea } from '~/concepts/areas';
import useIsAreaAvailable from '~/concepts/areas/useIsAreaAvailable';
import ModelRegistrySettingsRoutes from '~/pages/modelRegistrySettings/ModelRegistrySettingsRoutes';
import ConnectionTypeRoutes from '~/pages/connectionTypes/ConnectionTypeRoutes';

const HomePage = React.lazy(() => import('../pages/home/Home'));

Expand Down Expand Up @@ -125,6 +126,7 @@ const AppRoutes: React.FC = () => {
<Route path="/servingRuntimes/*" element={<CustomServingRuntimeRoutes />} />
<Route path="/modelRegistrySettings/*" element={<ModelRegistrySettingsRoutes />} />
<Route path="/groupSettings" element={<GroupSettingsPage />} />
<Route path="/connectionTypes/*" element={<ConnectionTypeRoutes />} />
</>
)}

Expand Down
26 changes: 25 additions & 1 deletion frontend/src/concepts/connectionTypes/__tests__/utils.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { mockConnectionTypeConfigMapObj } from '~/__mocks__/mockConnectionType';
import { DropdownField, HiddenField, TextField } from '~/concepts/connectionTypes/types';
import { DropdownField, HiddenField, TextField, UriField } from '~/concepts/connectionTypes/types';
import {
defaultValueToString,
fieldTypeToString,
toConnectionTypeConfigMap,
toConnectionTypeConfigMapObj,
} from '~/concepts/connectionTypes/utils';
Expand Down Expand Up @@ -201,3 +202,26 @@ describe('defaultValueToString', () => {
).toBe('Two, Three');
});
});

describe('fieldTypeToString', () => {
it('should return default value as string', () => {
expect(
fieldTypeToString({
type: 'text',
name: 'test',
envVar: 'test',
properties: {},
} satisfies TextField),
).toBe('Text');
expect(
fieldTypeToString({
type: 'uri',
name: 'test',
envVar: 'test',
properties: {
defaultValue: '',
},
} satisfies UriField),
).toBe('URI');
});
});
42 changes: 42 additions & 0 deletions frontend/src/concepts/connectionTypes/createConnectionTypeUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { ConnectionTypeConfigMapObj, ConnectionTypeField } from '~/concepts/connectionTypes/types';

export const extractConnectionTypeFromMap = (
configMap?: ConnectionTypeConfigMapObj,
): {
k8sName: string;
name: string;
description: string;
enabled: boolean;
fields: ConnectionTypeField[];
} => ({
k8sName: configMap?.metadata.name ?? '',
name: configMap?.metadata.annotations['openshift.io/display-name'] ?? '',
description: configMap?.metadata.annotations['openshift.io/description'] ?? '',
enabled: configMap?.metadata.annotations['opendatahub.io/enabled'] === 'true',
fields: configMap?.data?.fields ?? [],
});

export const createConnectionTypeObj = (
k8sName: string,
displayName: string,
description: string,
enabled: boolean,
username: string,
fields: ConnectionTypeField[],
): ConnectionTypeConfigMapObj => ({
kind: 'ConfigMap',
apiVersion: 'v1',
metadata: {
name: k8sName,
annotations: {
'openshift.io/display-name': displayName,
'openshift.io/description': description,
'opendatahub.io/enabled': enabled ? 'true' : 'false',
'opendatahub.io/username': username,
},
labels: { 'opendatahub.io/dashboard': 'true', 'opendatahub.io/connection-type': 'true' },
},
data: {
fields,
},
});
22 changes: 22 additions & 0 deletions frontend/src/concepts/connectionTypes/useConnectionType.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import * as React from 'react';
import { ConnectionTypeConfigMapObj } from '~/concepts/connectionTypes/types';
import { fetchConnectionType } from '~/services/connectionTypesService';
import useFetchState, {
FetchState,
FetchStateCallbackPromise,
NotReadyError,
} from '~/utilities/useFetchState';

export const useConnectionType = (
name?: string,
): FetchState<ConnectionTypeConfigMapObj | undefined> => {
const fetchData = React.useCallback<FetchStateCallbackPromise<ConnectionTypeConfigMapObj>>(() => {
if (!name) {
return Promise.reject(new NotReadyError('No connection type name'));
}

return fetchConnectionType(name);
}, [name]);

return useFetchState(fetchData, undefined);
};
11 changes: 11 additions & 0 deletions frontend/src/concepts/connectionTypes/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,14 @@ export const defaultValueToString = <T extends ConnectionTypeDataField>(
}
return defaultValue == null ? defaultValue : `${defaultValue}`;
};

export const fieldTypeToString = <T extends ConnectionTypeDataField>(field: T): string => {
if (field.type === ConnectionTypeFieldType.URI) {
return field.type.toUpperCase();
}

const withSpaces = field.type.replace(/-/g, ' ');
const withCapitalized = withSpaces[0].toUpperCase() + withSpaces.slice(1);

return withCapitalized;
};
12 changes: 9 additions & 3 deletions frontend/src/concepts/k8s/NameDescriptionField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,13 @@ import { isValidK8sName, translateDisplayNameForK8s } from '~/concepts/k8s/utils

type NameDescriptionFieldProps = {
nameFieldId: string;
nameFieldLabel?: string;
descriptionFieldId: string;
descriptionFieldLabel?: string;
data: NameDescType;
setData?: (data: NameDescType) => void;
autoFocusName?: boolean;
K8sLabelName?: string;
showK8sName?: boolean;
disableK8sName?: boolean;
maxLength?: number;
Expand All @@ -29,10 +32,13 @@ type NameDescriptionFieldProps = {

const NameDescriptionField: React.FC<NameDescriptionFieldProps> = ({
nameFieldId,
nameFieldLabel = 'Name',
descriptionFieldId,
descriptionFieldLabel = 'Description',
data,
setData,
autoFocusName,
K8sLabelName = 'Resource name',
showK8sName,
disableK8sName,
maxLength,
Expand All @@ -58,7 +64,7 @@ const NameDescriptionField: React.FC<NameDescriptionFieldProps> = ({
return (
<Stack hasGutter>
<StackItem>
<FormGroup label="Name" isRequired fieldId={nameFieldId}>
<FormGroup label={nameFieldLabel} isRequired fieldId={nameFieldId}>
<TextInput
aria-readonly={!setData}
isRequired
Expand Down Expand Up @@ -90,7 +96,7 @@ const NameDescriptionField: React.FC<NameDescriptionFieldProps> = ({
{showK8sName && (
<StackItem>
<FormGroup
label="Resource name"
label={K8sLabelName}
labelIcon={
<Tooltip
position="right"
Expand Down Expand Up @@ -145,7 +151,7 @@ const NameDescriptionField: React.FC<NameDescriptionFieldProps> = ({
</StackItem>
)}
<StackItem>
<FormGroup label="Description" fieldId={descriptionFieldId}>
<FormGroup label={descriptionFieldLabel} fieldId={descriptionFieldId}>
<TextArea
aria-readonly={!setData}
resizeOrientation="vertical"
Expand Down
18 changes: 18 additions & 0 deletions frontend/src/pages/connectionTypes/ConnectionTypeRoutes.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import * as React from 'react';
import { Navigate, Routes, Route } from 'react-router-dom';
import { CreateConnectionTypePage } from './create/CreateConnectionTypePage';
import { DuplicateConnectionTypePage } from './create/DuplicateConnectionTypePage';
import { EditConnectionTypePage } from './create/EditConnectionTypePage';

const ConnectionTypeRoutes: React.FC = () => (
<Routes>
<Route path="/">
<Route path="create" element={<CreateConnectionTypePage />} />
<Route path="duplicate/:name" element={<DuplicateConnectionTypePage />} />
<Route path="edit/:name" element={<EditConnectionTypePage />} />
<Route path="*" element={<Navigate to="." />} />
</Route>
</Routes>
);

export default ConnectionTypeRoutes;
Loading

0 comments on commit 12b1bb9

Please sign in to comment.