diff --git a/packages/manager/.changeset/pr-11773-upcoming-features-1741023864014.md b/packages/manager/.changeset/pr-11773-upcoming-features-1741023864014.md new file mode 100644 index 00000000000..32b7255939f --- /dev/null +++ b/packages/manager/.changeset/pr-11773-upcoming-features-1741023864014.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Upcoming Features +--- + +UI enhancements across Alerting feature: Notification Error message, Limits for Metrics, Dimensions, Notifications, and other UI enhancements with relevant unit test cases([#11773](https://github.com/linode/manager/pull/11773)) diff --git a/packages/manager/cypress/e2e/core/cloudpulse/create-user-alert.spec.ts b/packages/manager/cypress/e2e/core/cloudpulse/create-user-alert.spec.ts index 281e69dd407..e02eca8cb0b 100644 --- a/packages/manager/cypress/e2e/core/cloudpulse/create-user-alert.spec.ts +++ b/packages/manager/cypress/e2e/core/cloudpulse/create-user-alert.spec.ts @@ -190,11 +190,11 @@ describe('Create Alert', () => { cy.visitWithLogin('monitor/alerts/definitions/create'); // Enter Name and Description - cy.findByPlaceholderText('Enter Name') + cy.findByPlaceholderText('Enter a Name') .should('be.visible') .type(customAlertDefinition.label); - cy.findByPlaceholderText('Enter Description') + cy.findByPlaceholderText('Enter a Description') .should('be.visible') .type(customAlertDefinition.description ?? ''); @@ -227,7 +227,7 @@ describe('Create Alert', () => { const cpuUsageMetricDetails = { aggregationType: 'Average', dataField: 'CPU Utilization', - operator: '==', + operator: '=', ruleIndex: 0, threshold: '1000', }; @@ -273,7 +273,7 @@ describe('Create Alert', () => { const memoryUsageMetricDetails = { aggregationType: 'Average', dataField: 'Memory Usage', - operator: '==', + operator: '=', ruleIndex: 1, threshold: '1000', }; diff --git a/packages/manager/src/features/CloudPulse/Alerts/AlertsListing/AlertListTable.test.tsx b/packages/manager/src/features/CloudPulse/Alerts/AlertsListing/AlertListTable.test.tsx index 19a58afc4f3..3713e35e014 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/AlertsListing/AlertListTable.test.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/AlertsListing/AlertListTable.test.tsx @@ -21,11 +21,16 @@ queryMocks.useEditAlertDefinition.mockReturnValue({ mutateAsync: vi.fn().mockResolvedValue({}), reset: vi.fn(), }); - +const mockScroll = vi.fn(); describe('Alert List Table test', () => { it('should render the alert landing table ', async () => { const { getByText } = renderWithTheme( - + ); expect(getByText('Alert Name')).toBeVisible(); expect(getByText('Service')).toBeVisible(); @@ -40,6 +45,7 @@ describe('Alert List Table test', () => { alerts={[]} error={[{ reason: 'Error in fetching the alerts' }]} isLoading={false} + scrollToElement={mockScroll} services={[]} /> ); @@ -60,6 +66,7 @@ describe('Alert List Table test', () => { }), ]} isLoading={false} + scrollToElement={mockScroll} services={[{ label: 'Linode', value: 'linode' }]} /> ); @@ -82,6 +89,7 @@ describe('Alert List Table test', () => { ); @@ -98,6 +106,7 @@ describe('Alert List Table test', () => { ); @@ -118,6 +127,7 @@ describe('Alert List Table test', () => { ); @@ -139,6 +149,7 @@ describe('Alert List Table test', () => { ); diff --git a/packages/manager/src/features/CloudPulse/Alerts/AlertsListing/AlertListTable.tsx b/packages/manager/src/features/CloudPulse/Alerts/AlertsListing/AlertListTable.tsx index 7ed0692caad..ffead91fe1c 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/AlertsListing/AlertListTable.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/AlertsListing/AlertListTable.tsx @@ -33,6 +33,10 @@ export interface AlertsListTableProps { * A boolean indicating whether the alerts are loading */ isLoading: boolean; + /** + * Callback to scroll to the button element on page change + */ + scrollToElement: () => void; /** * The list of services to display in the table */ @@ -40,7 +44,7 @@ export interface AlertsListTableProps { } export const AlertsListTable = React.memo((props: AlertsListTableProps) => { - const { alerts, error, isLoading, services } = props; + const { alerts, error, isLoading, scrollToElement, services } = props; const _error = error ? getAPIErrorOrDefault(error, 'Error in fetching the alerts.') : undefined; @@ -85,7 +89,7 @@ export const AlertsListTable = React.memo((props: AlertsListTableProps) => { {({ data: orderedData, handleOrderChange, order, orderBy }) => ( @@ -109,11 +113,16 @@ export const AlertsListTable = React.memo((props: AlertsListTableProps) => { {AlertListingTableLabelMap.map((value) => ( { + if (order) { + handleOrderChange(orderBy, order); + handlePageChange(1); + } + }} active={orderBy === value.label} data-qa-header={value.label} data-qa-sorting={value.label} direction={order} - handleClick={handleOrderChange} key={value.label} label={value.label} noWrap @@ -147,12 +156,24 @@ export const AlertsListTable = React.memo((props: AlertsListTableProps) => { { + handlePageChange(page); + requestAnimationFrame(() => { + scrollToElement(); + }); + }} + handleSizeChange={(pageSize) => { + handlePageSizeChange(pageSize); + handlePageChange(1); + requestAnimationFrame(() => { + scrollToElement(); + }); + }} count={count} eventCategory="Alert Definitions Table" - handlePageChange={handlePageChange} - handleSizeChange={handlePageSizeChange} page={page} pageSize={pageSize} + sx={{ border: 0 }} /> )} diff --git a/packages/manager/src/features/CloudPulse/Alerts/AlertsListing/AlertListing.tsx b/packages/manager/src/features/CloudPulse/Alerts/AlertsListing/AlertListing.tsx index 76e354a1303..0fbd2921b46 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/AlertsListing/AlertListing.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/AlertsListing/AlertListing.tsx @@ -9,14 +9,16 @@ import { useAllAlertDefinitionsQuery } from 'src/queries/cloudpulse/alerts'; import { useCloudPulseServiceTypes } from 'src/queries/cloudpulse/services'; import { alertStatusOptions } from '../constants'; +import { scrollToElement } from '../Utils/AlertResourceUtils'; import { AlertsListTable } from './AlertListTable'; import type { Item } from '../constants'; import type { Alert, AlertServiceType, AlertStatusType } from '@linode/api-v4'; const searchAndSelectSx = { + lg: '250px', md: '300px', - sm: '500px', + sm: '400px', xs: '300px', }; @@ -30,6 +32,7 @@ export const AlertListing = () => { isLoading: serviceTypesLoading, } = useCloudPulseServiceTypes(true); + const topRef = React.useRef(null); const getServicesList = React.useMemo((): Item< string, AlertServiceType @@ -146,6 +149,7 @@ export const AlertListing = () => { flexWrap="wrap" gap={3} justifyContent="space-between" + ref={topRef} > { data-qa-filter="alert-service-filter" data-testid="alert-service-filter" label="" - limitTags={2} + limitTags={1} loading={serviceTypesLoading} multiple noMarginTop @@ -203,6 +207,7 @@ export const AlertListing = () => { data-qa-filter="alert-status-filter" data-testid="alert-status-filter" label="" + limitTags={1} multiple noMarginTop options={alertStatusOptions} @@ -219,7 +224,7 @@ export const AlertListing = () => { paddingBottom: 0, paddingTop: 0, whiteSpace: 'noWrap', - width: { md: '150px', xs: '200px' }, + width: { lg: '120px', md: '120px', sm: '150px', xs: '150px' }, }} buttonType="primary" data-qa-button="create-alert" @@ -233,6 +238,7 @@ export const AlertListing = () => { alerts={getAlertsList} error={error ?? undefined} isLoading={isLoading} + scrollToElement={() => scrollToElement(topRef.current ?? null)} services={getServicesList} /> 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 f9fe7a0e0f6..a7902921f52 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.test.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.test.tsx @@ -21,6 +21,7 @@ const queryMocks = vi.hoisted(() => ({ })); beforeEach(() => { + Element.prototype.scrollIntoView = vi.fn(); queryMocks.useResourcesQuery.mockReturnValue({ data: [], isError: false, @@ -79,9 +80,9 @@ describe('AlertDefinition Create', () => { await user.click( container.getByRole('button', { name: 'Add dimension filter' }) ); - const submitButton = container.getByText('Submit').closest('button'); + const submitButton = container.getByText('Submit'); await user.click(submitButton!); - expect(container.getAllByText('This field is required.').length).toBe(10); + expect(container.getAllByText('This field is required.').length).toBe(11); container.getAllByText(errorMessage).forEach((element) => { expect(element).toBeVisible(); }); @@ -101,5 +102,32 @@ describe('AlertDefinition Create', () => { expect( await container.findByText('The value should be a number.') ).toBeInTheDocument(); + + expect( + await container.findByText( + 'At least one notification channel is required.' + ) + ); + }); + + it('should validate the checks of Alert Name and Description', async () => { + const user = userEvent.setup(); + const container = renderWithTheme(); + const nameInput = container.getByLabelText('Name'); + const descriptionInput = container.getByLabelText('Description (optional)'); + await user.type(nameInput, '*#&+:<>"?@%'); + await user.type( + descriptionInput, + 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' + ); + await user.click(container.getByText('Submit')); + expect( + await container.findByText( + 'Name cannot contain special characters: * # & + : \< \> ? @ % { } \\ /.' + ) + ).toBeVisible(); + expect( + await container.findByText('Description must be 100 characters or less.') + ).toBeVisible(); }); }); diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx index 0971d003a6f..4d0158cf349 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx @@ -1,4 +1,5 @@ import { yupResolver } from '@hookform/resolvers/yup'; +import { isEmpty } from '@linode/api-v4'; import { Paper, TextField, Typography } from '@linode/ui'; import { useSnackbar } from 'notistack'; import * as React from 'react'; @@ -9,6 +10,7 @@ import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; import { Breadcrumb } from 'src/components/Breadcrumb/Breadcrumb'; import { DocumentTitleSegment } from 'src/components/DocumentTitle'; import { useCreateAlertDefinition } from 'src/queries/cloudpulse/alerts'; +import { scrollErrorIntoView } from 'src/utilities/scrollErrorIntoView'; import { MetricCriteriaField } from './Criteria/MetricCriteria'; import { TriggerConditions } from './Criteria/TriggerConditions'; @@ -77,8 +79,15 @@ export const CreateAlertDefinition = () => { CreateAlertDefinitionFormSchema as ObjectSchema ), }); + const { + control, + formState: { errors, isSubmitting, submitCount }, + getValues, + handleSubmit, + setError, + setValue, + } = formMethods; - const { control, formState, getValues, handleSubmit, setError } = formMethods; const { enqueueSnackbar } = useSnackbar(); const { mutateAsync: createAlert } = useCreateAlertDefinition( getValues('serviceType')! @@ -109,6 +118,26 @@ export const CreateAlertDefinition = () => { } }); + const previousSubmitCount = React.useRef(0); + React.useEffect(() => { + if (!isEmpty(errors) && submitCount > previousSubmitCount.current) { + scrollErrorIntoView(undefined, { behavior: 'smooth' }); + } + }, [errors, submitCount]); + + const handleServiceTypeChange = React.useCallback(() => { + // Reset the criteria to initial state + setValue('rule_criteria.rules', [ + { + aggregate_function: null, + dimension_filters: [], + metric: null, + operator: null, + threshold: 0, + }, + ]); + }, [setValue]); + return ( @@ -128,7 +157,7 @@ export const CreateAlertDefinition = () => { name="label" onBlur={field.onBlur} onChange={(e) => field.onChange(e.target.value)} - placeholder="Enter Name" + placeholder="Enter a Name" value={field.value ?? ''} /> )} @@ -144,14 +173,17 @@ export const CreateAlertDefinition = () => { onBlur={field.onBlur} onChange={(e) => field.onChange(e.target.value)} optional - placeholder="Enter Description" + placeholder="Enter a Description" value={field.value ?? ''} /> )} control={control} name="description" /> - + { { expect(container.queryByTestId(dimensionFilterID)).not.toBeInTheDocument() ); }); + it('should show tooltip when the max limit of dimension filters is reached', async () => { + const dimensionFilterValue: DimensionFilterForm = { + dimension_label: 'state', + operator: 'eq', + value: 'free', + }; + const { + getByText, + } = renderWithThemeAndHookFormContext({ + component: ( + + ), + useFormOptions: { + defaultValues: { + rule_criteria: { + rules: [ + { + ...mockData[0], + dimension_filters: Array(5).fill(dimensionFilterValue), + }, + ], + }, + serviceType: 'linode', + }, + }, + }); + const addButton = screen.getByRole('button', { + name: dimensionFilterButton, + }); + expect(addButton).toBeDisabled(); + userEvent.hover(addButton); + await waitFor(() => + expect( + getByText('You can add up to 5 dimension filters.') + ).toBeInTheDocument() + ); + }); }); diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/DimensionFilter.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/DimensionFilter.tsx index 95a16a1f587..0992d13374b 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/DimensionFilter.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/DimensionFilter.tsx @@ -1,7 +1,7 @@ import { Box } from '@linode/ui'; import { Button, Stack, Typography } from '@linode/ui'; import React from 'react'; -import { useFieldArray, useFormContext } from 'react-hook-form'; +import { useFieldArray, useFormContext, useWatch } from 'react-hook-form'; import { DimensionFilterField } from './DimensionFilterField'; @@ -31,6 +31,9 @@ export const DimensionFilters = (props: DimensionFilterProps) => { control, name, }); + + const dimensionFilterWatcher = useWatch({ control, name }); + return ( @@ -61,8 +64,11 @@ export const DimensionFilters = (props: DimensionFilterProps) => { buttonType="secondary" compactX data-qa-buttons="true" + disabled={dimensionFilterWatcher && dimensionFilterWatcher.length === 5} size="small" sx={{ justifyContent: 'start', width: '150px' }} + sxEndIcon={{ display: 'none' }} + tooltipText="You can add up to 5 dimension filters." > Add dimension filter diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/DimensionFilterField.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/DimensionFilterField.tsx index c2306ea0b09..0b5cb20fa1e 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/DimensionFilterField.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/DimensionFilterField.tsx @@ -86,11 +86,11 @@ export const DimensionFilterField = (props: DimensionFilterFieldProps) => { return ( { label="Data Field" onBlur={field.onBlur} options={dataFieldOptions} - placeholder="Select a Data field" + placeholder="Select a Data Field" /> )} control={control} @@ -147,6 +147,7 @@ export const DimensionFilterField = (props: DimensionFilterFieldProps) => { label="Operator" onBlur={field.onBlur} options={dimensionOperatorOptions} + placeholder="Select an Operator" /> )} control={control} diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/Metric.test.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/Metric.test.tsx index 95a825d6184..3c51d779c7a 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/Metric.test.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/Metric.test.tsx @@ -196,7 +196,7 @@ describe('Metric component tests', () => { expect( await container.findByRole('option', { name: '>' }) ).toBeInTheDocument(); - expect(container.getByRole('option', { name: '==' })).toBeInTheDocument(); + expect(container.getByRole('option', { name: '=' })).toBeInTheDocument(); expect(container.getByRole('option', { name: '<' })).toBeInTheDocument(); const option = await container.findByRole('option', { name: '>' }); await user.click(option); diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/Metric.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/Metric.tsx index 40c5f460626..c8ff0e0058c 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/Metric.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/Metric.tsx @@ -130,7 +130,7 @@ export const Metric = (props: MetricCriteriaProps) => { - + ( { name={`${name}.metric`} /> - + ( { name={`${name}.aggregate_function`} /> - + ( { name={`${name}.operator`} /> - + ( ) => event.target instanceof HTMLElement && event.target.blur() } + sx={{ + height: '34px', + }} data-qa-metric-threshold={`${name}-threshold`} data-qa-threshold="threshold" data-testid="threshold" @@ -264,7 +270,6 @@ export const Metric = (props: MetricCriteriaProps) => { noMarginTop onBlur={field.onBlur} onChange={(e) => field.onChange(e.target.value)} - sx={{ height: '34px', marginTop: { sm: 1, xs: 0 } }} type="number" value={field.value ?? 0} /> @@ -277,6 +282,7 @@ export const Metric = (props: MetricCriteriaProps) => { alignItems: 'flex-end', display: 'flex', height: '56px', + marginTop: { lg: '5px', md: '5px', sm: '5px' }, }} variant="body1" > diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/MetricCriteria.test.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/MetricCriteria.test.tsx index 4efd8ae0a29..4608d2eb10c 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/MetricCriteria.test.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/MetricCriteria.test.tsx @@ -245,4 +245,40 @@ describe('MetricCriteriaField', () => { expect(setMaxInterval).toBeCalledWith(firstOptionConvertedTime); }); + it('displays tooltip when the button is disabled', async () => { + const { + getByText, + } = renderWithThemeAndHookFormContext({ + component: ( + + ), + useFormOptions: { + defaultValues: { + rule_criteria: { + rules: [ + mockData.data[0], + mockData.data[0], + mockData.data[1], + mockData.data[1], + mockData.data[0], + ], + }, + }, + }, + }); + + const addButton = screen.getByRole('button', { + name: 'Add metric', + }); + + expect(addButton).toBeDisabled(); + userEvent.hover(addButton); + await waitFor(() => + expect(getByText('You can add up to 5 metrics.')).toBeInTheDocument() + ); + }); }); diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/MetricCriteria.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/MetricCriteria.tsx index 601a1d20315..20b5ae9bdcb 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/MetricCriteria.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/MetricCriteria.tsx @@ -64,6 +64,7 @@ export const MetricCriteriaField = (props: MetricCriteriaProps) => { control, name, }); + return ( ({ marginTop: theme.spacing(3) })}> { }) } buttonType="outlined" + disabled={metricCriteriaWatcher.length === 5} size="medium" sx={(theme) => ({ marginTop: theme.spacing(2) })} + sxEndIcon={{ display: 'none' }} + tooltipText="You can add up to 5 metrics." > Add metric diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/TriggerConditions.test.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/TriggerConditions.test.tsx index f9c58068090..ec0ce1b1592 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/TriggerConditions.test.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/TriggerConditions.test.tsx @@ -53,7 +53,8 @@ describe('Trigger Conditions', () => { const evaluationPeriodToolTip = within(evaluationPeriodContainer).getByRole( 'button', { - name: 'Choose how often you intend to evaluate the alert condition.', + name: + 'Defines the timeframe for collecting data in polling intervals to understand the service performance. Choose the data lookback period where the thresholds are applied to gather the information impactful for your business.', } ); const pollingIntervalContainer = container.getByTestId( @@ -62,8 +63,7 @@ describe('Trigger Conditions', () => { const pollingIntervalToolTip = within(pollingIntervalContainer).getByRole( 'button', { - name: - 'Defines the timeframe for collecting data in polling intervals to understand the service performance. Choose the data lookback period where the thresholds are applied to gather the information impactful for your business.', + name: 'Choose how often you intend to evaluate the alert condition.', } ); expect(evaluationPeriodToolTip).toBeInTheDocument(); diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/TriggerConditions.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/TriggerConditions.tsx index b8ea2d60399..b5804a79dc0 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/TriggerConditions.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/TriggerConditions.tsx @@ -79,7 +79,7 @@ export const TriggerConditions = (props: TriggerConditionProps) => { }} textFieldProps={{ labelTooltipText: - 'Choose how often you intend to evaluate the alert condition.', + 'Defines the timeframe for collecting data in polling intervals to understand the service performance. Choose the data lookback period where the thresholds are applied to gather the information impactful for your business.', }} value={ getEvaluationPeriodOptions().find( @@ -114,7 +114,7 @@ export const TriggerConditions = (props: TriggerConditionProps) => { }} textFieldProps={{ labelTooltipText: - 'Defines the timeframe for collecting data in polling intervals to understand the service performance. Choose the data lookback period where the thresholds are applied to gather the information impactful for your business.', + 'Choose how often you intend to evaluate the alert condition.', }} value={ getPollingIntervalOptions().find( @@ -127,7 +127,7 @@ export const TriggerConditions = (props: TriggerConditionProps) => { label="Polling Interval" onBlur={field.onBlur} options={getPollingIntervalOptions()} - placeholder="Select a Polling" + placeholder="Select a Polling Interval" /> )} control={control} @@ -136,49 +136,64 @@ export const TriggerConditions = (props: TriggerConditionProps) => { - + Trigger alert when all criteria are met for ( - - - event.target instanceof HTMLElement && event.target.blur() - } - sx={{ - height: '30px', - width: '30px', - }} - data-qa-trigger-occurrences - data-testid="trigger-occurences" - errorText={fieldState.error?.message} - label="" - min={0} - name={`${name}.trigger_occurrences`} - onBlur={field.onBlur} - onChange={(e) => field.onChange(e.target.value)} - type="number" - value={field.value ?? 0} - /> - + + event.target instanceof HTMLElement && event.target.blur() + } + sx={{ + height: '34px', + marginTop: { sm: '16px', xs: '0px' }, + width: '100px', + }} + data-qa-trigger-occurrences + data-testid="trigger-occurences" + errorText={fieldState.error?.message} + label="" + min={0} + name={`${name}.trigger_occurrences`} + noMarginTop + onBlur={field.onBlur} + onChange={(e) => field.onChange(e.target.value)} + type="number" + value={field.value ?? 0} + /> )} control={control} name={`${name}.trigger_occurrences`} /> - + consecutive occurrence(s). diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ServiceTypeSelect.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ServiceTypeSelect.tsx index a7f4783362a..a8716cb0b6f 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ServiceTypeSelect.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/ServiceTypeSelect.tsx @@ -10,6 +10,11 @@ import type { AlertServiceType } from '@linode/api-v4'; import type { FieldPathByValue } from 'react-hook-form'; interface CloudPulseServiceSelectProps { + /** + * @returns vsoid + * function to handle the service type change + */ + handleServiceTypeChange?: () => void; /** * Boolean value to check if service select is disabled in the edit flow */ @@ -23,7 +28,7 @@ interface CloudPulseServiceSelectProps { export const CloudPulseServiceSelect = ( props: CloudPulseServiceSelectProps ) => { - const { isDisabled, name } = props; + const { handleServiceTypeChange, isDisabled, name } = props; const { data: serviceOptions, error: serviceTypesError, @@ -62,6 +67,9 @@ export const CloudPulseServiceSelect = ( if (reason === 'clear') { field.onChange(null); } + if (handleServiceTypeChange !== undefined) { + handleServiceTypeChange(); + } }} value={ getServicesList.find((option) => option.value === field.value) ?? diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/NotificationChannels/AddChannelListing.test.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/NotificationChannels/AddChannelListing.test.tsx index b2894389303..e0b652f88b5 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/NotificationChannels/AddChannelListing.test.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/NotificationChannels/AddChannelListing.test.tsx @@ -1,5 +1,5 @@ import { capitalize } from '@linode/utilities'; -import { within } from '@testing-library/react'; +import { waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import * as React from 'react'; @@ -79,4 +79,31 @@ describe('Channel Listing component', () => { expect(notificationContainer).not.toBeInTheDocument(); }); + it('should show tooltip when the max limit of notification channels is reached', async () => { + // Mock the `notificationChannelWatcher` length to simulate the max limit + const mockMaxLimit = 5; + const { + getByRole, + getByText, + } = renderWithThemeAndHookFormContext({ + component: , + useFormOptions: { + defaultValues: { + channel_ids: Array(mockMaxLimit).fill(mockNotificationData[0].id), // simulate 5 channels + }, + }, + }); + + const addButton = getByRole('button', { + name: 'Add notification channel', + }); + + expect(addButton).toBeDisabled(); + userEvent.hover(addButton); + await waitFor(() => + expect( + getByText('You can add up to 5 notification channels.') + ).toBeInTheDocument() + ); + }); }); diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/NotificationChannels/AddChannelListing.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/NotificationChannels/AddChannelListing.tsx index dad46f05ad2..67885c4d3ad 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/NotificationChannels/AddChannelListing.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/NotificationChannels/AddChannelListing.tsx @@ -1,7 +1,7 @@ -import { Box, Button, Stack, Typography } from '@linode/ui'; +import { Box, Button, Notice, Stack, Typography } from '@linode/ui'; import { capitalize } from '@linode/utilities'; import React from 'react'; -import { useFormContext, useWatch } from 'react-hook-form'; +import { Controller, useFormContext, useWatch } from 'react-hook-form'; import { useAllAlertNotificationChannelsQuery } from 'src/queries/cloudpulse/alerts'; @@ -32,7 +32,7 @@ interface NotificationChannelsProps { */ notification: NotificationChannel; } -export const AddChannelListing = React.memo((props: AddChannelListingProps) => { +export const AddChannelListing = (props: AddChannelListingProps) => { const { name } = props; const { control, setValue } = useFormContext(); const [openAddNotification, setOpenAddNotification] = React.useState(false); @@ -67,7 +67,7 @@ export const AddChannelListing = React.memo((props: AddChannelListingProps) => { const handleRemove = (index: number) => { const newList = notificationChannelWatcher.filter((_, i) => i !== index); - setValue(name, newList); + setValue(name, newList, { shouldValidate: true }); }; const handleOpenDrawer = () => { @@ -79,7 +79,9 @@ export const AddChannelListing = React.memo((props: AddChannelListingProps) => { }; const handleAddNotification = (notificationId: number) => { - setValue(name, [...notificationChannelWatcher, notificationId]); + setValue(name, [...notificationChannelWatcher, notificationId], { + shouldValidate: true, + }); handleCloseDrawer(); }; @@ -135,39 +137,53 @@ export const AddChannelListing = React.memo((props: AddChannelListingProps) => { ); return ( - <> - - 4. Notification Channels - - - {selectedNotifications.length > 0 && - selectedNotifications.map((notification, id) => ( - - ))} - - + ( + <> + + 4. Notification Channels + + {(formState.isSubmitted || fieldState.isTouched) && fieldState.error && ( + + {fieldState.error.message} + + )} + + {selectedNotifications.length > 0 && + selectedNotifications.map((notification, id) => ( + + ))} + + - - + + + )} + control={control} + name={name} + /> ); -}); +}; diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/NotificationChannels/AddNotificationChannelDrawer.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/NotificationChannels/AddNotificationChannelDrawer.tsx index 16391e39c88..2fdac77629f 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/NotificationChannels/AddNotificationChannelDrawer.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/NotificationChannels/AddNotificationChannelDrawer.tsx @@ -90,12 +90,13 @@ export const AddNotificationChannelDrawer = ( (template) => template.label === channelLabelWatcher ); + const resetDrawer = () => { + handleCloseDrawer(); + reset(); + }; + return ( - +
option.label === field.value @@ -190,13 +196,13 @@ export const AddNotificationChannelDrawer = ( To: @@ -212,7 +218,7 @@ export const AddNotificationChannelDrawer = ( }} secondaryButtonProps={{ label: 'Cancel', - onClick: handleCloseDrawer, + onClick: resetDrawer, }} /> diff --git a/packages/manager/src/features/CloudPulse/Alerts/EditAlert/EditAlertDefinition.test.tsx b/packages/manager/src/features/CloudPulse/Alerts/EditAlert/EditAlertDefinition.test.tsx index a1d0e401499..59497427516 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/EditAlert/EditAlertDefinition.test.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/EditAlert/EditAlertDefinition.test.tsx @@ -23,6 +23,7 @@ vi.mock('src/queries/cloudpulse/alerts', () => ({ beforeEach(() => { vi.clearAllMocks(); + Element.prototype.scrollIntoView = vi.fn(); queryMocks.useEditAlertDefinition.mockReturnValue({ mutateAsync: vi.fn().mockResolvedValue({}), reset: vi.fn(), @@ -88,8 +89,8 @@ describe('EditAlertDefinition component', () => { ); const descriptionValue = 'Updated Description'; const nameValue = 'Updated Label'; - const nameInput = getByPlaceholderText('Enter Name'); - const descriptionInput = getByPlaceholderText('Enter Description'); + const nameInput = getByPlaceholderText('Enter a Name'); + const descriptionInput = getByPlaceholderText('Enter a Description'); await userEvent.clear(nameInput); await userEvent.clear(descriptionInput); await userEvent.type(nameInput, nameValue); diff --git a/packages/manager/src/features/CloudPulse/Alerts/EditAlert/EditAlertDefinition.tsx b/packages/manager/src/features/CloudPulse/Alerts/EditAlert/EditAlertDefinition.tsx index caee076ba54..92f5970f17e 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/EditAlert/EditAlertDefinition.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/EditAlert/EditAlertDefinition.tsx @@ -1,4 +1,5 @@ import { yupResolver } from '@hookform/resolvers/yup'; +import { isEmpty } from '@linode/api-v4'; import { Paper, TextField, Typography } from '@linode/ui'; import { useSnackbar } from 'notistack'; import React from 'react'; @@ -8,6 +9,7 @@ import { useHistory } from 'react-router-dom'; import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; import { Breadcrumb } from 'src/components/Breadcrumb/Breadcrumb'; import { useEditAlertDefinition } from 'src/queries/cloudpulse/alerts'; +import { scrollErrorIntoView } from 'src/utilities/scrollErrorIntoView'; import { MetricCriteriaField } from '../CreateAlert/Criteria/MetricCriteria'; import { TriggerConditions } from '../CreateAlert/Criteria/TriggerConditions'; @@ -19,10 +21,10 @@ import { convertAlertDefinitionValues } from '../Utils/utils'; import { EditAlertDefinitionFormSchema } from './schemas'; import type { + Alert, AlertServiceType, EditAlertDefinitionPayload, } from '@linode/api-v4'; -import type { Alert } from '@linode/api-v4'; import type { ObjectSchema } from 'yup'; export interface EditAlertProps { @@ -94,6 +96,16 @@ export const EditAlertDefinition = (props: EditAlertProps) => { }, ]; + const previousSubmitCount = React.useRef(0); + React.useEffect(() => { + if ( + !isEmpty(formState.errors) && + formState.submitCount > previousSubmitCount.current + ) { + scrollErrorIntoView(undefined, { behavior: 'smooth' }); + } + }, [formState.errors, formState.submitCount]); + return ( @@ -111,7 +123,7 @@ export const EditAlertDefinition = (props: EditAlertProps) => { name="label" onBlur={field.onBlur} onChange={(e) => field.onChange(e.target.value)} - placeholder="Enter Name" + placeholder="Enter a Name" value={field.value ?? ''} /> )} @@ -127,7 +139,7 @@ export const EditAlertDefinition = (props: EditAlertProps) => { onBlur={field.onBlur} onChange={(e) => field.onChange(e.target.value)} optional - placeholder="Enter Description" + placeholder="Enter a Description" value={field.value ?? ''} /> )} diff --git a/packages/manager/src/features/CloudPulse/Alerts/Utils/AlertResourceUtils.ts b/packages/manager/src/features/CloudPulse/Alerts/Utils/AlertResourceUtils.ts index 3b8e46442cf..61e6ae6cef9 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/Utils/AlertResourceUtils.ts +++ b/packages/manager/src/features/CloudPulse/Alerts/Utils/AlertResourceUtils.ts @@ -259,7 +259,9 @@ const applyAdditionalFilter = ( * This methods scrolls to the given HTML Element * @param scrollToElement The HTML Element to which we need to scroll */ -export const scrollToElement = (scrollToElement: HTMLDivElement | null) => { +export const scrollToElement = ( + scrollToElement: HTMLButtonElement | HTMLDivElement | null +) => { if (scrollToElement) { window.scrollTo({ behavior: 'smooth', diff --git a/packages/manager/src/features/CloudPulse/Alerts/constants.ts b/packages/manager/src/features/CloudPulse/Alerts/constants.ts index 8b91797b6bd..2d212eadf81 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/constants.ts +++ b/packages/manager/src/features/CloudPulse/Alerts/constants.ts @@ -48,7 +48,7 @@ export const metricOperatorOptions: Item[] = [ value: 'lte', }, { - label: '==', + label: '=', value: 'eq', }, ]; diff --git a/packages/manager/src/mocks/serverHandlers.ts b/packages/manager/src/mocks/serverHandlers.ts index 2445e5fdebc..a02ec3a24be 100644 --- a/packages/manager/src/mocks/serverHandlers.ts +++ b/packages/manager/src/mocks/serverHandlers.ts @@ -2557,7 +2557,7 @@ export const handlers = [ ), http.get('*/monitor/alert-channels', () => { return HttpResponse.json( - makeResourcePage(notificationChannelFactory.buildList(3)) + makeResourcePage(notificationChannelFactory.buildList(7)) ); }), http.get('*/monitor/services', () => { diff --git a/packages/validation/.changeset/pr-11773-upcoming-features-1741023892137.md b/packages/validation/.changeset/pr-11773-upcoming-features-1741023892137.md new file mode 100644 index 00000000000..0254d2878f9 --- /dev/null +++ b/packages/validation/.changeset/pr-11773-upcoming-features-1741023892137.md @@ -0,0 +1,5 @@ +--- +"@linode/validation": Upcoming Features +--- + +Add new rule for Name and Description of Create Alert form ([#11773](https://github.com/linode/manager/pull/11773)) \ No newline at end of file diff --git a/packages/validation/src/cloudpulse.schema.ts b/packages/validation/src/cloudpulse.schema.ts index ee5cf9f8d24..2781880a33b 100644 --- a/packages/validation/src/cloudpulse.schema.ts +++ b/packages/validation/src/cloudpulse.schema.ts @@ -3,7 +3,7 @@ import { array, number, object, string } from 'yup'; const fieldErrorMessage = 'This field is required.'; const dimensionFilters = object({ - dimension_label: string().required('Label for the filter is required.'), + dimension_label: string().required(fieldErrorMessage), operator: string().required(fieldErrorMessage), value: string().required(fieldErrorMessage), }); @@ -24,13 +24,36 @@ const triggerConditionValidation = object({ evaluation_period_seconds: number().required(fieldErrorMessage), trigger_occurrences: number() .required(fieldErrorMessage) - .positive("The value can't be 0.") - .typeError(fieldErrorMessage), + .positive('Enter a positive value.') + .typeError('The value should be a number.'), }); +const specialStartEndRegex = /^[^a-zA-Z0-9]/; export const createAlertDefinitionSchema = object({ - label: string().required(fieldErrorMessage), - description: string().optional(), + label: string() + .required(fieldErrorMessage) + .matches( + /^[^*#&+:<>"?@%{}\\\/]+$/, + 'Name cannot contain special characters: * # & + : < > ? @ % { } \\ /.' + ) + .max(100, 'Name must be 100 characters or less.') + .test( + 'no-special-start-end', + 'Name cannot start or end with a special character.', + (value) => { + return !specialStartEndRegex.test(value ?? ''); + } + ), + description: string() + .max(100, 'Description must be 100 characters or less.') + .test( + 'no-special-start-end', + 'Description cannot start or end with a special character.', + (value) => { + return !specialStartEndRegex.test(value ?? ''); + } + ) + .optional(), severity: number().oneOf([0, 1, 2, 3]).required(fieldErrorMessage), rule_criteria: object({ rules: array() @@ -38,6 +61,8 @@ export const createAlertDefinitionSchema = object({ .min(1, 'At least one metric criteria is required.'), }), trigger_conditions: triggerConditionValidation, - channel_ids: array(number()), + channel_ids: array() + .of(number()) + .min(1, 'At least one notification channel is required.'), tags: array().of(string()).notRequired(), });