From b70b3a02598c88bd58b7d07a85b45b3324d8e63b Mon Sep 17 00:00:00 2001 From: santoshp210-akamai <159890961+santoshp210-akamai@users.noreply.github.com> Date: Mon, 16 Dec 2024 23:07:22 +0530 Subject: [PATCH] upcoming: [DI-22184] - Review changes: replaced watch with useWatch, removed non-null assertion and using default values while filtering form values, modified code to include just side effects in the useEffect in MetricCriteria component --- .../CreateAlert/CreateAlertDefinition.tsx | 17 +++------- .../Alerts/CreateAlert/Criteria/Metric.tsx | 13 +++----- .../CreateAlert/Criteria/MetricCriteria.tsx | 32 +++++++++---------- .../AlertSeveritySelect.test.tsx | 4 +-- .../Alerts/CreateAlert/utilities.ts | 8 ++--- 5 files changed, 32 insertions(+), 42 deletions(-) diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx index d5f058df183..17bbb986b05 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/CreateAlertDefinition.tsx @@ -2,7 +2,7 @@ import { yupResolver } from '@hookform/resolvers/yup'; import { Paper, TextField, Typography } from '@linode/ui'; import { useSnackbar } from 'notistack'; import * as React from 'react'; -import { Controller, FormProvider, useForm } from 'react-hook-form'; +import { Controller, FormProvider, useForm, useWatch } from 'react-hook-form'; import { useHistory } from 'react-router-dom'; import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel'; @@ -74,14 +74,7 @@ export const CreateAlertDefinition = () => { ), }); - const { - control, - formState, - getValues, - handleSubmit, - setError, - watch, - } = formMethods; + const { control, formState, getValues, handleSubmit, setError } = formMethods; const { enqueueSnackbar } = useSnackbar(); const { mutateAsync: createAlert } = useCreateAlertDefinition( getValues('serviceType')! @@ -92,7 +85,7 @@ export const CreateAlertDefinition = () => { */ const [maxScrapeInterval, setMaxScrapeInterval] = React.useState(0); - const serviceTypeWatcher = watch('serviceType'); + const serviceTypeWatcher = useWatch({ control, name: 'serviceType' }); const onSubmit = handleSubmit(async (values) => { try { await createAlert(filterFormValues(values)); @@ -158,9 +151,9 @@ export const CreateAlertDefinition = () => { {serviceTypeWatcher === 'dbaas' && } 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 0db23ca9ece..7bbb5ae2d90 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/Metric.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/Metric.tsx @@ -2,7 +2,7 @@ import { Autocomplete, Box } from '@linode/ui'; import { Stack, TextField, Typography } from '@linode/ui'; import { Grid } from '@mui/material'; import React from 'react'; -import { Controller, useFormContext } from 'react-hook-form'; +import { Controller, useFormContext, useWatch } from 'react-hook-form'; import { MetricAggregationOptions, @@ -46,11 +46,7 @@ interface MetricCriteriaProps { export const Metric = (props: MetricCriteriaProps) => { const { apiError, data, name, onMetricDelete, showDeleteIcon } = props; const [isMetricDefinitionError, isMetricDefinitionLoading] = apiError; - const { - control, - setValue, - watch, - } = useFormContext(); + const { control, setValue } = useFormContext(); const handleDataFieldChange = ( selected: { label: string; unit: MetricUnitType; value: string }, @@ -83,7 +79,7 @@ export const Metric = (props: MetricCriteriaProps) => { : []; }, [data]); - const metricWatcher = watch(`${name}.metric`); + const metricWatcher = useWatch({ control, name: `${name}.metric` }); const selectedMetric = React.useMemo(() => { return data && metricWatcher @@ -103,6 +99,7 @@ export const Metric = (props: MetricCriteriaProps) => { : []; }, [selectedMetric]); + const serviceWatcher = useWatch({ control, name: 'serviceType' }); return ( ({ @@ -154,7 +151,7 @@ export const Metric = (props: MetricCriteriaProps) => { : null } data-testid={'Data-field'} - disabled={!watch('serviceType')} + disabled={!serviceWatcher} label="Data Field" loading={isMetricDefinitionLoading} onBlur={field.onBlur} 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 355fbc90a1c..213aa3f8b75 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/MetricCriteria.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/Criteria/MetricCriteria.tsx @@ -1,6 +1,6 @@ import { Box, Button, Stack, Typography } from '@linode/ui'; import * as React from 'react'; -import { useFieldArray, useFormContext } from 'react-hook-form'; +import { useFieldArray, useFormContext, useWatch } from 'react-hook-form'; import { useGetCloudPulseMetricDefinitionsByServiceType } from 'src/queries/cloudpulse/services'; @@ -39,24 +39,24 @@ export const MetricCriteriaField = (props: MetricCriteriaProps) => { serviceType !== null ); - const { control, watch } = useFormContext(); + const { control } = useFormContext(); - const metricCriteriaWatcher = watch(name); - React.useEffect(() => { - const formMetricValues = new Set( - metricCriteriaWatcher.map((item: MetricCriteriaForm) => item.metric) - ); + const metricCriteriaWatcher = useWatch({ control, name }); + + const intervalList = + metricDefinitions?.data + .filter((item) => + metricCriteriaWatcher.some( + (criteria: MetricCriteriaForm) => criteria.metric === item.metric + ) + ) + .map((item) => item.scrape_interval) || []; - const intervalList = - metricDefinitions && - metricDefinitions.data - .filter((item) => formMetricValues.has(item.metric)) - .map((item) => item.scrape_interval); - const maxInterval = Math.max( - ...convertToSeconds(intervalList ? intervalList : []) - ); + const maxInterval = Math.max(...convertToSeconds(intervalList)); + + React.useEffect(() => { setMaxInterval(maxInterval); - }, [setMaxInterval, metricCriteriaWatcher, metricDefinitions]); + }, [maxInterval, setMaxInterval]); const { append, fields, remove } = useFieldArray({ control, diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/AlertSeveritySelect.test.tsx b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/AlertSeveritySelect.test.tsx index 6ee98f68bf2..4d0dfd6bd4e 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/AlertSeveritySelect.test.tsx +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/GeneralInformation/AlertSeveritySelect.test.tsx @@ -6,8 +6,8 @@ import { renderWithThemeAndHookFormContext } from 'src/utilities/testHelpers'; import { CloudPulseAlertSeveritySelect } from './AlertSeveritySelect'; -describe('EngineOption component tests', () => { - it('should render the component when resource type is dbaas', () => { +describe('Severity component tests', () => { + it('should render the component', () => { const { getByLabelText, getByTestId } = renderWithThemeAndHookFormContext({ component: , }); diff --git a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts index 81ec2b00ca0..9a6bc907101 100644 --- a/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts +++ b/packages/manager/src/features/CloudPulse/Alerts/CreateAlert/utilities.ts @@ -18,7 +18,7 @@ export const filterFormValues = ( 'rule_criteria', ]); // 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 severity = formValues.severity ?? 1; const entityIds = formValues.entity_ids; const rules = formValues.rule_criteria.rules; return { @@ -36,10 +36,10 @@ export const filterMetricCriteriaFormValues = ( const values = omitProps(rule, ['aggregation_type', 'operator', 'metric']); return { ...values, - aggregation_type: rule.aggregation_type!, + aggregation_type: rule.aggregation_type ?? 'avg', dimension_filters: rule.dimension_filters, - metric: rule.metric!, - operator: rule.operator!, + metric: rule.metric ?? '', + operator: rule.operator ?? 'eq', }; }); };