Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upcoming: [DI-21694] - Added Resource Select component to the Create-alert form #11331

4 changes: 2 additions & 2 deletions packages/api-v4/src/cloudpulse/alerts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Alert>(
setURL(
`${API_ROOT}/monitor/services/${encodeURIComponent(
service_type!
serviceType!
)}/alert-definitions`
),
setMethod('POST'),
Expand Down
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-11331-added-1732627930598.md
Original file line number Diff line number Diff line change
@@ -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))
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export const AlertDefinitionLanding = () => {
/>
<Route
component={() => <CreateAlertDefinition />}
path="/monitor/cloudpulse/alerts/definitions/create"
path="/monitor/alerts/definitions/create"
/>
</Switch>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ export const AlertsLanding = React.memo(() => {
<Switch>
<Route
component={AlertDefinitionLanding}
exact
path={'/monitor/alerts/definitions'}
/>
<Redirect from="*" to="/monitor/alerts/definitions" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -33,34 +34,33 @@ 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,
};

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<CreateAlertDefinitionForm>({
defaultValues: initialValues,
Expand All @@ -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');
jaalah-akamai marked this conversation as resolved.
Show resolved Hide resolved
const onSubmit = handleSubmit(async (values) => {
try {
await createAlert(filterFormValues(values));
Expand Down Expand Up @@ -140,9 +140,15 @@ export const CreateAlertDefinition = () => {
control={control}
name="description"
/>
<CloudPulseServiceSelect name="service_type" />
{serviceWatcher === 'dbaas' && <EngineOption name="engine_type" />}
<CloudPulseServiceSelect name="serviceType" />
{serviceWatcher === 'dbaas' && <EngineOption name="engineType" />}
<CloudPulseRegionSelect name="region" />
<CloudPulseMultiResourceSelect
engine={watch('engineType')}
name="resource_ids"
region={watch('region')}
serviceType={serviceWatcher}
/>
<CloudPulseAlertSeveritySelect name="severity" />
<ActionsPanel
primaryButtonProps={{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ import { EngineOption } from './EngineOption';
describe('EngineOption component tests', () => {
it('should render the component when resource type is dbaas', () => {
const { getByLabelText, getByTestId } = renderWithThemeAndHookFormContext({
component: <EngineOption name={'engine_type'} />,
component: <EngineOption name={'engineType'} />,
});
expect(getByLabelText('Engine Option')).toBeInTheDocument();
expect(getByTestId('engine-option')).toBeInTheDocument();
});
it('should render the options happy path', async () => {
const user = userEvent.setup();
renderWithThemeAndHookFormContext({
component: <EngineOption name={'engine_type'} />,
component: <EngineOption name={'engineType'} />,
});
user.click(screen.getByRole('button', { name: 'Open' }));
expect(await screen.findByRole('option', { name: 'MySQL' }));
Expand All @@ -26,7 +26,7 @@ describe('EngineOption component tests', () => {
it('should be able to select an option', async () => {
const user = userEvent.setup();
renderWithThemeAndHookFormContext({
component: <EngineOption name={'engine_type'} />,
component: <EngineOption name={'engineType'} />,
});
user.click(screen.getByRole('button', { name: 'Open' }));
await user.click(await screen.findByRole('option', { name: 'MySQL' }));
Expand Down
Original file line number Diff line number Diff line change
@@ -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: (
<CloudPulseMultiResourceSelect
engine="mysql"
name="resource_ids"
region={undefined}
serviceType={null}
/>
),
});
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: (
<CloudPulseMultiResourceSelect
engine="mysql"
name="resource_ids"
region="us-east"
serviceType="linode"
/>
),
});
user.click(screen.getByRole('button', { name: 'Open' }));
expect(
await screen.findByRole('option', {
name: 'linode-3',
})
).toBeInTheDocument();
expect(
screen.getByRole('option', {
name: 'linode-4',
jaalah-akamai marked this conversation as resolved.
Show resolved Hide resolved
})
).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: (
<CloudPulseMultiResourceSelect
engine="mysql"
name="resource_ids"
region="us-east"
serviceType="linode"
/>
),
});
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: (
<CloudPulseMultiResourceSelect
engine="mysql"
name="resource_ids"
region="us-east"
serviceType="linode"
/>
),
});
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: (
<CloudPulseMultiResourceSelect
engine="mysql"
name="resource_ids"
region="us-east"
serviceType="linode"
/>
),
});
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: (
<CloudPulseMultiResourceSelect
engine="mysql"
name="resource_ids"
region="us-east"
serviceType="dbaas"
/>
),
});
expect(getByLabelText('Cluster'));
Copy link
Contributor

@coliu-akamai coliu-akamai Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will need to update this test depending on if label is changed to Clusters ^

and a small nitpick: if this test does need to be updated, could we also put a linebreak between each test while at it for better readability?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops looks like I submitted this comment early - meant to include it as part of my below review

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah @coliu-akamai , Clusters makes sense. I will make the changes and I'll add a linebreak too. Thank you for the approval

});
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: (
<CloudPulseMultiResourceSelect
engine="mysql"
name="resource_ids"
region="us-east"
serviceType="linode"
/>
),
});
expect(
screen.getByText('Failed to fetch the resources.')
).toBeInTheDocument();
});
});
Loading