From e812d13673dfdde5752518a15bb65bd577cf8d65 Mon Sep 17 00:00:00 2001 From: santoshp210-akamai <159890961+santoshp210-akamai@users.noreply.github.com> Date: Tue, 26 Nov 2024 15:12:50 +0530 Subject: [PATCH 1/7] upcoming: [DI-21694] - Added the Resources Select component, UT for that, converted few variables from snake case to camel case --- packages/api-v4/src/cloudpulse/alerts.ts | 4 +- .../AlertsLanding/AlertsDefinitionLanding.tsx | 2 +- .../Alerts/AlertsLanding/AlertsLanding.tsx | 1 - .../CreateAlertDefinition.test.tsx | 1 + .../CreateAlert/CreateAlertDefinition.tsx | 26 ++- .../GeneralInformation/EngineOption.test.tsx | 6 +- .../ResourceMultiSelect.test.tsx | 220 ++++++++++++++++++ .../ResourceMultiSelect.tsx | 102 ++++++++ .../ServiceTypeSelect.test.tsx | 8 +- .../CloudPulse/Alerts/CreateAlert/schemas.ts | 4 +- .../CloudPulse/Alerts/CreateAlert/types.ts | 5 +- .../Alerts/CreateAlert/utilities.ts | 11 +- .../manager/src/queries/cloudpulse/alerts.ts | 4 +- 13 files changed, 362 insertions(+), 32 deletions(-) create mode 100644 packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx create mode 100644 packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx diff --git a/packages/api-v4/src/cloudpulse/alerts.ts b/packages/api-v4/src/cloudpulse/alerts.ts index f7c4b732043..3c6f909b9db 100644 --- a/packages/api-v4/src/cloudpulse/alerts.ts +++ b/packages/api-v4/src/cloudpulse/alerts.ts @@ -5,12 +5,12 @@ import { BETA_API_ROOT as API_ROOT } from 'src/constants'; export const createAlertDefinition = ( data: CreateAlertDefinitionPayload, - service_type: AlertServiceType + serviceType: AlertServiceType ) => Request( setURL( `${API_ROOT}/monitor/services/${encodeURIComponent( - service_type! + serviceType! )}/alert-definitions` ), setMethod('POST'), diff --git a/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsDefinitionLanding.tsx b/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsDefinitionLanding.tsx index f6be4ae8b84..352ded4e3e2 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsDefinitionLanding.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsDefinitionLanding.tsx @@ -14,7 +14,7 @@ export const AlertDefinitionLanding = () => { /> } - path="/monitor/cloudpulse/alerts/definitions/create" + path="/monitor/alerts/definitions/create" /> ); diff --git a/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsLanding.tsx b/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsLanding.tsx index 50aaa6fa994..718050113c1 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsLanding.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/AlertsLanding/AlertsLanding.tsx @@ -84,7 +84,6 @@ export const AlertsLanding = React.memo(() => { diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.test.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.test.tsx index 35d4e4ad328..c8342ee6293 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.test.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.test.tsx @@ -34,5 +34,6 @@ describe('AlertDefinition Create', () => { expect(getByText('Severity is required.')).toBeVisible(); expect(getByText('Service is required.')).toBeVisible(); expect(getByText('Region is required.')).toBeVisible(); + expect(getByText('At least one resource is needed.')).toBeVisible(); }); }); diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx index 4b7cb07bd0f..36cb657f87d 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx @@ -12,6 +12,7 @@ import { useCreateAlertDefinition } from 'src/queries/cloudpulse/alerts'; import { CloudPulseAlertSeveritySelect } from './GeneralInformation/AlertSeveritySelect'; import { EngineOption } from './GeneralInformation/EngineOption'; import { CloudPulseRegionSelect } from './GeneralInformation/RegionSelect'; +import { CloudPulseMultiResourceSelect } from './GeneralInformation/ResourceMultiSelect'; import { CloudPulseServiceSelect } from './GeneralInformation/ServiceTypeSelect'; import { CreateAlertDefinitionFormSchema } from './schemas'; import { filterFormValues, filterMetricCriteriaFormValues } from './utilities'; @@ -33,14 +34,14 @@ const criteriaInitialValues: MetricCriteriaForm = { }; const initialValues: CreateAlertDefinitionForm = { channel_ids: [], - engine_type: null, + engineType: null, label: '', region: '', resource_ids: [], rule_criteria: { rules: filterMetricCriteriaFormValues(criteriaInitialValues), }, - service_type: null, + serviceType: null, severity: null, triggerCondition: triggerConditionInitialValues, }; @@ -48,19 +49,18 @@ const initialValues: CreateAlertDefinitionForm = { const overrides = [ { label: 'Definitions', - linkTo: '/monitor/cloudpulse/alerts/definitions', + linkTo: '/monitor/alerts/definitions', position: 1, }, { label: 'Details', - linkTo: `/monitor/cloudpulse/alerts/definitions/create`, + linkTo: `/monitor/alerts/definitions/create`, position: 2, }, ]; export const CreateAlertDefinition = () => { const history = useHistory(); - const alertCreateExit = () => - history.push('/monitor/cloudpulse/alerts/definitions'); + const alertCreateExit = () => history.push('/monitor/alerts/definitions'); const formMethods = useForm({ defaultValues: initialValues, @@ -78,10 +78,10 @@ export const CreateAlertDefinition = () => { } = formMethods; const { enqueueSnackbar } = useSnackbar(); const { mutateAsync: createAlert } = useCreateAlertDefinition( - getValues('service_type')! + getValues('serviceType')! ); - const serviceWatcher = watch('service_type'); + const serviceWatcher = watch('serviceType'); const onSubmit = handleSubmit(async (values) => { try { await createAlert(filterFormValues(values)); @@ -140,10 +140,16 @@ export const CreateAlertDefinition = () => { control={control} name="description" /> - - {serviceWatcher === 'dbaas' && } + + {serviceWatcher === 'dbaas' && } + { it('should render the component when resource type is dbaas', () => { const { getByLabelText, getByTestId } = renderWithThemeAndHookFormContext({ - component: , + component: , }); expect(getByLabelText('Engine Option')).toBeInTheDocument(); expect(getByTestId('engine-option')).toBeInTheDocument(); @@ -17,7 +17,7 @@ describe('EngineOption component tests', () => { it('should render the options happy path', async () => { const user = userEvent.setup(); renderWithThemeAndHookFormContext({ - component: , + component: , }); user.click(screen.getByRole('button', { name: 'Open' })); expect(await screen.findByRole('option', { name: 'MySQL' })); @@ -26,7 +26,7 @@ describe('EngineOption component tests', () => { it('should be able to select an option', async () => { const user = userEvent.setup(); renderWithThemeAndHookFormContext({ - component: , + component: , }); user.click(screen.getByRole('button', { name: 'Open' })); await user.click(await screen.findByRole('option', { name: 'MySQL' })); diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx new file mode 100644 index 00000000000..c6a331dfd7a --- /dev/null +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx @@ -0,0 +1,220 @@ +import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import * as React from 'react'; + +import { linodeFactory } from 'src/factories'; +import { renderWithThemeAndHookFormContext } from 'src/utilities/testHelpers'; + +import { CloudPulseMultiResourceSelect } from './ResourceMultiSelect'; + +const queryMocks = vi.hoisted(() => ({ + useResourcesQuery: vi.fn().mockReturnValue({}), +})); + +vi.mock('src/queries/cloudpulse/resources', async () => { + const actual = await vi.importActual('src/queries/cloudpulse/resources'); + return { + ...actual, + useResourcesQuery: queryMocks.useResourcesQuery, + }; +}); +const SELECT_ALL = 'Select All'; +const ARIA_SELECTED = 'aria-selected'; +describe('ResourceMultiSelect component tests', () => { + it('should render disabled component if the props are undefined or regions and service type does not have any values', () => { + queryMocks.useResourcesQuery.mockReturnValue({ + data: linodeFactory.buildList(2), + isError: false, + isLoading: false, + status: 'success', + }); + const { + getByPlaceholderText, + getByTestId, + } = renderWithThemeAndHookFormContext({ + component: ( + + ), + }); + expect(getByTestId('resource-select')).toBeInTheDocument(); + expect(getByPlaceholderText('Select Resources')).toBeInTheDocument(); + }); + it('should render resources happy path', async () => { + const user = userEvent.setup(); + queryMocks.useResourcesQuery.mockReturnValue({ + data: linodeFactory.buildList(2), + isError: false, + isLoading: false, + status: 'success', + }); + renderWithThemeAndHookFormContext({ + component: ( + + ), + }); + user.click(screen.getByRole('button', { name: 'Open' })); + expect( + await screen.findByRole('option', { + name: 'linode-3', + }) + ).toBeInTheDocument(); + expect( + screen.getByRole('option', { + name: 'linode-4', + }) + ).toBeInTheDocument(); + }); + it('should be able to select all resources', async () => { + const user = userEvent.setup(); + queryMocks.useResourcesQuery.mockReturnValue({ + data: linodeFactory.buildList(2), + isError: false, + isLoading: false, + status: 'success', + }); + renderWithThemeAndHookFormContext({ + component: ( + + ), + }); + user.click(await screen.findByRole('button', { name: 'Open' })); + await user.click(await screen.findByRole('option', { name: SELECT_ALL })); + expect( + await screen.findByRole('option', { + name: 'linode-5', + }) + ).toHaveAttribute(ARIA_SELECTED, 'true'); + expect( + screen.getByRole('option', { + name: 'linode-6', + }) + ).toHaveAttribute(ARIA_SELECTED, 'true'); + }); + it('should be able to deselect the selected resources', async () => { + const user = userEvent.setup(); + queryMocks.useResourcesQuery.mockReturnValue({ + data: linodeFactory.buildList(2), + isError: false, + isLoading: false, + status: 'success', + }); + renderWithThemeAndHookFormContext({ + component: ( + + ), + }); + user.click(screen.getByRole('button', { name: 'Open' })); + await user.click(await screen.findByRole('option', { name: SELECT_ALL })); + await user.click( + await screen.findByRole('option', { name: 'Deselect All' }) + ); + expect( + await screen.findByRole('option', { + name: 'linode-7', + }) + ).toHaveAttribute(ARIA_SELECTED, 'false'); + expect( + screen.getByRole('option', { + name: 'linode-8', + }) + ).toHaveAttribute(ARIA_SELECTED, 'false'); + }); + + it('should select multiple resources', async () => { + const user = userEvent.setup(); + queryMocks.useResourcesQuery.mockReturnValue({ + data: linodeFactory.buildList(3), + isError: false, + isLoading: false, + status: 'success', + }); + renderWithThemeAndHookFormContext({ + component: ( + + ), + }); + user.click(screen.getByRole('button', { name: 'Open' })); + await user.click(await screen.findByRole('option', { name: 'linode-9' })); + await user.click(await screen.findByRole('option', { name: 'linode-10' })); + + expect( + await screen.findByRole('option', { + name: 'linode-9', + }) + ).toHaveAttribute(ARIA_SELECTED, 'true'); + expect( + screen.getByRole('option', { + name: 'linode-10', + }) + ).toHaveAttribute(ARIA_SELECTED, 'true'); + expect( + screen.getByRole('option', { + name: 'linode-11', + }) + ).toHaveAttribute(ARIA_SELECTED, 'false'); + expect( + screen.getByRole('option', { + name: 'Select All', + }) + ).toHaveAttribute(ARIA_SELECTED, 'false'); + }); + it('should render the label as cluster when resource is of dbaas type', () => { + const { getByLabelText } = renderWithThemeAndHookFormContext({ + component: ( + + ), + }); + expect(getByLabelText('Cluster')); + }); + it('should render error messages when there is an API call failure', () => { + queryMocks.useResourcesQuery.mockReturnValue({ + data: undefined, + isError: true, + isLoading: false, + status: 'error', + }); + renderWithThemeAndHookFormContext({ + component: ( + + ), + }); + expect( + screen.getByText('Failed to fetch the resources.') + ).toBeInTheDocument(); + }); +}); diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx new file mode 100644 index 00000000000..998648fadfd --- /dev/null +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx @@ -0,0 +1,102 @@ +import * as React from 'react'; +import { Controller, useFormContext } from 'react-hook-form'; + +import { Autocomplete } from 'src/components/Autocomplete/Autocomplete'; +import { useResourcesQuery } from 'src/queries/cloudpulse/resources'; + +import type { Item } from '../../constants'; +import type { CreateAlertDefinitionForm } from '../types'; +import type { AlertServiceType } from '@linode/api-v4'; +import type { FieldPathByValue } from 'react-hook-form'; + +interface CloudPulseResourceSelectProps { + /** + * engine option type selected by the user + */ + engine: null | string; + /** + * name used for the component to set in the form + */ + name: FieldPathByValue; + /** + * region selected by the user + */ + region: string | undefined; + /** + * service type selected by the user + */ + serviceType: AlertServiceType | null; +} + +export const CloudPulseMultiResourceSelect = ( + props: CloudPulseResourceSelectProps +) => { + const { engine, name, region, serviceType } = { ...props }; + const { control, setValue } = useFormContext(); + + const { data: resources, isError, isLoading } = useResourcesQuery( + Boolean(region && serviceType), + serviceType?.toString(), + {}, + engine !== null ? { engine, region } : { region } + ); + + const getResourcesList = (): Item[] => { + return resources && resources.length > 0 + ? resources.map((resource) => ({ + label: resource.label, + value: resource.id, + })) + : []; + }; + + React.useEffect(() => { + setValue('resource_ids', []); + }, [region, serviceType, engine, setValue]); + + return ( + ( + { + const resourceIds = resources.map((resource) => resource.value); + field.onChange(resourceIds); + }} + textFieldProps={{ + InputProps: { + sx: { + maxHeight: '55px', + overflow: 'auto', + }, + }, + }} + value={ + field.value + ? getResourcesList().filter((resource) => + field.value.includes(resource.value) + ) + : [] + } + isOptionEqualToValue={(option, value) => option.value === value.value} + autoHighlight + clearOnBlur + data-testid="resource-select" + disabled={!Boolean(region && serviceType)} + label={serviceType === 'dbaas' ? 'Cluster' : 'Resources'} + limitTags={2} + loading={isLoading && Boolean(region && serviceType)} + multiple + onBlur={field.onBlur} + options={getResourcesList()} + placeholder="Select Resources" + /> + )} + control={control} + name={name} + /> + ); +}; diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ServiceTypeSelect.test.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ServiceTypeSelect.test.tsx index 5e14b886375..42e3f9e1e68 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ServiceTypeSelect.test.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ServiceTypeSelect.test.tsx @@ -41,7 +41,7 @@ queryMocks.useCloudPulseServiceTypes.mockReturnValue({ describe('ServiceTypeSelect component tests', () => { it('should render the Autocomplete component', () => { const { getAllByText, getByTestId } = renderWithThemeAndHookFormContext({ - component: , + component: , }); expect(getByTestId('servicetype-select')).toBeInTheDocument(); getAllByText('Service'); @@ -49,7 +49,7 @@ describe('ServiceTypeSelect component tests', () => { it('should render service types happy path', async () => { renderWithThemeAndHookFormContext({ - component: , + component: , }); userEvent.click(screen.getByRole('button', { name: 'Open' })); expect( @@ -66,7 +66,7 @@ describe('ServiceTypeSelect component tests', () => { it('should be able to select a service type', async () => { renderWithThemeAndHookFormContext({ - component: , + component: , }); userEvent.click(screen.getByRole('button', { name: 'Open' })); await userEvent.click( @@ -81,7 +81,7 @@ describe('ServiceTypeSelect component tests', () => { isLoading: false, }); renderWithThemeAndHookFormContext({ - component: , + component: , }); expect( screen.getByText('Failed to fetch the service types.') diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/schemas.ts b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/schemas.ts index 2e8fee3e200..5bbe4289000 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/schemas.ts +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/schemas.ts @@ -8,8 +8,8 @@ const engineOptionValidation = string().when('service_type', { }); export const CreateAlertDefinitionFormSchema = createAlertDefinitionSchema.concat( object({ - engine_type: engineOptionValidation, + engineType: engineOptionValidation, region: string().required('Region is required.'), - service_type: string().required('Service is required.').nullable(), + serviceType: string().required('Service is required.').nullable(), }) ); diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/types.ts b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/types.ts index c7c58fc1dcb..3f7377c5064 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/types.ts +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/types.ts @@ -9,9 +9,10 @@ import type { export interface CreateAlertDefinitionForm extends Omit { - engine_type: null | string; + engineType: null | string; region: string; - service_type: AlertServiceType | null; + resource_ids: string[]; + serviceType: AlertServiceType | null; severity: AlertSeverityType | null; } diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts index 3db313f7b1f..c2c9025d800 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts @@ -11,21 +11,22 @@ export const filterFormValues = ( formValues: CreateAlertDefinitionForm ): CreateAlertDefinitionPayload => { const values = omitProps(formValues, [ - 'service_type', + 'serviceType', 'region', - 'engine_type', + 'engineType', 'severity', ]); // severity has a need for null in the form for edge-cases, so null-checking and returning it as an appropriate type const severity = formValues.severity!; - return { ...values, severity }; + const resourceIds = formValues.resource_ids!; + return { ...values, resource_ids: resourceIds, severity }; }; export const filterMetricCriteriaFormValues = ( formValues: MetricCriteriaForm ): MetricCriteria[] => { - const aggregation_type = formValues.aggregation_type!; + const aggregationType = formValues.aggregation_type!; const operator = formValues.operator!; const values = omitProps(formValues, ['aggregation_type', 'operator']); - return [{ ...values, aggregation_type, operator }]; + return [{ ...values, aggregation_type: aggregationType, operator }]; }; diff --git a/packages/manager/src/queries/cloudpulse/alerts.ts b/packages/manager/src/queries/cloudpulse/alerts.ts index 4406a90e0c2..0da27a07093 100644 --- a/packages/manager/src/queries/cloudpulse/alerts.ts +++ b/packages/manager/src/queries/cloudpulse/alerts.ts @@ -10,10 +10,10 @@ import type { } from '@linode/api-v4/lib/cloudpulse'; import type { APIError } from '@linode/api-v4/lib/types'; -export const useCreateAlertDefinition = (service_type: AlertServiceType) => { +export const useCreateAlertDefinition = (serviceType: AlertServiceType) => { const queryClient = useQueryClient(); return useMutation({ - mutationFn: (data) => createAlertDefinition(data, service_type), + mutationFn: (data) => createAlertDefinition(data, serviceType), onSuccess() { queryClient.invalidateQueries(queryFactory.alerts); }, From c8b3e52fe5d2b9fec5da1c07e0b63a76a6d00c30 Mon Sep 17 00:00:00 2001 From: santoshp210-akamai <159890961+santoshp210-akamai@users.noreply.github.com> Date: Tue, 26 Nov 2024 17:55:13 +0530 Subject: [PATCH 2/7] upcoming: [DI-21694] - Added memoization to the resources fetching method --- .../GeneralInformation/ResourceMultiSelect.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx index 998648fadfd..0371b21ff76 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx @@ -41,14 +41,14 @@ export const CloudPulseMultiResourceSelect = ( engine !== null ? { engine, region } : { region } ); - const getResourcesList = (): Item[] => { + const getResourcesList = React.useMemo((): Item[] => { return resources && resources.length > 0 ? resources.map((resource) => ({ label: resource.label, value: resource.id, })) : []; - }; + }, [resources]); React.useEffect(() => { setValue('resource_ids', []); @@ -69,29 +69,29 @@ export const CloudPulseMultiResourceSelect = ( textFieldProps={{ InputProps: { sx: { - maxHeight: '55px', + maxHeight: '60px', overflow: 'auto', }, }, }} value={ field.value - ? getResourcesList().filter((resource) => + ? getResourcesList.filter((resource) => field.value.includes(resource.value) ) : [] } - isOptionEqualToValue={(option, value) => option.value === value.value} autoHighlight clearOnBlur data-testid="resource-select" disabled={!Boolean(region && serviceType)} + isOptionEqualToValue={(option, value) => option.value === value.value} label={serviceType === 'dbaas' ? 'Cluster' : 'Resources'} limitTags={2} loading={isLoading && Boolean(region && serviceType)} multiple onBlur={field.onBlur} - options={getResourcesList()} + options={getResourcesList} placeholder="Select Resources" /> )} From a6c58077fed1c14751688058cb645dab51af7b5c Mon Sep 17 00:00:00 2001 From: santoshp210-akamai <159890961+santoshp210-akamai@users.noreply.github.com> Date: Tue, 26 Nov 2024 18:42:07 +0530 Subject: [PATCH 3/7] upcoming: [DI-21694] - Rendering resources component before Severity --- .../CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx index 36cb657f87d..7e9f7676f4a 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx @@ -143,13 +143,13 @@ export const CreateAlertDefinition = () => { {serviceWatcher === 'dbaas' && } - + Date: Tue, 26 Nov 2024 19:02:10 +0530 Subject: [PATCH 4/7] Added changeset: ResourceMultiSelect component, along with UT. Changed case for few variables and properties --- packages/manager/.changeset/pr-11331-added-1732627930598.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 packages/manager/.changeset/pr-11331-added-1732627930598.md diff --git a/packages/manager/.changeset/pr-11331-added-1732627930598.md b/packages/manager/.changeset/pr-11331-added-1732627930598.md new file mode 100644 index 00000000000..6a2f1d24a13 --- /dev/null +++ b/packages/manager/.changeset/pr-11331-added-1732627930598.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Added +--- + +ResourceMultiSelect component, along with UT. Changed case for few variables and properties ([#11331](https://github.com/linode/manager/pull/11331)) From 2fc877aa044befd008c9db8e24907d9c69d39d5c Mon Sep 17 00:00:00 2001 From: santoshp210-akamai <159890961+santoshp210-akamai@users.noreply.github.com> Date: Wed, 27 Nov 2024 19:07:47 +0530 Subject: [PATCH 5/7] upcoming: [DI-21694] - Removed the styling for the Autocomplete in ResourceMultiSelect and modified and added few properties for the Alert interfaces as per the latest API-spec --- packages/api-v4/src/cloudpulse/types.ts | 5 +++-- .../manager/src/factories/cloudpulse/alerts.ts | 3 ++- .../Alerts/CreateAlert/CreateAlertDefinition.tsx | 4 ++-- .../ResourceMultiSelect.test.tsx | 14 +++++++------- .../GeneralInformation/ResourceMultiSelect.tsx | 12 ++---------- .../CloudPulse/Alerts/CreateAlert/types.ts | 2 +- .../CloudPulse/Alerts/CreateAlert/utilities.ts | 4 ++-- packages/validation/src/cloudpulse.schema.ts | 2 +- 8 files changed, 20 insertions(+), 26 deletions(-) diff --git a/packages/api-v4/src/cloudpulse/types.ts b/packages/api-v4/src/cloudpulse/types.ts index 6a863076a73..4b64bf16c30 100644 --- a/packages/api-v4/src/cloudpulse/types.ts +++ b/packages/api-v4/src/cloudpulse/types.ts @@ -143,7 +143,7 @@ export interface ServiceTypesList { export interface CreateAlertDefinitionPayload { label: string; description?: string; - resource_ids?: string[]; + entity_ids?: string[]; severity: AlertSeverityType; rule_criteria: { rules: MetricCriteria[]; @@ -174,11 +174,12 @@ export interface Alert { id: number; label: string; description: string; + has_more_resources: boolean; status: AlertStatusType; type: AlertDefinitionType; severity: AlertSeverityType; service_type: AlertServiceType; - resource_ids: string[]; + entity_ids: string[]; rule_criteria: { rules: MetricCriteria[]; }; diff --git a/packages/manager/src/factories/cloudpulse/alerts.ts b/packages/manager/src/factories/cloudpulse/alerts.ts index 4fbf4e0e222..a0bc2b6edf7 100644 --- a/packages/manager/src/factories/cloudpulse/alerts.ts +++ b/packages/manager/src/factories/cloudpulse/alerts.ts @@ -7,9 +7,10 @@ export const alertFactory = Factory.Sync.makeFactory({ created: new Date().toISOString(), created_by: 'user1', description: '', + entity_ids: ['0', '1', '2', '3'], + has_more_resources: true, id: Factory.each((i) => i), label: Factory.each((id) => `Alert-${id}`), - resource_ids: ['0', '1', '2', '3'], rule_criteria: { rules: [], }, diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx index 7e9f7676f4a..2a43b89c1f1 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx @@ -35,9 +35,9 @@ const criteriaInitialValues: MetricCriteriaForm = { const initialValues: CreateAlertDefinitionForm = { channel_ids: [], engineType: null, + entity_ids: [], label: '', region: '', - resource_ids: [], rule_criteria: { rules: filterMetricCriteriaFormValues(criteriaInitialValues), }, @@ -145,7 +145,7 @@ export const CreateAlertDefinition = () => { diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx index c6a331dfd7a..0698948651d 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx @@ -35,7 +35,7 @@ describe('ResourceMultiSelect component tests', () => { component: ( @@ -56,7 +56,7 @@ describe('ResourceMultiSelect component tests', () => { component: ( @@ -86,7 +86,7 @@ describe('ResourceMultiSelect component tests', () => { component: ( @@ -117,7 +117,7 @@ describe('ResourceMultiSelect component tests', () => { component: ( @@ -152,7 +152,7 @@ describe('ResourceMultiSelect component tests', () => { component: ( @@ -188,7 +188,7 @@ describe('ResourceMultiSelect component tests', () => { component: ( @@ -207,7 +207,7 @@ describe('ResourceMultiSelect component tests', () => { component: ( diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx index 0371b21ff76..38fcb766842 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx @@ -51,8 +51,8 @@ export const CloudPulseMultiResourceSelect = ( }, [resources]); React.useEffect(() => { - setValue('resource_ids', []); - }, [region, serviceType, engine, setValue]); + setValue(name, []); + }, [region, serviceType, engine, setValue, name]); return ( resource.value); field.onChange(resourceIds); }} - textFieldProps={{ - InputProps: { - sx: { - maxHeight: '60px', - overflow: 'auto', - }, - }, - }} value={ field.value ? getResourcesList.filter((resource) => diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/types.ts b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/types.ts index 3f7377c5064..844b47639a0 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/types.ts +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/types.ts @@ -10,8 +10,8 @@ import type { export interface CreateAlertDefinitionForm extends Omit { engineType: null | string; + entity_ids: string[]; region: string; - resource_ids: string[]; serviceType: AlertServiceType | null; severity: AlertSeverityType | null; } diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts index c2c9025d800..7459ca1c5da 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts @@ -18,8 +18,8 @@ export const filterFormValues = ( ]); // severity has a need for null in the form for edge-cases, so null-checking and returning it as an appropriate type const severity = formValues.severity!; - const resourceIds = formValues.resource_ids!; - return { ...values, resource_ids: resourceIds, severity }; + const entityIds = formValues.entity_ids; + return { ...values, entity_ids: entityIds, severity }; }; export const filterMetricCriteriaFormValues = ( diff --git a/packages/validation/src/cloudpulse.schema.ts b/packages/validation/src/cloudpulse.schema.ts index e8b7e63c414..18c5b59886b 100644 --- a/packages/validation/src/cloudpulse.schema.ts +++ b/packages/validation/src/cloudpulse.schema.ts @@ -30,7 +30,7 @@ const triggerCondition = object({ export const createAlertDefinitionSchema = object({ label: string().required('Name is required.'), description: string().optional(), - resource_ids: array().of(string()).min(1, 'At least one resource is needed.'), + entity_ids: array().of(string()).min(1, 'At least one resource is needed.'), severity: string().required('Severity is required.').nullable(), criteria: array() .of(metricCriteria) From 93c1c549f664c2a49f7dc988309e2ce632437743 Mon Sep 17 00:00:00 2001 From: santoshp210-akamai <159890961+santoshp210-akamai@users.noreply.github.com> Date: Wed, 27 Nov 2024 22:08:50 +0530 Subject: [PATCH 6/7] upcoming: [DI-21694] - Addressed the review changes: Made the values dynamic in the Unit Test, replaced the variable name from serviceWatcher to serviceTypeWatcher, added comment addressing need for useEffect() --- .../CreateAlert/CreateAlertDefinition.tsx | 6 +-- .../ResourceMultiSelect.test.tsx | 41 +++++++++++-------- .../ResourceMultiSelect.tsx | 4 ++ 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx index 2a43b89c1f1..61a6822075e 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx @@ -81,7 +81,7 @@ export const CreateAlertDefinition = () => { getValues('serviceType')! ); - const serviceWatcher = watch('serviceType'); + const serviceTypeWatcher = watch('serviceType'); const onSubmit = handleSubmit(async (values) => { try { await createAlert(filterFormValues(values)); @@ -141,13 +141,13 @@ export const CreateAlertDefinition = () => { name="description" /> - {serviceWatcher === 'dbaas' && } + {serviceTypeWatcher === 'dbaas' && } { it('should render disabled component if the props are undefined or regions and service type does not have any values', () => { + const mockLinodes = linodeFactory.buildList(2); queryMocks.useResourcesQuery.mockReturnValue({ - data: linodeFactory.buildList(2), + data: mockLinodes, isError: false, isLoading: false, status: 'success', @@ -46,8 +47,9 @@ describe('ResourceMultiSelect component tests', () => { }); it('should render resources happy path', async () => { const user = userEvent.setup(); + const mockLinodes = linodeFactory.buildList(2); queryMocks.useResourcesQuery.mockReturnValue({ - data: linodeFactory.buildList(2), + data: mockLinodes, isError: false, isLoading: false, status: 'success', @@ -65,19 +67,20 @@ describe('ResourceMultiSelect component tests', () => { user.click(screen.getByRole('button', { name: 'Open' })); expect( await screen.findByRole('option', { - name: 'linode-3', + name: mockLinodes[0].label, }) ).toBeInTheDocument(); expect( screen.getByRole('option', { - name: 'linode-4', + name: mockLinodes[1].label, }) ).toBeInTheDocument(); }); it('should be able to select all resources', async () => { const user = userEvent.setup(); + const mockLinodes = linodeFactory.buildList(2); queryMocks.useResourcesQuery.mockReturnValue({ - data: linodeFactory.buildList(2), + data: mockLinodes, isError: false, isLoading: false, status: 'success', @@ -96,19 +99,20 @@ describe('ResourceMultiSelect component tests', () => { await user.click(await screen.findByRole('option', { name: SELECT_ALL })); expect( await screen.findByRole('option', { - name: 'linode-5', + name: mockLinodes[0].label, }) ).toHaveAttribute(ARIA_SELECTED, 'true'); expect( screen.getByRole('option', { - name: 'linode-6', + name: mockLinodes[0].label, }) ).toHaveAttribute(ARIA_SELECTED, 'true'); }); it('should be able to deselect the selected resources', async () => { const user = userEvent.setup(); + const mockLinodes = linodeFactory.buildList(2); queryMocks.useResourcesQuery.mockReturnValue({ - data: linodeFactory.buildList(2), + data: mockLinodes, isError: false, isLoading: false, status: 'success', @@ -130,20 +134,21 @@ describe('ResourceMultiSelect component tests', () => { ); expect( await screen.findByRole('option', { - name: 'linode-7', + name: mockLinodes[0].label, }) ).toHaveAttribute(ARIA_SELECTED, 'false'); expect( screen.getByRole('option', { - name: 'linode-8', + name: mockLinodes[1].label, }) ).toHaveAttribute(ARIA_SELECTED, 'false'); }); it('should select multiple resources', async () => { const user = userEvent.setup(); + const mockLinodes = linodeFactory.buildList(3); queryMocks.useResourcesQuery.mockReturnValue({ - data: linodeFactory.buildList(3), + data: mockLinodes, isError: false, isLoading: false, status: 'success', @@ -159,22 +164,26 @@ describe('ResourceMultiSelect component tests', () => { ), }); user.click(screen.getByRole('button', { name: 'Open' })); - await user.click(await screen.findByRole('option', { name: 'linode-9' })); - await user.click(await screen.findByRole('option', { name: 'linode-10' })); + await user.click( + await screen.findByRole('option', { name: mockLinodes[0].label }) + ); + await user.click( + await screen.findByRole('option', { name: mockLinodes[1].label }) + ); expect( await screen.findByRole('option', { - name: 'linode-9', + name: mockLinodes[0].label, }) ).toHaveAttribute(ARIA_SELECTED, 'true'); expect( screen.getByRole('option', { - name: 'linode-10', + name: mockLinodes[1].label, }) ).toHaveAttribute(ARIA_SELECTED, 'true'); expect( screen.getByRole('option', { - name: 'linode-11', + name: mockLinodes[2].label, }) ).toHaveAttribute(ARIA_SELECTED, 'false'); expect( diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx index 38fcb766842..04f205f38a5 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx @@ -50,6 +50,10 @@ export const CloudPulseMultiResourceSelect = ( : []; }, [resources]); + /* useEffect is used here to reset the value of entity_ids back to [] when the region, engine, serviceType props are changed , + as the options to the Autocomplete component are dependent on those props , the values of the Autocomplete won't match with the given options that are passed + and this may raise a warning or error with the isOptionEqualToValue prop in the Autocomplete. + */ React.useEffect(() => { setValue(name, []); }, [region, serviceType, engine, setValue, name]); From cfe24f6cd40455155371d6e0d0fcc526fe499541 Mon Sep 17 00:00:00 2001 From: santoshp210-akamai <159890961+santoshp210-akamai@users.noreply.github.com> Date: Wed, 27 Nov 2024 23:50:39 +0530 Subject: [PATCH 7/7] upcoming: [DI-21694] - Addressed the review changes: Changed the Label to Clusters in case of Database service type --- .../GeneralInformation/ResourceMultiSelect.test.tsx | 7 ++++++- .../CreateAlert/GeneralInformation/ResourceMultiSelect.tsx | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx index 1e4c22c7bc7..d89fe9d3a24 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.test.tsx @@ -45,6 +45,7 @@ describe('ResourceMultiSelect component tests', () => { expect(getByTestId('resource-select')).toBeInTheDocument(); expect(getByPlaceholderText('Select Resources')).toBeInTheDocument(); }); + it('should render resources happy path', async () => { const user = userEvent.setup(); const mockLinodes = linodeFactory.buildList(2); @@ -76,6 +77,7 @@ describe('ResourceMultiSelect component tests', () => { }) ).toBeInTheDocument(); }); + it('should be able to select all resources', async () => { const user = userEvent.setup(); const mockLinodes = linodeFactory.buildList(2); @@ -108,6 +110,7 @@ describe('ResourceMultiSelect component tests', () => { }) ).toHaveAttribute(ARIA_SELECTED, 'true'); }); + it('should be able to deselect the selected resources', async () => { const user = userEvent.setup(); const mockLinodes = linodeFactory.buildList(2); @@ -192,6 +195,7 @@ describe('ResourceMultiSelect component tests', () => { }) ).toHaveAttribute(ARIA_SELECTED, 'false'); }); + it('should render the label as cluster when resource is of dbaas type', () => { const { getByLabelText } = renderWithThemeAndHookFormContext({ component: ( @@ -203,8 +207,9 @@ describe('ResourceMultiSelect component tests', () => { /> ), }); - expect(getByLabelText('Cluster')); + expect(getByLabelText('Clusters')); }); + it('should render error messages when there is an API call failure', () => { queryMocks.useResourcesQuery.mockReturnValue({ data: undefined, diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx index 04f205f38a5..fc19b4335f9 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ResourceMultiSelect.tsx @@ -82,7 +82,7 @@ export const CloudPulseMultiResourceSelect = ( data-testid="resource-select" disabled={!Boolean(region && serviceType)} isOptionEqualToValue={(option, value) => option.value === value.value} - label={serviceType === 'dbaas' ? 'Cluster' : 'Resources'} + label={serviceType === 'dbaas' ? 'Clusters' : 'Resources'} limitTags={2} loading={isLoading && Boolean(region && serviceType)} multiple