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): Implement hasEverEnteredErrorRecovery #15876

Merged
merged 3 commits into from
Aug 2, 2024

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented Aug 2, 2024

Closes EXEC-636 and EXEC-637 and RQA-2900

Overview

Now that the run can tell us if we've entered Error Recovery, we can:

Show drop tip CTAs conditionally

Because ER always requires the users to go through drop tip flows, it's redundant and mildly annoying for users to see a second set of now irrelevant CTAs. Now, we only show the CTAs at the end of the run if the run did not enter error recovery (and the conditions for detecting potential tip attachment occur).

I earlier hacked something together to make this work on the desktop, but it wasn't possible to do this on the ODD based on how routing logic works. I've cleaned up the desktop to use the same logic that the ODD uses.

Use ODD recovery analytics

There was one analytic that was impossible to implement on the ODD without hasEverEnteredErrorRecovery (again, due to the way we handle routing), so that is added here, too.

Current Behavior

Screen.Recording.2024-08-02.at.11.36.31.AM.mov

New Behavior

Screen.Recording.2024-08-02.at.11.35.22.AM.mov

Test Plan and Hands on Testing

  • See videos

Changelog

  • End of run drop tip CTAs will not appear if the user handled tips during Error Recovery.

Risk assessment

low

@mjhuff mjhuff requested a review from a team August 2, 2024 16:23
@mjhuff mjhuff requested a review from a team as a code owner August 2, 2024 16:23
@mjhuff mjhuff requested review from ncdiehl11 and removed request for a team and ncdiehl11 August 2, 2024 16:23
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.

Nice, looks good to me!

@mjhuff mjhuff merged commit 446cb2e into edge Aug 2, 2024
20 checks passed
@mjhuff mjhuff deleted the app_show-dt-conditionally branch August 2, 2024 17:36
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