From 32ba81778bcbd07b9e0233a6a6e2d5fb37cd57e3 Mon Sep 17 00:00:00 2001 From: Kevin Delemme Date: Thu, 19 Oct 2023 11:07:31 -0400 Subject: [PATCH] chore(slo): prefill slo edit form on update error (#168760) --- .../observability/common/locators/paths.ts | 2 + .../public/hooks/slo/use_create_slo.ts | 2 + .../public/hooks/slo/use_update_slo.ts | 24 +- .../slo_edit/components/slo_edit_form.tsx | 10 +- .../process_slo_form_values.test.ts.snap | 273 +++++++++ .../helpers/process_slo_form_values.test.ts | 124 ++--- .../helpers/process_slo_form_values.ts | 55 +- .../slo_edit/hooks/use_parse_url_state.ts | 6 +- .../public/pages/slo_edit/slo_edit.test.tsx | 526 ++++-------------- 9 files changed, 508 insertions(+), 514 deletions(-) create mode 100644 x-pack/plugins/observability/public/pages/slo_edit/helpers/__snapshots__/process_slo_form_values.test.ts.snap diff --git a/x-pack/plugins/observability/common/locators/paths.ts b/x-pack/plugins/observability/common/locators/paths.ts index a69062321dd298..dcd3c361c0e361 100644 --- a/x-pack/plugins/observability/common/locators/paths.ts +++ b/x-pack/plugins/observability/common/locators/paths.ts @@ -35,6 +35,8 @@ export const paths = { sloCreateWithEncodedForm: (encodedParams: string) => `${OBSERVABILITY_BASE_PATH}${SLO_CREATE_PATH}?_a=${encodedParams}`, sloEdit: (sloId: string) => `${OBSERVABILITY_BASE_PATH}${SLOS_PATH}/edit/${encodeURI(sloId)}`, + sloEditWithEncodedForm: (sloId: string, encodedParams: string) => + `${OBSERVABILITY_BASE_PATH}${SLOS_PATH}/edit/${encodeURI(sloId)}?_a=${encodedParams}`, sloDetails: (sloId: string, instanceId?: string) => !!instanceId ? `${OBSERVABILITY_BASE_PATH}${SLOS_PATH}/${encodeURI(sloId)}?instanceId=${encodeURI( diff --git a/x-pack/plugins/observability/public/hooks/slo/use_create_slo.ts b/x-pack/plugins/observability/public/hooks/slo/use_create_slo.ts index a1a79d51f5af52..15ba63d7809135 100644 --- a/x-pack/plugins/observability/public/hooks/slo/use_create_slo.ts +++ b/x-pack/plugins/observability/public/hooks/slo/use_create_slo.ts @@ -69,6 +69,8 @@ export function useCreateSlo() { values: { name: slo.name }, }) ); + + queryClient.invalidateQueries({ queryKey: sloKeys.lists(), exact: false }); }, onError: (error, { slo }, context) => { if (context?.previousData && context?.queryKey) { diff --git a/x-pack/plugins/observability/public/hooks/slo/use_update_slo.ts b/x-pack/plugins/observability/public/hooks/slo/use_update_slo.ts index 07f6991b9e82b2..8f81075f0f2df9 100644 --- a/x-pack/plugins/observability/public/hooks/slo/use_update_slo.ts +++ b/x-pack/plugins/observability/public/hooks/slo/use_update_slo.ts @@ -9,13 +9,16 @@ import { IHttpFetchError, ResponseErrorBody } from '@kbn/core/public'; import { i18n } from '@kbn/i18n'; import type { FindSLOResponse, UpdateSLOInput, UpdateSLOResponse } from '@kbn/slo-schema'; import { QueryKey, useMutation, useQueryClient } from '@tanstack/react-query'; +import { encode } from '@kbn/rison'; import { useKibana } from '../../utils/kibana_react'; +import { paths } from '../../../common/locators/paths'; import { sloKeys } from './query_key_factory'; type ServerError = IHttpFetchError; export function useUpdateSlo() { const { + application: { navigateToUrl }, http, notifications: { toasts }, } = useKibana().services; @@ -25,7 +28,7 @@ export function useUpdateSlo() { UpdateSLOResponse, ServerError, { sloId: string; slo: UpdateSLOInput }, - { previousData?: FindSLOResponse; queryKey?: QueryKey } + { previousData?: FindSLOResponse; queryKey?: QueryKey; sloId: string } >( ['updateSlo'], ({ sloId, slo }) => { @@ -57,7 +60,7 @@ export function useUpdateSlo() { queryClient.setQueryData(queryKey, optimisticUpdate); } - return { previousData, queryKey }; + return { previousData, queryKey, sloId }; }, onSuccess: (_data, { slo: { name } }) => { toasts.addSuccess( @@ -66,8 +69,10 @@ export function useUpdateSlo() { values: { name }, }) ); + + queryClient.invalidateQueries({ queryKey: sloKeys.lists(), exact: false }); }, - onError: (error, { slo: { name } }, context) => { + onError: (error, { slo }, context) => { if (context?.previousData && context?.queryKey) { queryClient.setQueryData(context.queryKey, context.previousData); } @@ -75,12 +80,17 @@ export function useUpdateSlo() { toasts.addError(new Error(error.body?.message ?? error.message), { title: i18n.translate('xpack.observability.slo.update.errorNotification', { defaultMessage: 'Something went wrong when updating {name}', - values: { name }, + values: { name: slo.name }, }), }); - }, - onSettled: () => { - queryClient.invalidateQueries({ queryKey: sloKeys.lists(), exact: false }); + + if (context?.sloId) { + navigateToUrl( + http.basePath.prepend( + paths.observability.sloEditWithEncodedForm(context.sloId, encode(slo)) + ) + ); + } }, } ); 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 0fb29d5980ccbc..b8fc9bdaa0a029 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 @@ -15,7 +15,7 @@ import { EuiSteps, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import type { SLOWithSummaryResponse } from '@kbn/slo-schema'; +import type { GetSLOResponse } from '@kbn/slo-schema'; import React, { useCallback, useEffect, useState } from 'react'; import { FormProvider, useForm } from 'react-hook-form'; import { sloFeatureId } from '../../../../common'; @@ -45,7 +45,7 @@ import { SloEditFormIndicatorSection } from './slo_edit_form_indicator_section'; import { SloEditFormObjectiveSection } from './slo_edit_form_objective_section'; export interface Props { - slo: SLOWithSummaryResponse | undefined; + slo?: GetSLOResponse; } export const maxWidth = 775; @@ -63,6 +63,8 @@ export function SloEditForm({ slo }: Props) { }); const sloFormValuesFromUrlState = useParseUrlState(); + const sloFormValuesFromSloResponse = transformSloResponseToCreateSloForm(slo); + const isAddRuleFlyoutOpen = useAddRuleFlyoutState(isEditMode); const [isCreateRuleCheckboxChecked, setIsCreateRuleCheckboxChecked] = useState(true); @@ -73,8 +75,8 @@ export function SloEditForm({ slo }: Props) { }, [isEditMode, rules, slo]); const methods = useForm({ - defaultValues: Object.assign({}, SLO_EDIT_FORM_DEFAULT_VALUES, sloFormValuesFromUrlState), - values: transformSloResponseToCreateSloForm(slo), + defaultValues: SLO_EDIT_FORM_DEFAULT_VALUES, + values: sloFormValuesFromUrlState ? sloFormValuesFromUrlState : sloFormValuesFromSloResponse, mode: 'all', }); const { watch, getFieldState, getValues, formState, trigger } = methods; diff --git a/x-pack/plugins/observability/public/pages/slo_edit/helpers/__snapshots__/process_slo_form_values.test.ts.snap b/x-pack/plugins/observability/public/pages/slo_edit/helpers/__snapshots__/process_slo_form_values.test.ts.snap new file mode 100644 index 00000000000000..6f1197f32c1570 --- /dev/null +++ b/x-pack/plugins/observability/public/pages/slo_edit/helpers/__snapshots__/process_slo_form_values.test.ts.snap @@ -0,0 +1,273 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Transform partial URL state into form state handles partial Custom Histogram state 1`] = ` +Object { + "budgetingMethod": "occurrences", + "description": "", + "groupBy": "*", + "indicator": Object { + "params": Object { + "filter": "", + "good": Object { + "aggregation": "value_count", + "field": "", + }, + "index": "override-index", + "timestampField": "", + "total": Object { + "aggregation": "value_count", + "field": "", + }, + }, + "type": "sli.histogram.custom", + }, + "name": "", + "objective": Object { + "target": 99, + }, + "tags": Array [], + "timeWindow": Object { + "duration": "30d", + "type": "rolling", + }, +} +`; + +exports[`Transform partial URL state into form state handles partial Custom Metric state 1`] = ` +Object { + "budgetingMethod": "occurrences", + "description": "", + "groupBy": "*", + "indicator": Object { + "params": Object { + "filter": "", + "good": Object { + "equation": "A", + "metrics": Array [ + Object { + "aggregation": "sum", + "field": "", + "name": "A", + }, + ], + }, + "index": "override-index", + "timestampField": "", + "total": Object { + "equation": "A", + "metrics": Array [ + Object { + "aggregation": "sum", + "field": "", + "name": "A", + }, + ], + }, + }, + "type": "sli.metric.custom", + }, + "name": "", + "objective": Object { + "target": 99, + }, + "tags": Array [], + "timeWindow": Object { + "duration": "30d", + "type": "rolling", + }, +} +`; + +exports[`Transform partial URL state into form state handles the 'budgetingMethod' URL state 1`] = ` +Object { + "budgetingMethod": "timeslices", + "description": "", + "groupBy": "*", + "indicator": Object { + "params": Object { + "filter": "", + "good": "", + "index": "", + "timestampField": "", + "total": "", + }, + "type": "sli.kql.custom", + }, + "name": "", + "objective": Object { + "target": 99, + }, + "tags": Array [], + "timeWindow": Object { + "duration": "30d", + "type": "rolling", + }, +} +`; + +exports[`Transform partial URL state into form state handles the 'objective' URL state 1`] = ` +Object { + "budgetingMethod": "occurrences", + "description": "", + "groupBy": "*", + "indicator": Object { + "params": Object { + "filter": "", + "good": "", + "index": "", + "timestampField": "", + "total": "", + }, + "type": "sli.kql.custom", + }, + "name": "", + "objective": Object { + "target": 94.5, + "timesliceTarget": 95, + "timesliceWindow": "2", + }, + "tags": Array [], + "timeWindow": Object { + "duration": "30d", + "type": "rolling", + }, +} +`; + +exports[`Transform partial URL state into form state handles the 'timeWindow' URL state 1`] = ` +Object { + "budgetingMethod": "occurrences", + "description": "", + "groupBy": "*", + "indicator": Object { + "params": Object { + "filter": "", + "good": "", + "index": "", + "timestampField": "", + "total": "", + }, + "type": "sli.kql.custom", + }, + "name": "", + "objective": Object { + "target": 99, + }, + "tags": Array [], + "timeWindow": Object { + "duration": "1M", + "type": "calendarAligned", + }, +} +`; + +exports[`Transform partial URL state into form state with 'indicator' in URL state handles partial APM Availability state 1`] = ` +Object { + "budgetingMethod": "occurrences", + "description": "", + "groupBy": "*", + "indicator": Object { + "params": Object { + "environment": "", + "filter": "", + "index": "", + "service": "override-service", + "transactionName": "", + "transactionType": "", + }, + "type": "sli.apm.transactionErrorRate", + }, + "name": "", + "objective": Object { + "target": 99, + }, + "tags": Array [], + "timeWindow": Object { + "duration": "30d", + "type": "rolling", + }, +} +`; + +exports[`Transform partial URL state into form state with 'indicator' in URL state handles partial APM Latency state 1`] = ` +Object { + "budgetingMethod": "occurrences", + "description": "", + "groupBy": "*", + "indicator": Object { + "params": Object { + "environment": "", + "filter": "", + "index": "", + "service": "override-service", + "threshold": 250, + "transactionName": "", + "transactionType": "", + }, + "type": "sli.apm.transactionDuration", + }, + "name": "", + "objective": Object { + "target": 99, + }, + "tags": Array [], + "timeWindow": Object { + "duration": "30d", + "type": "rolling", + }, +} +`; + +exports[`Transform partial URL state into form state with 'indicator' in URL state handles partial Custom KQL state 1`] = ` +Object { + "budgetingMethod": "occurrences", + "description": "", + "groupBy": "*", + "indicator": Object { + "params": Object { + "filter": "", + "good": "some.override.filter:'foo'", + "index": "override-index", + "timestampField": "", + "total": "", + }, + "type": "sli.kql.custom", + }, + "name": "", + "objective": Object { + "target": 99, + }, + "tags": Array [], + "timeWindow": Object { + "duration": "30d", + "type": "rolling", + }, +} +`; + +exports[`Transform partial URL state into form state with 'indicator' in URL state returns default form values when no indicator type is specified 1`] = ` +Object { + "budgetingMethod": "occurrences", + "description": "", + "groupBy": "*", + "indicator": Object { + "params": Object { + "filter": "", + "good": "", + "index": "", + "timestampField": "", + "total": "", + }, + "type": "sli.kql.custom", + }, + "name": "", + "objective": Object { + "target": 99, + }, + "tags": Array [], + "timeWindow": Object { + "duration": "30d", + "type": "rolling", + }, +} +`; 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 index 475dfb01b19989..dce74002baca75 100644 --- 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 @@ -7,10 +7,10 @@ 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({}); +describe('Transform partial URL state into form state', () => { + describe("with 'indicator' in URL state", () => { + it('returns default form values when no indicator type is specified', () => { + expect(transform({ indicator: { params: { index: 'my-index' } } })).toMatchSnapshot(); }); it('handles partial APM Availability state', () => { @@ -23,19 +23,7 @@ describe('Transform Partial URL State into partial State Form', () => { }, }, }) - ).toEqual({ - indicator: { - type: 'sli.apm.transactionErrorRate', - params: { - service: 'override-service', - environment: '', - filter: '', - index: '', - transactionName: '', - transactionType: '', - }, - }, - }); + ).toMatchSnapshot(); }); it('handles partial APM Latency state', () => { @@ -48,20 +36,7 @@ describe('Transform Partial URL State into partial State Form', () => { }, }, }) - ).toEqual({ - indicator: { - type: 'sli.apm.transactionDuration', - params: { - service: 'override-service', - environment: '', - filter: '', - index: '', - transactionName: '', - transactionType: '', - threshold: 250, - }, - }, - }); + ).toMatchSnapshot(); }); it('handles partial Custom KQL state', () => { @@ -75,78 +50,49 @@ describe('Transform Partial URL State into partial State Form', () => { }, }, }) - ).toEqual({ - indicator: { - type: 'sli.kql.custom', - params: { - index: 'override-index', - timestampField: '', - filter: '', - good: "some.override.filter:'foo'", - total: '', - }, - }, - }); + ).toMatchSnapshot(); }); + }); - it('handles partial Custom Metric state', () => { - expect( - transform({ - indicator: { - type: 'sli.metric.custom', - params: { - index: 'override-index', - }, - }, - }) - ).toEqual({ + it('handles partial Custom Metric state', () => { + expect( + transform({ 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' }], - }, }, }, - }); - }); + }) + ).toMatchSnapshot(); + }); - it('handles partial Custom Histogram state', () => { - expect( - transform({ - indicator: { - type: 'sli.histogram.custom', - params: { - index: 'override-index', - }, - }, - }) - ).toEqual({ + it('handles partial Custom Histogram state', () => { + expect( + transform({ indicator: { type: 'sli.histogram.custom', params: { index: 'override-index', - filter: '', - timestampField: '', - good: { - aggregation: 'value_count', - field: '', - }, - total: { - aggregation: 'value_count', - field: '', - }, }, }, - }); - }); + }) + ).toMatchSnapshot(); + }); + + it("handles the 'budgetingMethod' URL state", () => { + expect(transform({ budgetingMethod: 'timeslices' })).toMatchSnapshot(); + }); + + it("handles the 'timeWindow' URL state", () => { + expect( + transform({ timeWindow: { duration: '1M', type: 'calendarAligned' } }) + ).toMatchSnapshot(); + }); + + it("handles the 'objective' URL state", () => { + expect( + transform({ objective: { target: 0.945, timesliceTarget: 0.95, timesliceWindow: '2m' } }) + ).toMatchSnapshot(); }); }); 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 f523cc1ce1ce1f..2238d7848feeb8 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,9 +5,10 @@ * 2.0. */ -import { CreateSLOInput, Indicator, SLOWithSummaryResponse, UpdateSLOInput } from '@kbn/slo-schema'; +import { CreateSLOInput, GetSLOResponse, Indicator, UpdateSLOInput } from '@kbn/slo-schema'; import { assertNever } from '@kbn/std'; import { RecursivePartial } from '@kbn/utility-types'; +import { cloneDeep } from 'lodash'; import { toDuration } from '../../../utils/slo/duration'; import { APM_AVAILABILITY_DEFAULT_VALUES, @@ -15,12 +16,13 @@ import { CUSTOM_KQL_DEFAULT_VALUES, CUSTOM_METRIC_DEFAULT_VALUES, HISTOGRAM_DEFAULT_VALUES, + SLO_EDIT_FORM_DEFAULT_VALUES, TIMESLICE_METRIC_DEFAULT_VALUES, } from '../constants'; import { CreateSLOForm } from '../types'; export function transformSloResponseToCreateSloForm( - values: SLOWithSummaryResponse | undefined + values?: GetSLOResponse ): CreateSLOForm | undefined { if (!values) return undefined; @@ -144,12 +146,51 @@ function transformPartialIndicatorState( } export function transformPartialUrlStateToFormState( - values: RecursivePartial> -): Partial | {} { - const state: Partial = {}; + values: RecursivePartial +): CreateSLOForm { + const state: CreateSLOForm = cloneDeep(SLO_EDIT_FORM_DEFAULT_VALUES); - const parsedIndicator = transformPartialIndicatorState(values.indicator); - if (parsedIndicator !== undefined) state.indicator = parsedIndicator; + const indicator = transformPartialIndicatorState(values.indicator); + if (indicator !== undefined) { + state.indicator = indicator; + } + + if (values.name) { + state.name = values.name; + } + if (values.description) { + state.description = values.description; + } + if (!!values.tags) { + state.tags = values.tags as string[]; + } + + if (values.objective) { + if (values.objective.target) { + state.objective = { + target: values.objective.target * 100, + }; + + if (values.objective.timesliceTarget && values.objective.timesliceWindow) { + state.objective.timesliceTarget = values.objective.timesliceTarget * 100; + state.objective.timesliceWindow = String( + toDuration(values.objective.timesliceWindow).value + ); + } + } + } + + if (values.budgetingMethod) { + state.budgetingMethod = values.budgetingMethod; + } + + if (values.groupBy) { + state.groupBy = values.groupBy; + } + + if (values.timeWindow?.duration && values.timeWindow?.type) { + state.timeWindow = { duration: values.timeWindow.duration, type: values.timeWindow.type }; + } 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 f8354ae0304030..16edf6df757c1e 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 @@ -12,7 +12,7 @@ import { useHistory } from 'react-router-dom'; import { transformPartialUrlStateToFormState } from '../helpers/process_slo_form_values'; import { CreateSLOForm } from '../types'; -export function useParseUrlState(): Partial | null { +export function useParseUrlState(): CreateSLOForm | undefined { const history = useHistory(); const urlStateStorage = createKbnUrlStateStorage({ history, @@ -20,7 +20,7 @@ export function useParseUrlState(): Partial | null { useHashQuery: false, }); - const urlParams = urlStateStorage.get>('_a'); + const urlState = urlStateStorage.get>('_a'); - return !!urlParams ? transformPartialUrlStateToFormState(urlParams) : null; + return !!urlState ? transformPartialUrlStateToFormState(urlState) : undefined; } 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 b158094a67aabe..64e6552a78980e 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 @@ -5,27 +5,28 @@ * 2.0. */ +import { fireEvent, waitFor } from '@testing-library/dom'; +import { cleanup } from '@testing-library/react'; +import { createBrowserHistory } from 'history'; import React from 'react'; import Router from 'react-router-dom'; -import { createBrowserHistory } from 'history'; -import { waitFor, fireEvent, screen } from '@testing-library/dom'; -import { cleanup } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import { render } from '../../utils/test_helper'; -import { useKibana } from '../../utils/kibana_react'; -import { useLicense } from '../../hooks/use_license'; -import { useFetchIndices } from '../../hooks/use_fetch_indices'; -import { useFetchDataViews } from '../../hooks/use_fetch_data_views'; -import { useFetchSloDetails } from '../../hooks/slo/use_fetch_slo_details'; +import { paths } from '../../../common/locators/paths'; +import { buildSlo } from '../../data/slo/slo'; +import { useCapabilities } from '../../hooks/slo/use_capabilities'; import { useCreateSlo } from '../../hooks/slo/use_create_slo'; -import { useUpdateSlo } from '../../hooks/slo/use_update_slo'; import { useFetchApmSuggestions } from '../../hooks/slo/use_fetch_apm_suggestions'; +import { useFetchIndexPatternFields } from '../../hooks/slo/use_fetch_index_pattern_fields'; +import { useFetchSloDetails } from '../../hooks/slo/use_fetch_slo_details'; +import { useUpdateSlo } from '../../hooks/slo/use_update_slo'; +import { useFetchDataViews } from '../../hooks/use_fetch_data_views'; +import { useFetchIndices } from '../../hooks/use_fetch_indices'; +import { useLicense } from '../../hooks/use_license'; +import { useKibana } from '../../utils/kibana_react'; import { kibanaStartMock } from '../../utils/kibana_react.mock'; -import { buildSlo } from '../../data/slo/slo'; -import { paths } from '../../../common/locators/paths'; +import { render } from '../../utils/test_helper'; +import { SLO_EDIT_FORM_DEFAULT_VALUES } from './constants'; import { SloEditPage } from './slo_edit'; -import { useCapabilities } from '../../hooks/slo/use_capabilities'; jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), @@ -41,6 +42,7 @@ jest.mock('../../hooks/slo/use_create_slo'); jest.mock('../../hooks/slo/use_update_slo'); jest.mock('../../hooks/slo/use_fetch_apm_suggestions'); jest.mock('../../hooks/slo/use_capabilities'); +jest.mock('../../hooks/slo/use_fetch_index_pattern_fields'); const mockUseKibanaReturnValue = kibanaStartMock.startContract(); @@ -56,6 +58,7 @@ const useFetchSloMock = useFetchSloDetails as jest.Mock; const useCreateSloMock = useCreateSlo as jest.Mock; const useUpdateSloMock = useUpdateSlo as jest.Mock; const useFetchApmSuggestionsMock = useFetchApmSuggestions as jest.Mock; +const useFetchIndexPatternFieldsMock = useFetchIndexPatternFields as jest.Mock; const useCapabilitiesMock = useCapabilities as jest.Mock; const mockAddSuccess = jest.fn(); @@ -122,12 +125,50 @@ const mockKibana = () => { }; describe('SLO Edit Page', () => { + const mockCreate = jest.fn(); + const mockUpdate = jest.fn(); + beforeEach(() => { jest.clearAllMocks(); mockKibana(); // Silence all the ref errors in Eui components. + jest.spyOn(console, 'warn').mockImplementation(() => {}); jest.spyOn(console, 'error').mockImplementation(() => {}); + + const history = createBrowserHistory(); + history.replace(''); + jest.spyOn(Router, 'useHistory').mockReturnValueOnce(history); + + useFetchDataViewsMock.mockReturnValue({ + isLoading: false, + data: [{ getName: () => 'dataview', getIndexPattern: () => '.dataview-index' }], + }); + useFetchIndicesMock.mockReturnValue({ + isLoading: false, + data: ['some-index', 'index-2'], + }); + useFetchIndexPatternFieldsMock.mockReturnValue({ + isLoading: false, + data: [ + { name: 'field', type: 'date', aggregatable: false, searchable: false }, + { name: 'field_text', type: 'text', aggregatable: true, searchable: true }, + ], + }); + + useCreateSloMock.mockReturnValue({ + isLoading: false, + isSuccess: false, + isError: false, + mutateAsync: mockCreate, + }); + + useUpdateSloMock.mockReturnValue({ + isLoading: false, + isSuccess: false, + isError: false, + mutateAsync: mockUpdate, + }); }); afterEach(cleanup); @@ -149,28 +190,6 @@ describe('SLO Edit Page', () => { useFetchSloMock.mockReturnValue({ isLoading: false, data: undefined }); - useFetchIndicesMock.mockReturnValue({ - isLoading: false, - data: ['some-index'], - }); - useFetchDataViewsMock.mockReturnValue({ isLoading: false, data: [] }); - - useCreateSloMock.mockReturnValue({ - isLoading: false, - isSuccess: false, - isError: false, - mutate: jest.fn(), - mutateAsync: jest.fn(), - }); - - useUpdateSloMock.mockReturnValue({ - isLoading: false, - isSuccess: false, - isError: false, - mutate: jest.fn(), - mutateAsync: jest.fn(), - }); - render(); expect(mockNavigate).toBeCalledWith(mockBasePathPrepend(paths.observability.slos)); @@ -184,7 +203,6 @@ describe('SLO Edit Page', () => { hasReadCapabilities: true, }); useLicenseMock.mockReturnValue({ hasAtLeast: () => true }); - useFetchDataViewsMock.mockReturnValue({ isLoading: false, data: [] }); }); describe('with no write permission', () => { @@ -203,27 +221,6 @@ describe('SLO Edit Page', () => { useFetchSloMock.mockReturnValue({ isLoading: false, data: undefined }); - useFetchIndicesMock.mockReturnValue({ - isLoading: false, - data: ['some-index'], - }); - - useCreateSloMock.mockReturnValue({ - isLoading: false, - isSuccess: false, - isError: false, - mutate: jest.fn(), - mutateAsync: jest.fn(), - }); - - useUpdateSloMock.mockReturnValue({ - isLoading: false, - isSuccess: false, - isError: false, - mutate: jest.fn(), - mutateAsync: jest.fn(), - }); - render(); expect(mockNavigate).toBeCalledWith(mockBasePathPrepend(paths.observability.slos)); @@ -231,308 +228,119 @@ describe('SLO Edit Page', () => { }); describe('when no sloId route param is provided', () => { - it('renders the SLO Edit page in pristine state', async () => { - jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: undefined }); - jest - .spyOn(Router, 'useLocation') - .mockReturnValue({ pathname: 'foo', search: '', state: '', hash: '' }); - + beforeEach(() => { useFetchSloMock.mockReturnValue({ isLoading: false, data: undefined }); - - useFetchIndicesMock.mockReturnValue({ - isLoading: false, - data: ['some-index'], - }); - - useCreateSloMock.mockReturnValue({ - mutateAsync: jest.fn(), - isLoading: false, - isSuccess: false, - isError: false, - }); - - useUpdateSloMock.mockReturnValue({ - mutateAsync: jest.fn(), - isLoading: false, - isSuccess: false, - isError: false, - }); - - render(); - - expect(screen.queryByTestId('slosEditPage')).toBeTruthy(); - expect(screen.queryByTestId('sloForm')).toBeTruthy(); - - expect(screen.queryByTestId('sloEditFormIndicatorSection')).toBeTruthy(); - // Show default values from the kql indicator - expect(screen.queryByTestId('sloFormIndicatorTypeSelect')).toHaveValue('sli.kql.custom'); - expect(screen.queryByTestId('indexSelectionSelectedValue')).toBeNull(); - expect(screen.queryByTestId('customKqlIndicatorFormQueryFilterInput')).toHaveValue(''); - expect(screen.queryByTestId('customKqlIndicatorFormGoodQueryInput')).toHaveValue(''); - expect(screen.queryByTestId('customKqlIndicatorFormTotalQueryInput')).toHaveValue(''); - - // other sections are hidden - expect(screen.queryByTestId('sloEditFormObjectiveSection')).toBeNull(); - expect(screen.queryByTestId('sloEditFormDescriptionSection')).toBeNull(); }); - it.skip('calls the createSlo hook if all required values are filled in', async () => { + it('renders the SLO Edit page in pristine state', async () => { jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: undefined }); jest .spyOn(Router, 'useLocation') .mockReturnValue({ pathname: 'foo', search: '', state: '', hash: '' }); - useFetchIndicesMock.mockReturnValue({ - isLoading: false, - data: ['some-index'], - }); - - useFetchSloMock.mockReturnValue({ isLoading: false, data: undefined }); - - const mockCreate = jest.fn(); - const mockUpdate = jest.fn(); + const { queryByTestId } = render(); - useCreateSloMock.mockReturnValue({ - mutateAsync: mockCreate, - isLoading: false, - isSuccess: false, - isError: false, - }); + expect(queryByTestId('slosEditPage')).toBeTruthy(); + expect(queryByTestId('sloForm')).toBeTruthy(); - useUpdateSloMock.mockReturnValue({ - mutateAsync: mockUpdate, - isLoading: false, - isSuccess: false, - isError: false, - }); - - render(); - - userEvent.type(screen.getByTestId('indexSelection'), 'some-index'); - userEvent.type(screen.getByTestId('customKqlIndicatorFormQueryFilterInput'), 'irrelevant'); - userEvent.type(screen.getByTestId('customKqlIndicatorFormGoodQueryInput'), 'irrelevant'); - userEvent.type(screen.getByTestId('customKqlIndicatorFormTotalQueryInput'), 'irrelevant'); - userEvent.selectOptions(screen.getByTestId('sloFormBudgetingMethodSelect'), 'occurrences'); - userEvent.selectOptions(screen.getByTestId('sloFormTimeWindowDurationSelect'), '7d'); - userEvent.clear(screen.getByTestId('sloFormObjectiveTargetInput')); - userEvent.type(screen.getByTestId('sloFormObjectiveTargetInput'), '98.5'); - userEvent.type(screen.getByTestId('sloFormNameInput'), 'irrelevant'); - userEvent.type(screen.getByTestId('sloFormDescriptionTextArea'), 'irrelevant'); + expect(queryByTestId('sloEditFormIndicatorSection')).toBeTruthy(); + // Show default values from the kql indicator + expect(queryByTestId('sloFormIndicatorTypeSelect')).toHaveValue('sli.kql.custom'); + expect(queryByTestId('indexSelectionSelectedValue')).toBeNull(); - // all sections are visible - expect(screen.queryByTestId('sloEditFormIndicatorSection')).toBeTruthy(); - expect(screen.queryByTestId('sloEditFormObjectiveSection')).toBeTruthy(); - expect(screen.queryByTestId('sloEditFormDescriptionSection')).toBeTruthy(); - - expect(screen.getByTestId('sloFormSubmitButton')).toBeEnabled(); - - fireEvent.click(screen.getByTestId('sloFormSubmitButton')!); - - expect(mockCreate).toMatchInlineSnapshot(` - [MockFunction] { - "calls": Array [ - Array [ - Object { - "budgetingMethod": "occurrences", - "description": "irrelevant", - "indicator": Object { - "params": Object { - "filter": "irrelevant", - "good": "irrelevant", - "index": "some-index", - "total": "irrelevant", - }, - "type": "sli.kql.custom", - }, - "name": "irrelevant", - "objective": Object { - "target": 0.985, - }, - "timeWindow": Object { - "duration": "7d", - "type": "rolling", - }, - }, - ], - ], - "results": Array [ - Object { - "type": "return", - "value": undefined, - }, - ], - } - `); + // other sections are hidden + expect(queryByTestId('sloEditFormObjectiveSection')).toBeNull(); + expect(queryByTestId('sloEditFormDescriptionSection')).toBeNull(); }); - it('prefills the form with values when URL Search parameters are passed', () => { + it('prefills the form with values from URL', () => { jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: undefined }); const history = createBrowserHistory(); - history.push( + history.replace( '/slos/create?_a=(indicator:(params:(environment:prod,service:cartService),type:sli.apm.transactionDuration))' ); - jest.spyOn(Router, 'useHistory').mockReturnValue(history); + jest.spyOn(Router, 'useHistory').mockReturnValueOnce(history); jest .spyOn(Router, 'useLocation') .mockReturnValue({ pathname: 'foo', search: '', state: '', hash: '' }); - useFetchSloMock.mockReturnValue({ isLoading: false, data: undefined }); - useFetchApmSuggestionsMock.mockReturnValue({ suggestions: ['cartService'], isLoading: false, }); - useFetchIndicesMock.mockReturnValue({ - isLoading: false, - data: ['some-index'], - }); + const { queryByTestId } = render(); - useCreateSloMock.mockReturnValue({ - mutateAsync: jest.fn(), - isLoading: false, - isSuccess: false, - isError: false, - }); - - useUpdateSloMock.mockReturnValue({ - mutateAsync: jest.fn(), - isLoading: false, - isSuccess: false, - isError: false, - }); - - render(); - - expect(screen.queryByTestId('slosEditPage')).toBeTruthy(); - expect(screen.queryByTestId('sloForm')).toBeTruthy(); + expect(queryByTestId('slosEditPage')).toBeTruthy(); + expect(queryByTestId('sloForm')).toBeTruthy(); - expect(screen.queryByTestId('sloEditFormIndicatorSection')).toBeTruthy(); - expect(screen.queryByTestId('sloFormIndicatorTypeSelect')).toHaveValue( + expect(queryByTestId('sloEditFormIndicatorSection')).toBeTruthy(); + expect(queryByTestId('sloFormIndicatorTypeSelect')).toHaveValue( 'sli.apm.transactionDuration' ); - expect(screen.queryByTestId('apmLatencyServiceSelector')).toHaveTextContent('cartService'); - expect(screen.queryByTestId('apmLatencyEnvironmentSelector')).toHaveTextContent('prod'); + expect(queryByTestId('apmLatencyServiceSelector')).toHaveTextContent('cartService'); + expect(queryByTestId('apmLatencyEnvironmentSelector')).toHaveTextContent('prod'); - expect(screen.queryByTestId('sloEditFormObjectiveSection')).toBeFalsy(); - expect(screen.queryByTestId('sloEditFormDescriptionSection')).toBeFalsy(); + expect(queryByTestId('sloEditFormObjectiveSection')).toBeFalsy(); + expect(queryByTestId('sloEditFormDescriptionSection')).toBeFalsy(); }); }); describe('when a sloId route param is provided', () => { - it('renders the SLO Edit page with prefilled form values', async () => { - const slo = buildSlo({ id: '123' }); - jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: '123' }); + it('prefills the form with the SLO values', async () => { + const slo = buildSlo({ id: '123Foo' }); + useFetchSloMock.mockReturnValue({ isLoading: false, isInitialLoading: false, data: slo }); + jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: '123Foo' }); + jest .spyOn(Router, 'useLocation') .mockReturnValue({ pathname: 'foo', search: '', state: '', hash: '' }); - useFetchSloMock.mockReturnValue({ isLoading: false, data: slo }); - - useFetchIndicesMock.mockReturnValue({ - isLoading: false, - data: ['some-index'], - }); - - useCreateSloMock.mockReturnValue({ - mutateAsync: jest.fn(), - isLoading: false, - isSuccess: false, - isError: false, - }); - - useUpdateSloMock.mockReturnValue({ - mutateAsync: jest.fn(), - isLoading: false, - isSuccess: false, - isError: false, - }); - - render(); + const { queryByTestId } = render(); - expect(screen.queryByTestId('slosEditPage')).toBeTruthy(); - expect(screen.queryByTestId('sloForm')).toBeTruthy(); + expect(queryByTestId('slosEditPage')).toBeTruthy(); + expect(queryByTestId('sloForm')).toBeTruthy(); // all sections are visible - expect(screen.queryByTestId('sloEditFormIndicatorSection')).toBeTruthy(); - expect(screen.queryByTestId('sloEditFormObjectiveSection')).toBeTruthy(); - expect(screen.queryByTestId('sloEditFormDescriptionSection')).toBeTruthy(); + expect(queryByTestId('sloEditFormIndicatorSection')).toBeTruthy(); + expect(queryByTestId('sloEditFormObjectiveSection')).toBeTruthy(); + expect(queryByTestId('sloEditFormDescriptionSection')).toBeTruthy(); - expect(screen.queryByTestId('indexSelectionSelectedValue')).toHaveTextContent( - slo.indicator.params.index! - ); - expect(screen.queryByTestId('customKqlIndicatorFormQueryFilterInput')).toHaveValue( - slo.indicator.type === 'sli.kql.custom' ? slo.indicator.params.filter : '' - ); - expect(screen.queryByTestId('customKqlIndicatorFormGoodQueryInput')).toHaveValue( - slo.indicator.type === 'sli.kql.custom' ? slo.indicator.params.good : '' - ); - expect(screen.queryByTestId('customKqlIndicatorFormTotalQueryInput')).toHaveValue( - slo.indicator.type === 'sli.kql.custom' ? slo.indicator.params.total : '' - ); - - expect(screen.queryByTestId('sloFormBudgetingMethodSelect')).toHaveValue( - slo.budgetingMethod - ); - expect(screen.queryByTestId('sloFormTimeWindowDurationSelect')).toHaveValue( + expect(queryByTestId('sloFormBudgetingMethodSelect')).toHaveValue(slo.budgetingMethod); + expect(queryByTestId('sloFormTimeWindowDurationSelect')).toHaveValue( slo.timeWindow.duration ); - expect(screen.queryByTestId('sloFormObjectiveTargetInput')).toHaveValue( + expect(queryByTestId('sloFormObjectiveTargetInput')).toHaveValue( slo.objective.target * 100 ); - expect(screen.queryByTestId('sloFormNameInput')).toHaveValue(slo.name); - expect(screen.queryByTestId('sloFormDescriptionTextArea')).toHaveValue(slo.description); + expect(queryByTestId('sloFormNameInput')).toHaveValue(slo.name); + expect(queryByTestId('sloFormDescriptionTextArea')).toHaveValue(slo.description); }); it('calls the updateSlo hook if all required values are filled in', async () => { const slo = buildSlo({ id: '123' }); - jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: '123' }); - - useFetchIndicesMock.mockReturnValue({ - isLoading: false, - data: ['some-index'], - }); - useFetchSloMock.mockReturnValue({ isLoading: false, data: slo }); - const mockCreate = jest.fn(); - const mockUpdate = jest.fn(); + const { queryByTestId } = render(); - useCreateSloMock.mockReturnValue({ - mutateAsync: mockCreate, - isLoading: false, - isSuccess: false, - isError: false, - }); - - useUpdateSloMock.mockReturnValue({ - mutateAsync: mockUpdate, - isLoading: false, - isSuccess: false, - isError: false, - }); - - render(); - - expect(screen.queryByTestId('sloFormSubmitButton')).toBeEnabled(); - fireEvent.click(screen.queryByTestId('sloFormSubmitButton')!); + expect(queryByTestId('sloFormSubmitButton')).toBeEnabled(); + fireEvent.click(queryByTestId('sloFormSubmitButton')!); expect(mockUpdate).toMatchInlineSnapshot(`[MockFunction]`); }); - it('does not prefill the form with URL Search parameters when they are passed', () => { + it('prefills the form with the provided URL values and the default values', () => { const slo = buildSlo({ id: '123' }); - jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: '123' }); const history = createBrowserHistory(); history.push( - '/slos/create?_a=(name:%27prefilledSloName%27,indicator:(params:(environment:prod,service:cartService),type:sli.apm.transactionDuration))' + '/slos/123/edit?_a=(name:%27updated-name%27,indicator:(params:(environment:prod,service:cartService),type:sli.apm.transactionDuration),objective:(target:0.92))' ); - jest.spyOn(Router, 'useHistory').mockReturnValue(history); + jest.spyOn(Router, 'useHistory').mockReturnValueOnce(history); jest .spyOn(Router, 'useLocation') .mockReturnValue({ pathname: 'foo', search: '', state: '', hash: '' }); @@ -544,57 +352,24 @@ describe('SLO Edit Page', () => { isLoading: false, }); - useFetchIndicesMock.mockReturnValue({ - isLoading: false, - data: ['some-index'], - }); - - useCreateSloMock.mockReturnValue({ - mutateAsync: jest.fn(), - isLoading: false, - isSuccess: false, - isError: false, - }); - - useUpdateSloMock.mockReturnValue({ - mutateAsync: jest.fn(), - isLoading: false, - isSuccess: false, - isError: false, - }); - - render(); + const { queryByTestId } = render(); // all sections are visible - expect(screen.queryByTestId('sloEditFormIndicatorSection')).toBeTruthy(); - expect(screen.queryByTestId('sloEditFormObjectiveSection')).toBeTruthy(); - expect(screen.queryByTestId('sloEditFormDescriptionSection')).toBeTruthy(); + expect(queryByTestId('sloEditFormIndicatorSection')).toBeTruthy(); + expect(queryByTestId('sloEditFormObjectiveSection')).toBeTruthy(); + expect(queryByTestId('sloEditFormDescriptionSection')).toBeTruthy(); - expect(screen.queryByTestId('indexSelectionSelectedValue')).toHaveTextContent( - slo.indicator.params.index! - ); - expect(screen.queryByTestId('customKqlIndicatorFormQueryFilterInput')).toHaveValue( - slo.indicator.type === 'sli.kql.custom' ? slo.indicator.params.filter : '' - ); - expect(screen.queryByTestId('customKqlIndicatorFormGoodQueryInput')).toHaveValue( - slo.indicator.type === 'sli.kql.custom' ? slo.indicator.params.good : '' + expect(queryByTestId('indexSelectionSelectedValue')).toBeNull(); + expect(queryByTestId('sloFormBudgetingMethodSelect')).toHaveValue( + SLO_EDIT_FORM_DEFAULT_VALUES.budgetingMethod ); - expect(screen.queryByTestId('customKqlIndicatorFormTotalQueryInput')).toHaveValue( - slo.indicator.type === 'sli.kql.custom' ? slo.indicator.params.total : '' + expect(queryByTestId('sloFormTimeWindowDurationSelect')).toHaveValue( + SLO_EDIT_FORM_DEFAULT_VALUES.timeWindow.duration ); + expect(queryByTestId('sloFormObjectiveTargetInput')).toHaveValue(92); - expect(screen.queryByTestId('sloFormBudgetingMethodSelect')).toHaveValue( - slo.budgetingMethod - ); - expect(screen.queryByTestId('sloFormTimeWindowDurationSelect')).toHaveValue( - slo.timeWindow.duration - ); - expect(screen.queryByTestId('sloFormObjectiveTargetInput')).toHaveValue( - slo.objective.target * 100 - ); - - expect(screen.queryByTestId('sloFormNameInput')).toHaveValue(slo.name); - expect(screen.queryByTestId('sloFormDescriptionTextArea')).toHaveValue(slo.description); + expect(queryByTestId('sloFormNameInput')).toHaveValue('updated-name'); + expect(queryByTestId('sloFormDescriptionTextArea')).toHaveValue(''); }); }); @@ -609,31 +384,12 @@ describe('SLO Edit Page', () => { useFetchSloMock.mockReturnValue({ isLoading: false, data: slo }); - useFetchIndicesMock.mockReturnValue({ - isLoading: false, - data: ['some-index'], - }); + const { getByTestId } = render(); - useCreateSloMock.mockReturnValue({ - mutateAsync: jest.fn(), - isLoading: false, - isSuccess: false, - isError: false, - }); - - useUpdateSloMock.mockReturnValue({ - mutateAsync: jest.fn(), - isLoading: false, - isSuccess: false, - isError: false, - }); - - render(); - - expect(screen.queryByTestId('sloFormSubmitButton')).toBeEnabled(); + expect(getByTestId('sloFormSubmitButton')).toBeEnabled(); await waitFor(() => { - fireEvent.click(screen.getByTestId('sloFormSubmitButton')); + fireEvent.click(getByTestId('sloFormSubmitButton')); }); await waitFor(() => { expect(mockNavigate).toBeCalledWith(mockBasePathPrepend(paths.observability.slos)); @@ -650,32 +406,13 @@ describe('SLO Edit Page', () => { useFetchSloMock.mockReturnValue({ isLoading: false, data: slo }); - useFetchIndicesMock.mockReturnValue({ - isLoading: false, - data: ['some-index'], - }); - - useCreateSloMock.mockReturnValue({ - mutateAsync: jest.fn(), - isLoading: false, - isSuccess: false, - isError: false, - }); - - useUpdateSloMock.mockReturnValue({ - mutateAsync: jest.fn(), - isLoading: false, - isSuccess: false, - isError: false, - }); - - render(); + const { getByTestId } = render(); - expect(screen.queryByTestId('sloFormSubmitButton')).toBeEnabled(); + expect(getByTestId('sloFormSubmitButton')).toBeEnabled(); await waitFor(() => { - fireEvent.click(screen.getByTestId('createNewRuleCheckbox')); - fireEvent.click(screen.getByTestId('sloFormSubmitButton')); + fireEvent.click(getByTestId('createNewRuleCheckbox')); + fireEvent.click(getByTestId('sloFormSubmitButton')); }); await waitFor(() => { @@ -695,29 +432,10 @@ describe('SLO Edit Page', () => { useFetchSloMock.mockReturnValue({ isLoading: false, data: slo }); - useFetchIndicesMock.mockReturnValue({ - isLoading: false, - data: ['some-index'], - }); - - useCreateSloMock.mockReturnValue({ - mutateAsync: jest.fn(), - isLoading: false, - isSuccess: false, - isError: false, - }); - - useUpdateSloMock.mockReturnValue({ - mutateAsync: jest.fn(), - isLoading: false, - isSuccess: false, - isError: false, - }); - - render(); + const { getByTestId } = render(); await waitFor(() => { - expect(screen.getByTestId('add-rule-flyout')).toBeTruthy(); + expect(getByTestId('add-rule-flyout')).toBeTruthy(); }); }); });