From 9d6846d9e259192dd968c503fac40c44d42c13b9 Mon Sep 17 00:00:00 2001 From: Kevin Delemme Date: Thu, 8 Jun 2023 16:29:33 -0400 Subject: [PATCH] fix(slo): optimistic update (#159208) --- .../public/hooks/slo/query_key_factory.ts | 24 ++++++++--------- .../public/hooks/slo/use_clone_slo.ts | 26 ++++++++----------- .../public/hooks/slo/use_create_slo.ts | 14 ++++------ .../public/hooks/slo/use_delete_slo.ts | 13 ++++------ .../slo_details/components/header_control.tsx | 2 +- .../pages/slo_details/slo_details.test.tsx | 2 +- .../pages/slos/components/slo_list_item.tsx | 2 +- 7 files changed, 36 insertions(+), 47 deletions(-) diff --git a/x-pack/plugins/observability/public/hooks/slo/query_key_factory.ts b/x-pack/plugins/observability/public/hooks/slo/query_key_factory.ts index 270a74d38c073..73bdd9528dd75 100644 --- a/x-pack/plugins/observability/public/hooks/slo/query_key_factory.ts +++ b/x-pack/plugins/observability/public/hooks/slo/query_key_factory.ts @@ -20,33 +20,33 @@ interface CompositeSloKeyFilter { export const sloKeys = { all: ['slo'] as const, - lists: () => [...sloKeys.all, 'sloList'] as const, + lists: () => [...sloKeys.all, 'list'] as const, list: (filters: SloKeyFilter) => [...sloKeys.lists(), filters] as const, - details: () => [...sloKeys.all, 'sloDetails'] as const, + details: () => [...sloKeys.all, 'details'] as const, detail: (sloId?: string) => [...sloKeys.details(), sloId] as const, - rules: () => [...sloKeys.all, 'sloRules'] as const, + rules: () => [...sloKeys.all, 'rules'] as const, rule: (sloIds: string[]) => [...sloKeys.rules(), sloIds] as const, - activeAlerts: () => [...sloKeys.all, 'sloActiveAlerts'] as const, + activeAlerts: () => [...sloKeys.all, 'activeAlerts'] as const, activeAlert: (sloIds: string[]) => [...sloKeys.activeAlerts(), sloIds] as const, - historicalSummaries: () => [...sloKeys.all, 'sloHistoricalSummary'] as const, + historicalSummaries: () => [...sloKeys.all, 'historicalSummary'] as const, historicalSummary: (sloIds: string[]) => [...sloKeys.historicalSummaries(), sloIds] as const, - globalDiagnosis: () => [...sloKeys.all, 'sloGlobalDiagnosis'] as const, + globalDiagnosis: () => [...sloKeys.all, 'globalDiagnosis'] as const, }; export const compositeSloKeys = { all: ['compositeSlo'] as const, - lists: () => [...compositeSloKeys.all, 'compositeSloList'] as const, + lists: () => [...compositeSloKeys.all, 'list'] as const, list: (filters: CompositeSloKeyFilter) => [...compositeSloKeys.lists(), filters] as const, - details: () => [...compositeSloKeys.all, 'compositeSloDetails'] as const, + details: () => [...compositeSloKeys.all, 'details'] as const, detail: (sloId?: string) => [...compositeSloKeys.details(), sloId] as const, - rules: () => [...compositeSloKeys.all, 'compositeSloRules'] as const, + rules: () => [...compositeSloKeys.all, 'rules'] as const, rule: (sloIds: string[]) => [...compositeSloKeys.rules(), sloIds] as const, - activeAlerts: () => [...compositeSloKeys.all, 'compositeSloActiveAlerts'] as const, + activeAlerts: () => [...compositeSloKeys.all, 'activeAlerts'] as const, activeAlert: (sloIds: string[]) => [...compositeSloKeys.activeAlerts(), sloIds] as const, - historicalSummaries: () => [...compositeSloKeys.all, 'compositeSloHistoricalSummary'] as const, + historicalSummaries: () => [...compositeSloKeys.all, 'historicalSummary'] as const, historicalSummary: (sloIds: string[]) => [...compositeSloKeys.historicalSummaries(), sloIds] as const, - globalDiagnosis: () => [...compositeSloKeys.all, 'compositeSloGlobalDiagnosis'] as const, + globalDiagnosis: () => [...compositeSloKeys.all, 'globalDiagnosis'] as const, }; export type SloKeys = typeof compositeSloKeys | typeof sloKeys; diff --git a/x-pack/plugins/observability/public/hooks/slo/use_clone_slo.ts b/x-pack/plugins/observability/public/hooks/slo/use_clone_slo.ts index 320dd3c6efc7d..5e25dcad3de04 100644 --- a/x-pack/plugins/observability/public/hooks/slo/use_clone_slo.ts +++ b/x-pack/plugins/observability/public/hooks/slo/use_clone_slo.ts @@ -23,40 +23,36 @@ export function useCloneSlo() { return useMutation< CreateSLOResponse, string, - { slo: CreateSLOInput; idToCopyFrom?: string }, + { slo: CreateSLOInput; originalSloId?: string }, { previousSloList: FindSLOResponse | undefined } >( ['cloneSlo'], - ({ slo }: { slo: CreateSLOInput; idToCopyFrom?: string }) => { + ({ slo }: { slo: CreateSLOInput; originalSloId?: string }) => { const body = JSON.stringify(slo); return http.post(`/api/observability/slos`, { body }); }, { - onMutate: async ({ slo, idToCopyFrom }) => { + onMutate: async ({ slo, originalSloId }) => { // Cancel any outgoing refetches (so they don't overwrite our optimistic update) await queryClient.cancelQueries(sloKeys.lists()); - const latestFetchSloListRequest = ( - queryClient.getQueriesData(sloKeys.lists()) || [] + const latestQueriesData = ( + queryClient.getQueriesData(sloKeys.lists()) ?? [] ).at(0); + const [queryKey, data] = latestQueriesData ?? []; - const [queryKey, data] = latestFetchSloListRequest || []; - - const sloUsedToClone = data?.results.find((el) => el.id === idToCopyFrom); - + const originalSlo = data?.results?.find((el) => el.id === originalSloId); const optimisticUpdate = { ...data, results: [ - ...(data?.results || []), - { ...sloUsedToClone, name: slo.name, id: uuidv1(), summary: undefined }, + ...(data?.results ?? []), + { ...originalSlo, name: slo.name, id: uuidv1(), summary: undefined }, ], - total: data?.total && data.total + 1, + total: data?.total ? data.total + 1 : 1, }; // Optimistically update to the new value - if (queryKey) { - queryClient.setQueryData(queryKey, optimisticUpdate); - } + queryClient.setQueryData(queryKey ?? sloKeys.lists(), optimisticUpdate); toasts.addSuccess( i18n.translate('xpack.observability.slo.clone.successNotification', { 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 d353c74b8c508..7296a6d379156 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 @@ -48,24 +48,20 @@ export function useCreateSlo() { // Cancel any outgoing refetches (so they don't overwrite our optimistic update) await queryClient.cancelQueries(sloKeys.lists()); - const latestFetchSloListRequest = ( - queryClient.getQueriesData(sloKeys.lists()) || [] + const latestQueriesData = ( + queryClient.getQueriesData(sloKeys.lists()) ?? [] ).at(0); - const [queryKey, data] = latestFetchSloListRequest || []; + const [queryKey, data] = latestQueriesData || []; const newItem = { ...slo, id: uuidv1() }; - const optimisticUpdate = { ...data, - results: [...(data?.results || []), { ...newItem }], + results: [...(data?.results ?? []), newItem], total: data?.total ? data.total + 1 : 1, }; - // Optimistically update to the new value - if (queryKey) { - queryClient.setQueryData(queryKey, optimisticUpdate); - } + queryClient.setQueryData(queryKey ?? sloKeys.lists(), optimisticUpdate); // Return a context object with the snapshotted value return { previousSloList: data }; diff --git a/x-pack/plugins/observability/public/hooks/slo/use_delete_slo.ts b/x-pack/plugins/observability/public/hooks/slo/use_delete_slo.ts index a1b83bb3dd241..4b4de4d9f615b 100644 --- a/x-pack/plugins/observability/public/hooks/slo/use_delete_slo.ts +++ b/x-pack/plugins/observability/public/hooks/slo/use_delete_slo.ts @@ -37,22 +37,19 @@ export function useDeleteSlo() { // Cancel any outgoing refetches (so they don't overwrite our optimistic update) await queryClient.cancelQueries(sloKeys.lists()); - const latestFetchSloListRequest = ( + const latestQueriesData = ( queryClient.getQueriesData(sloKeys.lists()) || [] ).at(0); - - const [queryKey, data] = latestFetchSloListRequest || []; + const [queryKey, data] = latestQueriesData || []; const optimisticUpdate = { ...data, - results: data?.results.filter((result) => result.id !== slo.id), - total: data?.total && data.total - 1, + results: data?.results?.filter((result) => result.id !== slo.id) ?? [], + total: data?.total ? data.total - 1 : 0, }; // Optimistically update to the new value - if (queryKey) { - queryClient.setQueryData(queryKey, optimisticUpdate); - } + queryClient.setQueryData(queryKey ?? sloKeys.lists(), optimisticUpdate); toasts.addSuccess( i18n.translate('xpack.observability.slo.slo.delete.successNotification', { diff --git a/x-pack/plugins/observability/public/pages/slo_details/components/header_control.tsx b/x-pack/plugins/observability/public/pages/slo_details/components/header_control.tsx index ace0f1a6e5167..0306fb2c2875f 100644 --- a/x-pack/plugins/observability/public/pages/slo_details/components/header_control.tsx +++ b/x-pack/plugins/observability/public/pages/slo_details/components/header_control.tsx @@ -115,7 +115,7 @@ export function HeaderControl({ isLoading, slo }: Props) { transformSloResponseToCreateSloInput({ ...slo, name: `[Copy] ${slo.name}` })! ); - cloneSlo({ slo: newSlo, idToCopyFrom: slo.id }); + cloneSlo({ slo: newSlo, originalSloId: slo.id }); navigate(basePath.prepend(paths.observability.slos)); } diff --git a/x-pack/plugins/observability/public/pages/slo_details/slo_details.test.tsx b/x-pack/plugins/observability/public/pages/slo_details/slo_details.test.tsx index fbcdf4c46272f..73ba8be84953b 100644 --- a/x-pack/plugins/observability/public/pages/slo_details/slo_details.test.tsx +++ b/x-pack/plugins/observability/public/pages/slo_details/slo_details.test.tsx @@ -237,7 +237,7 @@ describe('SLO Details Page', () => { const { id, createdAt, enabled, revision, summary, updatedAt, ...newSlo } = slo; expect(mockClone).toBeCalledWith({ - idToCopyFrom: slo.id, + originalSloId: slo.id, slo: { ...newSlo, name: `[Copy] ${newSlo.name}`, diff --git a/x-pack/plugins/observability/public/pages/slos/components/slo_list_item.tsx b/x-pack/plugins/observability/public/pages/slos/components/slo_list_item.tsx index 4502e7a2d9d20..4564b133e4df9 100644 --- a/x-pack/plugins/observability/public/pages/slos/components/slo_list_item.tsx +++ b/x-pack/plugins/observability/public/pages/slos/components/slo_list_item.tsx @@ -115,7 +115,7 @@ export function SloListItem({ transformSloResponseToCreateSloInput({ ...slo, name: `[Copy] ${slo.name}` })! ); - cloneSlo({ slo: newSlo, idToCopyFrom: slo.id }); + cloneSlo({ slo: newSlo, originalSloId: slo.id }); setIsActionsPopoverOpen(false); };