[HOLD for Payment [2024-05-16][$250] Feature Request: Desktop app: Clicking on update button shouldn't lead to re download it

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Problem:

on the Mac Desktop app, have this "update" button update the current app directly, instead of leading you to re-download it.

Solution:

User should be able to update without downloading again

Context/Examples/Screenshots/Notes:

Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @puneetlath
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1712096487518269

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c264c5555e17f971
• Upwork Job ID: 1777186397013553152
• Last Price Increase: 2024-04-08
• Automatic offers:
• DylanDylann | Reviewer | 0
• gijoe0295 | Contributor | 0

[melvin-bot]

Triggered auto assignment to @Christinadobrzyn (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[melvin-bot]

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

[gijoe0295]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Feature Request: Desktop app: Clicking on update button shouldn't lead to re download it

What is the root cause of that problem?

Currently we only open desktop app download link:

App/src/libs/actions/AppUpdate/updateApp/index.desktop.ts

Line 5 in 207385a

Linking.openURL(CONST.APP_DOWNLOAD_LINKS.DESKTOP);

What changes do you think we should make in order to solve the problem?

We should replace the above by:

App/src/components/UpdateAppModal/index.desktop.tsx

Line 9 in 67aadf2

window.electron.send(ELECTRON_EVENTS.START_UPDATE);

We already handle the update event in electron:

App/desktop/main.ts

Line 211 in d22957d

ipcMain.on(ELECTRON_EVENTS.START_UPDATE, quitAndInstallWithUpdate);

By this, the app would restart to install the update automatically.

We also need to update the prompt text accordingly.

What alternative solutions did you explore? (Optional)

NA

[Christinadobrzyn]

I think this can be external

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01c264c5555e17f971

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @DylanDylann (External)

[melvin-bot]

📣 @ordinall! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[ordinall]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01e2212b454027f46b

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[DylanDylann]

@gijoe0295 Your proposal looks promising, but how you can verify that it works well? When testing your proposal, I see that quitAndInstallWithUpdate callback isn't triggered when we click Update button

[gijoe0295]

Updated proposal

Please re-state the problem that we are trying to solve in this issue.

Feature Request: Desktop app: Clicking on update button shouldn't lead to re download it

What is the root cause of that problem?

Currently we only open desktop app download link:

App/src/libs/actions/AppUpdate/updateApp/index.desktop.ts

Line 5 in 207385a

Linking.openURL(CONST.APP_DOWNLOAD_LINKS.DESKTOP);

What changes do you think we should make in order to solve the problem?

electron-updater's autoUpdater works like this:

• autoUpdater.checkForUpdates() checks if there's any update available and download it here.
• Once the download is done, the UPDATE_DOWNLOADED (an electron-updater built-in event) is fired which triggers the callback here:
  • If the app window is visible, we notify user there're an update available and if they press the notification to allow install, that triggers the START_UPDATE event to call quitAndInstallUpdate here.

We need to create a custom event for this and handle it appropriately following the instruction:

App/desktop/README.md

Lines 39 to 44 in d22957d

	3. The context bridge
	- Implemented in https://github.com/Expensify/App/blob/main/desktop/contextBridge.ts
	- The context bridge enables communication between the main and renderer processes. For example, if the renderer process needs to make use of a Node.js or Electron API it must:
	1. Define an event in https://github.com/Expensify/App/blob/main/desktop/ELECTRON_EVENTS.ts
	2. Add that event to the whitelist defined in the context bridge
	3. Set up a handler for the event in the main process that can respond to the renderer process back through the bridge, if necessary.

1. Create a new channel/event in contextBridge, for example MANUALLY_CHECK_FOR_UPDATE.
2. Trigger the event in updateApp/index.desktop.ts:

window.electron.send(ELECTRON_EVENTS.MANUALLY_CHECK_FOR_UPDATE);

3. Modify manuallyCheckForUpdates a bit because it was initially used as a callback for toolbar's menu item. We can handle this in the PR. Then bind it to the MANUALLY_CHECK_FOR_UPDATE event in desktop/main.ts:

App/desktop/main.ts

Line 131 in d22957d

const manuallyCheckForUpdates = (menuItem: MenuItem, browserWindow?: BrowserWindow) => {

ipcMain.on(ELECTRON_EVENTS.MANUALLY_CHECK_FOR_UPDATE, () => {
    manuallyCheckForUpdates(undefined, browserWindow);
});

4. As explained above, the UPDATE_DOWNLOADED event handler had a logic to notify user when downloading is done here, that requires user interaction to actually install the update, but I don't think we need it in this case because user already pressed Update in the modal.
   • Create a variable to indicate whether we want silently update isSilentUpdate and set it to true in MANUALLY_CHECK_FOR_UPDATE handler.
   • Then modify the UPDATE_DOWNLOADED event handler to call quitAndInstallWithUpdate immediately if isSilentUpdate.

By this, when user press Update in the modal, we check if there's available update and show a dialog to user, then download and install the update silently.

We also need to update the prompt text accordingly.

What alternative solutions did you explore? (Optional)

NA

[gijoe0295]

@DylanDylann I updated my proposal. We can follow this instruction to test this functionality locally but that's quite complicated and we could do it in implementation phase. For now we can hard-code some places to make it work.

[DylanDylann]

@gijoe0295 After testing I see that ipcMain.on(ELECTRON_EVENTS.MANUALLY_CHECK_FOR_UPDATE, ....) isn't triggered after we call window.electron.send(ELECTRON_EVENTS.MANUALLY_CHECK_FOR_UPDATE);

[gijoe0295]

@DylanDylann Note that console.log does not work in Electron runtime. You can use dialog.showMessageBox instead.

App/desktop/main.ts

Line 148 in 468ee2f

dialog.showMessageBox(browserWindow, {

[DylanDylann]

@gijoe0295 Ohh, cool. It works for me.

[DylanDylann]

@gijoe0295's proposal looks good to me

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @Beamanator, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

📣 @DylanDylann 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @gijoe0295 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[DylanDylann]

@gijoe0295 Any update here?

[DylanDylann]

@gijoe0295 Please prioritize this PR instead of proposing new issues

[gijoe0295]

@DylanDylann I do not propose any new 😄 I'm actively working on it as you could see in PR commit history. The code change and testing are good on my side but I'm looking for a way for you and QA team to test easily. I need to purchase a paid Apple Developer account for that.

[DylanDylann]

You proposed 2 hours ago #40316 (comment)

[Christinadobrzyn]

Hi @DylanDylann could you provide an updated when you have a moment! Thank you!

[DylanDylann]

Waiting for reviewing from @Beamanator

[Beamanator]

sorry for the delay, just merged!

[Christinadobrzyn]

monitoring PR - #40253

[Christinadobrzyn]

getting ready for payment...

Payouts due:

• Contributor: $500 @gijoe0295 (Upwork offer - https://www.upwork.com/nx/wm/offer/101808656)
• Contributor+: $500 @DylanDylann (Upwork offer - https://www.upwork.com/nx/wm/offer/101808653)

Upwork job is here.

Do we need a regression test for this?

[gijoe0295]

@DylanDylann Could you check ^?

[DylanDylann]

Regression Test Proposal

Precondition: The desktop app's outdated version and there is new update version

1. Open App
2. Press Update
3. Verify a dialog shows Update Available
4. Press Sounds good
5. Verify after a while, app is restarted
6. Press New Expensify menu >> About New Expensify
7. Verify the version is the latest production build

Do we agree 👍 or 👎

[Christinadobrzyn]

Created regression test - https://github.com/Expensify/Expensify/issues/396639

Paid out based on this payment summary - #39526 (comment)

I think we're good to close!

[DylanDylann]

@Christinadobrzyn Apology for catching up late here, this issue was created from April 3 (prior to the base price change), as such according to this announcement, it should remain as $500. So this payment summary needs adjustment.

Could you help to handle it (maybe as a bonus to the existing job), thanks!

[Christinadobrzyn]

HI @DylanDylann so sorry I didn't see this comment.

Thanks, @gijoe0295, for reaching out! I just added a $250 bonus to each of the Upwork job payments to account for the $500 price. Let me know if there's anything else!

[DylanDylann]

@Christinadobrzyn No problem at all. Thanks for stepping in!