Improve Push message when quitting

[epeicher]

Related issues

• Related to STU-751

Proposed Changes

• Changes ACTIVE_SYNC_OPERATIONS from a Set to a Map
• Adds the Push states to the ACTIVE_SYNC_OPERATIONS map
• When the user quits the application, checks the state of the Sync operation, if it is a Push and the operation is running server side, displays the following message: There's a sync operation in progress. The process will continue on WordPress.com servers even after quitting Studio. We will send you an email when it completes. Are you sure you want to quit?

Local operations	Remote operations

Note

The alignment of the dialog contents to the left is happening in trunk and it might seem related to the Electron version or the OS design changes, but it is not related to this PR

Testing Instructions

• Apply this branch and run npm start
• On the Sync tab, connect to a WP.com site
• Select the Push operation, select all checkboxes and click on Push
• Press Cmd+Q to quit Studio (or use the main Electron menu)
• Check that if the user is in Creating backup… state, the modal displays the previous message There's a sync operation in progress. Quitting the app will abort that operation. Are you sure you want to quit?
• Wait for the next states, from Uploading Studio site… onwards
• Press Cmd+Q to quit Studio (or use the main Electron menu)
• Check that the message is now different: There's a sync operation in progress. The process will continue on WordPress.com servers even after quitting Studio. We will send you an email when it completes. Are you sure you want to quit?

Pre-merge Checklist

• Have you checked for TypeScript, React or other console errors?

[github-actions]

📊 Performance Test Results

Comparing bec7259 vs trunk

site-editor

Metric	trunk	bec7259	Diff	Change
load	7873.00 ms	9755.00 ms	+1882.00 ms	🔴 23.9%

site-startup

Metric	trunk	bec7259	Diff	Change
siteCreation	10951.00 ms	11973.00 ms	+1022.00 ms	🔴 9.3%
siteStartup	3939.00 ms	3934.00 ms	-5.00 ms	🟢 -0.1%

---

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change

[bcotrim]

Recently we merged another PR #1864 related to cancelling, push and pull operations.
We added similar logic to determine if the operation can be cancelled: https://github.com/Automattic/studio/blob/trunk/src/hooks/use-sync-states-progress-info.ts#L288

I think we could extract this logic to a util function.

[epeicher]

Thanks @bcotrim, I saw that after updating this PR. I will definitely have a look at the changes as they are related

[epeicher]

@bcotrim, I have extracted the logic to a shared library as part of 478ccaf, please let me know your view about it

[bcotrim]

LGTM, thanks!

[sejas]

Great work @epeicher ! I really like the two different modals that set better expectations of what is the consequence of closing the app when uploading or when a restore is already in process.

I wonder if we should consider the edge case when the user tries to close the app just before the restore process starts, so the alert will display that will abort the process, but after a few seconds reading the modal, the upload will finish and the state will change to restore.

[epeicher]

I wonder if we should consider the edge case when the user tries to close the app just before the restore process starts, so the alert will display that will abort the process, but after a few seconds reading the modal, the upload will finish and the state will change to restore.

That's a good point, I have tested, and it can happen. When the dialog is displayed, the UI is not refreshed (new messages are not displayed), but the states continue to progress in the background. That could be addressed with another message, but it will be a "double dialog" scenario: one dialog appears when closing the first. We can adapt the text in that case to be something like "Sorry, the process has progressed to a non-cancellable step" or similar, but those double dialogs are a bit annoying. What do you think?

[bcotrim]

@sejas @epeicher that's a nice catch and a likely scenario.

We might be going into a rabbit hole, but we could add a paused status for the sync operations. When we display the warning, the operation is paused (if not in a remote operation state) and can be either resumed or cancelled.
What do you think?

[epeicher]

we could add a paused status for the sync operations. When we display the warning, the operation is paused (if not in a remote operation state) and can be either resumed or cancelled.
What do you think?

That's a great idea @bcotrim! I will work on this as I also see the 👍 of @sejas

[sejas]

The pause logic seems like the more solid implementation. Let me know if it becomes a rabbit hole.

[epeicher]

HI @sejas, @bcotrim, after some iterations trying to implement the Pause (you can see the reverted changes in 8fd150c) I found some issues with the UI being updated when sending the IPC event to the renderer (sendIpcEventToRenderer) and I have realized that the renderer process is frozen when the Dialog is displayed, so the steps are not progressing in the background as I assumed, as the renderer process executes them. So what I have done is to reduce the operations that are cancelable to only creatingBackup step in line with what the Cancel button does (it is actually coherent 😄 ).

So in summary, if the user opens the dialog, the renderer process will not progress to the next Uploading Studio site… step.

Could you please test the changes again and let me know if you find any issues? 🙏

[bcotrim]

LGTM 👍

I retested and confirm what you described. From what I could tell the operation is paused until the alert is closed.

[sejas]

Thanks for applying the changes 👍

[sejas]

• While testing #1943 , I noticed that the error message and the cancel button display that the push process cannot be cancelled even when the upload is in progress. Is that correct?

[epeicher]

• While testing Push: Check existing progress of push on start #1943 , I noticed that the error message and the cancel button display that the push process cannot be cancelled even when the upload is in progress. Is that correct?

That's a tricky scenario as it depends on a request that is in flight. That will depend on the time the cancel button is pressed or Studio is closed. Currrently, the cancel button does not have an AbortController for that request so I guess it was conservative in not allowing the user to cancel at that point (originally implemented here #1864).

In the case of the dialog message, I decided to be consistent with the cancel button, but I am happy to investigate other alternatives. One crucial detail is that a request in flight can be aborted, but I don't think it can be paused, so when the user sees the dialog, if it takes some time to click the button, the request could complete during that time and the process continues remotely. If alternatively, the user presses the quit button in the dialog before the request completes, the active request is aborted and the process stops.

[sentry]

Issues attributed to commits in this pull request

This pull request was merged and Sentry observed the following issues:

• ‼️ Error: No suitable backup handler found for the pr... in production