mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 09:48:58 -04:00
# Backport This will backport the following commits from `main` to `8.9`: - [fix(slo): optimistic update (#160804)](https://github.com/elastic/kibana/pull/160804) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Kevin Delemme","email":"kevin.delemme@elastic.co"},"sourceCommit":{"committedDate":"2023-06-29T16:55:23Z","message":"fix(slo): optimistic update (#160804)\n\nResolves https://github.com/elastic/kibana/issues/160805\n\n## ⭐ Summary\n\nThis PR fixes the optimistic updates when creating, updating, cloning or\ndeleting an SLO. In a previous PR, I've introduced an issue with react\nquery that breaks the optimistic updates in a few cases.\n\nThe main issue is that `getQueryData` was not doing what I was\nexpecting, therefore I reintroduced the previous `getQueriesData` usage.\n\nAlso when no `queryKey` and `previousData` are present in\n`getQueriesData`, we don't set the optimistic data as it was initially\ndone. This handles cases where you go directly to the creation form for\nexample.\n\nFinally, I forgot to update an invalidateQueries for the rules when one\nis created on an SLO.\n\n\n\nhttps://www.loom.com/share/c9521bd9d04549b1bd5cc59a0a6ba1cd?sid=a72ea1dc-aa4d-4536-a410-16fe2a1d9416","sha":"6f58507ea1a1af1ac3814e8973bd2cdebe7c80e2","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["backport","release_note:skip","Team: Actionable Observability","backport:prev-minor","v8.10.0"],"number":160804,"url":"https://github.com/elastic/kibana/pull/160804","mergeCommit":{"message":"fix(slo): optimistic update (#160804)\n\nResolves https://github.com/elastic/kibana/issues/160805\n\n## ⭐ Summary\n\nThis PR fixes the optimistic updates when creating, updating, cloning or\ndeleting an SLO. In a previous PR, I've introduced an issue with react\nquery that breaks the optimistic updates in a few cases.\n\nThe main issue is that `getQueryData` was not doing what I was\nexpecting, therefore I reintroduced the previous `getQueriesData` usage.\n\nAlso when no `queryKey` and `previousData` are present in\n`getQueriesData`, we don't set the optimistic data as it was initially\ndone. This handles cases where you go directly to the creation form for\nexample.\n\nFinally, I forgot to update an invalidateQueries for the rules when one\nis created on an SLO.\n\n\n\nhttps://www.loom.com/share/c9521bd9d04549b1bd5cc59a0a6ba1cd?sid=a72ea1dc-aa4d-4536-a410-16fe2a1d9416","sha":"6f58507ea1a1af1ac3814e8973bd2cdebe7c80e2"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/160804","number":160804,"mergeCommit":{"message":"fix(slo): optimistic update (#160804)\n\nResolves https://github.com/elastic/kibana/issues/160805\n\n## ⭐ Summary\n\nThis PR fixes the optimistic updates when creating, updating, cloning or\ndeleting an SLO. In a previous PR, I've introduced an issue with react\nquery that breaks the optimistic updates in a few cases.\n\nThe main issue is that `getQueryData` was not doing what I was\nexpecting, therefore I reintroduced the previous `getQueriesData` usage.\n\nAlso when no `queryKey` and `previousData` are present in\n`getQueriesData`, we don't set the optimistic data as it was initially\ndone. This handles cases where you go directly to the creation form for\nexample.\n\nFinally, I forgot to update an invalidateQueries for the rules when one\nis created on an SLO.\n\n\n\nhttps://www.loom.com/share/c9521bd9d04549b1bd5cc59a0a6ba1cd?sid=a72ea1dc-aa4d-4536-a410-16fe2a1d9416","sha":"6f58507ea1a1af1ac3814e8973bd2cdebe7c80e2"}}]}] BACKPORT-->
This commit is contained in:
parent
ae5053c447
commit
812b679467
7 changed files with 129 additions and 81 deletions
|
@ -7,7 +7,7 @@
|
|||
|
||||
import type { Indicator } from '@kbn/slo-schema';
|
||||
|
||||
interface SloKeyFilter {
|
||||
interface SloListFilter {
|
||||
name: string;
|
||||
page: number;
|
||||
sortBy: string;
|
||||
|
@ -23,7 +23,7 @@ interface CompositeSloKeyFilter {
|
|||
export const sloKeys = {
|
||||
all: ['slo'] as const,
|
||||
lists: () => [...sloKeys.all, 'list'] as const,
|
||||
list: (filters: SloKeyFilter) => [...sloKeys.lists(), filters] as const,
|
||||
list: (filters: SloListFilter) => [...sloKeys.lists(), filters] as const,
|
||||
details: () => [...sloKeys.all, 'details'] as const,
|
||||
detail: (sloId?: string) => [...sloKeys.details(), sloId] as const,
|
||||
rules: () => [...sloKeys.all, 'rules'] as const,
|
||||
|
|
|
@ -5,11 +5,10 @@
|
|||
* 2.0.
|
||||
*/
|
||||
|
||||
import { v1 as uuidv1 } from 'uuid';
|
||||
import { useMutation, useQueryClient } from '@tanstack/react-query';
|
||||
import { i18n } from '@kbn/i18n';
|
||||
import type { CreateSLOInput, CreateSLOResponse, FindSLOResponse } from '@kbn/slo-schema';
|
||||
|
||||
import { QueryKey, useMutation, useQueryClient } from '@tanstack/react-query';
|
||||
import { v1 as uuidv1 } from 'uuid';
|
||||
import { useKibana } from '../../utils/kibana_react';
|
||||
import { sloKeys } from './query_key_factory';
|
||||
|
||||
|
@ -24,7 +23,7 @@ export function useCloneSlo() {
|
|||
CreateSLOResponse,
|
||||
string,
|
||||
{ slo: CreateSLOInput; originalSloId?: string },
|
||||
{ previousSloList: FindSLOResponse | undefined }
|
||||
{ previousData?: FindSLOResponse; queryKey?: QueryKey }
|
||||
>(
|
||||
['cloneSlo'],
|
||||
({ slo }: { slo: CreateSLOInput; originalSloId?: string }) => {
|
||||
|
@ -33,34 +32,35 @@ export function useCloneSlo() {
|
|||
},
|
||||
{
|
||||
onMutate: async ({ slo, originalSloId }) => {
|
||||
// Cancel any outgoing refetches (so they don't overwrite our optimistic update)
|
||||
await queryClient.cancelQueries(sloKeys.lists());
|
||||
await queryClient.cancelQueries({ queryKey: sloKeys.lists(), exact: false });
|
||||
|
||||
const latestQueriesData = (
|
||||
queryClient.getQueriesData<FindSLOResponse>(sloKeys.lists()) ?? []
|
||||
).at(0);
|
||||
const [queryKey, data] = latestQueriesData ?? [];
|
||||
const queriesData = queryClient.getQueriesData<FindSLOResponse>({
|
||||
queryKey: sloKeys.lists(),
|
||||
exact: false,
|
||||
});
|
||||
const [queryKey, previousData] = queriesData?.at(0) ?? [];
|
||||
|
||||
const originalSlo = data?.results?.find((el) => el.id === originalSloId);
|
||||
const originalSlo = previousData?.results?.find((el) => el.id === originalSloId);
|
||||
const optimisticUpdate = {
|
||||
...data,
|
||||
page: previousData?.page ?? 1,
|
||||
perPage: previousData?.perPage ?? 25,
|
||||
total: previousData?.total ? previousData.total + 1 : 1,
|
||||
results: [
|
||||
...(data?.results ?? []),
|
||||
...(previousData?.results ?? []),
|
||||
{ ...originalSlo, name: slo.name, id: uuidv1(), summary: undefined },
|
||||
],
|
||||
total: data?.total ? data.total + 1 : 1,
|
||||
};
|
||||
|
||||
// Optimistically update to the new value
|
||||
queryClient.setQueryData(queryKey ?? sloKeys.lists(), optimisticUpdate);
|
||||
if (queryKey) {
|
||||
queryClient.setQueryData(queryKey, optimisticUpdate);
|
||||
}
|
||||
|
||||
// Return a context object with the snapshotted value
|
||||
return { previousSloList: data };
|
||||
return { queryKey, previousData };
|
||||
},
|
||||
// If the mutation fails, use the context returned from onMutate to roll back
|
||||
onError: (_err, { slo }, context) => {
|
||||
if (context?.previousSloList) {
|
||||
queryClient.setQueryData(sloKeys.lists(), context.previousSloList);
|
||||
if (context?.previousData && context?.queryKey) {
|
||||
queryClient.setQueryData(context.queryKey, context.previousData);
|
||||
}
|
||||
toasts.addDanger(
|
||||
i18n.translate('xpack.observability.slo.clone.errorNotification', {
|
||||
|
@ -76,7 +76,9 @@ export function useCloneSlo() {
|
|||
values: { name: slo.name },
|
||||
})
|
||||
);
|
||||
queryClient.invalidateQueries(sloKeys.lists());
|
||||
},
|
||||
onSettled: () => {
|
||||
queryClient.invalidateQueries({ queryKey: sloKeys.lists(), exact: false });
|
||||
},
|
||||
}
|
||||
);
|
||||
|
|
|
@ -8,7 +8,7 @@
|
|||
import { i18n } from '@kbn/i18n';
|
||||
import { encode } from '@kbn/rison';
|
||||
import type { CreateSLOInput, CreateSLOResponse, FindSLOResponse } from '@kbn/slo-schema';
|
||||
import { useMutation, useQueryClient } from '@tanstack/react-query';
|
||||
import { QueryKey, useMutation, useQueryClient } from '@tanstack/react-query';
|
||||
import { v1 as uuidv1 } from 'uuid';
|
||||
|
||||
import { paths } from '../../config/paths';
|
||||
|
@ -23,34 +23,42 @@ export function useCreateSlo() {
|
|||
} = useKibana().services;
|
||||
const queryClient = useQueryClient();
|
||||
|
||||
return useMutation(
|
||||
({ slo }: { slo: CreateSLOInput }) => {
|
||||
return useMutation<
|
||||
CreateSLOResponse,
|
||||
string,
|
||||
{ slo: CreateSLOInput },
|
||||
{ previousData?: FindSLOResponse; queryKey?: QueryKey }
|
||||
>(
|
||||
['createSlo'],
|
||||
({ slo }) => {
|
||||
const body = JSON.stringify(slo);
|
||||
return http.post<CreateSLOResponse>(`/api/observability/slos`, { body });
|
||||
},
|
||||
{
|
||||
mutationKey: ['createSlo'],
|
||||
onMutate: async ({ slo }) => {
|
||||
// Cancel any outgoing refetches (so they don't overwrite our optimistic update)
|
||||
await queryClient.cancelQueries(sloKeys.lists());
|
||||
await queryClient.cancelQueries({ queryKey: sloKeys.lists(), exact: false });
|
||||
|
||||
const latestQueriesData = (
|
||||
queryClient.getQueriesData<FindSLOResponse>(sloKeys.lists()) ?? []
|
||||
).at(0);
|
||||
const queriesData = queryClient.getQueriesData<FindSLOResponse>({
|
||||
queryKey: sloKeys.lists(),
|
||||
exact: false,
|
||||
});
|
||||
|
||||
const [queryKey, data] = latestQueriesData || [];
|
||||
const [queryKey, previousData] = queriesData?.at(0) ?? [];
|
||||
|
||||
const newItem = { ...slo, id: uuidv1() };
|
||||
|
||||
const optimisticUpdate = {
|
||||
...data,
|
||||
results: [...(data?.results ?? []), newItem],
|
||||
total: data?.total ? data.total + 1 : 1,
|
||||
page: previousData?.page ?? 1,
|
||||
perPage: previousData?.perPage ?? 25,
|
||||
total: previousData?.total ? previousData.total + 1 : 1,
|
||||
results: [...(previousData?.results ?? []), newItem],
|
||||
};
|
||||
|
||||
queryClient.setQueryData(queryKey ?? sloKeys.lists(), optimisticUpdate);
|
||||
if (queryKey) {
|
||||
queryClient.setQueryData(queryKey, optimisticUpdate);
|
||||
}
|
||||
|
||||
// Return a context object with the snapshotted value
|
||||
return { previousSloList: data };
|
||||
return { queryKey, previousData };
|
||||
},
|
||||
onSuccess: (_data, { slo }) => {
|
||||
toasts.addSuccess(
|
||||
|
@ -59,9 +67,12 @@ export function useCreateSlo() {
|
|||
values: { name: slo.name },
|
||||
})
|
||||
);
|
||||
queryClient.invalidateQueries(sloKeys.lists());
|
||||
},
|
||||
onError: (error, { slo }) => {
|
||||
onError: (error, { slo }, context) => {
|
||||
if (context?.previousData && context?.queryKey) {
|
||||
queryClient.setQueryData(context.queryKey, context.previousData);
|
||||
}
|
||||
|
||||
toasts.addError(new Error(String(error)), {
|
||||
title: i18n.translate('xpack.observability.slo.create.errorNotification', {
|
||||
defaultMessage: 'Something went wrong while creating {name}',
|
||||
|
@ -73,6 +84,9 @@ export function useCreateSlo() {
|
|||
http.basePath.prepend(paths.observability.sloCreateWithEncodedForm(encode(slo)))
|
||||
);
|
||||
},
|
||||
onSettled: () => {
|
||||
queryClient.invalidateQueries({ queryKey: sloKeys.lists(), exact: false });
|
||||
},
|
||||
}
|
||||
);
|
||||
}
|
||||
|
|
|
@ -5,7 +5,7 @@
|
|||
* 2.0.
|
||||
*/
|
||||
|
||||
import { useMutation, useQueryClient } from '@tanstack/react-query';
|
||||
import { QueryKey, useMutation, useQueryClient } from '@tanstack/react-query';
|
||||
import { i18n } from '@kbn/i18n';
|
||||
import { FindSLOResponse } from '@kbn/slo-schema';
|
||||
import { useKibana } from '../../utils/kibana_react';
|
||||
|
@ -18,11 +18,11 @@ export function useDeleteSlo() {
|
|||
} = useKibana().services;
|
||||
const queryClient = useQueryClient();
|
||||
|
||||
const deleteSlo = useMutation<
|
||||
return useMutation<
|
||||
string,
|
||||
string,
|
||||
{ id: string; name: string },
|
||||
{ previousSloList: FindSLOResponse | undefined }
|
||||
{ previousData?: FindSLOResponse; queryKey?: QueryKey }
|
||||
>(
|
||||
['deleteSlo'],
|
||||
({ id }) => {
|
||||
|
@ -34,30 +34,31 @@ export function useDeleteSlo() {
|
|||
},
|
||||
{
|
||||
onMutate: async (slo) => {
|
||||
// Cancel any outgoing refetches (so they don't overwrite our optimistic update)
|
||||
await queryClient.cancelQueries(sloKeys.lists());
|
||||
await queryClient.cancelQueries({ queryKey: sloKeys.lists(), exact: false });
|
||||
|
||||
const latestQueriesData = (
|
||||
queryClient.getQueriesData<FindSLOResponse>(sloKeys.lists()) || []
|
||||
).at(0);
|
||||
const [queryKey, data] = latestQueriesData || [];
|
||||
const queriesData = queryClient.getQueriesData<FindSLOResponse>({
|
||||
queryKey: sloKeys.lists(),
|
||||
exact: false,
|
||||
});
|
||||
const [queryKey, previousData] = queriesData?.at(0) ?? [];
|
||||
|
||||
const optimisticUpdate = {
|
||||
...data,
|
||||
results: data?.results?.filter((result) => result.id !== slo.id) ?? [],
|
||||
total: data?.total ? data.total - 1 : 0,
|
||||
page: previousData?.page ?? 1,
|
||||
perPage: previousData?.perPage ?? 25,
|
||||
total: previousData?.total ? previousData.total - 1 : 0,
|
||||
results: previousData?.results?.filter((result) => result.id !== slo.id) ?? [],
|
||||
};
|
||||
|
||||
// Optimistically update to the new value
|
||||
queryClient.setQueryData(queryKey ?? sloKeys.lists(), optimisticUpdate);
|
||||
if (queryKey) {
|
||||
queryClient.setQueryData(queryKey, optimisticUpdate);
|
||||
}
|
||||
|
||||
// Return a context object with the snapshotted value
|
||||
return { previousSloList: data };
|
||||
return { previousData, queryKey };
|
||||
},
|
||||
// If the mutation fails, use the context returned from onMutate to roll back
|
||||
onError: (_err, { name }, context) => {
|
||||
if (context?.previousSloList) {
|
||||
queryClient.setQueryData(sloKeys.lists(), context.previousSloList);
|
||||
if (context?.previousData && context?.queryKey) {
|
||||
queryClient.setQueryData(context.queryKey, context.previousData);
|
||||
}
|
||||
|
||||
toasts.addDanger(
|
||||
|
@ -74,15 +75,10 @@ export function useDeleteSlo() {
|
|||
values: { name },
|
||||
})
|
||||
);
|
||||
if (
|
||||
// @ts-ignore
|
||||
queryClient.getQueryCache().find(sloKeys.lists())?.options.refetchInterval === undefined
|
||||
) {
|
||||
queryClient.invalidateQueries(sloKeys.lists());
|
||||
}
|
||||
},
|
||||
onSettled: () => {
|
||||
queryClient.invalidateQueries({ queryKey: sloKeys.lists(), exact: false });
|
||||
},
|
||||
}
|
||||
);
|
||||
|
||||
return deleteSlo;
|
||||
}
|
||||
|
|
|
@ -93,6 +93,10 @@ export function useFetchSloList({
|
|||
return failureCount < 4;
|
||||
},
|
||||
onSuccess: ({ results }: FindSLOResponse) => {
|
||||
queryClient.invalidateQueries({ queryKey: sloKeys.historicalSummaries(), exact: false });
|
||||
queryClient.invalidateQueries({ queryKey: sloKeys.activeAlerts(), exact: false });
|
||||
queryClient.invalidateQueries({ queryKey: sloKeys.rules(), exact: false });
|
||||
|
||||
if (!shouldRefetch) {
|
||||
return;
|
||||
}
|
||||
|
@ -102,10 +106,6 @@ export function useFetchSloList({
|
|||
} else {
|
||||
setStateRefetchInterval(LONG_REFETCH_INTERVAL);
|
||||
}
|
||||
|
||||
queryClient.invalidateQueries(sloKeys.historicalSummaries());
|
||||
queryClient.invalidateQueries(sloKeys.activeAlerts());
|
||||
queryClient.invalidateQueries(sloKeys.rules());
|
||||
},
|
||||
onError: (error: Error) => {
|
||||
toasts.addError(error, {
|
||||
|
|
|
@ -5,10 +5,9 @@
|
|||
* 2.0.
|
||||
*/
|
||||
|
||||
import { useMutation, useQueryClient } from '@tanstack/react-query';
|
||||
import { i18n } from '@kbn/i18n';
|
||||
import type { UpdateSLOInput, UpdateSLOResponse } from '@kbn/slo-schema';
|
||||
|
||||
import type { FindSLOResponse, UpdateSLOInput, UpdateSLOResponse } from '@kbn/slo-schema';
|
||||
import { QueryKey, useMutation, useQueryClient } from '@tanstack/react-query';
|
||||
import { useKibana } from '../../utils/kibana_react';
|
||||
import { sloKeys } from './query_key_factory';
|
||||
|
||||
|
@ -19,13 +18,44 @@ export function useUpdateSlo() {
|
|||
} = useKibana().services;
|
||||
const queryClient = useQueryClient();
|
||||
|
||||
return useMutation(
|
||||
({ sloId, slo }: { sloId: string; slo: UpdateSLOInput }) => {
|
||||
return useMutation<
|
||||
UpdateSLOResponse,
|
||||
string,
|
||||
{ sloId: string; slo: UpdateSLOInput },
|
||||
{ previousData?: FindSLOResponse; queryKey?: QueryKey }
|
||||
>(
|
||||
['updateSlo'],
|
||||
({ sloId, slo }) => {
|
||||
const body = JSON.stringify(slo);
|
||||
return http.put<UpdateSLOResponse>(`/api/observability/slos/${sloId}`, { body });
|
||||
},
|
||||
{
|
||||
mutationKey: ['updateSlo'],
|
||||
onMutate: async ({ sloId, slo }) => {
|
||||
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 updatedItem = { ...slo, id: sloId };
|
||||
const optimisticUpdate = {
|
||||
page: previousData?.page ?? 1,
|
||||
perPage: previousData?.perPage ?? 25,
|
||||
total: previousData?.total ? previousData.total : 1,
|
||||
results: [
|
||||
...(previousData?.results?.filter((result) => result.id !== sloId) ?? []),
|
||||
updatedItem,
|
||||
],
|
||||
};
|
||||
|
||||
if (queryKey) {
|
||||
queryClient.setQueryData(queryKey, optimisticUpdate);
|
||||
}
|
||||
|
||||
return { previousData, queryKey };
|
||||
},
|
||||
onSuccess: (_data, { slo: { name } }) => {
|
||||
toasts.addSuccess(
|
||||
i18n.translate('xpack.observability.slo.update.successNotification', {
|
||||
|
@ -33,10 +63,12 @@ export function useUpdateSlo() {
|
|||
values: { name },
|
||||
})
|
||||
);
|
||||
queryClient.invalidateQueries(sloKeys.lists());
|
||||
queryClient.invalidateQueries(sloKeys.historicalSummaries());
|
||||
},
|
||||
onError: (error, { slo: { name } }) => {
|
||||
onError: (error, { slo: { name } }, context) => {
|
||||
if (context?.previousData && context?.queryKey) {
|
||||
queryClient.setQueryData(context.queryKey, context.previousData);
|
||||
}
|
||||
|
||||
toasts.addError(new Error(String(error)), {
|
||||
title: i18n.translate('xpack.observability.slo.update.errorNotification', {
|
||||
defaultMessage: 'Something went wrong when updating {name}',
|
||||
|
@ -44,6 +76,9 @@ export function useUpdateSlo() {
|
|||
}),
|
||||
});
|
||||
},
|
||||
onSettled: () => {
|
||||
queryClient.invalidateQueries({ queryKey: sloKeys.lists(), exact: false });
|
||||
},
|
||||
}
|
||||
);
|
||||
}
|
||||
|
|
|
@ -22,6 +22,7 @@ import { i18n } from '@kbn/i18n';
|
|||
|
||||
import { HistoricalSummaryResponse, SLOWithSummaryResponse } from '@kbn/slo-schema';
|
||||
import type { Rule } from '@kbn/triggers-actions-ui-plugin/public';
|
||||
import { sloKeys } from '../../../hooks/slo/query_key_factory';
|
||||
import { useCapabilities } from '../../../hooks/slo/use_capabilities';
|
||||
import { useKibana } from '../../../utils/kibana_react';
|
||||
import { useCloneSlo } from '../../../hooks/slo/use_clone_slo';
|
||||
|
@ -94,7 +95,7 @@ export function SloListItem({
|
|||
};
|
||||
|
||||
const handleSavedRule = async () => {
|
||||
queryClient.invalidateQueries(['fetchRulesForSlo']);
|
||||
queryClient.invalidateQueries({ queryKey: sloKeys.rules(), exact: false });
|
||||
};
|
||||
|
||||
const handleNavigateToRules = async () => {
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue