From 2e4f281427e5eb8542ff847cb23d7f37808cbb03 Mon Sep 17 00:00:00 2001 From: Anton Gilgur <4970083+agilgur5@users.noreply.github.com> Date: Tue, 26 Sep 2023 09:10:50 -0400 Subject: [PATCH] refactor(ui): use async/await in several components (#11882) Signed-off-by: Anton Gilgur --- .../cluster-workflow-template-details.tsx | 38 +++-- .../cluster-workflow-template-list.tsx | 28 ++-- .../event-source-details.tsx | 16 ++- .../shared/components/use-collect-event.ts | 6 +- .../workflow-details/workflow-details.tsx | 134 +++++++++--------- 5 files changed, 121 insertions(+), 101 deletions(-) diff --git a/ui/src/app/cluster-workflow-templates/components/cluster-workflow-template-details/cluster-workflow-template-details.tsx b/ui/src/app/cluster-workflow-templates/components/cluster-workflow-template-details/cluster-workflow-template-details.tsx index acb9e110ad69..0d3ca55f6dfa 100644 --- a/ui/src/app/cluster-workflow-templates/components/cluster-workflow-template-details/cluster-workflow-template-details.tsx +++ b/ui/src/app/cluster-workflow-templates/components/cluster-workflow-template-details/cluster-workflow-template-details.tsx @@ -7,6 +7,7 @@ import {ClusterWorkflowTemplate} from '../../../../models'; import {uiUrl} from '../../../shared/base'; import {ErrorNotice} from '../../../shared/components/error-notice'; import {Loading} from '../../../shared/components/loading'; +import {useCollectEvent} from '../../../shared/components/use-collect-event'; import {Context} from '../../../shared/context'; import {historyUrl} from '../../../shared/history'; import {services} from '../../../shared/services'; @@ -17,7 +18,7 @@ import {ClusterWorkflowTemplateEditor} from '../cluster-workflow-template-editor require('../../../workflows/components/workflow-details/workflow-details.scss'); -export const ClusterWorkflowTemplateDetails = ({history, location, match}: RouteComponentProps) => { +export function ClusterWorkflowTemplateDetails({history, location, match}: RouteComponentProps) { // boiler-plate const {navigation, notifications, popup} = useContext(Context); const queryParams = new URLSearchParams(location.search); @@ -45,23 +46,32 @@ export const ClusterWorkflowTemplateDetails = ({history, location, match}: Route }, [name, sidePanel, tab]); useEffect(() => { - services.clusterWorkflowTemplate - .get(name) - .then(setTemplate) - .then(() => setEdited(false)) // set back to false - .then(() => setError(null)) - .catch(setError); + (async () => { + try { + const newTemplate = await services.clusterWorkflowTemplate.get(name); + setTemplate(newTemplate); + setEdited(false); // set back to false + setError(null); + } catch (err) { + setError(err); + } + })(); }, [name]); useEffect(() => { - services.info - .getInfo() - .then(info => setNamespace(Utils.getNamespaceWithDefault(info.managedNamespace))) - .then(() => setError(null)) - .catch(setError); - services.info.collectEvent('openedClusterWorkflowTemplateDetails').then(); + (async () => { + try { + const info = await services.info.getInfo(); + setNamespace(Utils.getNamespaceWithDefault(info.managedNamespace)); + setError(null); + } catch (err) { + setError(err); + } + })(); }, []); + useCollectEvent('openedClusterWorkflowTemplateDetails'); + return ( ); -}; +} diff --git a/ui/src/app/cluster-workflow-templates/components/cluster-workflow-template-list/cluster-workflow-template-list.tsx b/ui/src/app/cluster-workflow-templates/components/cluster-workflow-template-list/cluster-workflow-template-list.tsx index 5ef0fbfa3a19..0ce5b899f850 100644 --- a/ui/src/app/cluster-workflow-templates/components/cluster-workflow-template-list/cluster-workflow-template-list.tsx +++ b/ui/src/app/cluster-workflow-templates/components/cluster-workflow-template-list/cluster-workflow-template-list.tsx @@ -9,6 +9,7 @@ import {ExampleManifests} from '../../../shared/components/example-manifests'; import {InfoIcon} from '../../../shared/components/fa-icons'; import {Loading} from '../../../shared/components/loading'; import {Timestamp} from '../../../shared/components/timestamp'; +import {useCollectEvent} from '../../../shared/components/use-collect-event'; import {ZeroState} from '../../../shared/components/zero-state'; import {Context} from '../../../shared/context'; import {useQueryParams} from '../../../shared/use-query-params'; @@ -18,20 +19,22 @@ import {services} from '../../../shared/services'; import {ClusterWorkflowTemplateCreator} from '../cluster-workflow-template-creator'; require('./cluster-workflow-template-list.scss'); -export const ClusterWorkflowTemplateList = ({history, location}: RouteComponentProps) => { +export function ClusterWorkflowTemplateList({history, location}: RouteComponentProps) { const {navigation} = useContext(Context); const queryParams = new URLSearchParams(location.search); const [templates, setTemplates] = useState(); const [error, setError] = useState(); const [sidePanel, setSidePanel] = useState(queryParams.get('sidePanel')); - const fetchClusterWorkflowTemplates = () => { - services.clusterWorkflowTemplate - .list() - .then(retrievedTemplates => setTemplates(retrievedTemplates)) - .then(() => setError(null)) - .catch(setError); - }; + async function fetchClusterWorkflowTemplates() { + try { + const retrievedTemplates = await services.clusterWorkflowTemplate.list(); + setTemplates(retrievedTemplates); + setError(null); + } catch (err) { + setError(err); + } + } useEffect( useQueryParams(history, p => { @@ -42,10 +45,11 @@ export const ClusterWorkflowTemplateList = ({history, location}: RouteComponentP useEffect(() => { fetchClusterWorkflowTemplates(); - services.info.collectEvent('openedClusterWorkflowTemplateList').then(); }, []); - const renderTemplates = () => { + useCollectEvent('openedClusterWorkflowTemplateList'); + + function renderTemplates() { if (error) { return ; } @@ -88,7 +92,7 @@ export const ClusterWorkflowTemplateList = ({history, location}: RouteComponentP ); - }; + } return ( ); -}; +} diff --git a/ui/src/app/event-sources/components/event-source-details/event-source-details.tsx b/ui/src/app/event-sources/components/event-source-details/event-source-details.tsx index 54f56d3a9b6a..9e712e9cb70a 100644 --- a/ui/src/app/event-sources/components/event-source-details/event-source-details.tsx +++ b/ui/src/app/event-sources/components/event-source-details/event-source-details.tsx @@ -63,12 +63,16 @@ export const EventSourceDetails = ({history, location, match}: RouteComponentPro })(); useEffect(() => { - services.eventSource - .get(name, namespace) - .then(setEventSource) - .then(() => setEdited(false)) // set back to false - .then(() => setError(null)) - .catch(setError); + (async () => { + try { + const newEventSource = await services.eventSource.get(name, namespace); + setEventSource(newEventSource); + setEdited(false); // set back to false + setError(null); + } catch (err) { + setError(err); + } + })(); }, [name, namespace]); useEffect(() => setEdited(true), [eventSource]); diff --git a/ui/src/app/shared/components/use-collect-event.ts b/ui/src/app/shared/components/use-collect-event.ts index 99b8c63256f5..e67cbfb99907 100644 --- a/ui/src/app/shared/components/use-collect-event.ts +++ b/ui/src/app/shared/components/use-collect-event.ts @@ -1,8 +1,8 @@ import {useEffect} from 'react'; import {services} from '../../shared/services'; -export const useCollectEvent = (name: string) => { +export function useCollectEvent(name: string) { useEffect(() => { - services.info.collectEvent(name).then(); + services.info.collectEvent(name); }, []); -}; +} diff --git a/ui/src/app/workflows/components/workflow-details/workflow-details.tsx b/ui/src/app/workflows/components/workflow-details/workflow-details.tsx index 822cb108f99e..be59dfc479af 100644 --- a/ui/src/app/workflows/components/workflow-details/workflow-details.tsx +++ b/ui/src/app/workflows/components/workflow-details/workflow-details.tsx @@ -12,6 +12,7 @@ import {ErrorNotice} from '../../../shared/components/error-notice'; import {ProcessURL} from '../../../shared/components/links'; import {Loading} from '../../../shared/components/loading'; import {SecurityNudge} from '../../../shared/components/security-nudge'; +import {useCollectEvent} from '../../../shared/components/use-collect-event'; import {hasArtifactGCError, hasWarningConditionBadge} from '../../../shared/conditions-panel'; import {Context} from '../../../shared/context'; import {historyUrl} from '../../../shared/history'; @@ -57,7 +58,7 @@ const ANIMATION_BUFFER_MS = 20; // component to render with the updated state. let globalDeleteArchived = false; -const DeleteCheck = (props: {isWfInDB: boolean; isWfInCluster: boolean}) => { +function DeleteCheck(props: {isWfInDB: boolean; isWfInCluster: boolean}) { // The local states are created intentionally so that the checkbox works as expected const [da, sda] = useState(false); if (props.isWfInDB && props.isWfInCluster) { @@ -86,9 +87,9 @@ const DeleteCheck = (props: {isWfInDB: boolean; isWfInCluster: boolean}) => { ); } -}; +} -export const WorkflowDetails = ({history, location, match}: RouteComponentProps) => { +export function WorkflowDetails({history, location, match}: RouteComponentProps) { // boiler-plate const {navigation, popup} = useContext(Context); const queryParams = new URLSearchParams(location.search); @@ -129,7 +130,7 @@ export const WorkflowDetails = ({history, location, match}: RouteComponentProps< [history] ); - const getInputParametersForNode = (selectedWorkflowNodeId: string): Parameter[] => { + function getInputParametersForNode(selectedWorkflowNodeId: string): Parameter[] { const selectedWorkflowNode = workflow && workflow.status && workflow.status.nodes && workflow.status.nodes[selectedWorkflowNodeId]; return ( selectedWorkflowNode?.inputs?.parameters?.map(param => { @@ -140,7 +141,7 @@ export const WorkflowDetails = ({history, location, match}: RouteComponentProps< return paramClone; }) || [] ); - }; + } useEffect(() => { // update the default Artifact Repository for the Template that corresponds to the selectedArtifact @@ -163,20 +164,25 @@ export const WorkflowDetails = ({history, location, match}: RouteComponentProps< }, [namespace, name, tab, nodeId, nodePanelView, sidePanel, uid]); useEffect(() => { - services.info - .getInfo() - .then(info => setLinks(info.links)) - .catch(setError); - services.info.collectEvent('openedWorkflowDetails').then(); + (async () => { + try { + const info = await services.info.getInfo(); + setLinks(info.links); + } catch (err) { + setError(err); + } + })(); }, []); + useCollectEvent('openedWorkflowDetails'); + useEffect(() => { setParameters(getInputParametersForNode(nodeId)); }, [nodeId, workflow]); const parsedSidePanel = parseSidePanelParam(sidePanel); - const getItems = () => { + function getItems() { const workflowOperationsMap: WorkflowOperations = Operations.WorkflowOperationsMap; const items = Object.keys(workflowOperationsMap) .filter(actionName => !workflowOperationsMap[actionName].disabled(workflow)) @@ -297,15 +303,15 @@ export const WorkflowDetails = ({history, location, match}: RouteComponentProps< } return items; - }; + } - const renderSecurityNudge = () => { + function renderSecurityNudge() { if (!execSpec(workflow).securityContext) { return This workflow does not have security context set. It maybe possible to set this to run it more securely.; } - }; + } - const renderCostOptimisations = () => { + function renderCostOptimisations() { const recommendations: string[] = []; if (!execSpec(workflow).activeDeadlineSeconds) { recommendations.push('activeDeadlineSeconds'); @@ -324,9 +330,9 @@ export const WorkflowDetails = ({history, location, match}: RouteComponentProps< You do not have {recommendations.join('/')} enabled for this workflow. Enabling these will reduce your costs. ); - }; + } - const renderSummaryTab = () => { + function renderSummaryTab() { return ( <> {!workflow ? ( @@ -353,7 +359,8 @@ export const WorkflowDetails = ({history, location, match}: RouteComponentProps< )} ); - }; + } + useEffect(() => { if (!isWorkflowInCluster(workflow)) { return; @@ -388,45 +395,40 @@ export const WorkflowDetails = ({history, location, match}: RouteComponentProps< // Get workflow useEffect(() => { - const getWf = async () => { + (async () => { let archivedWf: Workflow; if (uid !== '') { - await services.workflows - .getArchived(namespace, uid) - .then(wf => { - setError(null); - archivedWf = wf; - }) - .catch(err => { - if (err.status !== 404) { - setError(err); - } - }); - } - await services.workflows - .get(namespace, name) - .then(wf => { + try { + archivedWf = await services.workflows.getArchived(namespace, uid); setError(null); - // If we find live workflow which has same uid, we use live workflow. - if (!archivedWf || archivedWf.metadata.uid === wf.metadata.uid) { - setWorkflow(wf); - setUid(wf.metadata.uid); - } else { - setWorkflow(archivedWf); - } - }) - .catch(err => { - if (archivedWf) { - setWorkflow(archivedWf); - } else { + } catch (err) { + if (err.status !== 404) { setError(err); } - }); - }; - getWf(); + } + } + + try { + const wf = await services.workflows.get(namespace, name); + setError(null); + // If we find live workflow which has same uid, we use live workflow. + if (!archivedWf || archivedWf.metadata.uid === wf.metadata.uid) { + setWorkflow(wf); + setUid(wf.metadata.uid); + } else { + setWorkflow(archivedWf); + } + } catch (err) { + if (archivedWf) { + setWorkflow(archivedWf); + } else { + setError(err); + } + } + })(); }, [namespace, name, uid]); - const openLink = (link: Link) => { + function openLink(link: Link) { const object = { metadata: { namespace: workflow.metadata.namespace, @@ -445,9 +447,9 @@ export const WorkflowDetails = ({history, location, match}: RouteComponentProps< } else { document.location.href = url; } - }; + } - const setParameter = (key: string, value: string) => { + function setParameter(key: string, value: string) { setParameters(previous => { return previous?.map(parameter => { if (parameter.name === key) { @@ -456,33 +458,33 @@ export const WorkflowDetails = ({history, location, match}: RouteComponentProps< return parameter; }); }); - }; + } - const renderSuspendNodeOptions = () => { + function renderSuspendNodeOptions() { return ; - }; + } - const getParametersAsJsonString = () => { + function getParametersAsJsonString() { const outputVariables: {[x: string]: string} = {}; parameters.forEach(param => { outputVariables[param.name] = param.value; }); return JSON.stringify(outputVariables); - }; + } - const updateOutputParametersForNodeIfRequired = () => { + function updateOutputParametersForNodeIfRequired() { // No need to set outputs on node if there are no parameters if (parameters.length > 0) { return services.workflows.set(workflow.metadata.name, workflow.metadata.namespace, 'id=' + nodeId, getParametersAsJsonString()); } return Promise.resolve(null); - }; + } - const resumeNode = () => { + function resumeNode() { return services.workflows.resume(workflow.metadata.name, workflow.metadata.namespace, 'id=' + nodeId); - }; + } - const renderResumePopup = () => { + function renderResumePopup() { return popup.confirm('Confirm', renderSuspendNodeOptions).then(yes => { if (!yes) { return; @@ -492,9 +494,9 @@ export const WorkflowDetails = ({history, location, match}: RouteComponentProps< .then(resumeNode) .catch(setError); }); - }; + } - const ensurePodName = (wf: Workflow, node: NodeStatus, nodeID: string): string => { + function ensurePodName(wf: Workflow, node: NodeStatus, nodeID: string): string { if (workflow && node) { const annotations = workflow.metadata.annotations || {}; const version = annotations[ANNOTATION_KEY_POD_NAME_VERSION]; @@ -503,7 +505,7 @@ export const WorkflowDetails = ({history, location, match}: RouteComponentProps< } return nodeID; - }; + } const podName = ensurePodName(workflow, selectedNode, nodeId); @@ -602,4 +604,4 @@ export const WorkflowDetails = ({history, location, match}: RouteComponentProps< )} ); -}; +}