From 12b1bb9b3da18495398fadcfa3ad741a6fc50a7e Mon Sep 17 00:00:00 2001 From: Emily Samoylov <93456589+emilys314@users.noreply.github.com> Date: Tue, 13 Aug 2024 16:00:28 -0400 Subject: [PATCH] Duplicate connection page UI (#3059) * 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 --- .../cypress/cypress/pages/connectionTypes.ts | 73 ++++++++ .../cypress/cypress/support/commands/odh.ts | 19 ++ .../createConnectionType.cy.ts | 84 +++++++++ frontend/src/app/AppRoutes.tsx | 2 + .../connectionTypes/__tests__/utils.spec.ts | 26 ++- .../createConnectionTypeUtils.ts | 42 +++++ .../connectionTypes/useConnectionType.ts | 22 +++ .../src/concepts/connectionTypes/utils.ts | 11 ++ .../src/concepts/k8s/NameDescriptionField.tsx | 12 +- .../connectionTypes/ConnectionTypeRoutes.tsx | 18 ++ .../CreateConnectionTypeBreadcrumbs.tsx | 9 + .../CreateConnectionTypeFieldsTable.tsx | 65 +++++++ .../CreateConnectionTypeFieldsTableRow.tsx | 61 +++++++ .../create/CreateConnectionTypeFooter.tsx | 52 ++++++ .../create/CreateConnectionTypePage.tsx | 162 ++++++++++++++++++ .../create/DuplicateConnectionTypePage.tsx | 28 +++ .../create/EditConnectionTypePage.tsx | 4 + 17 files changed, 686 insertions(+), 4 deletions(-) create mode 100644 frontend/src/__tests__/cypress/cypress/pages/connectionTypes.ts create mode 100644 frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/createConnectionType.cy.ts create mode 100644 frontend/src/concepts/connectionTypes/createConnectionTypeUtils.ts create mode 100644 frontend/src/concepts/connectionTypes/useConnectionType.ts create mode 100644 frontend/src/pages/connectionTypes/ConnectionTypeRoutes.tsx create mode 100644 frontend/src/pages/connectionTypes/create/CreateConnectionTypeBreadcrumbs.tsx create mode 100644 frontend/src/pages/connectionTypes/create/CreateConnectionTypeFieldsTable.tsx create mode 100644 frontend/src/pages/connectionTypes/create/CreateConnectionTypeFieldsTableRow.tsx create mode 100644 frontend/src/pages/connectionTypes/create/CreateConnectionTypeFooter.tsx create mode 100644 frontend/src/pages/connectionTypes/create/CreateConnectionTypePage.tsx create mode 100644 frontend/src/pages/connectionTypes/create/DuplicateConnectionTypePage.tsx create mode 100644 frontend/src/pages/connectionTypes/create/EditConnectionTypePage.tsx diff --git a/frontend/src/__tests__/cypress/cypress/pages/connectionTypes.ts b/frontend/src/__tests__/cypress/cypress/pages/connectionTypes.ts new file mode 100644 index 0000000000..6d7ccb96b5 --- /dev/null +++ b/frontend/src/__tests__/cypress/cypress/pages/connectionTypes.ts @@ -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(); diff --git a/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts b/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts index d742cba552..cf3fc8d65d 100644 --- a/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts +++ b/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts @@ -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; @@ -590,6 +591,24 @@ declare global { path: { namespace: string }; }, response: OdhResponse, + ) => Cypress.Chainable) & + (( + type: 'GET /api/connection-types', + response: ConnectionTypeConfigMap[], + ) => Cypress.Chainable) & + (( + type: 'PATCH /api/connection-types/:name', + options: { + path: { name: string }; + }, + response: { success: boolean; error: string }, + ) => Cypress.Chainable) & + (( + type: 'GET /api/connection-types/:name', + options: { + path: { name: string }; + }, + response: ConnectionTypeConfigMap, ) => Cypress.Chainable); } } diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/createConnectionType.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/createConnectionType.cy.ts new file mode 100644 index 0000000000..82b532f2a1 --- /dev/null +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/createConnectionType.cy.ts @@ -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'); + }); +}); diff --git a/frontend/src/app/AppRoutes.tsx b/frontend/src/app/AppRoutes.tsx index cc6ee4516d..bd227c0b21 100644 --- a/frontend/src/app/AppRoutes.tsx +++ b/frontend/src/app/AppRoutes.tsx @@ -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')); @@ -125,6 +126,7 @@ const AppRoutes: React.FC = () => { } /> } /> } /> + } /> )} diff --git a/frontend/src/concepts/connectionTypes/__tests__/utils.spec.ts b/frontend/src/concepts/connectionTypes/__tests__/utils.spec.ts index 3cabf41ca8..cf93a5b726 100644 --- a/frontend/src/concepts/connectionTypes/__tests__/utils.spec.ts +++ b/frontend/src/concepts/connectionTypes/__tests__/utils.spec.ts @@ -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'; @@ -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'); + }); +}); diff --git a/frontend/src/concepts/connectionTypes/createConnectionTypeUtils.ts b/frontend/src/concepts/connectionTypes/createConnectionTypeUtils.ts new file mode 100644 index 0000000000..bcfd4314d3 --- /dev/null +++ b/frontend/src/concepts/connectionTypes/createConnectionTypeUtils.ts @@ -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, + }, +}); diff --git a/frontend/src/concepts/connectionTypes/useConnectionType.ts b/frontend/src/concepts/connectionTypes/useConnectionType.ts new file mode 100644 index 0000000000..00636e9339 --- /dev/null +++ b/frontend/src/concepts/connectionTypes/useConnectionType.ts @@ -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 => { + const fetchData = React.useCallback>(() => { + if (!name) { + return Promise.reject(new NotReadyError('No connection type name')); + } + + return fetchConnectionType(name); + }, [name]); + + return useFetchState(fetchData, undefined); +}; diff --git a/frontend/src/concepts/connectionTypes/utils.ts b/frontend/src/concepts/connectionTypes/utils.ts index 6ed8b7b535..473fc7cac8 100644 --- a/frontend/src/concepts/connectionTypes/utils.ts +++ b/frontend/src/concepts/connectionTypes/utils.ts @@ -46,3 +46,14 @@ export const defaultValueToString = ( } return defaultValue == null ? defaultValue : `${defaultValue}`; }; + +export const fieldTypeToString = (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; +}; diff --git a/frontend/src/concepts/k8s/NameDescriptionField.tsx b/frontend/src/concepts/k8s/NameDescriptionField.tsx index e8affd3f58..53d07703e5 100644 --- a/frontend/src/concepts/k8s/NameDescriptionField.tsx +++ b/frontend/src/concepts/k8s/NameDescriptionField.tsx @@ -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; @@ -29,10 +32,13 @@ type NameDescriptionFieldProps = { const NameDescriptionField: React.FC = ({ nameFieldId, + nameFieldLabel = 'Name', descriptionFieldId, + descriptionFieldLabel = 'Description', data, setData, autoFocusName, + K8sLabelName = 'Resource name', showK8sName, disableK8sName, maxLength, @@ -58,7 +64,7 @@ const NameDescriptionField: React.FC = ({ return ( - + = ({ {showK8sName && ( = ({ )} - +