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): No longer dismiss run if canceled while running on ODD #16142

Merged
merged 2 commits into from
Aug 27, 2024

Conversation

smb2268
Copy link
Contributor

@smb2268 smb2268 commented Aug 27, 2024

fix RQA-3100, RQA-3098

Overview

This PR changes the behavior of a run cancellation initiated on ODD. Previous behavior stopped the run, dismissed (or un-currented) the run, and then if the run was not yet started we'd manually redirect back to the protocols or quick transfer page. Now, we'll only dismiss the run if it has not yet started. This will fix the drop tip wizard not opening as expected after run cancellation.

Test Plan and Hands on Testing

  1. From ODD, cancel a run from the run setup screen. Ensure behavior has not changed and you are still redirected back to the protocol or quick transfer details page.
  2. From ODD, cancel a run that is in progress and has a tip attached. See that the drop tip wizard opens if you press "return to dashboard". When viewing the same run from the desktop app, verify that the drop tip wizard does not close after a few seconds without any user intervention

While testing this, you may run into https://opentrons.atlassian.net/browse/RQA-3109 which is not fixed in this PR. If this is too difficult to test because of the hanging cancellation modal, we can wait to test and merge until that ticket is resolved.

Changelog

  1. Moved dismissCurrentRun call in ConfirmCancelRunModal so that it is only called if a run is not active
  2. Moved the post dismissal navigation logic to the onSettled callback of useDismissCurrentRunMutation
  3. Removed some unnecessary duplicate logic in the returnToQuickTransfer function - closeCurrentRunIfValid handles the current run functionality for us

Review requests

Look over code changes

Risk assessment

Medium - we do not think this will have any side effects but it is possible that there are places in the app where we expect a run to be dismissed after ODD cancellation

@smb2268 smb2268 requested review from sfoster1 and mjhuff August 27, 2024 18:29
@smb2268 smb2268 self-assigned this Aug 27, 2024
@smb2268 smb2268 requested a review from a team as a code owner August 27, 2024 18:29
@smb2268 smb2268 requested a review from brenthagen August 27, 2024 18:29
@@ -55,9 +55,17 @@ export function ConfirmCancelRunModal({
dismissCurrentRun,
isLoading: isDismissing,
} = useDismissCurrentRunMutation({
onSuccess: () => {
if (isQuickTransfer && !isActiveRun) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This mutation will now only be called if !isActiveRun is true

@@ -251,15 +251,10 @@ export function RunSummary(): JSX.Element {
}, [isRunCurrent, enteredER])

const returnToQuickTransfer = (): void => {
if (!isRunCurrent) {
closeCurrentRunIfValid(() => {
Copy link
Contributor Author

@smb2268 smb2268 Aug 27, 2024

Choose a reason for hiding this comment

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

closeCurrentRunIfValid checks for !isRunCurrent and calls the passed in callback regardless

@smb2268 smb2268 requested a review from shlokamin August 27, 2024 18:38
Copy link
Contributor

@mjhuff mjhuff left a comment

Choose a reason for hiding this comment

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

Wooooooooooo!

@smb2268 smb2268 merged commit f74aa8a into chore_release-8.0.0 Aug 27, 2024
20 checks passed
@smb2268 smb2268 deleted the app_run-cancel-testing branch August 27, 2024 18:48
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