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(robot-server, app): Fix ODD routing back to previous screen #16021

Merged
merged 6 commits into from
Aug 15, 2024

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented Aug 15, 2024

Closes RQA-3036 and RQA-3038

Overview

The recent react-router update exposed an existing bug: MQTT did not properly publish a refetch to runs/:runId after a client sends a PATCH to runs/:runId. Although the exact reasoning concerning why the migration from history.push to navigate exposes this problem is unknown, this PR provides the necessary publish event to the app's top level redirection logic and prevents redirection back to the previous route. Note that this does mean we have to account for noticible client latency now, so this PR also adds loading state when "return to dashboard" is clicked.

Screen.Recording.2024-08-15.at.1.22.57.PM.mov

Test Plan and Hands on Testing

  • Verified clicking "Run again" on the ODD routes the user to the correct location with no unexpected redirection elsewhere.
  • Verified MQTT now sends a publish update after the app PATCHes run/:runId.

Changelog

  • Fixed the ODD routing back to the previous screen after clicking buttons.

Risk assessment

low

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.

very nice! LGTM!

@mjhuff mjhuff merged commit f1681f0 into chore_release-8.0.0 Aug 15, 2024
15 checks passed
@mjhuff mjhuff deleted the robot-server_notify-on-runid-update branch August 15, 2024 18: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