Skip to content

Commit

Permalink
fix: better error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
Birkbjo committed Feb 8, 2024
1 parent 9bf677b commit 09aac18
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 24 deletions.
63 changes: 47 additions & 16 deletions src/pages/data-integrity/details/CheckDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useConfig, useTimeZoneConversion } from '@dhis2/app-runtime'
import i18n from '@dhis2/d2-i18n'
import { Button, IconSync16 } from '@dhis2/ui'
import PropTypes from 'prop-types'

Check failure on line 4 in src/pages/data-integrity/details/CheckDetails.js

View workflow job for this annotation

GitHub Actions / lint

'PropTypes' is defined but never used
import React, { useEffect, useState } from 'react'
import React, { useEffect, useState, useRef } from 'react'
import {
getDurationWithUnitFromDelta,
selectedLocale,
Expand All @@ -15,14 +15,36 @@ import { Notice } from './Notice.js'
import { useDataIntegrityDetails } from './use-data-integrity-details.js'

export const CheckDetails = ({ check }) => {
const { startDetailsCheck, runningCheck, loading, details, currentJob } =
useDataIntegrityDetails(check.name)
// make sure detailsCheck is only started once
const hasStartedCheck = useRef(false)
const {
startDetailsCheck,
runningCheck,
loading,
details,
currentJob,
error,
} = useDataIntegrityDetails(check.name)

useEffect(() => {
if (!loading && !details && !runningCheck) {
if (
!loading &&
!details &&
!runningCheck &&
!error &&
!hasStartedCheck.current
) {
hasStartedCheck.current = true
startDetailsCheck({ name: check.name })
}
}, [loading, details, runningCheck, check.name, startDetailsCheck])
}, [
loading,
details,
runningCheck,
check.name,
error,
startDetailsCheck,
])

return (
<div className={css.wrapper}>
Expand All @@ -41,19 +63,23 @@ export const CheckDetails = ({ check }) => {
</div>

<div className={css.detailsRunWrapper}>
<DetailsContent
summaryCheck={check}
detailsCheck={details}
runningCheck={runningCheck}
currentJob={currentJob}
/>
{error ? (
<DetailsError />
) : (
<DetailsContent
summaryCheck={check}
detailsCheck={details}
runningCheck={runningCheck}
currentJob={currentJob}
/>
)}
</div>
</div>
)
}

CheckDetails.propTypes = {
check: checkProps
check: checkProps,
}

export const DetailsHeader = ({ name, description }) => {

Check failure on line 85 in src/pages/data-integrity/details/CheckDetails.js

View workflow job for this annotation

GitHub Actions / lint

'name' is missing in props validation

Check failure on line 85 in src/pages/data-integrity/details/CheckDetails.js

View workflow job for this annotation

GitHub Actions / lint

'description' is missing in props validation
Expand All @@ -65,8 +91,7 @@ export const DetailsHeader = ({ name, description }) => {
)
}

DetailsHeader.propTypes = {
}
DetailsHeader.propTypes = {}

const DetailsContent = ({
detailsCheck,

Check failure on line 97 in src/pages/data-integrity/details/CheckDetails.js

View workflow job for this annotation

GitHub Actions / lint

'detailsCheck' is missing in props validation
Expand Down Expand Up @@ -112,8 +137,7 @@ const DetailsRunCompleted = ({ detailsCheck }) => {
<header title={latestRun.getClientZonedISOString()}>
{i18n.t('Latest run completed {{time}}', {
time: formattedLatestRun,
interpolation: { escapeValue: false}

interpolation: { escapeValue: false },
})}
<StatusIcon count={detailsCheck.issues.length} />
</header>
Expand All @@ -133,6 +157,13 @@ const DetailsRunCompleted = ({ detailsCheck }) => {
)
}

const DetailsError = () => {
return (
<Notice status="error">
{i18n.t('An error occurred when running the job')}
</Notice>
)
}

const DetailsRunSuccess = () => {
return <Notice status={'success'}>{i18n.t('Passed with 0 errors.')}</Notice>
Expand Down
13 changes: 9 additions & 4 deletions src/pages/data-integrity/details/use-data-integrity-details.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,36 +35,41 @@ export const useDataIntegrityDetails = (name) => {
cancel,
started: isPolling,
} = useLazyInterval(fetchDetails, 500) // low due to long-polling
const [runMutation, { loading: mutationLoading }] =
const [runMutation, { loading: mutationLoading, error: mutationError }] =
useDataMutation(startDetailsCheckMutation, {
variables: { name },
onComplete: (data) => {
setLastJob(data.response)
start({ name, timeout: 5000 })
if(data?.response?.created) {
start({ name, timeout: 5000 })
}
},
})

const startDetailsCheck = useCallback(() => {
setLastJob(null)
runMutation()
}, [runMutation])

const details = detailsData?.result?.[name]
const anyError = detailsError || mutationError

useEffect(() => {
if(!isPolling) {
return
}
const ranByLastJob = details?.startTime >= lastJob?.created
if (detailsError || ranByLastJob || lastJob == null) {
if (anyError || ranByLastJob || lastJob == null) {
cancel()
}
}, [detailsError, details, lastJob, name, cancel, isPolling])
}, [anyError, details, lastJob, name, cancel, isPolling])

return {
startDetailsCheck,
runningCheck: mutationLoading || isPolling,
loading: detailsLoading,
details: details,
currentJob: isPolling ? lastJob : null,
error: anyError,
}
}
12 changes: 8 additions & 4 deletions src/pages/data-integrity/use-data-integrity-summary.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,14 @@ export const useDataIntegritySummary = () => {
} = useDataQuery(summaryQuery)

const { start, cancel, started: isPolling } = useLazyInterval(fetchSummary, 2000)
const [runMutation, { loading: mutationLoading }] =
const [runMutation, { loading: mutationLoading, error: mutationError }] =
useDataMutation(startDataIntegrityCheckMutation, {
onComplete: (data) => {
setLastJob(data.response)
start()
if(data?.response.created) {
start()

}
},
})

Expand Down Expand Up @@ -81,11 +84,12 @@ export const useDataIntegritySummary = () => {
})
}, [summaryData, checks, lastJob, isPolling, mutationLoading])

const anyError = summaryError || mutationError
useEffect(() => {
if (summaryError || formattedData?.every((check) => !check.loading)) {
if (anyError || formattedData?.every((check) => !check.loading)) {
cancel()
}
}, [summaryError, formattedData, cancel])
}, [anyError, formattedData, cancel])

return {
startDataIntegrityCheck,
Expand Down

0 comments on commit 09aac18

Please sign in to comment.