From 9e514bea01027d7c697cc0653a73723a45d0e899 Mon Sep 17 00:00:00 2001 From: Kevin Delemme Date: Fri, 29 Sep 2023 10:19:28 -0400 Subject: [PATCH] fix(slo): Handle partial indicator url state (#167247) (cherry picked from commit 9d3213e1372b72b5dd143e42b48e517d66cd3820) --- .../apm_availability_indicator_type_form.tsx | 20 ++- .../apm_latency_indicator_type_form.tsx | 20 ++- .../slo_edit/components/slo_edit_form.tsx | 4 +- .../helpers/process_slo_form_values.test.ts | 152 ++++++++++++++++++ .../helpers/process_slo_form_values.ts | 74 ++++++--- .../slo_edit/hooks/use_parse_url_state.ts | 7 +- .../slo_edit/hooks/use_unregister_fields.ts | 2 +- .../public/pages/slo_edit/slo_edit.test.tsx | 10 +- 8 files changed, 246 insertions(+), 43 deletions(-) create mode 100644 x-pack/plugins/observability/public/pages/slo_edit/helpers/process_slo_form_values.test.ts diff --git a/x-pack/plugins/observability/public/pages/slo_edit/components/apm_availability/apm_availability_indicator_type_form.tsx b/x-pack/plugins/observability/public/pages/slo_edit/components/apm_availability/apm_availability_indicator_type_form.tsx index bfb629f3c8071..d22509da43599 100644 --- a/x-pack/plugins/observability/public/pages/slo_edit/components/apm_availability/apm_availability_indicator_type_form.tsx +++ b/x-pack/plugins/observability/public/pages/slo_edit/components/apm_availability/apm_availability_indicator_type_form.tsx @@ -7,8 +7,9 @@ import { EuiFlexGroup, EuiFlexItem, EuiIconTip } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import React from 'react'; +import React, { useEffect } from 'react'; import { useFormContext } from 'react-hook-form'; +import { useFetchApmIndex } from '../../../../hooks/slo/use_fetch_apm_indices'; import { useFetchIndexPatternFields } from '../../../../hooks/slo/use_fetch_index_pattern_fields'; import { CreateSLOForm } from '../../types'; import { FieldSelector } from '../apm_common/field_selector'; @@ -17,10 +18,17 @@ import { IndexFieldSelector } from '../common/index_field_selector'; import { QueryBuilder } from '../common/query_builder'; export function ApmAvailabilityIndicatorTypeForm() { - const { watch } = useFormContext(); - const index = watch('indicator.params.index'); + const { watch, setValue } = useFormContext(); + const { data: apmIndex } = useFetchApmIndex(); + + useEffect(() => { + if (apmIndex !== '') { + setValue('indicator.params.index', apmIndex); + } + }, [setValue, apmIndex]); + const { isLoading: isIndexFieldsLoading, data: indexFields = [] } = - useFetchIndexPatternFields(index); + useFetchIndexPatternFields(apmIndex); const partitionByFields = indexFields.filter((field) => field.aggregatable); return ( @@ -144,8 +152,8 @@ export function ApmAvailabilityIndicatorTypeForm() { placeholder={i18n.translate('xpack.observability.slo.sloEdit.groupBy.placeholder', { defaultMessage: 'Select an optional field to partition by', })} - isLoading={!!index && isIndexFieldsLoading} - isDisabled={!index} + isLoading={!!apmIndex && isIndexFieldsLoading} + isDisabled={!apmIndex} /> diff --git a/x-pack/plugins/observability/public/pages/slo_edit/components/apm_latency/apm_latency_indicator_type_form.tsx b/x-pack/plugins/observability/public/pages/slo_edit/components/apm_latency/apm_latency_indicator_type_form.tsx index 8d21a0c7d2546..af357ea0c18d1 100644 --- a/x-pack/plugins/observability/public/pages/slo_edit/components/apm_latency/apm_latency_indicator_type_form.tsx +++ b/x-pack/plugins/observability/public/pages/slo_edit/components/apm_latency/apm_latency_indicator_type_form.tsx @@ -7,8 +7,9 @@ import { EuiFieldNumber, EuiFlexGroup, EuiFlexItem, EuiFormRow, EuiIconTip } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import React from 'react'; +import React, { useEffect } from 'react'; import { Controller, useFormContext } from 'react-hook-form'; +import { useFetchApmIndex } from '../../../../hooks/slo/use_fetch_apm_indices'; import { useFetchIndexPatternFields } from '../../../../hooks/slo/use_fetch_index_pattern_fields'; import { CreateSLOForm } from '../../types'; import { FieldSelector } from '../apm_common/field_selector'; @@ -17,10 +18,17 @@ import { IndexFieldSelector } from '../common/index_field_selector'; import { QueryBuilder } from '../common/query_builder'; export function ApmLatencyIndicatorTypeForm() { - const { control, watch, getFieldState } = useFormContext(); - const index = watch('indicator.params.index'); + const { control, watch, getFieldState, setValue } = useFormContext(); + const { data: apmIndex } = useFetchApmIndex(); + + useEffect(() => { + if (apmIndex !== '') { + setValue('indicator.params.index', apmIndex); + } + }, [setValue, apmIndex]); + const { isLoading: isIndexFieldsLoading, data: indexFields = [] } = - useFetchIndexPatternFields(index); + useFetchIndexPatternFields(apmIndex); const partitionByFields = indexFields.filter((field) => field.aggregatable); return ( @@ -187,8 +195,8 @@ export function ApmLatencyIndicatorTypeForm() { placeholder={i18n.translate('xpack.observability.slo.sloEdit.groupBy.placeholder', { defaultMessage: 'Select an optional field to partition by', })} - isLoading={!!index && isIndexFieldsLoading} - isDisabled={!index} + isLoading={!!apmIndex && isIndexFieldsLoading} + isDisabled={!apmIndex} /> diff --git a/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form.tsx b/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form.tsx index ba3970d93cab0..19c27928b292a 100644 --- a/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form.tsx +++ b/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form.tsx @@ -62,7 +62,7 @@ export function SloEditForm({ slo }: Props) { sloIds: slo?.id ? [slo.id] : undefined, }); - const sloFormValuesUrlState = useParseUrlState(); + const sloFormValuesFromUrlState = useParseUrlState(); const isAddRuleFlyoutOpen = useAddRuleFlyoutState(isEditMode); const [isCreateRuleCheckboxChecked, setIsCreateRuleCheckboxChecked] = useState(true); @@ -73,7 +73,7 @@ export function SloEditForm({ slo }: Props) { }, [isEditMode, rules, slo]); const methods = useForm({ - defaultValues: Object.assign({}, SLO_EDIT_FORM_DEFAULT_VALUES, sloFormValuesUrlState), + defaultValues: Object.assign({}, SLO_EDIT_FORM_DEFAULT_VALUES, sloFormValuesFromUrlState), values: transformSloResponseToCreateSloForm(slo), mode: 'all', }); diff --git a/x-pack/plugins/observability/public/pages/slo_edit/helpers/process_slo_form_values.test.ts b/x-pack/plugins/observability/public/pages/slo_edit/helpers/process_slo_form_values.test.ts new file mode 100644 index 0000000000000..475dfb01b1998 --- /dev/null +++ b/x-pack/plugins/observability/public/pages/slo_edit/helpers/process_slo_form_values.test.ts @@ -0,0 +1,152 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { transformPartialUrlStateToFormState as transform } from './process_slo_form_values'; + +describe('Transform Partial URL State into partial State Form', () => { + describe('indicators', () => { + it("returns an empty '{}' when no indicator type is specified", () => { + expect(transform({ indicator: { params: { index: 'my-index' } } })).toEqual({}); + }); + + it('handles partial APM Availability state', () => { + expect( + transform({ + indicator: { + type: 'sli.apm.transactionErrorRate', + params: { + service: 'override-service', + }, + }, + }) + ).toEqual({ + indicator: { + type: 'sli.apm.transactionErrorRate', + params: { + service: 'override-service', + environment: '', + filter: '', + index: '', + transactionName: '', + transactionType: '', + }, + }, + }); + }); + + it('handles partial APM Latency state', () => { + expect( + transform({ + indicator: { + type: 'sli.apm.transactionDuration', + params: { + service: 'override-service', + }, + }, + }) + ).toEqual({ + indicator: { + type: 'sli.apm.transactionDuration', + params: { + service: 'override-service', + environment: '', + filter: '', + index: '', + transactionName: '', + transactionType: '', + threshold: 250, + }, + }, + }); + }); + + it('handles partial Custom KQL state', () => { + expect( + transform({ + indicator: { + type: 'sli.kql.custom', + params: { + good: "some.override.filter:'foo'", + index: 'override-index', + }, + }, + }) + ).toEqual({ + indicator: { + type: 'sli.kql.custom', + params: { + index: 'override-index', + timestampField: '', + filter: '', + good: "some.override.filter:'foo'", + total: '', + }, + }, + }); + }); + + it('handles partial Custom Metric state', () => { + expect( + transform({ + indicator: { + type: 'sli.metric.custom', + params: { + index: 'override-index', + }, + }, + }) + ).toEqual({ + indicator: { + type: 'sli.metric.custom', + params: { + index: 'override-index', + filter: '', + timestampField: '', + good: { + equation: 'A', + metrics: [{ aggregation: 'sum', field: '', name: 'A' }], + }, + total: { + equation: 'A', + metrics: [{ aggregation: 'sum', field: '', name: 'A' }], + }, + }, + }, + }); + }); + + it('handles partial Custom Histogram state', () => { + expect( + transform({ + indicator: { + type: 'sli.histogram.custom', + params: { + index: 'override-index', + }, + }, + }) + ).toEqual({ + indicator: { + type: 'sli.histogram.custom', + params: { + index: 'override-index', + filter: '', + timestampField: '', + good: { + aggregation: 'value_count', + field: '', + }, + total: { + aggregation: 'value_count', + field: '', + }, + }, + }, + }); + }); + }); +}); diff --git a/x-pack/plugins/observability/public/pages/slo_edit/helpers/process_slo_form_values.ts b/x-pack/plugins/observability/public/pages/slo_edit/helpers/process_slo_form_values.ts index a4ecec640e2df..c6de2126adacf 100644 --- a/x-pack/plugins/observability/public/pages/slo_edit/helpers/process_slo_form_values.ts +++ b/x-pack/plugins/observability/public/pages/slo_edit/helpers/process_slo_form_values.ts @@ -5,8 +5,17 @@ * 2.0. */ -import { CreateSLOInput, SLOWithSummaryResponse, UpdateSLOInput } from '@kbn/slo-schema'; +import { CreateSLOInput, Indicator, SLOWithSummaryResponse, UpdateSLOInput } from '@kbn/slo-schema'; +import { assertNever } from '@kbn/std'; +import { RecursivePartial } from '@kbn/utility-types'; import { toDuration } from '../../../utils/slo/duration'; +import { + APM_AVAILABILITY_DEFAULT_VALUES, + APM_LATENCY_DEFAULT_VALUES, + CUSTOM_KQL_DEFAULT_VALUES, + CUSTOM_METRIC_DEFAULT_VALUES, + HISTOGRAM_DEFAULT_VALUES, +} from '../constants'; import { CreateSLOForm } from '../types'; export function transformSloResponseToCreateSloForm( @@ -91,21 +100,50 @@ export function transformValuesToUpdateSLOInput(values: CreateSLOForm): UpdateSL }; } -export function transformPartialCreateSLOInputToPartialCreateSLOForm( - values: Partial -): Partial { - return { - ...values, - ...(values.objective && { - objective: { - target: values.objective.target * 100, - ...(values.objective.timesliceTarget && { - timesliceTarget: values.objective.timesliceTarget * 100, - }), - ...(values.objective.timesliceWindow && { - timesliceWindow: String(toDuration(values.objective.timesliceWindow).value), - }), - }, - }), - }; +function transformPartialIndicatorState( + indicator?: RecursivePartial +): Indicator | undefined { + if (indicator === undefined || indicator.type === undefined) return undefined; + + const indicatorType = indicator.type; + switch (indicatorType) { + case 'sli.apm.transactionDuration': + return { + type: 'sli.apm.transactionDuration' as const, + params: Object.assign({}, APM_LATENCY_DEFAULT_VALUES.params, indicator.params ?? {}), + }; + case 'sli.apm.transactionErrorRate': + return { + type: 'sli.apm.transactionErrorRate' as const, + params: Object.assign({}, APM_AVAILABILITY_DEFAULT_VALUES.params, indicator.params ?? {}), + }; + case 'sli.histogram.custom': + return { + type: 'sli.histogram.custom' as const, + params: Object.assign({}, HISTOGRAM_DEFAULT_VALUES.params, indicator.params ?? {}), + }; + case 'sli.kql.custom': + return { + type: 'sli.kql.custom' as const, + params: Object.assign({}, CUSTOM_KQL_DEFAULT_VALUES.params, indicator.params ?? {}), + }; + case 'sli.metric.custom': + return { + type: 'sli.metric.custom' as const, + params: Object.assign({}, CUSTOM_METRIC_DEFAULT_VALUES.params, indicator.params ?? {}), + }; + default: + assertNever(indicatorType); + } +} + +export function transformPartialUrlStateToFormState( + values: RecursivePartial> +): Partial | {} { + const state: Partial = {}; + + const parsedIndicator = transformPartialIndicatorState(values.indicator); + if (parsedIndicator !== undefined) state.indicator = parsedIndicator; + + return state; } diff --git a/x-pack/plugins/observability/public/pages/slo_edit/hooks/use_parse_url_state.ts b/x-pack/plugins/observability/public/pages/slo_edit/hooks/use_parse_url_state.ts index 7ae38e9976f96..f8354ae030403 100644 --- a/x-pack/plugins/observability/public/pages/slo_edit/hooks/use_parse_url_state.ts +++ b/x-pack/plugins/observability/public/pages/slo_edit/hooks/use_parse_url_state.ts @@ -7,8 +7,9 @@ import { createKbnUrlStateStorage } from '@kbn/kibana-utils-plugin/public'; import { CreateSLOInput } from '@kbn/slo-schema'; +import { RecursivePartial } from '@kbn/utility-types'; import { useHistory } from 'react-router-dom'; -import { transformPartialCreateSLOInputToPartialCreateSLOForm } from '../helpers/process_slo_form_values'; +import { transformPartialUrlStateToFormState } from '../helpers/process_slo_form_values'; import { CreateSLOForm } from '../types'; export function useParseUrlState(): Partial | null { @@ -19,7 +20,7 @@ export function useParseUrlState(): Partial | null { useHashQuery: false, }); - const urlParams = urlStateStorage.get>('_a'); + const urlParams = urlStateStorage.get>('_a'); - return !!urlParams ? transformPartialCreateSLOInputToPartialCreateSLOForm(urlParams) : null; + return !!urlParams ? transformPartialUrlStateToFormState(urlParams) : null; } diff --git a/x-pack/plugins/observability/public/pages/slo_edit/hooks/use_unregister_fields.ts b/x-pack/plugins/observability/public/pages/slo_edit/hooks/use_unregister_fields.ts index 973a023bdae77..c15b5cb7fbbfc 100644 --- a/x-pack/plugins/observability/public/pages/slo_edit/hooks/use_unregister_fields.ts +++ b/x-pack/plugins/observability/public/pages/slo_edit/hooks/use_unregister_fields.ts @@ -32,8 +32,8 @@ export function useUnregisterFields({ isEditMode }: { isEditMode: boolean }) { const [indicatorTypeState, setIndicatorTypeState] = useState( watch('indicator.type') ); - const indicatorType = watch('indicator.type'); + const indicatorType = watch('indicator.type'); useEffect(() => { if (indicatorType !== indicatorTypeState && !isEditMode) { setIndicatorTypeState(indicatorType); diff --git a/x-pack/plugins/observability/public/pages/slo_edit/slo_edit.test.tsx b/x-pack/plugins/observability/public/pages/slo_edit/slo_edit.test.tsx index 06d3285f8945d..c38689200a164 100644 --- a/x-pack/plugins/observability/public/pages/slo_edit/slo_edit.test.tsx +++ b/x-pack/plugins/observability/public/pages/slo_edit/slo_edit.test.tsx @@ -370,7 +370,7 @@ describe('SLO Edit Page', () => { const history = createBrowserHistory(); history.push( - '/slos/create?_a=(name:%27prefilledSloName%27,indicator:(params:(environment:prod,service:cartService),type:sli.apm.transactionDuration))' + '/slos/create?_a=(indicator:(params:(environment:prod,service:cartService),type:sli.apm.transactionDuration))' ); jest.spyOn(Router, 'useHistory').mockReturnValue(history); jest @@ -409,18 +409,14 @@ describe('SLO Edit Page', () => { expect(screen.queryByTestId('sloForm')).toBeTruthy(); expect(screen.queryByTestId('sloEditFormIndicatorSection')).toBeTruthy(); - // Show default values from the kql indicator expect(screen.queryByTestId('sloFormIndicatorTypeSelect')).toHaveValue( 'sli.apm.transactionDuration' ); - - expect(screen.queryByTestId('sloEditFormObjectiveSection')).toBeTruthy(); - expect(screen.queryByTestId('sloEditFormDescriptionSection')).toBeTruthy(); - expect(screen.queryByTestId('apmLatencyServiceSelector')).toHaveTextContent('cartService'); expect(screen.queryByTestId('apmLatencyEnvironmentSelector')).toHaveTextContent('prod'); - expect(screen.queryByTestId('sloFormNameInput')).toHaveValue('prefilledSloName'); + expect(screen.queryByTestId('sloEditFormObjectiveSection')).toBeFalsy(); + expect(screen.queryByTestId('sloEditFormDescriptionSection')).toBeFalsy(); }); });