Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(app, odd): run summery error list bug fixes #15953

Merged
merged 16 commits into from
Aug 12, 2024
Merged

fix(app, odd): run summery error list bug fixes #15953

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
9bc4866
small bug fixes
TamarZanzouri Aug 8, 2024
5d20145
banner colors and warnings/errors
TamarZanzouri Aug 8, 2024
91523b5
WIP odd warnings
TamarZanzouri Aug 9, 2024
8d8113a
plurale warnings/errors
TamarZanzouri Aug 9, 2024
d850730
revert debug change
TamarZanzouri Aug 9, 2024
f7e5db1
text localization
TamarZanzouri Aug 9, 2024
b3993b4
run status injection
TamarZanzouri Aug 9, 2024
713b9c4
pluralization
TamarZanzouri Aug 9, 2024
8fb6aa0
plural try
TamarZanzouri Aug 9, 2024
5da9748
fixed failing test and removed redundant
TamarZanzouri Aug 10, 2024
1e02e9d
formatting
TamarZanzouri Aug 10, 2024
12511b5
reverted plurar changes
TamarZanzouri Aug 12, 2024
e4ddf56
run completed with warnings odd
TamarZanzouri Aug 12, 2024
f4c54e2
linting fix
TamarZanzouri Aug 12, 2024
a1af052
Merge branch 'chore_release-8.0.0' into RQA-2923-odd-not-showing-view…
TamarZanzouri Aug 12, 2024
da53912
removed redundant test
TamarZanzouri Aug 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion app/src/assets/localization/en/run_details.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
"end_step_time": "End",
"error_info": "Error {{errorCode}}: {{errorType}}",
"error_type": "Error: {{errorType}}",
"error_details": "Error details",
"error": "error",
"failed_step": "Failed step",
"final_step": "Final Step",
"ignore_stored_data": "Ignore stored data",
Expand Down Expand Up @@ -98,6 +100,8 @@
"run_complete": "Run completed",
"run_complete_splash": "Run completed",
"run_completed": "Run completed.",
"run_completed_with_errors": "Run completed with errors.",
"run_completed_with_warnings": "Run completed with warnings.",
"run_cta_disabled": "Complete required steps on Protocol tab before starting the run",
"run_failed": "Run failed.",
"run_failed_modal_body": "Error occurred when protocol was {{command}}",
Expand Down Expand Up @@ -144,5 +148,7 @@
"view_analysis_error_details": "View <errorLink>error details</errorLink>",
"view_current_step": "View current step",
"view_error": "View error",
"view_error_details": "View error details"
"view_error_details": "View error details",
"warning_details": "Warning details",
"warning": "warning"
}
1 change: 0 additions & 1 deletion app/src/molecules/Modal/ModalHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export function ModalHeader(props: ModalHeaderBaseProps): JSX.Element {
color={iconColor}
size="2rem"
alignSelf={ALIGN_CENTER}
marginRight={SPACING.spacing16}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really? this will affect all the rest of the modals on the ODD

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have a gap in the div above it. I think we can remove this. can test more if needed!

/>
) : null}
<LegacyStyledText
Expand Down
22 changes: 15 additions & 7 deletions app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@
setShowRunFailedModal={setShowRunFailedModal}
highestPriorityError={highestPriorityError}
commandErrorList={commandErrorList}
runStatus={runStatus}

Check failure on line 348 in app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx

View workflow job for this annotation

GitHub Actions / js checks 

Type 'RunStatus | null' is not assignable to type 'RunStatus'.

Check failure on line 348 in app/src/organisms/Devices/ProtocolRun/ProtocolRunHeader.tsx

View workflow job for this annotation

GitHub Actions / js checks 

Type 'RunStatus | null' is not assignable to type 'RunStatus'.
/>
) : null}
<Flex
Expand Down Expand Up @@ -899,7 +900,8 @@
isRunCurrent,
} = props
const { t } = useTranslation('run_details')

const completedWithErrors =
commandErrorList?.data != null && commandErrorList.data.length > 0
const handleRunSuccessClick = (): void => {
handleClearClick()
}
Expand All @@ -926,15 +928,22 @@

const buildErrorBanner = (): JSX.Element => {
return (
<Banner type="error" iconMarginLeft={SPACING.spacing4}>
<Banner
type={runStatus === RUN_STATUS_SUCCEEDED ? 'warning' : 'error'}
iconMarginLeft={SPACING.spacing4}
>
<Flex justifyContent={JUSTIFY_SPACE_BETWEEN} width="100%">
<LegacyStyledText>
{highestPriorityError != null
? t('error_info', {
errorType: highestPriorityError?.errorType,
errorCode: highestPriorityError?.errorCode,
})
: 'Run completed with errors.'}
: `${
runStatus === RUN_STATUS_SUCCEEDED
? t('run_completed_with_warnings')
: t('run_completed_with_errors')
}`}
</LegacyStyledText>

<LinkButton
Expand All @@ -951,14 +960,13 @@
if (
runStatus === RUN_STATUS_SUCCEEDED &&
isRunCurrent &&
!isResetRunLoading
!isResetRunLoading &&
!completedWithErrors
) {
return buildSuccessBanner()
} else if (
highestPriorityError != null ||
(commandErrorList?.data != null &&
commandErrorList.data.length > 0 &&
!isResetRunLoading)
(completedWithErrors && !isResetRunLoading)
) {
return buildErrorBanner()
} else {
Expand Down
22 changes: 18 additions & 4 deletions app/src/organisms/Devices/ProtocolRun/RunFailedModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ import {
import { LegacyModal } from '../../../molecules/LegacyModal'
import { useDownloadRunLog } from '../hooks'

import type { RunError, RunCommandErrors } from '@opentrons/api-client'
import type {
RunError,
RunCommandErrors,
RunStatus,
} from '@opentrons/api-client'
import { RUN_STATUS_SUCCEEDED } from '@opentrons/api-client'
import type { LegacyModalProps } from '../../../molecules/LegacyModal'
import type { RunCommandError } from '@opentrons/shared-data'

Expand All @@ -45,6 +50,7 @@ interface RunFailedModalProps {
setShowRunFailedModal: (showRunFailedModal: boolean) => void
highestPriorityError?: RunError | null
commandErrorList?: RunCommandErrors | null
runStatus: RunStatus
}

export function RunFailedModal({
Expand All @@ -53,11 +59,17 @@ export function RunFailedModal({
setShowRunFailedModal,
highestPriorityError,
commandErrorList,
runStatus,
}: RunFailedModalProps): JSX.Element | null {
const { i18n, t } = useTranslation(['run_details', 'shared', 'branded'])
const modalProps: LegacyModalProps = {
type: 'error',
title: t('run_failed_modal_title'),
type: runStatus === RUN_STATUS_SUCCEEDED ? 'warning' : 'error',
title:
commandErrorList == null || commandErrorList?.data.length === 0
? t('run_failed_modal_title')
: runStatus === RUN_STATUS_SUCCEEDED
? t('warning_details')
: t('error_details'),
onClose: () => {
setShowRunFailedModal(false)
},
Expand Down Expand Up @@ -95,7 +107,9 @@ export function RunFailedModal({
errorType: errors[0].errorType,
errorCode: errors[0].errorCode,
})
: `${errors.length} errors`}
: `${errors.length} ${
TamarZanzouri marked this conversation as resolved.
Show resolved Hide resolved
runStatus === RUN_STATUS_SUCCEEDED ? t('warning') : t('error')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this won't translate properly, right? if you translated it into say chinese, if there was more than one error it would have an "s" at the end of the chinese characters, and because the translation isn't number-aware it wouldn't have the right counting word. we should be using the i18n pluralization tools here

}${errors.length > 1 ? 's' : ''}`}
</LegacyStyledText>
<Flex css={ERROR_MESSAGE_STYLE}>
{' '}
Expand Down
20 changes: 17 additions & 3 deletions app/src/organisms/OnDeviceDisplay/RunningProtocol/RunFailedModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ import { SmallButton } from '../../../atoms/buttons'
import { Modal } from '../../../molecules/Modal'

import type { ModalHeaderBaseProps } from '../../../molecules/Modal/types'
import type { RunCommandErrors, RunError } from '@opentrons/api-client'
import type {
RunCommandErrors,
RunError,
RunStatus,
} from '@opentrons/api-client'
import { RUN_STATUS_SUCCEEDED } from '@opentrons/api-client'

import type { RunCommandError } from '@opentrons/shared-data'

Expand All @@ -29,13 +34,15 @@ interface RunFailedModalProps {
setShowRunFailedModal: (showRunFailedModal: boolean) => void
errors?: RunError[]
commandErrorList?: RunCommandErrors
runStatus: RunStatus
}

export function RunFailedModal({
runId,
setShowRunFailedModal,
errors,
commandErrorList,
runStatus,
}: RunFailedModalProps): JSX.Element | null {
const { t, i18n } = useTranslation(['run_details', 'shared', 'branded'])
const navigate = useNavigate()
Expand All @@ -48,7 +55,12 @@ export function RunFailedModal({
)
return null
const modalHeader: ModalHeaderBaseProps = {
title: t('run_failed_modal_title'),
title:
commandErrorList == null || commandErrorList?.data.length === 0
? t('run_failed_modal_title')
: runStatus === RUN_STATUS_SUCCEEDED
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same thing re: plural details

? t('warning_details')
: t('error_details'),
}

const highestPriorityError = getHighestPriorityError(errors ?? [])
Expand Down Expand Up @@ -85,7 +97,9 @@ export function RunFailedModal({
errorType: errors[0].errorType,
errorCode: errors[0].errorCode,
})
: `${errors.length} errors`}
: `${errors.length} ${
runStatus === RUN_STATUS_SUCCEEDED ? t('warning') : t('error')
}${errors.length > 1 ? 's' : ''}`}
</LegacyStyledText>
<Flex
width="100%"
Expand Down
9 changes: 6 additions & 3 deletions app/src/pages/RunSummary/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@
padding={SPACING.spacing40}
>
{showRunFailedModal ? (
<RunFailedModal

Check failure on line 330 in app/src/pages/RunSummary/index.tsx

View workflow job for this annotation

GitHub Actions / js checks 

Property 'runStatus' is missing in type '{ runId: string; setShowRunFailedModal: Dispatch<SetStateAction<boolean>>; errors: RunCommandError[] | undefined; commandErrorList: RunCommandErrors | undefined; }' but required in type 'RunFailedModalProps'.

Check failure on line 330 in app/src/pages/RunSummary/index.tsx

View workflow job for this annotation

GitHub Actions / js checks 

Property 'runStatus' is missing in type '{ runId: string; setShowRunFailedModal: Dispatch<SetStateAction<boolean>>; errors: RunCommandError[] | undefined; commandErrorList: RunCommandErrors | undefined; }' but required in type 'RunFailedModalProps'.
runId={runId}
setShowRunFailedModal={setShowRunFailedModal}
errors={runRecord?.data.errors}
Expand Down Expand Up @@ -399,7 +399,8 @@
height="17rem"
css={showRunAgainSpinner ? RUN_AGAIN_CLICKED_STYLE : undefined}
/>
{!didRunSucceed ? (
{(commandErrorList != null && commandErrorList?.data.length > 0) ||
!didRunSucceed ? (
<LargeButton
flex="1"
iconName="info"
Expand All @@ -408,8 +409,10 @@
buttonText={t('view_error_details')}
height="17rem"
disabled={
runRecord?.data.errors == null ||
runRecord?.data.errors.length === 0
(runRecord?.data.errors == null ||
runRecord?.data.errors.length === 0) &&
(commandErrorList == null ||
commandErrorList?.data.length === 0)
}
/>
) : null}
Expand Down
Loading