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.

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

feat(app): error handling for wrong pip attached #12706

Merged
merged 4 commits into from
May 20, 2023

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented May 16, 2023

closes RLIQ-393

Overview

When attaching a pipette in run setup, if you attach the wrong pipette, the results page now renders the error modal (designs). When you click detach and retry it goes back to detach and reattach the correct pipette.

Test Plan

test this flow on a robot to make sure it works as expected

Changelog

  • add error logic to Results page and add test cases

Review requests

see test plan

Risk assessment

low

@jerader jerader requested a review from a team as a code owner May 16, 2023 12:20
@jerader jerader requested review from koji, smb2268 and a team and removed request for a team and koji May 16, 2023 12:20
@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Merging #12706 (13513c8) into edge (b8ebbbe) will increase coverage by 0.04%.
The diff coverage is 95.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #12706      +/-   ##
==========================================
+ Coverage   73.35%   73.39%   +0.04%     
==========================================
  Files        2261     2265       +4     
  Lines       61480    61971     +491     
  Branches     6782     6984     +202     
==========================================
+ Hits        45098    45486     +388     
- Misses      14763    14831      +68     
- Partials     1619     1654      +35     
Flag Coverage Δ
app 71.87% <95.23%> (+0.20%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
app/src/organisms/PipetteWizardFlows/index.tsx 64.70% <50.00%> (-0.30%) ⬇️
...rganisms/PipetteWizardFlows/CheckPipetteButton.tsx 90.00% <100.00%> (ø)
app/src/organisms/PipetteWizardFlows/Results.tsx 91.93% <100.00%> (+2.35%) ⬆️

... and 39 files with indirect coverage changes

@jerader jerader force-pushed the app_pip-flow-wrong-insturment-attached branch from ebd1d1b to 87f1c47 Compare May 16, 2023 13:02
@@ -94,7 +112,9 @@ export const Results = (props: ResultsProps): JSX.Element => {
}

const handleProceed = (): void => {
if (currentStepIndex === totalStepCount || !isSuccess) {
if (requiredPipette != null && Boolean(!isCorrectPipette)) {
goBack()
Copy link
Contributor

Choose a reason for hiding this comment

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

This takes us back to the before you begin screen which I'm not sure makes sense from a user perspective - on the OT-2 flows we just have the CheckPipetteButton here in this case, so we recheck for attachment of the correct pipette instead of rerouting the user through the flow again which might make more sense?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wonder if we should add a subheader that says which pipette is the correct one to attach?

@jerader jerader force-pushed the app_pip-flow-wrong-insturment-attached branch from ab91f60 to 91f7f03 Compare May 16, 2023 18:34
@jerader jerader requested a review from smb2268 May 18, 2023 12:40
Copy link
Contributor

@smb2268 smb2268 left a comment

Choose a reason for hiding this comment

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

Tested this and it worked as expected!

/>
)
}
if (
Copy link
Contributor

Choose a reason for hiding this comment

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

can we make this an else if?

@jerader jerader merged commit 4344a03 into edge May 20, 2023
@jerader jerader deleted the app_pip-flow-wrong-insturment-attached branch May 20, 2023 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants