feat(app, app-shell): add download progress to robot update flows #13915
Conversation
If the robot update cache files are still downloading, no longer is an error thrown. Instead, the update modal renders and a downloading stage of the update flow begins. Additionally, release files are downloaded and returned to the browser layer separately from system files, so users may see release notes as soon as they are downloaded.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## edge #13915 +/- ##
=======================================
Coverage 70.66% 70.67%
=======================================
Files 2508 2508
Lines 70738 70835 +97
Branches 8688 8736 +48
=======================================
+ Hits 49985 50060 +75
- Misses 18634 18641 +7
- Partials 2119 2134 +15
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| if (checkingForUpdates) { | ||
| dispatch({ | ||
| type: 'robotUpdate:CHECKING_FOR_UPDATE', | ||
| payload: target, | ||
| }) | ||
| } else { |
There was a problem hiding this comment.
This condition is hit if a user actively starts a robot update, but the file has not yet been downloaded. The CHECKING_FOR_UPDATE action creates an update session for the robot with a downloadFile stage. The UI layer uses this session to alert the user that the file is downloading.
| return Promise.all([ | ||
| downloadAndNotify(true, urls.releaseNotes, directory, dispatch, target), | ||
| downloadAndNotify( | ||
| false, | ||
| urls.system, | ||
| directory, | ||
| dispatch, | ||
| target, | ||
| onProgress | ||
| ), | ||
| ]).then(([releaseNotes, system]) => ({ releaseNotes, system })) |
There was a problem hiding this comment.
If a user clicks an the robot update banner while the file is still downloading, there will not be release notes (as seen in the before vide). Let's separate the release notes from the system file download.
If a user somehow navigates to the robot update banner before 20kb have been downloaded, they won't see the notes, but this won't prevent the download.
| } | ||
| // This action will only have an effect if the user is actively waiting for the download to complete. | ||
| else { | ||
| return dispatch({ |
There was a problem hiding this comment.
This dispatch occurs after the system files are downloaded but not yet cleaned up. The redux store will update the robot's session from "waiting on download to finish" to done if the robot is of the same version as the file that finished downloading.
All the file cleanup can happen asynchronously without any problem.
There was a problem hiding this comment.
All the changes here are a refactor. I'll revisit this again in the future as it's still obtuse.
The guarded transitions can be safely removed now, since the app-shell guards the transitions safely.
| @@ -144,6 +145,19 @@ export const startUpdateEpic: Epic = (action$, state$) => | |||
| }) | |||
| ) | |||
|
|
|||
| export const startUpdateAfterFileDownload: Epic = (_, state$) => { | |||
There was a problem hiding this comment.
After the robot update file download completes, this pipe sends us back to the app-shell to kick off the installation process.
This time on READ_SYSTEM_FILE in app-shell, the filename will not be null. Note that the process isn't recursive (checkingForUpdates can never occur more than 1x unless the app is hard restarted. If the file doesn't exist the second time we get here, we'll throw the UNEXPECTED_ERROR.
|
|
||
| const { target, ...session } = state.session | ||
| const isThisRobotDownloadDone = | ||
| session?.step === Constants.DOWNLOAD_FILE && target === action.payload |
There was a problem hiding this comment.
This check prevents us initiating an update session when the user isn't actively engaged in the robot update flows, and it makes sure the correct system file update has completed (ex, the OT-2 file completing download won't trigger a Flex waiting for download to register as "done").
app-shell/src/http.ts
Outdated
|
|
||
| function handleSuccess(): void { | ||
| resolve(readStream) | ||
| readStream.removeListener('error', handleError) |
There was a problem hiding this comment.
this technically has the same pitfall that #13923 fixes on the odd side (errors from the read stream will be lost) but here I think those errors would only happen if the user deleted the cached file while the update was running
| @@ -105,7 +98,9 @@ export function RobotUpdateProgressModal({ | |||
| progressPercent | |||
| ) | |||
|
|
|||
| let modalBodyText = t('installing_update') | |||
| let modalBodyText = '' | |||
There was a problem hiding this comment.
unless we can prove exhaustion in the if cases after this I think we should give this a meaningful default value
let modalBodyText = t('downloading_update')
if (updateStep === 'install') modalBodyText = t('downloading_update')
or even const it and do a chained ternay assign, or have a function that takes the step and returns the text
…3915) * feat(app): add download progress to robot update flows If the robot update cache files are still downloading, no longer is an error thrown. Instead, the update modal renders and a downloading stage of the update flow begins. Additionally, release files are downloaded and returned to the browser layer separately from system files, so users may see release notes as soon as they are downloaded.
Closes RAUT-789, RAUT-827
Overview
If the robot update cache files are still downloading, no longer is an error thrown. Instead, the update modal renders and a downloading stage of the update flow begins. Additionally, release files are downloaded and returned to the browser layer separately from system files, so users may see release notes as soon as they are downloaded.
What's Changed
The substantive changes are in app-shell/src/robot-update/index & release-files and app/src/redux/robot-update/epic & reducer. I've left comments to help explain what's happening.
Before
before.mov
After
after.mov
Test Plan
Changelog
Review requests
Risk assessment
Medium - Getting the logic right is important, but it's relatively (keyword) straightforward as far as update flows are concerned...