diff --git a/packages/kbn-slo-schema/src/rest_specs/slo.ts b/packages/kbn-slo-schema/src/rest_specs/slo.ts index 629a7ac5b5738..246e50ba1383e 100644 --- a/packages/kbn-slo-schema/src/rest_specs/slo.ts +++ b/packages/kbn-slo-schema/src/rest_specs/slo.ts @@ -96,7 +96,7 @@ const updateSLOParamsSchema = t.type({ timeWindow: timeWindowSchema, budgetingMethod: budgetingMethodSchema, objective: objectiveSchema, - settings: settingsSchema, + settings: optionalSettingsSchema, }), }); @@ -112,11 +112,13 @@ const findSLOResponseSchema = t.type({ type SLOResponse = t.OutputOf; type SLOWithSummaryResponse = t.OutputOf; -type CreateSLOParams = t.TypeOf; -type CreateSLOResponse = t.TypeOf; +type CreateSLOInput = t.OutputOf; // Raw payload sent by the frontend +type CreateSLOParams = t.TypeOf; // Parsed payload used by the backend +type CreateSLOResponse = t.TypeOf; // Raw response sent to the frontend type GetSLOResponse = t.OutputOf; +type UpdateSLOInput = t.OutputOf; type UpdateSLOParams = t.TypeOf; type UpdateSLOResponse = t.OutputOf; @@ -139,6 +141,7 @@ export { }; export type { BudgetingMethod, + CreateSLOInput, CreateSLOParams, CreateSLOResponse, FindSLOParams, @@ -146,6 +149,7 @@ export type { GetSLOResponse, SLOResponse, SLOWithSummaryResponse, + UpdateSLOInput, UpdateSLOParams, UpdateSLOResponse, }; diff --git a/x-pack/plugins/observability/public/data/slo.ts b/x-pack/plugins/observability/public/data/slo.ts index 73154263783af..806c5f07231b2 100644 --- a/x-pack/plugins/observability/public/data/slo.ts +++ b/x-pack/plugins/observability/public/data/slo.ts @@ -100,7 +100,7 @@ export const sloList: FindSLOResponse = { }, { ...baseSlo, - id: 'c0f8d669-9177-4706-9098-f397a88173a7', + id: 'c0f8d669-9177-4706-9098-f397a88173a8', budgetingMethod: 'timeslices', objective: { target: 0.98, timesliceTarget: 0.9, timesliceWindow: '5m' }, summary: { 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 77bf736ae46fd..31f8202c3c77e 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 @@ -6,25 +6,31 @@ */ import { useCallback, useState } from 'react'; -import type { CreateSLOParams, CreateSLOResponse } from '@kbn/slo-schema'; +import type { + CreateSLOInput, + CreateSLOResponse, + UpdateSLOInput, + UpdateSLOResponse, +} from '@kbn/slo-schema'; import { useKibana } from '../../utils/kibana_react'; -interface UseCreateSlo { +interface UseCreateOrUpdateSlo { loading: boolean; success: boolean; error: string | undefined; - createSlo: (slo: CreateSLOParams) => void; + createSlo: (slo: CreateSLOInput) => void; + updateSlo: (sloId: string, slo: UpdateSLOInput) => void; } -export function useCreateSlo(): UseCreateSlo { +export function useCreateOrUpdateSlo(): UseCreateOrUpdateSlo { const { http } = useKibana().services; const [loading, setLoading] = useState(false); const [success, setSuccess] = useState(false); const [error, setError] = useState(undefined); const createSlo = useCallback( - async (slo: CreateSLOParams) => { + async (slo: CreateSLOInput) => { setLoading(true); setError(''); setSuccess(false); @@ -40,10 +46,28 @@ export function useCreateSlo(): UseCreateSlo { [http] ); + const updateSlo = useCallback( + async (sloId: string, slo: UpdateSLOInput) => { + setLoading(true); + setError(''); + setSuccess(false); + const body = JSON.stringify(slo); + + try { + await http.put(`/api/observability/slos/${sloId}`, { body }); + setSuccess(true); + } catch (e) { + setError(e); + } + }, + [http] + ); + return { loading, error, success, createSlo, + updateSlo, }; } diff --git a/x-pack/plugins/observability/public/pages/slo_edit/__snapshots__/index.test.tsx.snap b/x-pack/plugins/observability/public/pages/slo_edit/__snapshots__/index.test.tsx.snap new file mode 100644 index 0000000000000..252c2843bf2b0 --- /dev/null +++ b/x-pack/plugins/observability/public/pages/slo_edit/__snapshots__/index.test.tsx.snap @@ -0,0 +1,79 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`SLO Edit Page when a sloId route param is provided calls the updateSlo hook if all required values are filled in 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "2f17deb0-725a-11ed-ab7c-4bb641cfc57e", + Object { + "budgetingMethod": "occurrences", + "description": "irrelevant", + "indicator": Object { + "params": Object { + "filter": "baz: foo and bar > 2", + "good": "http_status: 2xx", + "index": "some-index", + "total": "a query", + }, + "type": "sli.kql.custom", + }, + "name": "irrelevant", + "objective": Object { + "target": 0.98, + }, + "settings": Object { + "frequency": "1m", + "syncDelay": "1m", + "timestampField": "@timestamp", + }, + "timeWindow": Object { + "duration": "30d", + "isRolling": true, + }, + }, + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + +exports[`SLO Edit Page when no sloId route param is provided calls the createSlo hook if all required values are filled in 1`] = ` +[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", + "isRolling": true, + }, + }, + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; 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 723f4e01af90b..97bca4635635a 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 @@ -23,14 +23,15 @@ import { Controller, useForm } from 'react-hook-form'; import type { SLOWithSummaryResponse } from '@kbn/slo-schema'; import { useKibana } from '../../../utils/kibana_react'; -import { useCreateSlo } from '../../../hooks/slo/use_create_slo'; +import { useCreateOrUpdateSlo } from '../../../hooks/slo/use_create_slo'; import { useCheckFormPartialValidities } from '../helpers/use_check_form_partial_validities'; import { SloEditFormDefinitionCustomKql } from './slo_edit_form_definition_custom_kql'; import { SloEditFormDescription } from './slo_edit_form_description'; import { SloEditFormObjectives } from './slo_edit_form_objectives'; import { - processValues, - transformGetSloToCreateSloParams, + transformValuesToCreateSLOInput, + transformSloResponseToCreateSloInput, + transformValuesToUpdateSLOInput, } from '../helpers/process_slo_form_values'; import { paths } from '../../../config'; import { SLI_OPTIONS, SLO_EDIT_FORM_DEFAULT_VALUES } from '../constants'; @@ -42,6 +43,7 @@ export interface Props { const maxWidth = 775; export function SloEditForm({ slo }: Props) { + const isEditMode = slo !== undefined; const { application: { navigateToUrl }, http: { basePath }, @@ -50,7 +52,7 @@ export function SloEditForm({ slo }: Props) { const { control, watch, getFieldState, getValues, formState, trigger } = useForm({ defaultValues: SLO_EDIT_FORM_DEFAULT_VALUES, - values: transformGetSloToCreateSloParams(slo), + values: transformSloResponseToCreateSloInput(slo), mode: 'all', }); @@ -58,31 +60,36 @@ export function SloEditForm({ slo }: Props) { { getFieldState, formState } ); - const { - loading: loadingCreatingSlo, - success: successCreatingSlo, - error: errorCreatingSlo, - createSlo, - } = useCreateSlo(); + const { loading, success, error, createSlo, updateSlo } = useCreateOrUpdateSlo(); - const handleCreateSlo = () => { + const handleSubmit = () => { const values = getValues(); - const processedValues = processValues(values); - createSlo(processedValues); + if (isEditMode) { + const processedValues = transformValuesToUpdateSLOInput(values); + updateSlo(slo.id, processedValues); + } else { + const processedValues = transformValuesToCreateSLOInput(values); + createSlo(processedValues); + } }; - if (successCreatingSlo) { + if (success) { toasts.addSuccess( - i18n.translate('xpack.observability.slos.sloEdit.creation.success', { - defaultMessage: 'Successfully created {name}', - values: { name: getValues().name }, - }) + isEditMode + ? i18n.translate('xpack.observability.slos.sloEdit.update.success', { + defaultMessage: 'Successfully updated {name}', + values: { name: getValues().name }, + }) + : i18n.translate('xpack.observability.slos.sloEdit.creation.success', { + defaultMessage: 'Successfully created {name}', + values: { name: getValues().name }, + }) ); navigateToUrl(basePath.prepend(paths.observability.slos)); } - if (errorCreatingSlo) { - toasts.addError(new Error(errorCreatingSlo), { + if (error) { + toasts.addError(new Error(error), { title: i18n.translate('xpack.observability.slos.sloEdit.creation.error', { defaultMessage: 'Something went wrong', }), @@ -197,13 +204,17 @@ export function SloEditForm({ slo }: Props) { fill color="primary" data-test-subj="sloFormSubmitButton" - onClick={handleCreateSlo} + onClick={handleSubmit} disabled={!formState.isValid} - isLoading={loadingCreatingSlo && !errorCreatingSlo} + isLoading={loading && !error} > - {i18n.translate('xpack.observability.slos.sloEdit.createSloButton', { - defaultMessage: 'Create SLO', - })} + {isEditMode + ? i18n.translate('xpack.observability.slos.sloEdit.editSloButton', { + defaultMessage: 'Update SLO', + }) + : i18n.translate('xpack.observability.slos.sloEdit.createSloButton', { + defaultMessage: 'Create SLO', + })} diff --git a/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_definition_custom_kql.stories.tsx b/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_definition_custom_kql.stories.tsx index 08c634b45faaf..f0ed441a56a1a 100644 --- a/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_definition_custom_kql.stories.tsx +++ b/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_definition_custom_kql.stories.tsx @@ -26,7 +26,7 @@ const Template: ComponentStory = (props: Props) => { const methods = useForm({ defaultValues: SLO_EDIT_FORM_DEFAULT_VALUES }); return ( - + ); }; diff --git a/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_definition_custom_kql.tsx b/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_definition_custom_kql.tsx index dae887c595e77..2d13548c7c8e9 100644 --- a/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_definition_custom_kql.tsx +++ b/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_definition_custom_kql.tsx @@ -9,13 +9,13 @@ import React, { useEffect } from 'react'; import { EuiButtonEmpty, EuiFlexGroup, EuiFlexItem, EuiFormLabel, EuiSuggest } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { Control, Controller, UseFormTrigger } from 'react-hook-form'; -import type { CreateSLOParams } from '@kbn/slo-schema'; +import type { CreateSLOInput } from '@kbn/slo-schema'; import { useFetchIndices } from '../../../hooks/use_fetch_indices'; export interface Props { - control: Control; - trigger: UseFormTrigger; + control: Control; + trigger: UseFormTrigger; } export function SloEditFormDefinitionCustomKql({ control, trigger }: Props) { @@ -35,6 +35,18 @@ export function SloEditFormDefinitionCustomKql({ control, trigger }: Props) { } }, [indices.length, loading, trigger]); + function valueMatchIndex(value: string | undefined, index: string): boolean { + if (value === undefined) { + return false; + } + + if (value.length > 0 && value.substring(value.length - 1) === '*') { + return index.indexOf(value.substring(0, value.length - 1), 0) > -1; + } + + return index === value; + } + return ( @@ -49,9 +61,9 @@ export function SloEditFormDefinitionCustomKql({ control, trigger }: Props) { control={control} rules={{ required: true, - validate: (value) => Boolean(indices.find((index) => index.name === value)), + validate: (value) => indices.some((index) => valueMatchIndex(value, index.name)), }} - render={({ field }) => ( + render={({ field, fieldState }) => ( { field.onChange(label); }} - isInvalid={!Boolean(indicesNames.find((index) => index.label === field.value))} + isInvalid={ + fieldState.isDirty && + !indicesNames.some((index) => valueMatchIndex(field.value, index.label)) + } placeholder={i18n.translate( 'xpack.observability.slos.sloEdit.sloDefinition.customKql.index.selectIndex', { diff --git a/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_description.tsx b/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_description.tsx index c8442d5c5d56c..c169b3316a681 100644 --- a/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_description.tsx +++ b/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_description.tsx @@ -16,10 +16,10 @@ import { import { i18n } from '@kbn/i18n'; import React from 'react'; import { Control, Controller } from 'react-hook-form'; -import type { CreateSLOParams } from '@kbn/slo-schema'; +import type { CreateSLOInput } from '@kbn/slo-schema'; export interface Props { - control: Control; + control: Control; } export function SloEditFormDescription({ control }: Props) { diff --git a/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_objectives.tsx b/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_objectives.tsx index 6f32b8d749cb1..419c32bededd8 100644 --- a/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_objectives.tsx +++ b/x-pack/plugins/observability/public/pages/slo_edit/components/slo_edit_form_objectives.tsx @@ -17,26 +17,14 @@ import { } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { Control, Controller, UseFormWatch } from 'react-hook-form'; -import type { BudgetingMethod, CreateSLOParams } from '@kbn/slo-schema'; +import type { CreateSLOInput } from '@kbn/slo-schema'; import { SloEditFormObjectivesTimeslices } from './slo_edit_form_objectives_timeslices'; - -export const BUDGETING_METHOD_OPTIONS: Array<{ value: BudgetingMethod; text: string }> = [ - { value: 'occurrences', text: 'Occurences' }, - { value: 'timeslices', text: 'Timeslices' }, -]; - -export const TIMEWINDOW_OPTIONS = [30, 7].map((number) => ({ - value: `${number}d`, - text: i18n.translate('xpack.observability.slos.sloEdit.objectives.days', { - defaultMessage: '{number} days', - values: { number }, - }), -})); +import { BUDGETING_METHOD_OPTIONS, TIMEWINDOW_OPTIONS } from '../constants'; export interface Props { - control: Control; - watch: UseFormWatch; + control: Control; + watch: UseFormWatch; } export function SloEditFormObjectives({ control, watch }: Props) { @@ -48,7 +36,7 @@ export function SloEditFormObjectives({ control, watch }: Props) { - {i18n.translate('xpack.observability.slos.sloEdit.objectives.budgetingMethod', { + {i18n.translate('xpack.observability.slos.sloEdit.budgetingMethod.label', { defaultMessage: 'Budgeting method', })} @@ -70,7 +58,7 @@ export function SloEditFormObjectives({ control, watch }: Props) { - {i18n.translate('xpack.observability.slos.sloEdit.objectives.timeWindow', { + {i18n.translate('xpack.observability.slos.sloEdit.timeWindow.label', { defaultMessage: 'Time window', })} @@ -93,7 +81,7 @@ export function SloEditFormObjectives({ control, watch }: Props) { - {i18n.translate('xpack.observability.slos.sloEdit.objectives.targetSlo', { + {i18n.translate('xpack.observability.slos.sloEdit.targetSlo.label', { defaultMessage: 'Target / SLO (%)', })} @@ -110,6 +98,7 @@ export function SloEditFormObjectives({ control, watch }: Props) { ; + control: Control; } export function SloEditFormObjectivesTimeslices({ control }: Props) { @@ -20,7 +20,7 @@ export function SloEditFormObjectivesTimeslices({ control }: Props) { - {i18n.translate('xpack.observability.slos.sloEdit.objectives.timeSliceTarget', { + {i18n.translate('xpack.observability.slos.sloEdit.timeSliceTarget.label', { defaultMessage: 'Timeslice target (%)', })} @@ -36,6 +36,7 @@ export function SloEditFormObjectivesTimeslices({ control }: Props) { render={({ field }) => ( - {i18n.translate('xpack.observability.slos.sloEdit.objectives.timesliceWindow', { + {i18n.translate('xpack.observability.slos.sloEdit.timesliceWindow.label', { defaultMessage: 'Timeslice window (minutes)', })} ( @@ -65,7 +67,7 @@ export function SloEditFormObjectivesTimeslices({ control }: Props) { min={1} max={120} step={1} - onChange={(event) => field.onChange(String(event.target.value))} + onChange={(event) => field.onChange(String(Number(event.target.value)))} /> )} /> diff --git a/x-pack/plugins/observability/public/pages/slo_edit/constants.ts b/x-pack/plugins/observability/public/pages/slo_edit/constants.ts index fc036f5ba6ee7..9d44bc707ce36 100644 --- a/x-pack/plugins/observability/public/pages/slo_edit/constants.ts +++ b/x-pack/plugins/observability/public/pages/slo_edit/constants.ts @@ -6,12 +6,7 @@ */ import { i18n } from '@kbn/i18n'; -import type { CreateSLOParams } from '@kbn/slo-schema'; - -import { - BUDGETING_METHOD_OPTIONS, - TIMEWINDOW_OPTIONS, -} from './components/slo_edit_form_objectives'; +import { BudgetingMethod, CreateSLOInput } from '@kbn/slo-schema'; export const SLI_OPTIONS = [ { @@ -22,7 +17,30 @@ export const SLI_OPTIONS = [ }, ]; -export const SLO_EDIT_FORM_DEFAULT_VALUES: CreateSLOParams = { +export const BUDGETING_METHOD_OPTIONS: Array<{ value: BudgetingMethod; text: string }> = [ + { + value: 'occurrences', + text: i18n.translate('xpack.observability.slos.sloEdit.budgetingMethod.occurrences', { + defaultMessage: 'Occurrences', + }), + }, + { + value: 'timeslices', + text: i18n.translate('xpack.observability.slos.sloEdit.budgetingMethod.timeslices', { + defaultMessage: 'Timeslices', + }), + }, +]; + +export const TIMEWINDOW_OPTIONS = [90, 30, 7].map((number) => ({ + value: `${number}d`, + text: i18n.translate('xpack.observability.slos.sloEdit.timeWindow.days', { + defaultMessage: '{number} days', + values: { number }, + }), +})); + +export const SLO_EDIT_FORM_DEFAULT_VALUES: CreateSLOInput = { name: '', description: '', indicator: { @@ -35,7 +53,8 @@ export const SLO_EDIT_FORM_DEFAULT_VALUES: CreateSLOParams = { }, }, timeWindow: { - duration: TIMEWINDOW_OPTIONS[0].value as any, // Get this to be a proper Duration + duration: + TIMEWINDOW_OPTIONS[TIMEWINDOW_OPTIONS.findIndex((option) => option.value === '30d')].value, isRolling: true, }, budgetingMethod: BUDGETING_METHOD_OPTIONS[0].value, 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 35ad905d46fea..0c2d2a5989477 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,25 +5,31 @@ * 2.0. */ -import type { CreateSLOParams, GetSLOResponse } from '@kbn/slo-schema'; +import omit from 'lodash/omit'; +import type { CreateSLOInput, SLOWithSummaryResponse, UpdateSLOInput } from '@kbn/slo-schema'; -export function transformGetSloToCreateSloParams( - values: GetSLOResponse | undefined -): CreateSLOParams | undefined { +import { toDuration } from '../../../utils/slo/duration'; + +export function transformSloResponseToCreateSloInput( + values: SLOWithSummaryResponse | undefined +): CreateSLOInput | undefined { if (!values) return undefined; return { - ...values, + ...omit(values, ['id', 'revision', 'createdAt', 'updatedAt', 'summary']), objective: { target: values.objective.target * 100, ...(values.objective.timesliceTarget && { timesliceTarget: values.objective.timesliceTarget * 100, }), + ...(values.objective.timesliceWindow && { + timesliceWindow: String(toDuration(values.objective.timesliceWindow).value), + }), }, - } as unknown as CreateSLOParams; + }; } -export function processValues(values: CreateSLOParams): CreateSLOParams { +export function transformValuesToCreateSLOInput(values: CreateSLOInput): CreateSLOInput { return { ...values, objective: { @@ -31,6 +37,24 @@ export function processValues(values: CreateSLOParams): CreateSLOParams { ...(values.objective.timesliceTarget && { timesliceTarget: values.objective.timesliceTarget / 100, }), + ...(values.objective.timesliceWindow && { + timesliceWindow: `${values.objective.timesliceWindow}m`, + }), + }, + }; +} + +export function transformValuesToUpdateSLOInput(values: CreateSLOInput): UpdateSLOInput { + return { + ...values, + objective: { + target: values.objective.target / 100, + ...(values.objective.timesliceTarget && { + timesliceTarget: values.objective.timesliceTarget / 100, + }), + ...(values.objective.timesliceWindow && { + timesliceWindow: `${values.objective.timesliceWindow}m`, + }), }, }; } diff --git a/x-pack/plugins/observability/public/pages/slo_edit/helpers/use_check_form_partial_validities.ts b/x-pack/plugins/observability/public/pages/slo_edit/helpers/use_check_form_partial_validities.ts index 5ae5d5b7fa283..392cfd0ef5204 100644 --- a/x-pack/plugins/observability/public/pages/slo_edit/helpers/use_check_form_partial_validities.ts +++ b/x-pack/plugins/observability/public/pages/slo_edit/helpers/use_check_form_partial_validities.ts @@ -5,12 +5,12 @@ * 2.0. */ +import { CreateSLOInput } from '@kbn/slo-schema'; import { FormState, UseFormGetFieldState } from 'react-hook-form'; -import type { CreateSLOParams } from '@kbn/slo-schema'; interface Props { - getFieldState: UseFormGetFieldState; - formState: FormState; + getFieldState: UseFormGetFieldState; + formState: FormState; } export function useCheckFormPartialValidities({ getFieldState, formState }: Props) { @@ -24,7 +24,13 @@ export function useCheckFormPartialValidities({ getFieldState, formState }: Prop ).every((field) => getFieldState(field, formState).error === undefined); const isObjectiveValid = ( - ['budgetingMethod', 'timeWindow.duration', 'objective.target'] as const + [ + 'budgetingMethod', + 'timeWindow.duration', + 'objective.target', + 'objective.timesliceTarget', + 'objective.timesliceWindow', + ] as const ).every((field) => getFieldState(field, formState).error === undefined); const isDescriptionValid = (['name', 'description'] as const).every( diff --git a/x-pack/plugins/observability/public/pages/slo_edit/index.test.tsx b/x-pack/plugins/observability/public/pages/slo_edit/index.test.tsx index 816693cd6a9f1..2fb53fb8a5096 100644 --- a/x-pack/plugins/observability/public/pages/slo_edit/index.test.tsx +++ b/x-pack/plugins/observability/public/pages/slo_edit/index.test.tsx @@ -10,12 +10,13 @@ import Router from 'react-router-dom'; import { waitFor, fireEvent, screen } from '@testing-library/dom'; import { BasePath } from '@kbn/core-http-server-internal'; 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 { useFetchIndices } from '../../hooks/use_fetch_indices'; import { useFetchSloDetails } from '../../hooks/slo/use_fetch_slo_details'; -import { useCreateSlo } from '../../hooks/slo/use_create_slo'; +import { useCreateOrUpdateSlo } from '../../hooks/slo/use_create_slo'; import { kibanaStartMock } from '../../utils/kibana_react.mock'; import { ConfigSchema } from '../../plugin'; import { Subset } from '../../typings'; @@ -42,7 +43,7 @@ jest.mock('../../utils/kibana_react', () => ({ const useFetchIndicesMock = useFetchIndices as jest.Mock; const useFetchSloMock = useFetchSloDetails as jest.Mock; -const useCreateSloMock = useCreateSlo as jest.Mock; +const useCreateOrUpdateSloMock = useCreateOrUpdateSlo as jest.Mock; const mockAddSuccess = jest.fn(); const mockAddError = jest.fn(); @@ -79,7 +80,7 @@ describe('SLO Edit Page', () => { afterEach(cleanup); - describe('when the feature flag is not enabled', () => { + describe('when the feature flag is disabled', () => { it('renders the not found page when no sloId param is passed', async () => { jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: undefined }); @@ -101,8 +102,8 @@ describe('SLO Edit Page', () => { }); }); - describe('when the feature flag is enabled', () => { - it('renders the SLO Edit page in pristine state when no sloId route param is passed', async () => { + 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 }); useFetchSloMock.mockReturnValue({ loading: false, slo: undefined }); @@ -110,11 +111,12 @@ describe('SLO Edit Page', () => { loading: false, indices: [{ name: 'some-index' }], }); - useCreateSloMock.mockReturnValue({ + useCreateOrUpdateSloMock.mockReturnValue({ loading: false, success: false, error: '', createSlo: jest.fn(), + updateSlo: jest.fn(), }); render(, config); @@ -163,7 +165,48 @@ describe('SLO Edit Page', () => { ); }); - it('renders the SLO Edit page with prefilled form values if sloId route param is passed', async () => { + it.skip('calls the createSlo hook if all required values are filled in', async () => { + jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: undefined }); + useFetchIndicesMock.mockReturnValue({ + loading: false, + indices: [{ name: 'some-index' }], + }); + useFetchSloMock.mockReturnValue({ loading: false, slo: undefined }); + const mockCreate = jest.fn(); + const mockUpdate = jest.fn(); + useCreateOrUpdateSloMock.mockReturnValue({ + loading: false, + success: false, + error: '', + createSlo: mockCreate, + updateSlo: mockUpdate, + }); + + render(, config); + + userEvent.type(screen.getByTestId('sloFormCustomKqlIndexInput'), 'some-index'); + userEvent.type(screen.getByTestId('sloFormCustomKqlFilterQueryInput'), 'irrelevant'); + userEvent.type(screen.getByTestId('sloFormCustomKqlGoodQueryInput'), 'irrelevant'); + userEvent.type(screen.getByTestId('sloFormCustomKqlTotalQueryInput'), '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'); + + const t = Date.now(); + await waitFor(() => expect(screen.getByTestId('sloFormSubmitButton')).toBeEnabled()); + console.log('end waiting for submit button: ', Math.ceil(Date.now() - t)); + + fireEvent.click(screen.getByTestId('sloFormSubmitButton')!); + + expect(mockCreate).toMatchSnapshot(); + }); + }); + + describe('when a sloId route param is provided', () => { + it('renders the SLO Edit page with prefilled form values', async () => { jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: '123' }); useFetchSloMock.mockReturnValue({ loading: false, slo: anSLO }); @@ -171,11 +214,12 @@ describe('SLO Edit Page', () => { loading: false, indices: [{ name: 'some-index' }], }); - useCreateSloMock.mockReturnValue({ + useCreateOrUpdateSloMock.mockReturnValue({ loading: false, success: false, error: '', createSlo: jest.fn(), + updateSlo: jest.fn(), }); render(, config); @@ -211,7 +255,7 @@ describe('SLO Edit Page', () => { expect(screen.queryByTestId('sloFormDescriptionTextArea')).toHaveValue(anSLO.description); }); - it('enables submitting if all required values are filled in', async () => { + it('calls the updateSlo hook if all required values are filled in', async () => { // Note: the `anSLO` object is considered to have (at least) // values for all required fields. @@ -223,11 +267,13 @@ describe('SLO Edit Page', () => { }); useFetchSloMock.mockReturnValue({ loading: false, slo: anSLO }); const mockCreate = jest.fn(); - useCreateSloMock.mockReturnValue({ + const mockUpdate = jest.fn(); + useCreateOrUpdateSloMock.mockReturnValue({ loading: false, success: false, error: '', createSlo: mockCreate, + updateSlo: mockUpdate, }); render(, config); @@ -236,7 +282,7 @@ describe('SLO Edit Page', () => { fireEvent.click(screen.queryByTestId('sloFormSubmitButton')!); - expect(mockCreate).toBeCalledWith(anSLO); + expect(mockUpdate).toMatchSnapshot(); }); it('blocks submitting if not all required values are filled in', async () => { @@ -257,66 +303,69 @@ describe('SLO Edit Page', () => { expect(screen.queryByTestId('sloFormSubmitButton')).toBeDisabled(); }); }); + }); - describe('if submitting has completed successfully', () => { - it('renders a success toast', async () => { - // Note: the `anSLO` object is considered to have (at least) - // values for all required fields. - jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: '123' }); - useFetchSloMock.mockReturnValue({ loading: false, slo: anSLO }); - useFetchIndicesMock.mockReturnValue({ - loading: false, - indices: [{ name: 'some-index' }], - }); - useCreateSloMock.mockReturnValue({ - loading: false, - success: true, - error: '', - createSlo: jest.fn(), - }); - render(, config); - expect(mockAddSuccess).toBeCalled(); + describe('if submitting has completed successfully', () => { + it('renders a success toast', async () => { + // Note: the `anSLO` object is considered to have (at least) + // values for all required fields. + jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: '123' }); + useFetchSloMock.mockReturnValue({ loading: false, slo: anSLO }); + useFetchIndicesMock.mockReturnValue({ + loading: false, + indices: [{ name: 'some-index' }], + }); + useCreateOrUpdateSloMock.mockReturnValue({ + loading: false, + success: true, + error: '', + createSlo: jest.fn(), + updateSlo: jest.fn(), }); + render(, config); + expect(mockAddSuccess).toBeCalled(); + }); - it('navigates to the SLO List page', async () => { - // Note: the `anSLO` object is considered to have (at least) - // values for all required fields. - jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: '123' }); - useFetchSloMock.mockReturnValue({ loading: false, slo: anSLO }); - useFetchIndicesMock.mockReturnValue({ - loading: false, - indices: [{ name: 'some-index' }], - }); - useCreateSloMock.mockReturnValue({ - loading: false, - success: true, - error: '', - createSlo: jest.fn(), - }); - render(, config); - expect(mockNavigate).toBeCalledWith(paths.observability.slos); + it('navigates to the SLO List page', async () => { + // Note: the `anSLO` object is considered to have (at least) + // values for all required fields. + jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: '123' }); + useFetchSloMock.mockReturnValue({ loading: false, slo: anSLO }); + useFetchIndicesMock.mockReturnValue({ + loading: false, + indices: [{ name: 'some-index' }], + }); + useCreateOrUpdateSloMock.mockReturnValue({ + loading: false, + success: true, + error: '', + createSlo: jest.fn(), + updateSlo: jest.fn(), }); + render(, config); + expect(mockNavigate).toBeCalledWith(paths.observability.slos); }); + }); - describe('if submitting has not completed successfully', () => { - it('renders an error toast', async () => { - // Note: the `anSLO` object is considered to have (at least) - // values for all required fields. - jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: '123' }); - useFetchSloMock.mockReturnValue({ loading: false, slo: anSLO }); - useFetchIndicesMock.mockReturnValue({ - loading: false, - indices: [{ name: 'some-index' }], - }); - useCreateSloMock.mockReturnValue({ - loading: false, - success: false, - error: 'Argh, API died', - createSlo: jest.fn(), - }); - render(, config); - expect(mockAddError).toBeCalled(); + describe('if submitting has not completed successfully', () => { + it('renders an error toast', async () => { + // Note: the `anSLO` object is considered to have (at least) + // values for all required fields. + jest.spyOn(Router, 'useParams').mockReturnValue({ sloId: '123' }); + useFetchSloMock.mockReturnValue({ loading: false, slo: anSLO }); + useFetchIndicesMock.mockReturnValue({ + loading: false, + indices: [{ name: 'some-index' }], }); + useCreateOrUpdateSloMock.mockReturnValue({ + loading: false, + success: false, + error: 'Argh, API died', + createSlo: jest.fn(), + updateSlo: jest.fn(), + }); + render(, config); + expect(mockAddError).toBeCalled(); }); }); }); diff --git a/x-pack/plugins/observability/public/pages/slos/components/slo_list_search_filter_sort_bar.tsx b/x-pack/plugins/observability/public/pages/slos/components/slo_list_search_filter_sort_bar.tsx index 255b1c3ea9168..0e7ccc31d54ee 100644 --- a/x-pack/plugins/observability/public/pages/slos/components/slo_list_search_filter_sort_bar.tsx +++ b/x-pack/plugins/observability/public/pages/slos/components/slo_list_search_filter_sort_bar.tsx @@ -27,10 +27,10 @@ export interface SloListSearchFilterSortBarProps { onChangeIndicatorTypeFilter: (filter: FilterType[]) => void; } -export type SortType = 'name' | 'indicator_type'; +export type SortType = 'name' | 'indicatorType'; export type FilterType = - | 'sli.apm.transaction_duration' - | 'sli.apm.transaction_error_rate' + | 'sli.apm.transactionDuration' + | 'sli.apm.transactionErrorRate' | 'sli.kql.custom'; export type Item = EuiSelectableOption & { @@ -51,7 +51,7 @@ const SORT_OPTIONS: Array> = [ label: i18n.translate('xpack.observability.slos.list.sortBy.indicatorType', { defaultMessage: 'Indicator type', }), - type: 'indicator_type', + type: 'indicatorType', }, ]; @@ -60,13 +60,13 @@ const INDICATOR_TYPE_OPTIONS: Array> = [ label: i18n.translate('xpack.observability.slos.list.indicatorTypeFilter.apmLatency', { defaultMessage: 'APM latency', }), - type: 'sli.apm.transaction_duration', + type: 'sli.apm.transactionDuration', }, { label: i18n.translate('xpack.observability.slos.list.indicatorTypeFilter.apmAvailability', { defaultMessage: 'APM availability', }), - type: 'sli.apm.transaction_error_rate', + type: 'sli.apm.transactionErrorRate', }, { label: i18n.translate('xpack.observability.slos.list.indicatorTypeFilter.customKql', { @@ -109,7 +109,7 @@ export function SloListSearchFilterSortBar({ }; useEffect(() => { - if (selectedSort?.type === 'name' || selectedSort?.type === 'indicator_type') { + if (selectedSort?.type === 'name' || selectedSort?.type === 'indicatorType') { onChangeSort(selectedSort.type); } }, [onChangeSort, selectedSort]); diff --git a/x-pack/plugins/observability/server/services/slo/update_slo.test.ts b/x-pack/plugins/observability/server/services/slo/update_slo.test.ts index 42fabf8c0e957..166ed2a319c77 100644 --- a/x-pack/plugins/observability/server/services/slo/update_slo.test.ts +++ b/x-pack/plugins/observability/server/services/slo/update_slo.test.ts @@ -39,7 +39,7 @@ describe('UpdateSLO', () => { expectTransformManagerNeverCalled(); expect(mockEsClient.deleteByQuery).not.toBeCalled(); expect(mockRepository.save).toBeCalledWith( - expect.objectContaining({ ...slo, name: newName, updated_at: expect.anything() }) + expect.objectContaining({ ...slo, name: newName, updatedAt: expect.anything() }) ); expect(response.name).toBe(newName); expect(response.updatedAt).not.toBe(slo.updatedAt); @@ -61,7 +61,7 @@ describe('UpdateSLO', () => { ...slo, settings: newSettings, revision: 2, - updated_at: expect.anything(), + updatedAt: expect.anything(), }) ); expectInstallationOfNewSLOTransform(); @@ -82,7 +82,7 @@ describe('UpdateSLO', () => { ...slo, indicator: newIndicator, revision: 2, - updated_at: expect.anything(), + updatedAt: expect.anything(), }) ); expectInstallationOfNewSLOTransform(); diff --git a/x-pack/plugins/observability/server/services/slo/update_slo.ts b/x-pack/plugins/observability/server/services/slo/update_slo.ts index 88621b3a7e18a..02b3055fb0bf1 100644 --- a/x-pack/plugins/observability/server/services/slo/update_slo.ts +++ b/x-pack/plugins/observability/server/services/slo/update_slo.ts @@ -6,6 +6,7 @@ */ import deepEqual from 'fast-deep-equal'; +import merge from 'lodash/merge'; import { ElasticsearchClient } from '@kbn/core/server'; import { UpdateSLOParams, UpdateSLOResponse, updateSLOResponseSchema } from '@kbn/slo-schema'; @@ -41,7 +42,7 @@ export class UpdateSLO { private updateSLO(originalSlo: SLO, params: UpdateSLOParams) { let hasBreakingChange = false; - const updatedSlo: SLO = Object.assign({}, originalSlo, params, { updated_at: new Date() }); + const updatedSlo: SLO = merge({}, originalSlo, params, { updatedAt: new Date() }); validateSLO(updatedSlo); if (!deepEqual(originalSlo.indicator, updatedSlo.indicator)) {