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

app(fix): Fix error recovery on desktop DQA #16916

Merged
merged 5 commits into from
Nov 20, 2024

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented Nov 20, 2024

Overview

As with the previous DQA tickets, the proof is linked in each ticket. There are two important fixes, the first two commits.

Commit 1 - RQA-3632 Step number doesn't translate 1-1 to 0 based indexing :)
Commit 2 - RQA-3642 The "meaty" one. QA hasn't seen alpha.3, so they haven't reported it yet, but there are times we accidentally show a generic "error" recovery modal after cancelling the run, and that's because we now gate recovery behind there being a failed command OR there being a stop requested run status. The problem is these data are derived from separate HTTP resources, so there can be torn state, in which we get the flicker behavior shown in the ticket video. The solution is just to reduce the complexity of the hook. We don't need intermediate state, hasSeenAwaitingRecovery, as this creates an extra render cycle that leads to the flickering.
Commit 3 - RQA-3629
Commit 4 - RQA-3627

Test Plan and Hands on Testing

  • See tickets. For commit 2, I did test a lot of permutations of ER.

Risk assessment

low

@mjhuff mjhuff requested a review from a team as a code owner November 20, 2024 20:25
@mjhuff mjhuff changed the title App fix desktop recovery dqa app(fix): Fix error recovery on desktop DQA Nov 20, 2024
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.

useLayoutEffect looks sick

@mjhuff mjhuff merged commit 6009a29 into chore_release-8.2.0 Nov 20, 2024
35 checks passed
@mjhuff mjhuff deleted the app_fix-desktop-recovery-dqa branch November 20, 2024 22:29
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