Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(slo): slo edit form improvements #148419

Merged
merged 21 commits into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions packages/kbn-slo-schema/src/rest_specs/slo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ const updateSLOParamsSchema = t.type({
timeWindow: timeWindowSchema,
budgetingMethod: budgetingMethodSchema,
objective: objectiveSchema,
settings: settingsSchema,
settings: optionalSettingsSchema,
}),
});

Expand All @@ -112,11 +112,13 @@ const findSLOResponseSchema = t.type({
type SLOResponse = t.OutputOf<typeof sloResponseSchema>;
type SLOWithSummaryResponse = t.OutputOf<typeof sloWithSummaryResponseSchema>;

type CreateSLOParams = t.TypeOf<typeof createSLOParamsSchema.props.body>;
type CreateSLOResponse = t.TypeOf<typeof createSLOResponseSchema>;
type CreateSLOInput = t.OutputOf<typeof createSLOParamsSchema.props.body>; // Raw payload sent by the frontend
type CreateSLOParams = t.TypeOf<typeof createSLOParamsSchema.props.body>; // Parsed payload used by the backend
type CreateSLOResponse = t.TypeOf<typeof createSLOResponseSchema>; // Raw response sent to the frontend

type GetSLOResponse = t.OutputOf<typeof getSLOResponseSchema>;

type UpdateSLOInput = t.OutputOf<typeof updateSLOParamsSchema.props.body>;
type UpdateSLOParams = t.TypeOf<typeof updateSLOParamsSchema.props.body>;
type UpdateSLOResponse = t.OutputOf<typeof updateSLOResponseSchema>;

Expand All @@ -139,13 +141,15 @@ export {
};
export type {
BudgetingMethod,
CreateSLOInput,
CreateSLOParams,
CreateSLOResponse,
FindSLOParams,
FindSLOResponse,
GetSLOResponse,
SLOResponse,
SLOWithSummaryResponse,
UpdateSLOInput,
UpdateSLOParams,
UpdateSLOResponse,
};
2 changes: 1 addition & 1 deletion x-pack/plugins/observability/public/data/slo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export const sloList: FindSLOResponse = {
},
{
...baseSlo,
id: 'c0f8d669-9177-4706-9098-f397a88173a7',
id: 'c0f8d669-9177-4706-9098-f397a88173a8',
kdelemme marked this conversation as resolved.
Show resolved Hide resolved
budgetingMethod: 'timeslices',
objective: { target: 0.98, timesliceTarget: 0.9, timesliceWindow: '5m' },
summary: {
Expand Down
34 changes: 29 additions & 5 deletions x-pack/plugins/observability/public/hooks/slo/use_create_slo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string | undefined>(undefined);

const createSlo = useCallback(
async (slo: CreateSLOParams) => {
async (slo: CreateSLOInput) => {
setLoading(true);
setError('');
setSuccess(false);
Expand All @@ -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<UpdateSLOResponse>(`/api/observability/slos/${sloId}`, { body });
setSuccess(true);
} catch (e) {
setError(e);
}
},
[http]
);

return {
loading,
error,
success,
createSlo,
updateSlo,
};
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -42,6 +43,7 @@ export interface Props {
const maxWidth = 775;

export function SloEditForm({ slo }: Props) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this context, SloEdit is also covering create, right? Does it make sense to reflect it somehow in the name? It was a bit hard to understand what is covered on this page in the code.

Maybe we can remove the slo_edit prefix from the components and have SloEditCreatefor the page level.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's a good idea, I'll rename it.
cc @CoenWarmer Except if you strongly disagree

const isEditMode = slo !== undefined;
const {
application: { navigateToUrl },
http: { basePath },
Expand All @@ -50,39 +52,44 @@ 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',
});

const { isDefinitionValid, isDescriptionValid, isObjectiveValid } = useCheckFormPartialValidities(
{ getFieldState, formState }
);

const {
loading: loadingCreatingSlo,
success: successCreatingSlo,
error: errorCreatingSlo,
createSlo,
} = useCreateSlo();
const { loading, success, error, createSlo, updateSlo } = useCreateOrUpdateSlo();
kdelemme marked this conversation as resolved.
Show resolved Hide resolved

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',
}),
Expand Down Expand Up @@ -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',
})}
</EuiButton>

<EuiSpacer size="xl" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const Template: ComponentStory<typeof Component> = (props: Props) => {
const methods = useForm({ defaultValues: SLO_EDIT_FORM_DEFAULT_VALUES });
return (
<FormProvider {...methods}>
<Component {...props} control={methods.control} />
<Component {...props} control={methods.control} trigger={methods.trigger} />
</FormProvider>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<CreateSLOParams>;
trigger: UseFormTrigger<CreateSLOParams>;
control: Control<CreateSLOInput>;
trigger: UseFormTrigger<CreateSLOInput>;
}

export function SloEditFormDefinitionCustomKql({ control, trigger }: Props) {
Expand All @@ -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 (
<EuiFlexGroup direction="column" gutterSize="l">
<EuiFlexItem>
Expand All @@ -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 }) => (
<EuiSuggest
fullWidth
isClearable
Expand All @@ -61,7 +73,10 @@ export function SloEditFormDefinitionCustomKql({ control, trigger }: Props) {
onItemClick={({ label }) => {
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',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<CreateSLOParams>;
control: Control<CreateSLOInput>;
}

export function SloEditFormDescription({ control }: Props) {
Expand Down
Loading