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

fix(app): Fix post run drop tip wizard always displaying after error recovery #16893

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented Nov 19, 2024

Closes RQA-3614

Overview

We recently updated post-run tip checking to get the last run command via useCurrentRunCommands with parameters

{
      includeFixitCommands: false,
      pageLength: 1,
     cursor: null
}

Something on the server (likely Pydantic) deserializes the cursor: 'null' string to 0, which causes unexpected behavior (ex, we always pop drop tip wizard post-run, which we don't always want to do).

412e29f - The api-client typing involving command queries requires supplying a cursor: null if none is specified, so to fix, we must change the types and then update all usages of the relevant queries.

0cdec49 - This fixes some unexpected behavior on the desktop app in which the post run CTA sometimes appears while "run again" is cloning the run. The fix is just to clear tip status while the run is resetting (on click on the ActionButton).

Test Plan and Hands on Testing

  • Ran a lot of runs with various types of drop tip wizard CTAs, both in and out of Error Recovery, confirming expected behavior.
  • Confirmed that "run again" does not flash the post-run drop tip CTA.

Changelog

  • Fixed the post-run drop tip CTA always appearing after error recovery.

Risk assessment

  • low. This touches a lot of files, but the only real functional change is the one liner in useActionButtonProperties.ts

@mjhuff mjhuff requested a review from a team as a code owner November 19, 2024 20:58
Copy link
Contributor

@TamarZanzouri TamarZanzouri left a comment

Choose a reason for hiding this comment

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

thanks for fixing this! sorry it was such a nightmare :-)

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Lol.

@mjhuff mjhuff merged commit 90389eb into chore_release-8.2.0 Nov 19, 2024
38 checks passed
@mjhuff mjhuff deleted the app_fix-post-run-drop-tip-wiz branch November 19, 2024 22:17
SyntaxColoring added a commit that referenced this pull request Nov 26, 2024
Conflicts:

api/src/opentrons/protocol_engine/commands/absorbance_reader/read.py:
Conflict in error message wording between PR #16941 in release and PR #16920 in edge. I took the one in release because the one in edge was an outdated cherry pick.

app/src/organisms/Desktop/Devices/ProtocolRun/ProtocolRunHeader/RunHeaderModalContainer/hooks/useRunHeaderDropTip.ts:
Just import statements.

app/src/organisms/DropTipWizardFlows/hooks/useTipAttachmentStatus/getPipettesWithTipAttached.ts:
Release removed `cursor: null` and `includeFixitCommands: null` from some network requests, leaving those fields unspecified instead (PR ##16893). Edge deleted this whole file in favor of a different one (PR #16904). I removed `cursor: null` from the new file; `includeFixitCommands: null` was already removed.
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.

3 participants