From 153356ab4bf9b24438ac57dc8ad974ff48b769f3 Mon Sep 17 00:00:00 2001 From: YanJin Date: Tue, 6 Feb 2024 12:21:40 +0100 Subject: [PATCH 1/4] ARTESCA-11299: prevent create a new storage service if it's already exist --- src/react/locations/LocationEditor.tsx | 8 +-- .../__tests__/LocationEditor.test.tsx | 49 ++++++++++++++++++- src/react/utils/storageOptions.ts | 7 +++ 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/react/locations/LocationEditor.tsx b/src/react/locations/LocationEditor.tsx index cda25f64d..f4e71bd70 100644 --- a/src/react/locations/LocationEditor.tsx +++ b/src/react/locations/LocationEditor.tsx @@ -62,7 +62,8 @@ function LocationEditor() { } = useAccountsLocationsAndEndpoints({ accountsLocationsEndpointsAdapter, }); - const locationEditing = accountsLocationsAndEndpoints?.locations.find( + const locations = accountsLocationsAndEndpoints?.locations; + const locationEditing = locations?.find( (location) => location.name === locationName, ); const capabilities = useSelector( @@ -73,9 +74,10 @@ function LocationEditor() { convertToForm({ ...newLocationDetails(), ...locationEditing }), ); const selectOptions = useMemo(() => { + if(!locations) return []; //@ts-expect-error fix this when you are working on it - return selectStorageOptions(capabilities, makeLabel, !editingExisting); - }, [capabilities, editingExisting]); + return selectStorageOptions(capabilities, locations, makeLabel, !editingExisting); + }, [capabilities, editingExisting, locations]); useMemo(() => { if (locationEditing) { setLocation(convertToForm(locationEditing)); diff --git a/src/react/locations/__tests__/LocationEditor.test.tsx b/src/react/locations/__tests__/LocationEditor.test.tsx index 27f7e840c..49c4734a7 100644 --- a/src/react/locations/__tests__/LocationEditor.test.tsx +++ b/src/react/locations/__tests__/LocationEditor.test.tsx @@ -3,20 +3,28 @@ import LocationEditor from '../LocationEditor'; import { fireEvent, + render, screen, + waitFor, waitForElementToBeRemoved, } from '@testing-library/react'; import { notFalsyTypeGuard } from '../../../types/typeGuards'; import { TEST_API_BASE_URL, + Wrapper, mockOffsetSize, reduxRender, selectClick, } from '../../utils/testUtil'; import { setupServer } from 'msw/node'; -import { getConfigOverlay } from '../../../js/mock/managementClientMSWHandlers'; +import { + ENDPOINTS, + USERS, + getConfigOverlay, +} from '../../../js/mock/managementClientMSWHandlers'; import { INSTANCE_ID } from '../../actions/__tests__/utils/testUtil'; import userEvent from '@testing-library/user-event'; +import { rest } from 'msw'; const server = setupServer(getConfigOverlay(TEST_API_BASE_URL, INSTANCE_ID)); @@ -69,4 +77,43 @@ describe('LocationEditor', () => { ).toBe(locationName); }); }); + const selectors = { + loadingLocation: () => screen.getByText('Loading location...'), + locationType: () => screen.getByLabelText(/location type \*/i), + }; + + it('should hide the artesca storage service if it is already created', async () => { + //S + server.use( + rest.get( + `${TEST_API_BASE_URL}/api/v1/config/overlay/view/${INSTANCE_ID}`, + (_, res, ctx) => + res( + ctx.json({ + locations: { + 'us-east-2': { + details: { + bootstrapList: [ + 'artesca-storage-service-hdservice-proxy.xcore.svc:18888', + ], + repoId: null, + }, + locationType: 'location-scality-hdclient-v2', + name: 'us-east-2', + objectId: '22f31240-4bd3-11ee-98b3-1e5b6f897bc7', + }, + }, + }), + ), + ), + ); + render(, { wrapper: Wrapper }); + await waitForElementToBeRemoved(() => selectors.loadingLocation()); + //E + selectClick(selectors.locationType()); + //V + await waitFor(() => { + expect(screen.queryByText('Storage Service for ARTESCA')).toBeNull(); + }); + }); }); diff --git a/src/react/utils/storageOptions.ts b/src/react/utils/storageOptions.ts index e77058ce7..46bf4883e 100644 --- a/src/react/utils/storageOptions.ts +++ b/src/react/utils/storageOptions.ts @@ -105,11 +105,18 @@ export const getLocationTypeShort = ( export function selectStorageOptions( capabilities: Pick, + locations: Location[], labelFn?: LabelFunction, exceptHidden = true, ): Array { + const hdLocation = locations?.find( + (l) => l.type === 'location-scality-hdclient-v2', + ); return Object.keys(storageOptions) .filter((o) => { + if (hdLocation && o === 'location-scality-hdclient-v2') { + return false; + } if (exceptHidden) { const hidden = !!storageOptions[o].hidden; From 863ed429a34c913f7fbf7621192ff9ff6f610cf4 Mon Sep 17 00:00:00 2001 From: YanJin Date: Tue, 6 Feb 2024 13:00:31 +0100 Subject: [PATCH 2/4] ARTESCA-11299: disabled edit location for hd location --- src/react/locations/LocationEditor.tsx | 10 ++++-- src/react/locations/LocationsList.tsx | 20 +++++++++--- .../__tests__/LocationEditor.test.tsx | 6 +--- src/react/utils/storageOptions.ts | 12 +++---- src/types/stats.ts | 31 ++++++++++--------- 5 files changed, 45 insertions(+), 34 deletions(-) diff --git a/src/react/locations/LocationEditor.tsx b/src/react/locations/LocationEditor.tsx index f4e71bd70..fde8596d8 100644 --- a/src/react/locations/LocationEditor.tsx +++ b/src/react/locations/LocationEditor.tsx @@ -74,9 +74,13 @@ function LocationEditor() { convertToForm({ ...newLocationDetails(), ...locationEditing }), ); const selectOptions = useMemo(() => { - if(!locations) return []; - //@ts-expect-error fix this when you are working on it - return selectStorageOptions(capabilities, locations, makeLabel, !editingExisting); + if (!locations) return []; + return selectStorageOptions( + capabilities, + locations, + makeLabel, + !editingExisting, + ); }, [capabilities, editingExisting, locations]); useMemo(() => { if (locationEditing) { diff --git a/src/react/locations/LocationsList.tsx b/src/react/locations/LocationsList.tsx index aeea45c1b..f758c5031 100644 --- a/src/react/locations/LocationsList.tsx +++ b/src/react/locations/LocationsList.tsx @@ -138,6 +138,9 @@ const ActionButtons = ({ ); }; + const isEditButtonDisabled = + rowValues.isBuiltin || rowValues.type === 'location-scality-hdclient-v2'; + return (
history.push(`/locations/${locationName}/edit`)} type="button" aria-label="Edit Location" - tooltip={{ - overlay: 'Edit Location', - placement: 'top', - }} - disabled={rowValues.isBuiltin} + tooltip={ + isEditButtonDisabled + ? { + overlay: 'Edit Location is disabled for this location', + placement: 'top', + } + : { + overlay: 'Edit Location', + placement: 'top', + } + } + disabled={isEditButtonDisabled} /> } diff --git a/src/react/locations/__tests__/LocationEditor.test.tsx b/src/react/locations/__tests__/LocationEditor.test.tsx index 49c4734a7..e5f939060 100644 --- a/src/react/locations/__tests__/LocationEditor.test.tsx +++ b/src/react/locations/__tests__/LocationEditor.test.tsx @@ -17,11 +17,7 @@ import { selectClick, } from '../../utils/testUtil'; import { setupServer } from 'msw/node'; -import { - ENDPOINTS, - USERS, - getConfigOverlay, -} from '../../../js/mock/managementClientMSWHandlers'; +import { getConfigOverlay } from '../../../js/mock/managementClientMSWHandlers'; import { INSTANCE_ID } from '../../actions/__tests__/utils/testUtil'; import userEvent from '@testing-library/user-event'; import { rest } from 'msw'; diff --git a/src/react/utils/storageOptions.ts b/src/react/utils/storageOptions.ts index 46bf4883e..b2da27dda 100644 --- a/src/react/utils/storageOptions.ts +++ b/src/react/utils/storageOptions.ts @@ -1,4 +1,4 @@ -import type { InstanceStateSnapshot } from '../../types/stats'; +import type { Capabilities, InstanceStateSnapshot } from '../../types/stats'; import type { LabelFunction, StorageOptionSelect, @@ -8,7 +8,6 @@ import { JAGUAR_S3_ENDPOINT, JAGUAR_S3_LOCATION_KEY, Location as LegacyLocation, - Locations, ORANGE_S3_ENDPOINT, ORANGE_S3_LOCATION_KEY, OUTSCALE_PUBLIC_S3_ENDPOINT, @@ -20,6 +19,7 @@ import { LocationForm } from '../../types/location'; import { Location } from '../next-architecture/domain/entities/location'; import { LocationInfo } from '../next-architecture/adapters/accounts-locations/ILocationsAdapter'; import { LocationV1 } from '../../js/managementClient/api'; + export function checkSupportsReplicationTarget( locations: LocationInfo[], ): boolean { @@ -104,13 +104,13 @@ export const getLocationTypeShort = ( }; export function selectStorageOptions( - capabilities: Pick, - locations: Location[], + capabilities: Capabilities, + locations: LocationInfo[], labelFn?: LabelFunction, exceptHidden = true, ): Array { - const hdLocation = locations?.find( - (l) => l.type === 'location-scality-hdclient-v2', + const hdLocation = locations.find( + (l) => l.type === LocationV1.LocationTypeEnum.ScalityHdclientV2, ); return Object.keys(storageOptions) .filter((o) => { diff --git a/src/types/stats.ts b/src/types/stats.ts index 80b8368c5..8e9f59c33 100644 --- a/src/types/stats.ts +++ b/src/types/stats.ts @@ -72,22 +72,23 @@ export type MetricsUnit = { 'crr-schedule'?: CrrScheduleUnit; 'ingest-schedule'?: IngestScheduleUnit; }; +export type Capabilities = { + readonly locationTypeCephRadosGW: boolean; + readonly locationTypeDigitalOcean: boolean; + readonly locationTypeHyperdriveV2: boolean; + readonly locationTypeLocal: boolean; + readonly locationTypeNFS: boolean; + readonly locationTypeS3Custom: boolean; + readonly locationTypeSproxyd: boolean; + readonly managedLifecycle: boolean; + readonly managedLifecycleTransition: boolean; + readonly preferredReadLocation: boolean; + readonly s3cIngestLocation: boolean; + readonly secureChannel: boolean; + readonly secureChannelOptimizedPath: boolean; +}; export type InstanceStateSnapshot = { - readonly capabilities: { - readonly locationTypeCephRadosGW: boolean; - readonly locationTypeDigitalOcean: boolean; - readonly locationTypeHyperdriveV2: boolean; - readonly locationTypeLocal: boolean; - readonly locationTypeNFS: boolean; - readonly locationTypeS3Custom: boolean; - readonly locationTypeSproxyd: boolean; - readonly managedLifecycle: boolean; - readonly managedLifecycleTransition: boolean; - readonly preferredReadLocation: boolean; - readonly s3cIngestLocation: boolean; - readonly secureChannel: boolean; - readonly secureChannelOptimizedPath: boolean; - }; + readonly capabilities: Capabilities; readonly latestConfigurationOverlay: ConfigurationOverlay; readonly runningConfigurationVersion: number; readonly lastSeen: string; From e02d4837ea1d87963e4d86deb898430c82a5d710 Mon Sep 17 00:00:00 2001 From: YanJin Date: Tue, 2 Jul 2024 11:50:09 +0200 Subject: [PATCH 3/4] ARTESCA-11299: In case of no location created, display all the storage options --- src/react/locations/LocationEditor.tsx | 1 - src/react/utils/storageOptions.ts | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/react/locations/LocationEditor.tsx b/src/react/locations/LocationEditor.tsx index fde8596d8..791acd682 100644 --- a/src/react/locations/LocationEditor.tsx +++ b/src/react/locations/LocationEditor.tsx @@ -74,7 +74,6 @@ function LocationEditor() { convertToForm({ ...newLocationDetails(), ...locationEditing }), ); const selectOptions = useMemo(() => { - if (!locations) return []; return selectStorageOptions( capabilities, locations, diff --git a/src/react/utils/storageOptions.ts b/src/react/utils/storageOptions.ts index b2da27dda..81457db5c 100644 --- a/src/react/utils/storageOptions.ts +++ b/src/react/utils/storageOptions.ts @@ -105,11 +105,11 @@ export const getLocationTypeShort = ( export function selectStorageOptions( capabilities: Capabilities, - locations: LocationInfo[], + locations?: LocationInfo[], labelFn?: LabelFunction, exceptHidden = true, ): Array { - const hdLocation = locations.find( + const hdLocation = locations?.find( (l) => l.type === LocationV1.LocationTypeEnum.ScalityHdclientV2, ); return Object.keys(storageOptions) From 894e452bc90f2420cfab86894d0d8dd4ad03bec4 Mon Sep 17 00:00:00 2001 From: YanJin Date: Mon, 1 Jul 2024 15:31:35 +0200 Subject: [PATCH 4/4] ARTESCA-11299: Add test to prevent the edition of Artesca default location --- src/js/mock/managementClientMSWHandlers.ts | 17 +++-- .../__tests__/LocationEditor.test.tsx | 4 +- .../locations/__tests__/LocationList.test.tsx | 65 +++++++++++++++++++ .../__tests__/TransitionForm.test.tsx | 18 ++--- 4 files changed, 86 insertions(+), 18 deletions(-) create mode 100644 src/react/locations/__tests__/LocationList.test.tsx diff --git a/src/js/mock/managementClientMSWHandlers.ts b/src/js/mock/managementClientMSWHandlers.ts index f02b6a7b7..1cefd7873 100644 --- a/src/js/mock/managementClientMSWHandlers.ts +++ b/src/js/mock/managementClientMSWHandlers.ts @@ -144,17 +144,22 @@ export const LOCATIONS = { name: 'ring-nick', objectId: '99a06f79-c62c-11ec-b993-7e8a0ab79998', }, - 'us-east-1': { - isBuiltin: true, - locationType: 'location-file-v1', - name: 'us-east-1', - objectId: '95dbedf5-9888-11ec-8565-1ac2af7d1e53', - }, [azureblobstorage]: { locationType: 'location-azure-v1', name: azureblobstorage, details: {}, }, + 'us-east-1': { + details: { + bootstrapList: [ + 'artesca-storage-service-hdservice-proxy.xcore.svc:18888', + ], + repoId: null, + }, + locationType: 'location-scality-hdclient-v2', + name: 'us-east-1', + objectId: '22f31240-4bd3-11ee-98b3-1e5b6f897bc7', + }, }; export const ENDPOINTS = [ diff --git a/src/react/locations/__tests__/LocationEditor.test.tsx b/src/react/locations/__tests__/LocationEditor.test.tsx index e5f939060..d837c587f 100644 --- a/src/react/locations/__tests__/LocationEditor.test.tsx +++ b/src/react/locations/__tests__/LocationEditor.test.tsx @@ -49,13 +49,11 @@ describe('LocationEditor', () => { ); await selectClick(selector); await userEvent.keyboard('{arrowup}'); - expect( container.querySelector('.sc-select__option--is-focused')?.textContent, - ).toBe('Storage Service for ARTESCA'); + ).toBe('Scality ARTESCA S3'); [ - 'Scality ARTESCA S3', 'Scality RING with S3 Connector', 'Amazon S3', 'Google Cloud Storage', diff --git a/src/react/locations/__tests__/LocationList.test.tsx b/src/react/locations/__tests__/LocationList.test.tsx new file mode 100644 index 000000000..27ed87e32 --- /dev/null +++ b/src/react/locations/__tests__/LocationList.test.tsx @@ -0,0 +1,65 @@ +import { + screen, + waitFor, + waitForElementToBeRemoved, + within, +} from '@testing-library/react'; +import { setupServer } from 'msw/node'; +import { rest } from 'msw'; +import { + getConfigOverlay, + getStorageConsumptionMetricsHandlers, +} from '../../../js/mock/managementClientMSWHandlers'; +import { + TEST_API_BASE_URL, + mockOffsetSize, + renderWithRouterMatch, + zenkoUITestConfig, +} from '../../utils/testUtil'; +import { INSTANCE_ID } from '../../actions/__tests__/utils/testUtil'; +import { LocationsList } from '../LocationsList'; + +const server = setupServer( + getConfigOverlay(TEST_API_BASE_URL, INSTANCE_ID), + ...getStorageConsumptionMetricsHandlers( + zenkoUITestConfig.managementEndpoint, + INSTANCE_ID, + ), + rest.get( + `${TEST_API_BASE_URL}/api/v1/instance/${INSTANCE_ID}/status`, + (req, res, ctx) => res(ctx.json({})), + ), +); + +describe('LocationList', () => { + beforeAll(() => { + jest.setTimeout(30_000); + mockOffsetSize(500, 100); + server.listen({ onUnhandledRequest: 'error' }); + }); + afterEach(() => { + server.resetHandlers(); + }); + afterAll(() => { + server.close(); + }); + it('should disable the delete button for default location', async () => { + //S + renderWithRouterMatch(); + //E + await waitForElementToBeRemoved(() => [ + ...screen.queryAllByText(/Loading/i), + ]); + const defaultArtescaLocationRow = screen.getByRole('row', { + name: /us-east-1 Storage Service for ARTESCA/i, + }); + //V + await waitFor(() => { + expect( + within(defaultArtescaLocationRow).getByRole('button', { + name: /Edit Location/i, + }), + ).toBeDisabled(); + }); + }); +}); diff --git a/src/react/workflow/__tests__/TransitionForm.test.tsx b/src/react/workflow/__tests__/TransitionForm.test.tsx index 15bd25d5d..9d2989451 100644 --- a/src/react/workflow/__tests__/TransitionForm.test.tsx +++ b/src/react/workflow/__tests__/TransitionForm.test.tsx @@ -200,7 +200,7 @@ describe('TransitionForm', () => { await waitFor(() => screen.getByRole('option', { name: new RegExp( - `${notVersionedBucket} \\(us-east-1 / Local Filesystem \\)`, + `${notVersionedBucket} \\(us-east-1 / Storage Service \\)`, 'i', ), }), @@ -208,7 +208,7 @@ describe('TransitionForm', () => { await userEvent.click( screen.getByRole('option', { name: new RegExp( - `${notVersionedBucket} \\(us-east-1 / Local Filesystem \\)`, + `${notVersionedBucket} \\(us-east-1 / Storage Service \\)`, 'i', ), }), @@ -233,7 +233,7 @@ describe('TransitionForm', () => { await waitFor(() => screen.getByRole('option', { name: new RegExp( - `${versionedBucket} \\(us-east-1 / Local Filesystem \\)`, + `${versionedBucket} \\(us-east-1 / Storage Service \\)`, 'i', ), }), @@ -241,7 +241,7 @@ describe('TransitionForm', () => { await userEvent.click( screen.getByRole('option', { name: new RegExp( - `${versionedBucket} \\(us-east-1 / Local Filesystem \\)`, + `${versionedBucket} \\(us-east-1 / Storage Service \\)`, 'i', ), }), @@ -336,7 +336,7 @@ describe('TransitionForm', () => { await waitFor(() => screen.getByRole('option', { name: new RegExp( - `${notVersionedBucket} \\(us-east-1 / Local Filesystem \\)`, + `${notVersionedBucket} \\(us-east-1 / Storage Service \\)`, 'i', ), }), @@ -344,7 +344,7 @@ describe('TransitionForm', () => { await userEvent.click( screen.getByRole('option', { name: new RegExp( - `${notVersionedBucket} \\(us-east-1 / Local Filesystem \\)`, + `${notVersionedBucket} \\(us-east-1 / Storage Service \\)`, 'i', ), }), @@ -384,7 +384,7 @@ describe('TransitionForm', () => { await waitFor(() => screen.getByRole('option', { name: new RegExp( - `${notVersionedBucket} \\(us-east-1 / Local Filesystem \\)`, + `${notVersionedBucket} \\(us-east-1 / Storage Service \\)`, 'i', ), }), @@ -392,7 +392,7 @@ describe('TransitionForm', () => { await userEvent.click( screen.getByRole('option', { name: new RegExp( - `${notVersionedBucket} \\(us-east-1 / Local Filesystem \\)`, + `${notVersionedBucket} \\(us-east-1 / Storage Service \\)`, 'i', ), }), @@ -435,7 +435,7 @@ describe('TransitionForm', () => { userEvent.click( screen.getByRole('option', { name: new RegExp( - `${versionedBucket} \\(us-east-1 / Local Filesystem \\)`, + `${versionedBucket} \\(us-east-1 / Storage Service \\)`, 'i', ), }),