Skip to content

fix(app): fix applying offsets implicitly when navigating on the desktop app#17967

Merged
mjhuff merged 1 commit into
chore_release-8.4.0from
app_fix-lpc-client-store
Apr 3, 2025
Merged

fix(app): fix applying offsets implicitly when navigating on the desktop app#17967
mjhuff merged 1 commit into
chore_release-8.4.0from
app_fix-lpc-client-store

Conversation

@mjhuff
Copy link
Copy Markdown
Contributor

@mjhuff mjhuff commented Apr 2, 2025

Closes RQA-4032

Overview

Woops, the clientData effect for applying offsets for apps that did not do LPC had some logic issues. It was possible to navigate back and forth on the desktop app quickly and cause offsets to automatically apply, because some of the state used in the effect hadn't had the chance to properly initialize.

To fix, let's not update the client store as an effect. Since only one user can actually click the apply offsets button, that user should do the client store update on the CTA itself. This simplies the effect a bit and everything works.

Current Behavior

Screen.Recording.2025-04-02.at.1.11.57.PM.mov

Fixed Behavior

Screen.Recording.2025-04-02.at.12.57.17.PM.mov

Test Plan and Hands on Testing

  • See videos.
  • Ran the exact test case in the ticket (same protocol, etc), and the issue no longer occurs.

Changelog

  • Fixed offsets sometimes getting automatically applied on the desktop app when navigating back to a run.

Risk assessment

low

@mjhuff mjhuff requested review from a team, SyntaxColoring and sfoster1 April 2, 2025 17:49
@mjhuff mjhuff requested a review from a team as a code owner April 2, 2025 17:49
@mjhuff mjhuff requested review from smb2268 and removed request for a team and smb2268 April 2, 2025 17:49
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 2, 2025

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 27.91%. Comparing base (7afe1f1) to head (f135104).
Report is 3 commits behind head on chore_release-8.4.0.

Files with missing lines Patch % Lines
...s/Desktop/Devices/ProtocolRun/ProtocolRunSetup.tsx 0.00% 2 Missing ⚠️
...ck/LPCFlows/hooks/useHandleClientAppliedOffsets.ts 0.00% 2 Missing ⚠️
...lSetup/ProtocolSetupOffsets/SetupOffsetsHeader.tsx 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                   Coverage Diff                   @@
##           chore_release-8.4.0   #17967      +/-   ##
=======================================================
+ Coverage                27.65%   27.91%   +0.25%     
=======================================================
  Files                     3113     3113              
  Lines                   236796   236797       +1     
  Branches                 19220    19224       +4     
=======================================================
+ Hits                     65482    66095     +613     
+ Misses                  171296   170679     -617     
- Partials                    18       23       +5     
Flag Coverage Δ
app 2.32% <0.00%> (+0.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...s/Desktop/Devices/ProtocolRun/ProtocolRunSetup.tsx 0.00% <0.00%> (ø)
...ck/LPCFlows/hooks/useHandleClientAppliedOffsets.ts 0.00% <0.00%> (ø)
...lSetup/ProtocolSetupOffsets/SetupOffsetsHeader.tsx 0.00% <0.00%> (ø)

... and 15 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mjhuff mjhuff force-pushed the app_fix-lpc-client-store branch from f135104 to 78a1d50 Compare April 2, 2025 17:55
Copy link
Copy Markdown
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.

Looks good to me!

@mjhuff mjhuff merged commit eab3d0c into chore_release-8.4.0 Apr 3, 2025
30 checks passed
@mjhuff mjhuff deleted the app_fix-lpc-client-store branch April 3, 2025 19:55
y3rsh added a commit that referenced this pull request Apr 9, 2025
…y-path

* chore_release-8.4.0: (30 commits)
  fix(api): make error message clearer for lld issues (#18005)
  chore(locize): sync for translations needed (#18009)
  chore(locize): sync translations (#18009)
  docs(api): API reference entries for liquid class methods (#17887)
  refactor(api): change the names of liquid classes based transfers (#18006)
  feat(app, labware-library): add evotip definition assets (#18007)
  fix(api, shared-data): Flex Stacker engine command optional fields (#17989)
  fix(shared-data): ethanol aspirate position reference (#17991)
  fix(app): Labware setup UI fixes (#17987)
  refactor(app): adjust protocol setup offsets table header (#17985)
  fix(app): do not show post run drop tip prompt if just handled in Error Recovery (#17981)
  fix(app): fix LPC disabled reasons not including fixture mismatch (#17979)
  refactor(app): adjust width on "calibrate now" button (#17978)
  fix(app): fix applying offsets implicitly when navigating on the desktop app (#17967)
  feat(robot-server): Populate `locationSequence` on old runs and make it faster to filter out deleted offsets (#17946)
  feat(app): add inline notification when setting default offsets with a 96ch (#17977)
  fix(app): ER tip selection crashes when trying to get labware def (#17975)
  feat(robot-server,system-server): Return server timing metrics in HTTP responses (#17970)
  fix(app): fix accumulating offsets on run record (#17969)
  fix(app): Fix local state issues when "resetting to default" in LPC (#17965)
  ...
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