From 89f1da11bf5b3e916b720bbb97e1233efeea64f0 Mon Sep 17 00:00:00 2001 From: lucasgarfield Date: Tue, 10 Oct 2023 14:09:15 +0200 Subject: [PATCH] API: Move notification dispatch to Image Builder API slice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit moves the notification dispatching for creating composes and clones into a more sensible location – the Image Builder API slice. It is more sensible because it separates the logic of the React component (the wizard or share images modal) from the logic of handling the request life cycle (which is now handled entirely in the slice). There is a subtle but significant change – a new request will be dispatched for every request. This is the correct way to do things as it is possible that some requests succeed, and that others fail. Insights causes the notifications to stack on top of each other neatly, so there is no UI problem. To facilitate this, we also need to use use Promise.allSettled instead of Promise.all. --- .../CreateImageWizard/CreateImageWizard.js | 40 ++--------- .../ShareImageModal/RegionsSelect.tsx | 31 +-------- src/store/enhancedImageBuilderApi.ts | 68 +++++++++++++++---- 3 files changed, 60 insertions(+), 79 deletions(-) diff --git a/src/Components/CreateImageWizard/CreateImageWizard.js b/src/Components/CreateImageWizard/CreateImageWizard.js index 17a09d67..348f8125 100644 --- a/src/Components/CreateImageWizard/CreateImageWizard.js +++ b/src/Components/CreateImageWizard/CreateImageWizard.js @@ -1,9 +1,7 @@ import React from 'react'; import componentTypes from '@data-driven-forms/react-form-renderer/component-types'; -import { addNotification } from '@redhat-cloud-services/frontend-components-notifications/redux'; import { useFlag } from '@unleash/proxy-client-react'; -import { useDispatch, useStore } from 'react-redux'; import { useNavigate, useParams } from 'react-router-dom'; import ImageCreator from './ImageCreator'; @@ -533,7 +531,6 @@ const formStepHistory = (composeRequest, contentSourcesEnabled, isBeta) => { const CreateImageWizard = () => { const [composeImage] = useComposeImageMutation(); - const dispatch = useDispatch(); const navigate = useNavigate(); // composeId is an optional param that is used for Recreate image const { composeId } = useParams(); @@ -596,39 +593,10 @@ const CreateImageWizard = () => { onSubmit={async ({ values, setIsSaving }) => { setIsSaving(true); const requests = onSave(values); - // https://redux-toolkit.js.org/rtk-query/usage/mutations#frequently-used-mutation-hook-return-values - // If you want to immediately access the result of a mutation, you need to chain `.unwrap()` - // if you actually want the payload or to catch the error. - // We do this so we can dispatch the appropriate notification (success or failure). - await Promise.all( - requests.map((composeRequest) => - composeImage({ composeRequest }).unwrap() - ) - ) - .then(() => { - navigate(resolveRelPath('')); - dispatch( - addNotification({ - variant: 'success', - title: 'Your image is being created', - }) - ); - }) - .catch((err) => { - let msg = err.response.statusText; - if (err.response.data?.errors[0]?.detail) { - msg = err.response.data?.errors[0]?.detail; - } - - navigate(resolveRelPath('')); - dispatch( - addNotification({ - variant: 'danger', - title: 'Your image could not be created', - description: 'Status code ' + err.response.status + ': ' + msg, - }) - ); - }); + await Promise.allSettled( + requests.map((composeRequest) => composeImage({ composeRequest })) + ); + navigate(resolveRelPath('')); }} defaultArch="x86_64" customValidatorMapper={{ diff --git a/src/Components/ShareImageModal/RegionsSelect.tsx b/src/Components/ShareImageModal/RegionsSelect.tsx index e4a9f7d7..eef78255 100644 --- a/src/Components/ShareImageModal/RegionsSelect.tsx +++ b/src/Components/ShareImageModal/RegionsSelect.tsx @@ -12,8 +12,6 @@ import { ValidatedOptions, } from '@patternfly/react-core'; import { ExclamationCircleIcon, HelpIcon } from '@patternfly/react-icons'; -import { addNotification } from '@redhat-cloud-services/frontend-components-notifications/redux'; -import { useDispatch } from 'react-redux'; import { useNavigate } from 'react-router-dom'; import { AWS_REGIONS } from '../../constants'; @@ -64,7 +62,6 @@ const RegionsSelect = ({ isOpen, setIsOpen, }: RegionsSelectPropTypes) => { - const dispatch = useDispatch(); const navigate = useNavigate(); const [isSaving, setIsSaving] = useState(false); const [selected, setSelected] = useState([]); @@ -113,32 +110,8 @@ const RegionsSelect = ({ const handleSubmit = async () => { setIsSaving(true); const requests = generateRequests(composeId, composeStatus, selected); - // https://redux-toolkit.js.org/rtk-query/usage/mutations#frequently-used-mutation-hook-return-values - // If you want to immediately access the result of a mutation, you need to chain `.unwrap()` - // if you actually want the payload or to catch the error. - // We do this so we can dispatch the appropriate notification (success or failure). - await Promise.all(requests.map((request) => cloneCompose(request).unwrap())) - .then(() => { - setIsSaving(false); - navigate(resolveRelPath('')); - dispatch( - addNotification({ - variant: 'success', - title: 'Your image is being shared', - }) - ); - }) - .catch((err) => { - navigate(resolveRelPath('')); - // TODO The error should be typed. - dispatch( - addNotification({ - variant: 'danger', - title: 'Your image could not be shared', - description: `Status code ${err.status}: ${err.data.errors[0].detail}`, - }) - ); - }); + await Promise.allSettled(requests.map((request) => cloneCompose(request))); + navigate(resolveRelPath('')); }; return ( diff --git a/src/store/enhancedImageBuilderApi.ts b/src/store/enhancedImageBuilderApi.ts index 2849b070..e3bb243f 100644 --- a/src/store/enhancedImageBuilderApi.ts +++ b/src/store/enhancedImageBuilderApi.ts @@ -1,3 +1,5 @@ +import { addNotification } from '@redhat-cloud-services/frontend-components-notifications/redux'; + import { imageBuilderApi } from './imageBuilderApi'; const enhancedApi = imageBuilderApi.enhanceEndpoints({ @@ -18,15 +20,32 @@ const enhancedApi = imageBuilderApi.enhanceEndpoints({ { composeId, cloneRequest }, { dispatch, queryFulfilled } ) => { - queryFulfilled.then(() => { - dispatch( - imageBuilderApi.util.invalidateTags([ - // Typescript is unaware of tag types being defined concurrently in enhanceEndpoints() - // @ts-expect-error - { type: 'Clone', id: composeId }, - ]) - ); - }); + queryFulfilled + .then(() => { + dispatch( + imageBuilderApi.util.invalidateTags([ + // Typescript is unaware of tag types being defined concurrently in enhanceEndpoints() + // @ts-expect-error + { type: 'Clone', id: composeId }, + ]) + ); + + dispatch( + addNotification({ + variant: 'success', + title: 'Your image is being shared', + }) + ); + }) + .catch((err) => { + dispatch( + addNotification({ + variant: 'danger', + title: 'Your image could not be shared', + description: `Status code ${err.status}: ${err.data.errors[0].detail}`, + }) + ); + }); }, }, composeImage: { @@ -34,11 +53,32 @@ const enhancedApi = imageBuilderApi.enhanceEndpoints({ { composeRequest }, { dispatch, queryFulfilled } ) => { - queryFulfilled.then(() => { - // Typescript is unaware of tag types being defined concurrently in enhanceEndpoints() - // @ts-expect-error - dispatch(imageBuilderApi.util.invalidateTags(['Compose'])); - }); + queryFulfilled + .then(() => { + // Typescript is unaware of tag types being defined concurrently in enhanceEndpoints() + // @ts-expect-error + dispatch(imageBuilderApi.util.invalidateTags(['Compose'])); + dispatch( + addNotification({ + variant: 'success', + title: 'Your image is being created', + }) + ); + }) + .catch((err) => { + let msg = err.response.statusText; + if (err.response.data?.errors[0]?.detail) { + msg = err.response.data?.errors[0]?.detail; + } + + dispatch( + addNotification({ + variant: 'danger', + title: 'Your image could not be created', + description: 'Status code ' + err.response.status + ': ' + msg, + }) + ); + }); }, }, },