Merge branch 'respect-notification-setting'

6 files changed, 171 insertions, 18 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 78e0370985..0b095497ee 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -48,6 +48,8 @@ Line wrap the file at 100 chars.                                              Th
 - Fix location search in desktop app only searching for English location names.
 - Fix automatic WireGuard key rotation not being initialized correctly when not running the GUI.
 - Fix duplicated notifications in some situations.
+- Fix notification setting being inverted. When non-important notifications were disabled it instead
+  disabled important ones and showed non-important ones.
 
 #### Android
 - Fix adaptive app icon which previously had a displaced nose and some other oddities.
diff --git a/gui/package.json b/gui/package.json
index 6d204b013a..d903ac2da7 100644
--- a/gui/package.json
+++ b/gui/package.json
@@ -101,7 +101,7 @@
     "e2e:sequential:no-build": "xvfb-maybe -- playwright test --workers 1",
     "e2e:update-snapshots": "npm run e2e:no-build -- --update-snapshots",
     "develop": "gulp develop",
-    "test": "cross-env NODE_ENV=test electron-mocha --renderer --reporter spec --require ts-node/register --require \"test/unit/setup/renderer.ts\" \"test/unit/**/*.{ts,tsx}\"",
+    "test": "cross-env NODE_ENV=test electron-mocha --renderer --reporter spec --require ts-node/register --require \"test/unit/setup.ts\" \"test/unit/**/*.{ts,tsx}\"",
     "update-translations": "node scripts/extract-translations",
     "pack:mac": "gulp pack-mac",
     "pack:win": "gulp pack-win",
diff --git a/gui/src/main/notification-controller.ts b/gui/src/main/notification-controller.ts
index cbc1afd66b..c83fc11eff 100644
--- a/gui/src/main/notification-controller.ts
+++ b/gui/src/main/notification-controller.ts
@@ -51,7 +51,8 @@ export default class NotificationController {
   private dismissedNotifications: Set<SystemNotification> = new Set();
   private throttledNotifications: Map<SystemNotification, Scheduler> = new Map();
 
-  private notificationTitle = process.platform === 'linux' ? app.name : '';
+  private notificationTitle =
+    process.platform === 'linux' && process.env.NODE_ENV !== 'test' ? app.name : '';
   private notificationIcon?: NativeImage;
 
   constructor(private notificationControllerDelegate: NotificationControllerDelegate) {
@@ -85,7 +86,7 @@ export default class NotificationController {
     hasExcludedApps: boolean,
     isWindowVisible: boolean,
     areSystemNotificationsEnabled: boolean,
-  ) {
+  ): boolean {
     const notificationProviders: SystemNotificationProvider[] = [
       new ConnectingNotificationProvider({ tunnelState, reconnecting: this.reconnecting }),
       new ConnectedNotificationProvider(tunnelState),
@@ -98,11 +99,14 @@ export default class NotificationController {
       notification.mayDisplay(),
     );
 
+    this.reconnecting =
+      tunnelState.state === 'disconnecting' && tunnelState.details === 'reconnect';
+
     if (notificationProvider) {
       const notification = notificationProvider.getSystemNotification();
 
       if (notification) {
-        this.notify(notification, isWindowVisible, areSystemNotificationsEnabled);
+        return this.notify(notification, isWindowVisible, areSystemNotificationsEnabled);
       } else {
         log.error(
           `Notification providers mayDisplay() returned true but getSystemNotification() returned undefined for ${notificationProvider.constructor.name}`,
@@ -112,8 +116,7 @@ export default class NotificationController {
       this.closeNotificationsInCategory(SystemNotificationCategory.tunnelState);
     }
 
-    this.reconnecting =
-      tunnelState.state === 'disconnecting' && tunnelState.details === 'reconnect';
+    return false;
   }
 
   // Closes still relevant notifications but still lets them affect notification dot in tray icon.
@@ -150,7 +153,7 @@ export default class NotificationController {
     systemNotification: SystemNotification,
     windowVisible: boolean,
     infoNotificationsEnabled: boolean,
-  ) {
+  ): boolean {
     const notificationSuppressReason = this.evaluateNotification(
       systemNotification,
       windowVisible,
@@ -165,7 +168,7 @@ export default class NotificationController {
         this.updateNotificationIcon();
       }
 
-      return;
+      return false;
     }
 
     // Cancel throttled notifications within the same category
@@ -186,8 +189,10 @@ export default class NotificationController {
       }, THROTTLE_DELAY);
 
       this.throttledNotifications.set(systemNotification, scheduler);
+      return true;
     } else {
       this.notifyImpl(systemNotification);
+      return true;
     }
   }
 
@@ -210,14 +215,7 @@ export default class NotificationController {
   }
 
   private createNotification(systemNotification: SystemNotification): Notification {
-    const notification = new ElectronNotification({
-      title: this.notificationTitle,
-      body: systemNotification.message,
-      silent: true,
-      icon: this.notificationIcon,
-      timeoutType:
-        systemNotification.severity == SystemNotificationSeverityType.high ? 'never' : 'default',
-    });
+    const notification = this.createElectronNotification(systemNotification);
 
     // Action buttons are only available on macOS.
     if (process.platform === 'darwin') {
@@ -242,6 +240,17 @@ export default class NotificationController {
     return { specification: systemNotification, notification };
   }
 
+  private createElectronNotification(systemNotification: SystemNotification): ElectronNotification {
+    return new ElectronNotification({
+      title: this.notificationTitle,
+      body: systemNotification.message,
+      silent: true,
+      icon: this.notificationIcon,
+      timeoutType:
+        systemNotification.severity == SystemNotificationSeverityType.high ? 'never' : 'default',
+    });
+  }
+
   private performAction(action?: NotificationAction) {
     if (action && action.type === 'open-url') {
       void this.notificationControllerDelegate.openLink(action.url, action.withAuth);
@@ -287,7 +296,7 @@ export default class NotificationController {
       return NotificationSuppressReason.windowVisible;
     } else if (
       !areSystemNotificationsEnabled &&
-      notification.severity >= SystemNotificationSeverityType.low
+      notification.severity <= SystemNotificationSeverityType.low
     ) {
       return NotificationSuppressReason.preference;
     } else if (this.suppressDueToAlreadyPresented(notification)) {
diff --git a/gui/test/unit/setup/changelog.spec.ts b/gui/test/unit/changelog.spec.ts
index 912a8a3397..249c26e1db 100644
--- a/gui/test/unit/setup/changelog.spec.ts
+++ b/gui/test/unit/changelog.spec.ts
@@ -1,6 +1,6 @@
 import { expect } from 'chai';
 import { after, it, describe } from 'mocha';
-import { parseChangelog } from '../../../src/main/changelog';
+import { parseChangelog } from '../../src/main/changelog';
 
 // It should be handled the same no matter if the platforms are split with a space or not.
 const changelogItems = [
diff --git a/gui/test/unit/notification-evaluation.spec.ts b/gui/test/unit/notification-evaluation.spec.ts
new file mode 100644
index 0000000000..33a86328bd
--- /dev/null
+++ b/gui/test/unit/notification-evaluation.spec.ts
@@ -0,0 +1,142 @@
+import { expect } from 'chai';
+import { it, describe } from 'mocha';
+import sinon from 'sinon';
+
+import {
+  UnsupportedVersionNotificationProvider, UpdateAvailableNotificationProvider,
+  // UpdateAvailableNotificationProvider,
+} from '../../src/shared/notifications/notification';
+import NotificationController from '../../src/main/notification-controller';
+import { TunnelState } from '../../src/shared/daemon-rpc-types';
+import { ErrorStateCause } from '../../src/shared/daemon-rpc-types';
+import { FirewallPolicyErrorType } from '../../src/shared/daemon-rpc-types';
+
+function createController() {
+  return new NotificationController({
+    openApp: () => { /* no-op */ },
+    openLink: (_url: string, _withAuth?: boolean) => Promise.resolve(),
+    showNotificationIcon: (_value: boolean) => { /* no-op */ },
+  });
+}
+
+describe('System notifications', () => {
+  let sandbox: sinon.SinonSandbox;
+
+  before(() => {
+    sandbox = sinon.createSandbox();
+    // @ts-ignore
+    sandbox.stub(NotificationController.prototype, 'createElectronNotification').returns({
+      show: () => { /* no-op */ },
+      close: () => { /* no-op */ },
+      on: () => { /* no-op */ },
+      removeAllListeners: () => { /* no-op */ },
+    });
+  });
+
+  it('should evaluate unspupported version notification to show', () => {
+    const controller1 = createController();
+    const controller2 = createController();
+    const notification = new UnsupportedVersionNotificationProvider({
+      supported: false,
+      consistent: true,
+      suggestedUpgrade: '2100.1',
+      suggestedIsBeta: false,
+    });
+
+    expect(notification.mayDisplay()).to.be.true;
+
+    const systemNotification = notification.getSystemNotification();
+    const result1 = controller1.notify(systemNotification, false, true);
+    const result2 = controller2.notify(systemNotification, false, false);
+
+    expect(result1).to.be.true;
+    expect(result2).to.be.true;
+  });
+
+  it('should evaluate update available notification to show', () => {
+    const controller1 = createController();
+    const controller2 = createController();
+    const notification = new UpdateAvailableNotificationProvider({
+      suggestedUpgrade: '2100.1',
+      suggestedIsBeta: false,
+    });
+
+    expect(notification.mayDisplay()).to.be.true;
+
+    const systemNotification = notification.getSystemNotification();
+    const result1 = controller1.notify(systemNotification, false, true);
+    const result2 = controller2.notify(systemNotification, false, false);
+
+    expect(result1).to.be.true;
+    expect(result2).to.be.true;
+  });
+
+  it('should show unsupported version notification only once', () => {
+    const controller = createController();
+    const notification = new UnsupportedVersionNotificationProvider({
+      supported: false,
+      consistent: true,
+      suggestedUpgrade: '2100.1',
+      suggestedIsBeta: false,
+    });
+
+    const systemNotification = notification.getSystemNotification();
+    const result1 = controller.notify(systemNotification, false, true);
+    const result2 = controller.notify(systemNotification, false, true);
+
+    expect(result1).to.be.true;
+    expect(result2).to.be.false;
+  });
+
+  it('should not show notification when window is open', () => {
+    const controller = createController();
+    const notification = new UnsupportedVersionNotificationProvider({
+      supported: false,
+      consistent: true,
+      suggestedUpgrade: '2100.1',
+      suggestedIsBeta: false,
+    });
+
+    const systemNotification = notification.getSystemNotification();
+    const result = controller.notify(systemNotification, true, true);
+
+    expect(result).to.be.false;
+  });
+
+  it('Tunnel state notifications should respect notification setting', () => {
+    const controller = createController();
+
+    const disconnectedState: TunnelState = { state: 'disconnected' };
+    const connectingState: TunnelState = { state: 'connecting' };
+    const result1 = controller.notifyTunnelState(disconnectedState, false, false, false, true);
+    const result2 = controller.notifyTunnelState(disconnectedState, false, false, false, false);
+    const result3 = controller.notifyTunnelState(connectingState, false, false, false, true);
+    const result4 = controller.notifyTunnelState(connectingState, false, false, false, false);
+
+    expect(result1).to.be.true;
+    expect(result2).to.be.false;
+    expect(result3).to.be.true;
+    expect(result4).to.be.false;
+
+    const blockingErrorState: TunnelState = {
+      state: 'error',
+      details: {
+        cause: ErrorStateCause.isOffline,
+      },
+    };
+    const result5 = controller.notifyTunnelState(blockingErrorState, false, false, false, false);
+    expect(result5).to.be.false;
+
+    const nonBlockingErrorState: TunnelState = {
+      state: 'error',
+      details: {
+        cause: ErrorStateCause.isOffline,
+        blockingError: {
+          type: FirewallPolicyErrorType.generic,
+        }
+      },
+    };
+    const result6 = controller.notifyTunnelState(nonBlockingErrorState, false, false, false, false);
+    expect(result6).to.be.true;
+  });
+});
diff --git a/gui/test/unit/setup/renderer.ts b/gui/test/unit/setup.ts
index 65644e28d9..65644e28d9 100644
--- a/gui/test/unit/setup/renderer.ts
+++ b/gui/test/unit/setup.ts