Desktop: Seamless-Updates - fixing bugs #11121
Conversation
| autoUpdateEnabled: { value: true, type: SettingItemType.Bool, storage: SettingStorage.File, isGlobal: true, section: 'application', public: false, appTypes: [AppType.Desktop], label: () => _('Automatically check for updates') }, | ||
| 'autoUpdate.includePreReleases': { value: false, type: SettingItemType.Bool, section: 'application', storage: SettingStorage.File, isGlobal: true, public: true, appTypes: [AppType.Desktop], label: () => _('Get pre-releases when checking for updates'), description: () => _('See the pre-release page for more details: %s. Restart app (quit app from system tray) to start getting them', 'https://joplinapp.org/help/about/prereleases') }, |
There was a problem hiding this comment.
We talked about this before - we can't have this enabled by default
There was a problem hiding this comment.
This flag is not the flag for my service.
https://github.com/laurent22/joplin/pull/11079/files
I set it to false in the last PR, and I switched it back to true here (like how it was before). I also hid it.
Would you like to have both flags set to false ? In this case, when the user triggers a manual check, what should happen ?
There was a problem hiding this comment.
Ok I missed you switched that off - but why did you do this? That's exactly what we did not want to happen, we talked about it and the whole reason why we have an extra feature flag for you. Now everybody who got this version will no longer be notified of new versions. I'm going to turn that back on and create another prerelease.
| bridge().electronApp().initializeAutoUpdaterService( | ||
| Logger.create('AutoUpdaterService'), | ||
| Setting.value('env') === 'dev', | ||
| Setting.value('autoUpdate.includePreReleases'), | ||
| ); |
There was a problem hiding this comment.
Likewise we can't have this running if the user didn't enable the feature. That's the purpose of this flag, to enable it in a controlled way. I don't really understand the logic here, why we need to run this if the flag is not on? If the user needs to restart the app after enabling the flag, we just document it. The flag is a temporary, advanced feature anyway
There was a problem hiding this comment.
In the explanation below, by saying "default flag" I refer to the flag previously used for auto updates, together with the existing logic from before. By saying "implemented flag", I refer to the flag I added for my service.
Explanation: let's say the user has the default flag set to true and the implemented flag set to false. If he/she goes to Options to set the implemented flag to true, and then goes back into the application to manually check for an update without closing or quitting the app, the above error will be shown, because the service was never initialized in the first place (since the implemented flag was initially false).
I even tried to add this check so we could avoid this situation:
if (typeof this.updaterService_ != 'undefined' && this.updaterService_ && this.updaterService_?.checkForUpdates)
Unfortunately it still throws the error.
There was a problem hiding this comment.
Not sure about the technical details, but it should be possible to turn on your feature flag and it only uses your new updater service, or turn it off and it only uses the old method. The fact that something is not working when one of the flag is on or off means some logic is missing somewhere
This PR fixes multiple issues: