feature: silently update desktop app #40253
Conversation
gijoe0295
changed the title
melvin-bot
bot
requested review from
DylanDylann
and removed request for
a team
|
@DylanDylann Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
@Beamanator Can you add Build label here? @DylanDylann Hold up reviewing, let me thoroughly test on the adhoc build first, I'll notify you once it's ready. |
|
Added label & kicked off build 👍 - https://github.com/Expensify/App/actions/runs/8717438546 |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
…at-silently-update-on-desktop
|
Adhoc builds do not support auto updater so we need to set up local our own release channel locally as instructed here. I'm having problems with signing certificate with my apple developer account, I already contact them about this. Will notify when it's ready for testing. |
|
Hi @Beamanator, I had problem testing the auto updating feature. The Adhoc build, by all means, couldn't be used for that purpose. I also followed the local test instruction here but struggled with the Apple Developer program enrollment. I already paid for it twice (through app and web) but still couldn't use it for unknown reason. I filed a support ticket to Apple but hasn't been replied. Additionally, I spotted many developers had the same problem but couldn't resolve for months. So it's definitely Apple's issue. Thus I want to quick bump you here to see whether either:
Hope to get your support, thanks! |
There was a problem hiding this comment.
Overall this is looking good, just a few tiny changes requested.
As for testing the actual functionality, you're right this can be pretty hard to test & sorry that has been very difficult for you to get the apple developer credentials...
This is a difficult process so it may be enough to just test:
- The local app runs fine (
npm run desktop) - The staging build runs fine locally (even without signing the build -
npm run desktop-build-staging) - Make sure there's no CORS errors, logging in is fine, performance seems fine, etc
…b.com/gijoe0295/App into gijoe/feat-silently-update-on-desktop
|
cc @DylanDylann on #40253 (review). I verified all the above requirements are met. I think for this case we only need to verify if pressing Note that we can use Also note that in order to build desktop, you need to omit the |
|
@DylanDylann So sorry for this inconvenience again, Apple finally activated my account so I think I have to do the testing. Please hold up your review, I'll complete it today. |
Ooh these are pretty interesting suggestions, would you mind bringing these up in #expensify-open-source so we can hopefully make that script better? I'm honestly not super well versed with that script so it could be great to start a convo in slack so more people can see your suggestions 🙏 |
@Beamanator Hmm I do not quite understand your points. The log output is the current behavior already pointed out in |
…at-silently-update-on-desktop
|
@gijoe0295 Any update here? |
|
Before I test this solution by using cc @Beamanator |
|
@DylanDylann I am stuck on Apple notarization API call for like 3 hours: electron-notarize:spawn spawning cmd: xcrun args: [
'notarytool',
'submit',
'/var/folders/vw/p4pxkyc56r7cx6qwnz15240r0000gn/T/electron-notarize-B6qu5k/New Expensify.zip',
'--apple-id',
'*********',
'--password',
'*********',
'--team-id',
'*********',
'--wait',
'--output-format',
'json'
] opts: {} +0msYesterday and today morning, they had an outage for |
…at-silently-update-on-desktop
|
@gijoe0295 I think we should use a tricky way to test this PR on dev ( using dialog.showMessageBox to see log) and leave the rest to the QA team after this PR is deployed @Beamanator What do you think about this way? |
|
@DylanDylann To be honest i don't know how that works, but feel free to try it!! |
…at-silently-update-on-desktop
|
@DylanDylann If that's the case, I think we're good for testing and reviewing. I updated test steps and screenshots accordingly. |
desktop/main.ts
Outdated
| /** Menu Item callback to triggers an update check */ | ||
| const manuallyCheckForUpdates = (menuItem: MenuItem, browserWindow?: BrowserWindow) => { | ||
| /** Menu Item callback to trigger an update check */ | ||
| const manuallyCheckForUpdates = (menuItem?: MenuItem, browserWindow?: BrowserWindow) => { | ||
| // Disable item until the check (and download) is complete |
There was a problem hiding this comment.
Move this line to the below code block
There was a problem hiding this comment.
@gijoe0295 Small change, please address it
| @@ -201,7 +210,7 @@ const electronUpdater = (browserWindow: BrowserWindow): PlatformSpecificUpdater | |||
| if (checkForUpdatesMenuItem) { | |||
| checkForUpdatesMenuItem.visible = false; | |||
| } | |||
| if (browserWindow.isVisible()) { | |||
| if (browserWindow.isVisible() && !isSilentUpdating) { | |||
There was a problem hiding this comment.
@gijoe0295 I still think that we should show a notification to the user after updating version successfully
cc @Beamanator
There was a problem hiding this comment.
Hmm well this is why we're calling it "silent" updating I guess haha. At this point, if the user already clicked "Update" I'm not sure this would require another notification to the user -> Like on Web, we would just refresh & BAM it's updated, no extra notification 🤔
|
@Beamanator Testing well Screen.Recording.2024-04-30.at.22.59.01.movI change somethings to test this PR manually diff --git a/desktop/main.ts b/desktop/main.ts
index be3d8722fa..85506eed3a 100644
--- a/desktop/main.ts
+++ b/desktop/main.ts
@@ -139,10 +139,12 @@ const manuallyCheckForUpdates = (menuItem?: MenuItem, browserWindow?: BrowserWin
autoUpdater
.checkForUpdates()
.catch((error) => {
+ dialog.showMessageBox({message: "error"});
isSilentUpdating = false;
return {error};
})
.then((result) => {
+ dialog.showMessageBox({message: "successful"});
const downloadPromise = result && 'downloadPromise' in result ? result.downloadPromise : undefined;
if (!browserWindow) {
@@ -615,6 +617,7 @@ const mainWindow = (): Promise<void> => {
// Automatically check for and install the latest version in the background
ipcMain.on(ELECTRON_EVENTS.SILENT_UPDATE, () => {
+ dialog.showMessageBox({message: "trigger SLIENT UPDATE event"});
if (isSilentUpdating) {
return;
}
diff --git a/src/App.tsx b/src/App.tsx
index 6316fa80fb..27ebfb08b1 100644
--- a/src/App.tsx
+++ b/src/App.tsx
@@ -54,6 +54,10 @@ const fill = {flex: 1};
function App({url}: AppProps) {
useDefaultDragAndDrop();
OnyxUpdateManager();
+ const ErrorComponent = () => {
:...skipping...
diff --git a/desktop/main.ts b/desktop/main.ts
index be3d8722fa..85506eed3a 100644
--- a/desktop/main.ts
+++ b/desktop/main.ts
@@ -139,10 +139,12 @@ const manuallyCheckForUpdates = (menuItem?: MenuItem, browserWindow?: BrowserWin
autoUpdater
.checkForUpdates()
.catch((error) => {
+ dialog.showMessageBox({message: "error"});
isSilentUpdating = false;
return {error};
})
.then((result) => {
+ dialog.showMessageBox({message: "successful"});
const downloadPromise = result && 'downloadPromise' in result ? result.downloadPromise : undefined;
if (!browserWindow) {
@@ -615,6 +617,7 @@ const mainWindow = (): Promise<void> => {
// Automatically check for and install the latest version in the background
ipcMain.on(ELECTRON_EVENTS.SILENT_UPDATE, () => {
+ dialog.showMessageBox({message: "trigger SLIENT UPDATE event"});
if (isSilentUpdating) {
return;
}
diff --git a/src/App.tsx b/src/App.tsx
index 6316fa80fb..27ebfb08b1 100644
--- a/src/App.tsx
+++ b/src/App.tsx
@@ -54,6 +54,10 @@ const fill = {flex: 1};
function App({url}: AppProps) {
useDefaultDragAndDrop();
OnyxUpdateManager();
+ const ErrorComponent = () => {
+ throw new Error('Error in ErrorComponent');
+ // This will throw an error when the component renders
+ };
return (
<InitialURLContextProvider url={url}>
<GestureHandlerRootView style={fill}>
@@ -88,9 +92,12 @@ function App({url}: AppProps) {
>
<CustomStatusBarAndBackground />
<ErrorBoundary errorMessage="NewExpensify crash caught by error boundary">
- <ColorSchemeWrapper>
+ {/* <ColorSchemeWrapper>
<Expensify />
</ColorSchemeWrapper>
+ */}
+
+ <ErrorComponent />
</ErrorBoundary>
</ComposeProviders>
</GestureHandlerRootView>
:...skipping...
diff --git a/desktop/main.ts b/desktop/main.ts
index be3d8722fa..85506eed3a 100644
--- a/desktop/main.ts
+++ b/desktop/main.ts
@@ -139,10 +139,12 @@ const manuallyCheckForUpdates = (menuItem?: MenuItem, browserWindow?: BrowserWin
autoUpdater
.checkForUpdates()
.catch((error) => {
+ dialog.showMessageBox({message: "error"});
isSilentUpdating = false;
return {error};
})
.then((result) => {
+ dialog.showMessageBox({message: "successful"});
const downloadPromise = result && 'downloadPromise' in result ? result.downloadPromise : undefined;
if (!browserWindow) {
@@ -615,6 +617,7 @@ const mainWindow = (): Promise<void> => {
// Automatically check for and install the latest version in the background
ipcMain.on(ELECTRON_EVENTS.SILENT_UPDATE, () => {
+ dialog.showMessageBox({message: "trigger SLIENT UPDATE event"});
if (isSilentUpdating) {
return;
}
diff --git a/src/App.tsx b/src/App.tsx
index 6316fa80fb..27ebfb08b1 100644
--- a/src/App.tsx
+++ b/src/App.tsx
@@ -54,6 +54,10 @@ const fill = {flex: 1};
function App({url}: AppProps) {
useDefaultDragAndDrop();
OnyxUpdateManager();
+ const ErrorComponent = () => {
+ throw new Error('Error in ErrorComponent');
+ // This will throw an error when the component renders
+ };
return (
<InitialURLContextProvider url={url}>
<GestureHandlerRootView style={fill}>
@@ -88,9 +92,12 @@ function App({url}: AppProps) {
>
<CustomStatusBarAndBackground />
<ErrorBoundary errorMessage="NewExpensify crash caught by error boundary">
- <ColorSchemeWrapper>
+ {/* <ColorSchemeWrapper>
<Expensify />
</ColorSchemeWrapper>
+ */}
+
+ <ErrorComponent />
</ErrorBoundary>
</ComposeProviders>
</GestureHandlerRootView>
diff --git a/src/components/ErrorBoundary/BaseErrorBoundary.tsx b/src/components/ErrorBoundary/BaseErrorBoundary.tsx
index 75f65a06a2..3a36f91822 100644
--- a/src/components/ErrorBoundary/BaseErrorBoundary.tsx
+++ b/src/components/ErrorBoundary/BaseErrorBoundary.tsx
@@ -24,7 +24,7 @@ function BaseErrorBoundary({logError = () => {}, errorMessage, children}: BaseEr
return (
<ErrorBoundary
- fallback={updateRequired ? <UpdateRequiredView /> : <GenericErrorPage />}
+ fallback={true ? <UpdateRequiredView /> : <GenericErrorPage />}
onError={catchError}
>
{children}
diff --git a/src/libs/actions/AppUpdate/updateApp/index.desktop.ts b/src/libs/actions/AppUpdate/updateApp/index.desktop.ts
index 5c1ecbe057..873577b03a 100644
--- a/src/libs/actions/AppUpdate/updateApp/index.desktop.ts
+++ b/src/libs/actions/AppUpdate/updateApp/index.desktop.ts
@@ -1,5 +1,7 @@
import ELECTRON_EVENTS from '@desktop/ELECTRON_EVENTS';
export default function updateApp() {
+ console.log("Start Updating")
+
window.electron.send(ELECTRON_EVENTS.SILENT_UPDATE);
}
:
|
Reviewer Checklist
Only need to test on Desktop App Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / SafariMacOS: DesktopScreen.Recording.2024-04-30.at.22.59.01.mov |
…at-silently-update-on-desktop
|
@Beamanator Bump, could you help to take a look at this PR? |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
@b1tjoy @Beamanator Could someone internally verify this PR? Unfortunately, we don't have anyone with the Desktop app's version 1.4.57-5 or below. |
|
🚀 Deployed to production by https://github.com/marcaaron in version: 1.4.71-6 🚀
|
Details
Press
Updatein Update required modal should automatically check and install the latest update in the background.Fixed Issues
$ #39526
PROPOSAL: #39526 (comment)
Tests
package.json'sversionto1.4.57-5(the minimum required version since the group chat releases)build-desktop.sh, omit the--publish alwaysparam (to disable Apple's notarization process)npm run desktop-buildOffline tests
NA
QA Steps
Precondition: Desktop app's version is
1.4.57-5or below.PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2024-04-26.at.19.59.01-source.mov
Android: mWeb Chrome
Screen.Recording.2024-04-26.at.19.52.31-source.mov
iOS: Native
Screen.Recording.2024-04-26.at.20.06.22-source.mov
iOS: mWeb Safari
Screen.Recording.2024-04-26.at.19.43.30-source.mov
MacOS: Chrome / Safari
Screen.Recording.2024-04-26.at.19.40.43-source.mov
MacOS: Desktop
Screen.Recording.2024-04-26.at.19.36.55-source.mov