From 8e083f41f3c02bbcadcf9b0db1ffd48549daa8f4 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 29 Dec 2020 00:00:57 +0100 Subject: [PATCH 1/2] fix(scaffolder): Allow user to retry creating a new software without refreshing the page This is more of a bug fix than a new feature. The commit fixes 2 problems mainly. 1. When user clicks the close button on the JobStatusModal, it does not set the `modalOpen` state of its parent component. Thus, even after closing, JobStatusModal still persists in DOM, and modalOpen is still set to true. That is why, retrying by clicking on "Submit" again does not open the modal. 2. The useJobPolling hook does not consider starting the timer again when Job changes. Even if jobId changes, the logic to set `shouldBeRunningInterval` still uses a previous value of `currentJob` since it was never reset since the timer stopped. Using a effect hook and having jobId as a dependency to reset currentJob is the way to fix this. --- .changeset/famous-shrimps-double.md | 5 +++++ .../JobStatusModal/JobStatusModal.tsx | 18 ++++++++++++------ .../components/TemplatePage/TemplatePage.tsx | 7 ++++++- .../src/components/hooks/useJobPolling.ts | 15 ++++++++++++++- 4 files changed, 37 insertions(+), 8 deletions(-) create mode 100644 .changeset/famous-shrimps-double.md diff --git a/.changeset/famous-shrimps-double.md b/.changeset/famous-shrimps-double.md new file mode 100644 index 0000000000..198c0dc8f5 --- /dev/null +++ b/.changeset/famous-shrimps-double.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder': patch +--- + +Bug fix: User can retry creating a new component if an error occurs, without having to reload the page. diff --git a/plugins/scaffolder/src/components/JobStatusModal/JobStatusModal.tsx b/plugins/scaffolder/src/components/JobStatusModal/JobStatusModal.tsx index 01a6346d24..5051475ae8 100644 --- a/plugins/scaffolder/src/components/JobStatusModal/JobStatusModal.tsx +++ b/plugins/scaffolder/src/components/JobStatusModal/JobStatusModal.tsx @@ -23,18 +23,23 @@ import { LinearProgress, } from '@material-ui/core'; -import React, { useCallback, useState } from 'react'; +import React, { useCallback } from 'react'; import { Job } from '../../types'; import { JobStage } from '../JobStage/JobStage'; type Props = { job: Job | null; toCatalogLink?: string; + open: boolean; + setOpen: (newState: boolean) => void; }; -export const JobStatusModal = ({ job, toCatalogLink }: Props) => { - const [isOpen, setOpen] = useState(true); - +export const JobStatusModal = ({ + job, + toCatalogLink, + open, + setOpen, +}: Props) => { const renderTitle = () => { switch (job?.status) { case 'COMPLETED': @@ -47,16 +52,17 @@ export const JobStatusModal = ({ job, toCatalogLink }: Props) => { }; const onClose = useCallback(() => { + setOpen(false); if (!job) { return; } if (job.status === 'COMPLETED' || job.status === 'FAILED') { setOpen(false); } - }, [job]); + }, [job, setOpen]); return ( - + {renderTitle()} {!job ? ( diff --git a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx index 0754b64d7f..cdf9b05b4c 100644 --- a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx +++ b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx @@ -161,7 +161,12 @@ export const TemplatePage = () => { /> {loading && } - {modalOpen && } + {template && ( { const scaffolderApi = useApi(scaffolderApiRef); const [currentJob, setCurrentJob] = useState(null); + + useEffect(() => { + const resetCurrentJob = async () => { + if (jobId) { + const job = await scaffolderApi.getJob(jobId); + setCurrentJob(job); + } + }; + + resetCurrentJob(); + }, [jobId, scaffolderApi]); + const shouldBeRunningInterval = jobId && currentJob?.status !== 'COMPLETED' && currentJob?.status !== 'FAILED'; + useInterval( async () => { if (jobId) { From 2d0a45d0942cdcfc63ee72d1e05b5cf66971cfed Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 29 Dec 2020 15:39:01 +0100 Subject: [PATCH 2/2] fix(scaffolder): Instead of state setter, pass a callback as prop --- .../src/components/JobStatusModal/JobStatusModal.tsx | 12 ++++++------ .../src/components/TemplatePage/TemplatePage.tsx | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/plugins/scaffolder/src/components/JobStatusModal/JobStatusModal.tsx b/plugins/scaffolder/src/components/JobStatusModal/JobStatusModal.tsx index 5051475ae8..d35385ec3b 100644 --- a/plugins/scaffolder/src/components/JobStatusModal/JobStatusModal.tsx +++ b/plugins/scaffolder/src/components/JobStatusModal/JobStatusModal.tsx @@ -31,14 +31,14 @@ type Props = { job: Job | null; toCatalogLink?: string; open: boolean; - setOpen: (newState: boolean) => void; + onModalClose: () => void; }; export const JobStatusModal = ({ job, toCatalogLink, open, - setOpen, + onModalClose, }: Props) => { const renderTitle = () => { switch (job?.status) { @@ -52,14 +52,14 @@ export const JobStatusModal = ({ }; const onClose = useCallback(() => { - setOpen(false); if (!job) { return; } + // Disallow closing modal if the job is in progress. if (job.status === 'COMPLETED' || job.status === 'FAILED') { - setOpen(false); + onModalClose(); } - }, [job, setOpen]); + }, [job, onModalClose]); return ( @@ -87,7 +87,7 @@ export const JobStatusModal = ({ )} {job?.status === 'FAILED' && ( - setOpen(false)}>Close + Close )} diff --git a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx index cdf9b05b4c..fd4e4c3ded 100644 --- a/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx +++ b/plugins/scaffolder/src/components/TemplatePage/TemplatePage.tsx @@ -165,7 +165,7 @@ export const TemplatePage = () => { job={job} toCatalogLink={catalogLink} open={modalOpen} - setOpen={setModalOpen} + onModalClose={() => setModalOpen(false)} /> {template && (