From 4b1e286ad14b407ab25a79cb1887250f4c6c4ac0 Mon Sep 17 00:00:00 2001 From: jpuzz0 <96431149+jpuzz0@users.noreply.github.com> Date: Wed, 18 Sep 2024 12:15:01 -0400 Subject: [PATCH] [RHOAIENG-12310] Error cases for storage class admin table (#3213) --- frontend/src/__mocks__/mockStorageClasses.ts | 19 +- .../cypress/cypress/pages/storageClasses.ts | 36 +- .../storageClasses/storageClasses.cy.ts | 323 ++++++++++++++++-- .../storageClasses/CorruptedMetadataAlert.tsx | 46 +++ .../ResetCorruptConfigValueAlert.tsx | 55 +++ .../StorageClassDefaultRadio.tsx | 3 +- .../storageClasses/StorageClassEditModal.tsx | 68 ++-- .../StorageClassEnableSwitch.tsx | 3 +- .../storageClasses/StorageClassesTableRow.tsx | 194 +++++++++-- frontend/src/pages/storageClasses/utils.ts | 19 ++ 10 files changed, 673 insertions(+), 93 deletions(-) create mode 100644 frontend/src/pages/storageClasses/CorruptedMetadataAlert.tsx create mode 100644 frontend/src/pages/storageClasses/ResetCorruptConfigValueAlert.tsx diff --git a/frontend/src/__mocks__/mockStorageClasses.ts b/frontend/src/__mocks__/mockStorageClasses.ts index 35ac9ee2f6..13d5bbb88b 100644 --- a/frontend/src/__mocks__/mockStorageClasses.ts +++ b/frontend/src/__mocks__/mockStorageClasses.ts @@ -19,19 +19,28 @@ export const mockStorageClassList = ( items: storageClasses, }); -export const buildMockStorageClass = ( +export const buildMockStorageClassConfig = ( mockStorageClass: MockStorageClass, config: Partial, +): string => + JSON.stringify({ + ...JSON.parse( + String(mockStorageClass.metadata.annotations?.['opendatahub.io/sc-config'] ?? null), + ), + ...config, + }); + +export const buildMockStorageClass = ( + mockStorageClass: MockStorageClass, + config: Partial | string, ): MockStorageClass => ({ ...mockStorageClass, metadata: { ...mockStorageClass.metadata, annotations: { ...mockStorageClass.metadata.annotations, - 'opendatahub.io/sc-config': JSON.stringify({ - ...JSON.parse(String(mockStorageClass.metadata.annotations?.['opendatahub.io/sc-config'])), - ...config, - }), + 'opendatahub.io/sc-config': + typeof config !== 'string' ? buildMockStorageClassConfig(mockStorageClass, config) : config, }, }, }); diff --git a/frontend/src/__tests__/cypress/cypress/pages/storageClasses.ts b/frontend/src/__tests__/cypress/cypress/pages/storageClasses.ts index f582c33e59..1f538f14e6 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/storageClasses.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/storageClasses.ts @@ -29,16 +29,32 @@ class StorageClassesTableRow extends TableRow { return this.find().findByTestId('openshift-sc-default-label'); } - findEnableSwitch() { + findEnableSwitchInput() { return this.find().findByTestId('enable-switch'); } - findDefaultRadio() { + findDefaultRadioInput() { return this.find().findByTestId('set-default-radio'); } - findOpenshiftScName() { - return this.find().find(`[data-label="Openshift storage class"]`); + findDisplayNameValue() { + return this.find().find(`[data-label="Display name"]`); + } + + findEnableValue() { + return this.find().find(`[data-label="Enable"]`); + } + + findDefaultValue() { + return this.find().find(`[data-label="Default"]`); + } + + findLastModifiedValue() { + return this.find().find(`[data-label="Last modified"]`); + } + + findCorruptedMetadataAlert() { + return cy.findByTestId('corrupted-metadata-alert'); } } @@ -48,13 +64,19 @@ class StorageClassesTable { } getRowByName(name: string) { + return new StorageClassesTableRow(() => + this.find().find(`[data-label="Openshift storage class"]`).contains(name).parents('tr'), + ); + } + + getRowByConfigName(name: string) { return new StorageClassesTableRow(() => this.find().find(`[data-label="Display name"]`).contains(name).parents('tr'), ); } findRowByName(name: string) { - return this.getRowByName(name).find(); + return this.getRowByConfigName(name).find(); } mockUpdateStorageClass(storageClassName: string, times?: number) { @@ -119,6 +141,10 @@ class StorageClassEditModal extends Modal { return this.findFooter().findByTestId('modal-submit-button'); } + findInfoAlert() { + return this.find().findByTestId('edit-sc-modal-info-alert'); + } + mockUpdateStorageClass(storageClassName: string, times?: number) { return cy.interceptOdh( `PUT /api/storage-class/:name/config`, diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/storageClasses/storageClasses.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/storageClasses/storageClasses.cy.ts index 94a84bdac2..f2858f93a6 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/storageClasses/storageClasses.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/storageClasses/storageClasses.cy.ts @@ -1,4 +1,9 @@ -import { buildMockStorageClass, mockStorageClassList, mockStorageClasses } from '~/__mocks__'; +import { + buildMockStorageClass, + buildMockStorageClassConfig, + mockStorageClassList, + mockStorageClasses, +} from '~/__mocks__'; import { asProductAdminUser } from '~/__tests__/cypress/cypress/utils/mockUsers'; import { pageNotfound } from '~/__tests__/cypress/cypress/pages/pageNotFound'; import { @@ -52,19 +57,22 @@ describe('Storage classes', () => { ); storageClassesPage.visit(); - const openshiftDefaultTableRow = storageClassesTable.getRowByName('openshift-default-sc'); + const openshiftDefaultTableRow = + storageClassesTable.getRowByConfigName('openshift-default-sc'); openshiftDefaultTableRow.findOpenshiftDefaultLabel().should('be.visible'); - openshiftDefaultTableRow.findEnableSwitch().should('have.attr', 'aria-checked', 'true'); - openshiftDefaultTableRow.findEnableSwitch().should('have.attr', 'disabled'); - openshiftDefaultTableRow.findDefaultRadio().should('have.attr', 'checked'); - openshiftDefaultTableRow.findDefaultRadio().should('not.have.attr', 'disabled'); + openshiftDefaultTableRow.findEnableSwitchInput().should('have.attr', 'aria-checked', 'true'); + openshiftDefaultTableRow.findEnableSwitchInput().should('have.attr', 'disabled'); + openshiftDefaultTableRow.findDefaultRadioInput().should('have.attr', 'checked'); + openshiftDefaultTableRow.findDefaultRadioInput().should('not.have.attr', 'disabled'); - const otherStorageClassTableRow = storageClassesTable.getRowByName('Test SC 1'); + const otherStorageClassTableRow = storageClassesTable.getRowByConfigName('Test SC 1'); otherStorageClassTableRow.findOpenshiftDefaultLabel().should('not.exist'); - otherStorageClassTableRow.findEnableSwitch().should('have.attr', 'aria-checked', 'false'); - otherStorageClassTableRow.findEnableSwitch().should('not.have.attr', 'disabled'); - otherStorageClassTableRow.findDefaultRadio().should('not.have.attr', 'checked'); - otherStorageClassTableRow.findDefaultRadio().should('have.attr', 'disabled'); + otherStorageClassTableRow + .findEnableSwitchInput() + .should('have.attr', 'aria-checked', 'false'); + otherStorageClassTableRow.findEnableSwitchInput().should('not.have.attr', 'disabled'); + otherStorageClassTableRow.findDefaultRadioInput().should('not.have.attr', 'checked'); + otherStorageClassTableRow.findDefaultRadioInput().should('have.attr', 'disabled'); storageClassesTable .mockUpdateStorageClass(otherStorageClass.metadata.name, 1) @@ -80,12 +88,12 @@ describe('Storage classes', () => { .as('refreshStorageClasses-1'); // Enable the other storage class so that the RHOAI default can be toggled - otherStorageClassTableRow.findEnableSwitch().click({ force: true }); + otherStorageClassTableRow.findEnableSwitchInput().click({ force: true }); cy.wait('@updateStorageClass-1'); cy.wait('@refreshStorageClasses-1'); - otherStorageClassTableRow.findEnableSwitch().should('have.attr', 'aria-checked', 'true'); - otherStorageClassTableRow.findDefaultRadio().should('not.have.attr', 'disabled'); + otherStorageClassTableRow.findEnableSwitchInput().should('have.attr', 'aria-checked', 'true'); + otherStorageClassTableRow.findDefaultRadioInput().should('not.have.attr', 'disabled'); storageClassesTable .mockUpdateStorageClass(otherStorageClass.metadata.name, 1) @@ -104,13 +112,13 @@ describe('Storage classes', () => { .as('refreshStorageClasses-2'); // Set the other storage class as the RHOAI default - otherStorageClassTableRow.findDefaultRadio().click(); + otherStorageClassTableRow.findDefaultRadioInput().click(); cy.wait('@updateStorageClass-2'); cy.wait('@updateStorageClass-3'); cy.wait('@refreshStorageClasses-2'); - openshiftDefaultTableRow.findEnableSwitch().should('not.have.attr', 'disabled'); - openshiftDefaultTableRow.findDefaultRadio().should('have.attr', 'checked'); + openshiftDefaultTableRow.findEnableSwitchInput().should('not.have.attr', 'disabled'); + openshiftDefaultTableRow.findDefaultRadioInput().should('have.attr', 'checked'); }); it('can edit storage class display name and description', () => { @@ -121,11 +129,14 @@ describe('Storage classes', () => { ); storageClassesPage.visit(); - storageClassesTable.getRowByName('openshift-default-sc').findKebabAction('Edit').click(); + storageClassesTable + .getRowByConfigName('openshift-default-sc') + .findKebabAction('Edit') + .click(); storageClassEditModal.findOpenshiftDefaultLabel().should('be.visible'); storageClassEditModal.findCloseButton().click(); - storageClassesTable.getRowByName('Test SC 1').findKebabAction('Edit').click(); + storageClassesTable.getRowByConfigName('Test SC 1').findKebabAction('Edit').click(); storageClassEditModal.findOpenshiftScName().should('have.text', 'test-storage-class-1'); storageClassEditModal.findProvisioner().should('have.text', 'manila.csi.openstack.org'); storageClassEditModal.findOpenshiftDefaultLabel().should('not.exist'); @@ -145,9 +156,281 @@ describe('Storage classes', () => { storageClassEditModal.findSaveButton().click(); cy.wait('@updateStorageClass'); - const updatedRow = storageClassesTable.getRowByName('Updated name'); + const updatedRow = storageClassesTable.getRowByConfigName('Updated name'); updatedRow.find().should('contain.text', 'Updated name'); updatedRow.find().should('contain.text', 'Updated description'); }); + + it('can reset an unreadable storage class config', () => { + const storageClassName = 'unreadable-config'; + const storageClass = { + ...otherStorageClass, + metadata: { ...otherStorageClass.metadata, name: storageClassName }, + }; + const unreadableConfigStorageClass = buildMockStorageClass(storageClass, '{“FAIL:}'); + + cy.interceptOdh( + 'GET /api/k8s/apis/storage.k8s.io/v1/storageclasses', + {}, + mockStorageClassList([unreadableConfigStorageClass]), + ); + + storageClassesPage.visit(); + + const storageClassTableRow = storageClassesTable.getRowByName(storageClassName); + storageClassTableRow.findDisplayNameValue().should('have.text', '-'); + storageClassTableRow.findEnableValue().should('have.text', '-'); + storageClassTableRow.findDefaultValue().should('have.text', '-'); + storageClassTableRow.findLastModifiedValue().should('have.text', '-'); + storageClassTableRow.findEnableValue().should('have.text', '-'); + storageClassTableRow.find().findByTestId('corrupted-metadata-alert').should('be.visible'); + storageClassTableRow.findKebabAction('Edit').click(); + storageClassEditModal.findInfoAlert().should('contain.text', 'Reset the metadata'); + storageClassEditModal.fillDisplayNameInput('Readable config'); + + storageClassEditModal.mockUpdateStorageClass(storageClassName, 1); + storageClassesTable + .mockGetStorageClasses([ + buildMockStorageClass(storageClass, { + displayName: 'Readable config', + isEnabled: false, + isDefault: false, + }), + ]) + .as('updateStorageClass'); + storageClassEditModal.findSaveButton().click(); + + cy.wait('@updateStorageClass'); + + storageClassTableRow.findDisplayNameValue().should('have.text', 'Readable config'); + storageClassTableRow.findEnableSwitchInput().should('have.attr', 'aria-checked', 'false'); + storageClassTableRow.findDefaultRadioInput().should('not.have.attr', 'checked'); + storageClassTableRow.findLastModifiedValue().should('not.have.text', '-'); + }); + + it('can reset individual config non-name fields when invalid', () => { + const storageClassName = 'invalid-non-name-fields'; + const storageClassConfig = + '{"isDefault":"","isEnabled":"","displayName":"Test malformed default, enable, lastmodified","lastModified":"","description":""}'; + const storageClass = buildMockStorageClass( + { ...otherStorageClass, metadata: { name: storageClassName } }, + storageClassConfig, + ); + const existingConfig = JSON.parse(storageClassConfig); + + cy.interceptOdh( + 'GET /api/k8s/apis/storage.k8s.io/v1/storageclasses', + {}, + mockStorageClassList([storageClass]), + ); + + storageClassesPage.visit(); + + const storageClassTableRow = storageClassesTable.getRowByName(storageClassName); + storageClassTableRow + .findEnableValue() + .findByTestId('corrupted-metadata-alert') + .should('be.visible'); + storageClassTableRow + .findDefaultValue() + .findByTestId('corrupted-metadata-alert') + .should('be.visible'); + storageClassTableRow + .findLastModifiedValue() + .findByTestId('corrupted-metadata-alert') + .should('be.visible'); + + // Reset enable + storageClassesTable.mockUpdateStorageClass(storageClassName, 1); + storageClassesTable + .mockGetStorageClasses([ + buildMockStorageClass(storageClass, { + ...existingConfig, + isEnabled: false, + }), + ]) + .as('resetEnable'); + + storageClassTableRow + .findEnableValue() + .findByTestId('corrupted-metadata-alert-action') + .click(); + + cy.wait('@resetEnable'); + storageClassTableRow.findEnableSwitchInput().should('have.attr', 'aria-checked', 'false'); + + // Reset default + storageClassesTable.mockUpdateStorageClass(storageClassName, 1); + storageClassesTable + .mockGetStorageClasses([ + buildMockStorageClass(storageClass, { + ...existingConfig, + isEnabled: false, + isDefault: false, + }), + ]) + .as('resetDefault'); + + storageClassTableRow + .findDefaultValue() + .findByTestId('corrupted-metadata-alert-action') + .click(); + + cy.wait('@resetDefault'); + storageClassTableRow.findDefaultRadioInput().should('not.have.attr', 'checked'); + + // Reset last modified + storageClassesTable.mockUpdateStorageClass(storageClassName, 1); + storageClassesTable + .mockGetStorageClasses([ + buildMockStorageClass(storageClass, { + ...existingConfig, + lastModified: '2023-08-22T15:42:53.101Z', + isEnabled: false, + isDefault: false, + }), + ]) + .as('resetLastModified'); + + storageClassTableRow + .findLastModifiedValue() + .findByTestId('corrupted-metadata-alert-action') + .click(); + + cy.wait('@resetLastModified'); + storageClassTableRow.findLastModifiedValue().should('contain.text', '8/22/2023'); + }); + + it('can reset invalid config display name', () => { + const storageClass = buildMockStorageClass( + { ...otherStorageClass, metadata: { name: 'invalid-name' } }, + '{"isDefault":false,"isEnabled":false,"displayName":{},"lastModified":"2024-09-09T17:45:05.299Z","description":"Test malformed displayName"}', + ); + + cy.interceptOdh( + 'GET /api/k8s/apis/storage.k8s.io/v1/storageclasses', + {}, + mockStorageClassList([storageClass]), + ); + + storageClassesPage.visit(); + + const storageClassTableRow = storageClassesTable.getRowByName('invalid-name'); + storageClassTableRow + .findDisplayNameValue() + .findByTestId('corrupted-metadata-alert-action') + .click(); + storageClassEditModal.findDisplayNameInput().should('be.empty'); + storageClassEditModal + .findDescriptionInput() + .should('have.value', 'Test malformed displayName'); + storageClassEditModal.findInfoAlert().should('contain.text', 'Edit the invalid field'); + storageClassEditModal.fillDisplayNameInput('New name'); + + storageClassEditModal.mockUpdateStorageClass('invalid-name', 1); + storageClassesTable + .mockGetStorageClasses([ + buildMockStorageClass( + storageClass, + buildMockStorageClassConfig(storageClass, { + displayName: 'New name', + }), + ), + ]) + .as('refreshStorageClasses'); + + storageClassEditModal.findSaveButton().click(); + + cy.wait('@refreshStorageClasses'); + storageClassTableRow.findDisplayNameValue().should('contain.text', 'New name'); + }); + + it('can reset invalid config description', () => { + const storageClass = buildMockStorageClass( + { ...otherStorageClass, metadata: { name: 'invalid-description' } }, + '{"isDefault":false,"isEnabled":false,"displayName":"Test malformed description","lastModified":"2024-09-09T17:45:05.299Z","description":{}}', + ); + + cy.interceptOdh( + 'GET /api/k8s/apis/storage.k8s.io/v1/storageclasses', + {}, + mockStorageClassList([storageClass]), + ); + + storageClassesPage.visit(); + + const storageClassTableRow = storageClassesTable.getRowByName('invalid-description'); + storageClassTableRow + .findDisplayNameValue() + .findByTestId('corrupted-metadata-alert-action') + .click(); + storageClassEditModal + .findDisplayNameInput() + .should('have.value', 'Test malformed description'); + storageClassEditModal.findDescriptionInput().should('be.empty'); + storageClassEditModal.findInfoAlert().should('contain.text', 'Edit the invalid field'); + + storageClassEditModal.mockUpdateStorageClass('invalid-description', 1); + storageClassesTable + .mockGetStorageClasses([ + buildMockStorageClass( + storageClass, + buildMockStorageClassConfig(storageClass, { + description: '', + }), + ), + ]) + .as('refreshStorageClasses'); + + storageClassEditModal.findSaveButton().click(); + + cy.wait('@refreshStorageClasses'); + storageClassTableRow.findDisplayNameValue().should('have.text', 'Test malformed description'); + }); + + it('can reset invalid config display name & description', () => { + const storageClass = buildMockStorageClass( + { ...otherStorageClass, metadata: { name: 'invalid-name-and-desc' } }, + '{"isDefault":false,"isEnabled":false,"displayName":{},"description":{},"lastModified":"2024-09-09T17:45:05.299Z"}', + ); + + cy.interceptOdh( + 'GET /api/k8s/apis/storage.k8s.io/v1/storageclasses', + {}, + mockStorageClassList([storageClass]), + ); + + storageClassesPage.visit(); + + const storageClassTableRow = storageClassesTable.getRowByName('invalid-name-and-desc'); + storageClassTableRow + .findDisplayNameValue() + .findByTestId('corrupted-metadata-alert-action') + .click(); + storageClassEditModal.findDisplayNameInput().should('be.empty'); + storageClassEditModal.findDescriptionInput().should('be.empty'); + storageClassEditModal.findInfoAlert().should('contain.text', 'Edit the invalid field'); + storageClassEditModal.fillDisplayNameInput('New name'); + storageClassEditModal.fillDescriptionInput('New description'); + + storageClassEditModal.mockUpdateStorageClass('invalid-name-and-desc', 1); + storageClassesTable + .mockGetStorageClasses([ + buildMockStorageClass( + storageClass, + buildMockStorageClassConfig(storageClass, { + displayName: 'New name', + description: 'New description', + }), + ), + ]) + .as('refreshStorageClasses'); + + storageClassEditModal.findSaveButton().click(); + + cy.wait('@refreshStorageClasses'); + storageClassTableRow.findDisplayNameValue().should('contain.text', 'New name'); + storageClassTableRow.findDisplayNameValue().should('contain.text', 'New description'); + }); }); }); diff --git a/frontend/src/pages/storageClasses/CorruptedMetadataAlert.tsx b/frontend/src/pages/storageClasses/CorruptedMetadataAlert.tsx new file mode 100644 index 0000000000..3682bf1414 --- /dev/null +++ b/frontend/src/pages/storageClasses/CorruptedMetadataAlert.tsx @@ -0,0 +1,46 @@ +import React from 'react'; +import { Alert, AlertProps, Flex, FlexItem, Icon, Popover, Tooltip } from '@patternfly/react-core'; +import { ExclamationCircleIcon } from '@patternfly/react-icons'; + +interface CorruptedMetadataAlertProps extends Pick { + popoverText: React.ReactNode; + action?: React.ReactNode; + errorText?: React.ReactNode; +} + +export const CorruptedMetadataAlert: React.FC = ({ + variant = 'warning', + action, + errorText, + popoverText, +}) => ( + + + + + + + + {action && {action}} + + {errorText && ( + + + + + + + + )} + +); diff --git a/frontend/src/pages/storageClasses/ResetCorruptConfigValueAlert.tsx b/frontend/src/pages/storageClasses/ResetCorruptConfigValueAlert.tsx new file mode 100644 index 0000000000..0e8f2dedb7 --- /dev/null +++ b/frontend/src/pages/storageClasses/ResetCorruptConfigValueAlert.tsx @@ -0,0 +1,55 @@ +import React from 'react'; + +import { AlertProps, Button } from '@patternfly/react-core'; +import { SyncAltIcon } from '@patternfly/react-icons'; + +import { StorageClassConfig, StorageClassKind } from '~/k8sTypes'; +import { updateStorageClassConfig } from '~/services/StorageClassService'; +import { CorruptedMetadataAlert } from './CorruptedMetadataAlert'; + +interface ResetCorruptConfigValueAlertProps extends Pick { + storageClassName: string; + storageClassConfig: StorageClassConfig; + refresh: () => Promise; +} + +export const ResetCorruptConfigValueAlert: React.FC = ({ + storageClassName, + storageClassConfig, + variant, + refresh, +}) => { + const [error, setError] = React.useState(); + const [isUpdating, setIsUpdating] = React.useState(false); + + const onClick = React.useCallback(async () => { + setIsUpdating(true); + + try { + await updateStorageClassConfig(storageClassName, storageClassConfig); + await refresh(); + } catch { + setError('Failed to refresh the field. Try again later.'); + } finally { + setIsUpdating(false); + } + }, [storageClassName, storageClassConfig, refresh]); + + return ( + + + + } + /> + ); +}; diff --git a/frontend/src/pages/storageClasses/StorageClassDefaultRadio.tsx b/frontend/src/pages/storageClasses/StorageClassDefaultRadio.tsx index 4455443540..8e056140f0 100644 --- a/frontend/src/pages/storageClasses/StorageClassDefaultRadio.tsx +++ b/frontend/src/pages/storageClasses/StorageClassDefaultRadio.tsx @@ -30,8 +30,7 @@ export const StorageClassDefaultRadio: React.FC = try { await updateStorageClassConfig(storageClassName, { isDefault: true }); await onChange(); - setIsUpdating(false); - } catch { + } finally { setIsUpdating(false); } diff --git a/frontend/src/pages/storageClasses/StorageClassEditModal.tsx b/frontend/src/pages/storageClasses/StorageClassEditModal.tsx index e6e9998737..0e10adde62 100644 --- a/frontend/src/pages/storageClasses/StorageClassEditModal.tsx +++ b/frontend/src/pages/storageClasses/StorageClassEditModal.tsx @@ -14,29 +14,40 @@ import { TextArea, Flex, FlexItem, + AlertProps, } from '@patternfly/react-core'; import { StorageClassKind } from '~/k8sTypes'; import { updateStorageClassConfig } from '~/services/StorageClassService'; import DashboardModalFooter from '~/concepts/dashboard/DashboardModalFooter'; -import { getStorageClassConfig, isOpenshiftDefaultStorageClass } from './utils'; +import { getStorageClassConfig, isOpenshiftDefaultStorageClass, isValidConfigValue } from './utils'; import { OpenshiftDefaultLabel } from './OpenshiftDefaultLabel'; interface StorageClassEditModalProps { storageClass: StorageClassKind; + alert?: Partial>; onSuccess: () => Promise; onClose: () => void; } export const StorageClassEditModal: React.FC = ({ storageClass, + alert, onSuccess, onClose, }) => { const { name: storageClassName } = storageClass.metadata; const storageClassConfig = getStorageClassConfig(storageClass); - const [displayName, setDisplayName] = React.useState(storageClassConfig?.displayName ?? ''); - const [description, setDescription] = React.useState(storageClassConfig?.description ?? ''); + const [displayName, setDisplayName] = React.useState( + isValidConfigValue('displayName', storageClassConfig?.displayName) + ? storageClassConfig?.displayName + : '', + ); + const [description, setDescription] = React.useState( + isValidConfigValue('description', storageClassConfig?.description) + ? storageClassConfig?.description + : '', + ); const [updateError, setUpdateError] = React.useState(); const [isSubmitting, setIsSubmitting] = React.useState(false); @@ -44,7 +55,12 @@ export const StorageClassEditModal: React.FC = ({ setIsSubmitting(true); try { - await updateStorageClassConfig(storageClassName, { displayName, description }); + await updateStorageClassConfig(storageClassName, { + displayName, + description, + ...(storageClassConfig?.isDefault === undefined && { isDefault: false }), + ...(storageClassConfig?.isEnabled === undefined && { isEnabled: false }), + }); await onSuccess(); onClose(); } catch (error) { @@ -79,31 +95,35 @@ export const StorageClassEditModal: React.FC = ({ isInline variant="info" title="Editing these details will not affect the storage class in OpenShift." + {...alert} className="pf-v5-u-mb-lg" + data-testid="edit-sc-modal-info-alert" />
- - - OpenShift storage class - - - {storageClassName} - {isOpenshiftDefaultStorageClass(storageClass) && } - - - + {(!alert || alert.children) && ( + + + OpenShift storage class + + + {storageClassName} + {isOpenshiftDefaultStorageClass(storageClass) && } + + + - - Provisioner - - {storageClass.provisioner} - - - + + Provisioner + + {storageClass.provisioner} + + + + )} = try { await updateStorageClassConfig(storageClassName, { isEnabled: checked }); await onChange(); - setIsUpdating(false); - } catch { + } finally { setIsUpdating(false); } diff --git a/frontend/src/pages/storageClasses/StorageClassesTableRow.tsx b/frontend/src/pages/storageClasses/StorageClassesTableRow.tsx index 69e7d5d09e..80cf233c01 100644 --- a/frontend/src/pages/storageClasses/StorageClassesTableRow.tsx +++ b/frontend/src/pages/storageClasses/StorageClassesTableRow.tsx @@ -9,9 +9,10 @@ import { DescriptionListTerm, DescriptionListDescription, Timestamp, + Button, } from '@patternfly/react-core'; import { Tr, Td, ActionsColumn, TableText } from '@patternfly/react-table'; -import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons'; +import { PencilAltIcon, OutlinedQuestionCircleIcon } from '@patternfly/react-icons'; import { StorageClassConfig, StorageClassKind } from '~/k8sTypes'; import { TableRowTitleDescription } from '~/components/table'; @@ -20,11 +21,13 @@ import { updateStorageClassConfig } from '~/services/StorageClassService'; import DashboardPopupIconButton from '~/concepts/dashboard/DashboardPopupIconButton'; import { NoValue } from '~/components/NoValue'; import { ColumnLabel } from './constants'; -import { isOpenshiftDefaultStorageClass } from './utils'; +import { isOpenshiftDefaultStorageClass, isValidConfigValue } from './utils'; import { StorageClassEnableSwitch } from './StorageClassEnableSwitch'; import { StorageClassDefaultRadio } from './StorageClassDefaultRadio'; import { StorageClassEditModal } from './StorageClassEditModal'; import { OpenshiftDefaultLabel } from './OpenshiftDefaultLabel'; +import { CorruptedMetadataAlert } from './CorruptedMetadataAlert'; +import { ResetCorruptConfigValueAlert } from './ResetCorruptConfigValueAlert'; interface StorageClassesTableRowProps { storageClass: StorageClassKind; @@ -38,10 +41,13 @@ export const StorageClassesTableRow: React.FC = ({ refresh, }) => { const [isEditModalOpen, setIsEditModalOpen] = React.useState(false); - const storageClassConfig = storageClassConfigMap[storageClass.metadata.name]; const isOpenshiftDefault = isOpenshiftDefaultStorageClass(storageClass); const { metadata, provisioner, reclaimPolicy, volumeBindingMode, allowVolumeExpansion } = storageClass; + const storageClassConfig = storageClassConfigMap[metadata.name]; + const hasReadableConfig = storageClassConfig !== undefined; + const editModalAlertRef = + React.useRef['alert']>(); const storageClassInfoItems = [ { @@ -72,8 +78,7 @@ export const StorageClassesTableRow: React.FC = ({ const isEnableSwitchDisabled = React.useMemo(() => { const hasOtherStorageClassesEnabled = Object.entries(storageClassConfigMap).some( - ([storageClassName, config]) => - storageClassName !== storageClass.metadata.name && config?.isEnabled, + ([storageClassName, config]) => storageClassName !== metadata.name && config?.isEnabled, ); // Prevent disabling when the storage class is enabled and all @@ -87,7 +92,7 @@ export const StorageClassesTableRow: React.FC = ({ return false; }, [ - storageClass.metadata.name, + metadata.name, storageClassConfig?.isDefault, storageClassConfig?.isEnabled, storageClassConfigMap, @@ -95,7 +100,7 @@ export const StorageClassesTableRow: React.FC = ({ const onDefaultRadioChange = React.useCallback(async () => { const existingDefaultConfigMap = Object.entries(storageClassConfigMap).find( - ([name, config]) => storageClass.metadata.name !== name && config?.isDefault, + ([name, config]) => metadata.name !== name && config?.isDefault, ); if (existingDefaultConfigMap) { @@ -103,15 +108,48 @@ export const StorageClassesTableRow: React.FC = ({ await updateStorageClassConfig(name, { ...config, isDefault: false }); refresh(); } - }, [storageClass.metadata.name, storageClassConfigMap, refresh]); + }, [metadata.name, storageClassConfigMap, refresh]); return ( - {storageClassConfig?.displayName}} - description={storageClassConfig?.description} - /> + {hasReadableConfig ? ( + { + editModalAlertRef.current = { + title: + 'Edit the invalid field(s) and save your changes to correct the corrupted metadata.', + }; + setIsEditModalOpen(true); + }} + > + + + } + /> + } + > + {isValidConfigValue('displayName', storageClassConfig.displayName) && + (!storageClassConfig.description || + isValidConfigValue('description', storageClassConfig.description)) && ( + {storageClassConfig.displayName} + } + description={storageClassConfig.description} + /> + )} + + ) : ( + '-' + )} @@ -151,40 +189,114 @@ export const StorageClassesTableRow: React.FC = ({ - + {hasReadableConfig ? ( + + } + > + {isValidConfigValue('isEnabled', storageClassConfig.isEnabled) && ( + + )} + + ) : ( + '-' + )} - + {hasReadableConfig ? ( + + } + > + {isValidConfigValue('isDefault', storageClassConfig.isDefault) && ( + + )} + + ) : ( + '-' + )} - {storageClassConfig?.lastModified ? ( - + {hasReadableConfig ? ( + + } + > + {isValidConfigValue('lastModified', storageClassConfig.lastModified) && ( + + )} + ) : ( - 'Unknown' + '-' )} - setIsEditModalOpen(true), - }, - ]} - /> + + {!hasReadableConfig && ( + + )} + + { + editModalAlertRef.current = !hasReadableConfig + ? { + title: 'Reset the metadata', + children: + 'Correct any invalid fields and save your changes to reset the corrupted metadata. Upon saving, Display name and Last modified will receive valid values, and the Enable and Default parameters will be reset to their default values.', + } + : undefined; + setIsEditModalOpen(true); + }, + }, + ]} + /> + {isEditModalOpen && ( @@ -192,8 +304,20 @@ export const StorageClassesTableRow: React.FC = ({ storageClass={storageClass} onSuccess={refresh} onClose={() => setIsEditModalOpen(false)} + alert={editModalAlertRef.current} /> )} ); }; + +const StrorageClassConfigValue: React.FC = ({ + alert, + children, +}) => { + if (!children) { + return alert; + } + + return children; +}; diff --git a/frontend/src/pages/storageClasses/utils.ts b/frontend/src/pages/storageClasses/utils.ts index 3619f8315d..64b5fb5fda 100644 --- a/frontend/src/pages/storageClasses/utils.ts +++ b/frontend/src/pages/storageClasses/utils.ts @@ -1,3 +1,4 @@ +import { isValidDate } from '@patternfly/react-core'; import { MetadataAnnotation, StorageClassConfig, StorageClassKind } from '~/k8sTypes'; export const getStorageClassConfig = ( @@ -16,3 +17,21 @@ export const getStorageClassConfig = ( export const isOpenshiftDefaultStorageClass = (storageClass: StorageClassKind): boolean => storageClass.metadata.annotations?.[MetadataAnnotation.StorageClassIsDefault] === 'true'; + +export const isValidConfigValue = ( + configKey: keyof StorageClassConfig, + value: string | boolean | undefined, +): boolean => { + switch (configKey) { + case 'displayName': + case 'description': + return !!value && typeof value === 'string'; + case 'isEnabled': + case 'isDefault': + return typeof value === 'boolean'; + case 'lastModified': + return typeof value === 'string' && isValidDate(new Date(value)); + default: + return false; + } +};