#2430 remove app.relaunch on desktop update

[dklymenk]

Details

Removes unnecessary from update function that breaks notifications after update.

Fixed Issues

$ #2430

Tests

1. Follow the instructions on how to test electron updates, on the last step you should get an "update ready" notification.
2. Start update
3. After the app has restarted, open dev toools (cmd+option+i) and type new Notification('Test notification'); in console. You see a notification.
4. (Optional) Assuming you have account A open in desktop app after update, send a message to it from account B open in browser or mobile app. You also get a notification.

QA Steps

1. Log in to the desktop app with account A
2. Wait for the update to arrive
3. Update the desktop app
4. Right after app restart send a message from account B to account A
5. You should see a notification

Tested On

The change only affects desktop, no need to test anything else.

• Desktop

Screenshots

Desktop

2430-desktop.mp4

[chiragsalian]

LGTM

[OSBotify]

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

[OSBotify]

🚀 Deployed to staging in version: 1.0.82-8🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[roryabraham]

I am worried this caused a regression where the update app modal doesn't display after the update notification is clicked? Would love it someone could independently verify the bug. This PR seems like the most likely culprit, but I'm not sure.

[dklymenk]

I actually haven't seen any update modal during any of my tests. I don't believe there is any reason to assume this PR has caused it, because the change is to update callback only. The logic behind showing the update modal wasn't changed.

[chiragsalian]

Looks like this PR may not be the cause of the issue since we've been able to reproduce the issue on production while this code is only on staging.

[roryabraham]

Yep, my mistake this PR was not the cause.

[parasharrajat]

Coming here from the Update App modal issue.

App/desktop/main.js

Lines 213 to 224 in 3ec2137

	app.on('activate', () => {
	if (!expectedUpdateVersion || app.getVersion() === expectedUpdateVersion) {
	browserWindow.show();
	}
	});
	// Hide the app if we expected to upgrade to a new version but never did.
	if (expectedUpdateVersion && app.getVersion() !== expectedUpdateVersion) {
	browserWindow.hide();
	app.hide();
	}

was dependent on the code removed. I know that relaunching is the issue but Shouldn't something be added to keep the other functionality intact.

cc: @roryabraham @chiragsalian

[OSBotify]

🚀 Deployed to production by @francoisl in version: 1.0.83-1 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	failure ❌
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[roryabraham]

Yeah, it looks like a fair amount of code related to the expected update version flag was not removed along with the flag itself.

[dklymenk]

What exactly is that code's function? Is that something I should have reimplemented in some other way?

[parasharrajat]

Yes. That code determines the Window visibility after the update. (Casually saying 🐳) But It's shocking that you removed a piece of code without tracing its usages.