Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: more auto-update logging, disable if not supported COMPASS-7220 #4868

Merged
merged 3 commits into from
Sep 20, 2023
Merged

fix: more auto-update logging, disable if not supported COMPASS-7220 #4868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1d6759a
more auto-update logging, disable if not supported
lerouxb Sep 19, 2023
10f07a2
add ticket numbers, remove extra logging
lerouxb Sep 19, 2023
6a1046f
tweak comment
lerouxb Sep 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 36 additions & 12 deletions packages/compass/src/main/auto-update-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import COMPASS_ICON from './icon';
import type { FeedURLOptions } from 'electron';
import { app, dialog, BrowserWindow } from 'electron';
import { setTimeout as wait } from 'timers/promises';
import autoUpdater from './auto-updater';
import autoUpdater, { supportsAutoupdater } from './auto-updater';
import preferences from 'compass-preferences-model';
import fetch from 'node-fetch';
import dl from 'electron-dl';
Expand Down Expand Up @@ -579,9 +579,41 @@ class CompassAutoUpdateManager {
...options,
};

// TODO(COMPASS-7232): If auto-updates are not supported, then there is
// still a menu item to check for updates and then if it finds an update but
// auto-updates aren't supported it will still display a popup with an
// Install button that does nothing.
compassApp.on('check-for-updates', () => {
this.setState(AutoUpdateManagerState.ManualCheck);
});

const supported = supportsAutoupdater();
const enabled = !!preferences.getPreferences().autoUpdates;

log.info(
mongoLogId(1001000133),
'AutoUpdateManager',
'Setting up updateManager',
{ ...this.autoUpdateOptions, supported, enabled }
);

if (!supported) {
this.setState(AutoUpdateManagerState.Disabled);
return;
}

// If autoupdate is supported, then enable/disable it depending on preferences

preferences.onPreferenceValueChanged('autoUpdates', (enabled) => {
log.info(
mongoLogId(1_001_000_247),
'AutoUpdateManager',
`autoUpdate preference toggled to ${enabled ? 'enabled' : 'disabled'}`,
{
enabled,
}
);

if (enabled) {
track('Autoupdate Enabled');
this.setState(AutoUpdateManagerState.CheckingForUpdates);
Expand All @@ -591,17 +623,9 @@ class CompassAutoUpdateManager {
}
});

compassApp.on('check-for-updates', () => {
this.setState(AutoUpdateManagerState.ManualCheck);
});

log.info(
mongoLogId(1001000133),
'AutoUpdateManager',
'Setting up updateManager',
{ ...this.autoUpdateOptions, enabled }
);

// TODO(COMPASS-7233): This is kinda pointless at the moment because the
// preferences start as disabled and then become enabled the moment they are
// loaded. Which would immediately kick off the state change.
Comment on lines +626 to +628
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think this is not a race condition and just a case for first-time compass run where some of those default properties are not set until we show the welcome modal first time

if (enabled) {
// Do not kick off update check immediately, wait a little before that so
// that we 1) don't waste time checking on the application start 2) don't
Expand Down
2 changes: 1 addition & 1 deletion packages/compass/src/main/auto-updater.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function hasSquirrel() {
return fs.existsSync(updateExe);
}

function supportsAutoupdater() {
export function supportsAutoupdater() {
if (process.platform === 'linux') {
return false;
}
Expand Down
Loading