Skip to content

Commit

Permalink
[SLOs] Fix cloning SLO by opening pre filled form (#172927)
Browse files Browse the repository at this point in the history
  • Loading branch information
shahzad31 authored Dec 18, 2023
1 parent a61b864 commit 3419469
Show file tree
Hide file tree
Showing 15 changed files with 109 additions and 212 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export function SloOverview({

return (
<div ref={containerRef} style={{ width: '100%' }}>
<SloCardChart slo={slo} historicalSliData={historicalSliData ?? []} cardsPerRow={4} />
<SloCardChart slo={slo} historicalSliData={historicalSliData ?? []} />
<SloCardBadgesPortal containerRef={containerRef}>
<SloCardItemBadges
slo={slo}
Expand Down
87 changes: 16 additions & 71 deletions x-pack/plugins/observability/public/hooks/slo/use_clone_slo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,83 +5,28 @@
* 2.0.
*/

import { IHttpFetchError, ResponseErrorBody } from '@kbn/core/public';
import { i18n } from '@kbn/i18n';
import type { CreateSLOInput, CreateSLOResponse, FindSLOResponse } from '@kbn/slo-schema';
import { QueryKey, useMutation, useQueryClient } from '@tanstack/react-query';
import { v4 as uuidv4 } from 'uuid';
import { encode } from '@kbn/rison';
import { useCallback } from 'react';
import { SLOWithSummaryResponse } from '@kbn/slo-schema';
import { paths } from '../../../common/locators/paths';
import { useKibana } from '../../utils/kibana_react';
import { sloKeys } from './query_key_factory';

type ServerError = IHttpFetchError<ResponseErrorBody>;

export function useCloneSlo() {
const {
http,
notifications: { toasts },
http: { basePath },
application: { navigateToUrl },
} = useKibana().services;
const queryClient = useQueryClient();

return useMutation<
CreateSLOResponse,
ServerError,
{ slo: CreateSLOInput; originalSloId?: string },
{ previousData?: FindSLOResponse; queryKey?: QueryKey }
>(
['cloneSlo'],
({ slo }: { slo: CreateSLOInput; originalSloId?: string }) => {
const body = JSON.stringify(slo);
return http.post<CreateSLOResponse>(`/api/observability/slos`, { body });
return useCallback(
(slo: SLOWithSummaryResponse) => {
navigateToUrl(
basePath.prepend(
paths.observability.sloCreateWithEncodedForm(
encode({ ...slo, name: `[Copy] ${slo.name}`, id: undefined })
)
)
);
},
{
onMutate: async ({ slo, originalSloId }) => {
await queryClient.cancelQueries({ queryKey: sloKeys.lists(), exact: false });

const queriesData = queryClient.getQueriesData<FindSLOResponse>({
queryKey: sloKeys.lists(),
exact: false,
});
const [queryKey, previousData] = queriesData?.at(0) ?? [];

const originalSlo = previousData?.results?.find((el) => el.id === originalSloId);
const optimisticUpdate = {
page: previousData?.page ?? 1,
perPage: previousData?.perPage ?? 25,
total: previousData?.total ? previousData.total + 1 : 1,
results: [
...(previousData?.results ?? []),
{ ...originalSlo, name: slo.name, id: uuidv4(), summary: undefined },
],
};

if (queryKey) {
queryClient.setQueryData(queryKey, optimisticUpdate);
}

return { queryKey, previousData };
},
// If the mutation fails, use the context returned from onMutate to roll back
onError: (error, { slo }, context) => {
if (context?.previousData && context?.queryKey) {
queryClient.setQueryData(context.queryKey, context.previousData);
}

toasts.addError(new Error(error.body?.message ?? error.message), {
title: i18n.translate('xpack.observability.slo.clone.errorNotification', {
defaultMessage: 'Failed to clone {name}',
values: { name: slo.name },
}),
});
},
onSuccess: (_data, { slo }) => {
toasts.addSuccess(
i18n.translate('xpack.observability.slo.clone.successNotification', {
defaultMessage: 'Successfully created {name}',
values: { name: slo.name },
})
);
queryClient.invalidateQueries({ queryKey: sloKeys.lists(), exact: false });
},
}
[navigateToUrl, basePath]
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,16 @@ import { i18n } from '@kbn/i18n';
import { EuiButton, EuiContextMenuItem, EuiContextMenuPanel, EuiPopover } from '@elastic/eui';
import { SLOWithSummaryResponse } from '@kbn/slo-schema';

import { useCloneSlo } from '../../../hooks/slo/use_clone_slo';
import { SloDeleteConfirmationModal } from '../../../components/slo/delete_confirmation_modal/slo_delete_confirmation_modal';
import { useCapabilities } from '../../../hooks/slo/use_capabilities';
import { useKibana } from '../../../utils/kibana_react';
import { useCloneSlo } from '../../../hooks/slo/use_clone_slo';
import { useDeleteSlo } from '../../../hooks/slo/use_delete_slo';
import { isApmIndicatorType } from '../../../utils/slo/indicator';
import { convertSliApmParamsToApmAppDeeplinkUrl } from '../../../utils/slo/convert_sli_apm_params_to_apm_app_deeplink_url';
import { SLO_BURN_RATE_RULE_TYPE_ID } from '../../../../common/constants';
import { rulesLocatorID, sloFeatureId } from '../../../../common';
import { paths } from '../../../../common/locators/paths';
import {
transformSloResponseToCreateSloForm,
transformCreateSLOFormToCreateSLOInput,
} from '../../slo_edit/helpers/process_slo_form_values';
import type { RulesParams } from '../../../locators/rules';

export interface Props {
Expand All @@ -47,7 +43,6 @@ export function HeaderControl({ isLoading, slo }: Props) {
const [isRuleFlyoutVisible, setRuleFlyoutVisibility] = useState<boolean>(false);
const [isDeleteConfirmationModalOpen, setDeleteConfirmationModalOpen] = useState(false);

const { mutate: cloneSlo } = useCloneSlo();
const { mutate: deleteSlo } = useDeleteSlo();

const handleActionsClick = () => setIsPopoverOpen((value) => !value);
Expand Down Expand Up @@ -101,17 +96,12 @@ export function HeaderControl({ isLoading, slo }: Props) {
}
};

const navigateToClone = useCloneSlo();

const handleClone = async () => {
if (slo) {
setIsPopoverOpen(false);

const newSlo = transformCreateSLOFormToCreateSLOInput(
transformSloResponseToCreateSloForm({ ...slo, name: `[Copy] ${slo.name}` })!
);

cloneSlo({ slo: newSlo, originalSloId: slo.id });

navigate(basePath.prepend(paths.observability.slos));
navigateToClone(slo);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { useFetchSloDetails } from '../../hooks/slo/use_fetch_slo_details';
import { useFetchHistoricalSummary } from '../../hooks/slo/use_fetch_historical_summary';
import { useFetchActiveAlerts } from '../../hooks/slo/use_fetch_active_alerts';
import { ActiveAlerts } from '../../hooks/slo/active_alerts';
import { useCloneSlo } from '../../hooks/slo/use_clone_slo';
import { useDeleteSlo } from '../../hooks/slo/use_delete_slo';
import { render } from '../../utils/test_helper';
import { SloDetailsPage } from './slo_details';
Expand All @@ -30,6 +29,7 @@ import {
import { chartPluginMock } from '@kbn/charts-plugin/public/mocks';
import { buildApmAvailabilityIndicator } from '../../data/slo/indicator';
import { ALL_VALUE } from '@kbn/slo-schema';
import { encode } from '@kbn/rison';

jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
Expand All @@ -44,7 +44,6 @@ jest.mock('../../hooks/slo/use_capabilities');
jest.mock('../../hooks/slo/use_fetch_active_alerts');
jest.mock('../../hooks/slo/use_fetch_slo_details');
jest.mock('../../hooks/slo/use_fetch_historical_summary');
jest.mock('../../hooks/slo/use_clone_slo');
jest.mock('../../hooks/slo/use_delete_slo');

const useKibanaMock = useKibana as jest.Mock;
Expand All @@ -55,12 +54,10 @@ const useCapabilitiesMock = useCapabilities as jest.Mock;
const useFetchActiveAlertsMock = useFetchActiveAlerts as jest.Mock;
const useFetchSloDetailsMock = useFetchSloDetails as jest.Mock;
const useFetchHistoricalSummaryMock = useFetchHistoricalSummary as jest.Mock;
const useCloneSloMock = useCloneSlo as jest.Mock;
const useDeleteSloMock = useDeleteSlo as jest.Mock;

const mockNavigate = jest.fn();
const mockLocator = jest.fn();
const mockClone = jest.fn();
const mockDelete = jest.fn();
const mockCapabilities = {
apm: { show: true },
Expand Down Expand Up @@ -120,7 +117,6 @@ describe('SLO Details Page', () => {
data: historicalSummaryData,
});
useFetchActiveAlertsMock.mockReturnValue({ isLoading: false, data: new ActiveAlerts() });
useCloneSloMock.mockReturnValue({ mutate: mockClone });
useDeleteSloMock.mockReturnValue({ mutate: mockDelete });
useLocationMock.mockReturnValue({ search: '' });
});
Expand Down Expand Up @@ -248,29 +244,12 @@ describe('SLO Details Page', () => {

fireEvent.click(button!);

const {
id,
createdAt,
enabled,
revision,
summary,
settings,
updatedAt,
instanceId,
version,
...newSlo
} = slo;

expect(mockClone).toBeCalledWith({
originalSloId: slo.id,
slo: {
...newSlo,
name: `[Copy] ${newSlo.name}`,
},
});

await waitFor(() => {
expect(mockNavigate).toBeCalledWith(paths.observability.slos);
expect(mockNavigate).toBeCalledWith(
paths.observability.sloCreateWithEncodedForm(
encode({ ...slo, name: `[Copy] ${slo.name}`, id: undefined })
)
);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ import { i18n } from '@kbn/i18n';
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';
import { SLO_BURN_RATE_RULE_TYPE_ID } from '../../../../common/constants';
import { BurnRateRuleFlyout } from '../../slos/components/common/burn_rate_rule_flyout';
import { paths } from '../../../../common/locators/paths';
import { useCreateSlo } from '../../../hooks/slo/use_create_slo';
import { useFetchRulesForSlo } from '../../../hooks/slo/use_fetch_rules_for_slo';
Expand Down Expand Up @@ -54,7 +53,6 @@ export function SloEditForm({ slo }: Props) {
const {
application: { navigateToUrl },
http: { basePath },
triggersActionsUi: { getAddRuleFlyout: AddRuleFlyout },
} = useKibana().services;

const isEditMode = slo !== undefined;
Expand Down Expand Up @@ -146,10 +144,6 @@ export function SloEditForm({ slo }: Props) {
setIsCreateRuleCheckboxChecked(!isCreateRuleCheckboxChecked);
};

const handleCloseRuleFlyout = async () => {
navigateToUrl(basePath.prepend(paths.observability.slos));
};

return (
<>
<FormProvider {...methods}>
Expand Down Expand Up @@ -256,17 +250,11 @@ export function SloEditForm({ slo }: Props) {
</EuiFlexGroup>
</FormProvider>

{isAddRuleFlyoutOpen && slo ? (
<AddRuleFlyout
canChangeTrigger={false}
consumer={sloFeatureId}
initialValues={{ name: `${watch('name')} burn rate rule`, params: { sloId: slo.id } }}
ruleTypeId={SLO_BURN_RATE_RULE_TYPE_ID}
onClose={handleCloseRuleFlyout}
onSave={handleCloseRuleFlyout}
useRuleProducer
/>
) : null}
<BurnRateRuleFlyout
slo={slo as GetSLOResponse}
isAddRuleFlyoutOpen={isAddRuleFlyoutOpen}
canChangeTrigger={false}
/>
</>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export function SloEditPage() {
const { sloId } = useParams<{ sloId: string | undefined }>();
const { hasAtLeast } = useLicense();
const hasRightLicense = hasAtLeast('platinum');
const { data: slo, isInitialLoading } = useFetchSloDetails({ sloId });
const { data: slo } = useFetchSloDetails({ sloId });

useBreadcrumbs([
{
Expand Down Expand Up @@ -66,10 +66,6 @@ export function SloEditPage() {
navigateToUrl(basePath.prepend(paths.observability.slos));
}

if (sloId && isInitialLoading) {
return null;
}

return (
<ObservabilityPageTemplate
pageHeader={{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { EuiIcon, EuiPanel, useEuiBackgroundColor } from '@elastic/eui';
import { ALL_VALUE, HistoricalSummaryResponse, SLOWithSummaryResponse } from '@kbn/slo-schema';
import { Rule } from '@kbn/triggers-actions-ui-plugin/public';
import { i18n } from '@kbn/i18n';
import { css } from '@emotion/react';
import { SloCardBadgesPortal } from './badges_portal';
import { useSloListActions } from '../../hooks/use_slo_list_actions';
import { BurnRateRuleFlyout } from '../common/burn_rate_rule_flyout';
Expand Down Expand Up @@ -52,7 +53,7 @@ export const useSloCardColor = (status?: SLOWithSummaryResponse['summary']['stat
return colors[status ?? 'NO_DATA'];
};

const getSubTitle = (slo: SLOWithSummaryResponse, cardsPerRow: number) => {
const getSubTitle = (slo: SLOWithSummaryResponse) => {
return slo.groupBy && slo.groupBy !== ALL_VALUE ? `${slo.groupBy}: ${slo.instanceId}` : '';
};

Expand Down Expand Up @@ -88,14 +89,14 @@ export function SloCardItem({ slo, rules, activeAlerts, historicalSummary, cards
}
}}
paddingSize="none"
style={{
height: '182px',
overflow: 'hidden',
position: 'relative',
}}
css={css`
height: 182px;
overflow: hidden;
position: relative;
`}
title={slo.summary.status}
>
<SloCardChart slo={slo} historicalSliData={historicalSliData} cardsPerRow={cardsPerRow} />
<SloCardChart slo={slo} historicalSliData={historicalSliData} />
{(isMouseOver || isActionsPopoverOpen) && (
<SloCardItemActions
slo={slo}
Expand Down Expand Up @@ -135,19 +136,17 @@ export function SloCardItem({ slo, rules, activeAlerts, historicalSummary, cards

export function SloCardChart({
slo,
cardsPerRow,
historicalSliData,
}: {
slo: SLOWithSummaryResponse;
cardsPerRow: number;
historicalSliData?: Array<{ key?: number; value?: number }>;
}) {
const {
application: { navigateToUrl },
} = useKibana().services;

const cardColor = useSloCardColor(slo.summary.status);
const subTitle = getSubTitle(slo, cardsPerRow);
const subTitle = getSubTitle(slo);
const { sliValue, sloTarget, sloDetailsUrl } = useSloFormattedSummary(slo);

return (
Expand Down
Loading

0 comments on commit 3419469

Please sign in to comment.