From da88b8643e02922e8eb8b7da74d4160ff1beb1f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20V=C3=A9lez?= Date: Fri, 18 Oct 2024 15:39:56 +0200 Subject: [PATCH] MM-60782 - use default download directory on windows (#3161) * MM-60782 - use default download directory on windows * fix unit tests --- src/app/serverViewState.test.js | 3 + src/common/config/defaultPreferences.ts | 4 +- src/common/config/index.test.js | 6 ++ src/common/config/upgradePreferences.test.js | 6 ++ src/main/app/config.test.js | 66 +++++++++++++++++++ src/main/downloadsManager.test.js | 1 + src/main/tray/tray.test.js | 1 + src/main/views/MattermostBrowserView.test.js | 1 + .../views/downloadsDropdownMenuView.test.js | 1 + src/main/views/downloadsDropdownView.test.js | 1 + src/main/views/serverDropdownView.test.js | 3 + src/main/views/viewManager.test.js | 1 + 12 files changed, 93 insertions(+), 1 deletion(-) diff --git a/src/app/serverViewState.test.js b/src/app/serverViewState.test.js index d0fb4299..85d9a233 100644 --- a/src/app/serverViewState.test.js +++ b/src/app/serverViewState.test.js @@ -15,6 +15,9 @@ import MainWindow from 'main/windows/mainWindow'; import {ServerViewState} from './serverViewState'; jest.mock('electron', () => ({ + app: { + getPath: jest.fn(() => '/valid/downloads/path'), + }, ipcMain: { on: jest.fn(), handle: jest.fn(), diff --git a/src/common/config/defaultPreferences.ts b/src/common/config/defaultPreferences.ts index f6ad2821..5587371e 100644 --- a/src/common/config/defaultPreferences.ts +++ b/src/common/config/defaultPreferences.ts @@ -5,6 +5,8 @@ import os from 'os'; import path from 'path'; +import {app} from 'electron'; + /** * Default user preferences. End-users can change these parameters by editing config.json * @param {number} version - Scheme version. (Not application version) @@ -24,7 +26,7 @@ export const getDefaultDownloadLocation = (): string | undefined => { return process.env.XDG_DOWNLOAD_DIR; } - return path.join(os.homedir(), 'Downloads'); + return app.getPath('downloads') || path.join(os.homedir(), 'Downloads'); }; const defaultPreferences: ConfigV3 = { diff --git a/src/common/config/index.test.js b/src/common/config/index.test.js index 8caaaf50..72f88af1 100644 --- a/src/common/config/index.test.js +++ b/src/common/config/index.test.js @@ -94,6 +94,12 @@ jest.mock('common/config/RegistryConfig', () => { return jest.fn(); }); +jest.mock('electron', () => ({ + app: { + getPath: jest.fn(() => '/valid/downloads/path'), + }, +})); + describe('common/config', () => { it('should load buildConfig', () => { const config = new Config(); diff --git a/src/common/config/upgradePreferences.test.js b/src/common/config/upgradePreferences.test.js index 446afc45..9e9f1804 100644 --- a/src/common/config/upgradePreferences.test.js +++ b/src/common/config/upgradePreferences.test.js @@ -18,6 +18,12 @@ jest.mock('common/views/View', () => ({ }), })); +jest.mock('electron', () => ({ + app: { + getPath: jest.fn(() => '/valid/downloads/path'), + }, +})); + describe('common/config/upgradePreferences', () => { describe('upgradeV0toV1', () => { it('should upgrade from v0', () => { diff --git a/src/main/app/config.test.js b/src/main/app/config.test.js index 475d73b1..eb1e1689 100644 --- a/src/main/app/config.test.js +++ b/src/main/app/config.test.js @@ -1,10 +1,14 @@ // Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. +import os from 'os'; +import path from 'path'; + import {app} from 'electron'; import {RELOAD_CONFIGURATION} from 'common/communication'; import Config from 'common/config'; +import {getDefaultDownloadLocation} from 'common/config/defaultPreferences'; import {setLoggingLevel} from 'common/log'; import {handleConfigUpdate} from 'main/app/config'; import {handleMainWindowIsShown} from 'main/app/intercom'; @@ -16,6 +20,7 @@ jest.mock('electron', () => ({ getAppPath: () => '/path/to/app', isReady: jest.fn(), setPath: jest.fn(), + getPath: jest.fn(() => '/valid/downloads/path'), }, ipcMain: { emit: jest.fn(), @@ -23,6 +28,10 @@ jest.mock('electron', () => ({ }, })); +jest.mock('os', () => ({ + homedir: jest.fn(), +})); + jest.mock('main/app/utils', () => ({ handleUpdateMenuEvent: jest.fn(), updateSpellCheckerLocales: jest.fn(), @@ -51,6 +60,8 @@ jest.mock('main/windows/mainWindow', () => ({ describe('main/app/config', () => { describe('handleConfigUpdate', () => { + const originalPlatform = process.platform; + const originalXDGDownloadDir = process.env.XDG_DOWNLOAD_DIR; beforeEach(() => { AutoLauncher.enable.mockResolvedValue({}); AutoLauncher.disable.mockResolvedValue({}); @@ -58,6 +69,17 @@ describe('main/app/config', () => { afterEach(() => { delete Config.registryConfigData; + Object.defineProperty(process, 'platform', { + value: originalPlatform, + }); + + if (originalXDGDownloadDir) { + process.env.XDG_DOWNLOAD_DIR = originalXDGDownloadDir; + } else { + delete process.env.XDG_DOWNLOAD_DIR; + } + // eslint-disable-next-line no-underscore-dangle + global.__IS_MAC_APP_STORE__ = false; }); it('should reload renderer config only when app is ready', () => { @@ -74,6 +96,50 @@ describe('main/app/config', () => { expect(app.setPath).toHaveBeenCalledWith('downloads', '/a/download/location'); }); + it('should return undefined for Mac App Store builds', () => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + // eslint-disable-next-line no-underscore-dangle + global.__IS_MAC_APP_STORE__ = true; + + expect(getDefaultDownloadLocation()).toBeUndefined(); + }); + + it('should return XDG_DOWNLOAD_DIR if running on Linux and environment variable is set', () => { + Object.defineProperty(process, 'platform', { + value: 'linux', + }); + + process.env.XDG_DOWNLOAD_DIR = '/home/user/xdg-downloads'; + const result = getDefaultDownloadLocation(); + expect(result).toBe('/home/user/xdg-downloads'); + }); + + it('should return app.getPath("downloads") if available', () => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + // eslint-disable-next-line no-underscore-dangle + global.__IS_MAC_APP_STORE__ = false; + + (app.getPath).mockReturnValue('/custom/downloads'); + + const result = getDefaultDownloadLocation(); + + expect(result).toBe('/custom/downloads'); + expect(app.getPath).toHaveBeenCalledWith('downloads'); + }); + + it('should fallback to home directory if app.getPath("downloads") is not available', () => { + (app.getPath).mockReturnValue(null); + + (os.homedir).mockReturnValue('/home/user'); + + const result = getDefaultDownloadLocation(); + + expect(result).toBe(path.join('/home/user', 'Downloads')); + expect(app.getPath).toHaveBeenCalledWith('downloads'); + }); + it('should enable/disable auto launch on windows/linux', () => { const originalPlatform = process.platform; Object.defineProperty(process, 'platform', { diff --git a/src/main/downloadsManager.test.js b/src/main/downloadsManager.test.js index fc16e4e8..8b9cfe1a 100644 --- a/src/main/downloadsManager.test.js +++ b/src/main/downloadsManager.test.js @@ -28,6 +28,7 @@ jest.mock('electron', () => { return { app: { getAppPath: jest.fn(), + getPath: jest.fn(() => '/valid/downloads/path'), }, BrowserView: jest.fn().mockImplementation(() => ({ webContents: { diff --git a/src/main/tray/tray.test.js b/src/main/tray/tray.test.js index 2dac7fc2..eb39a5f9 100644 --- a/src/main/tray/tray.test.js +++ b/src/main/tray/tray.test.js @@ -30,6 +30,7 @@ jest.mock('electron', () => { getAppPath: () => '/path/to/app', isReady: jest.fn(), setPath: jest.fn(), + getPath: jest.fn(() => '/valid/downloads/path'), }, ipcMain: { emit: jest.fn(), diff --git a/src/main/views/MattermostBrowserView.test.js b/src/main/views/MattermostBrowserView.test.js index bb300b8c..033f1b4e 100644 --- a/src/main/views/MattermostBrowserView.test.js +++ b/src/main/views/MattermostBrowserView.test.js @@ -17,6 +17,7 @@ import MainWindow from '../windows/mainWindow'; jest.mock('electron', () => ({ app: { getVersion: () => '5.0.0', + getPath: jest.fn(() => '/valid/downloads/path'), }, BrowserView: jest.fn().mockImplementation(() => ({ webContents: { diff --git a/src/main/views/downloadsDropdownMenuView.test.js b/src/main/views/downloadsDropdownMenuView.test.js index cec506f7..c805d966 100644 --- a/src/main/views/downloadsDropdownMenuView.test.js +++ b/src/main/views/downloadsDropdownMenuView.test.js @@ -28,6 +28,7 @@ jest.mock('electron', () => { return { app: { getAppPath: () => '', + getPath: jest.fn(() => '/valid/downloads/path'), }, BrowserView: jest.fn().mockImplementation(() => ({ webContents: { diff --git a/src/main/views/downloadsDropdownView.test.js b/src/main/views/downloadsDropdownView.test.js index a4278887..e5d76bff 100644 --- a/src/main/views/downloadsDropdownView.test.js +++ b/src/main/views/downloadsDropdownView.test.js @@ -36,6 +36,7 @@ jest.mock('electron', () => { return { app: { getAppPath: () => '', + getPath: jest.fn(() => '/valid/downloads/path'), }, BrowserView: jest.fn().mockImplementation(() => ({ webContents: { diff --git a/src/main/views/serverDropdownView.test.js b/src/main/views/serverDropdownView.test.js index 91d88890..a98e42ec 100644 --- a/src/main/views/serverDropdownView.test.js +++ b/src/main/views/serverDropdownView.test.js @@ -25,6 +25,9 @@ jest.mock('electron', () => ({ ipcMain: { on: jest.fn(), }, + app: { + getPath: jest.fn(() => '/valid/downloads/path'), + }, })); jest.mock('main/windows/mainWindow', () => ({ on: jest.fn(), diff --git a/src/main/views/viewManager.test.js b/src/main/views/viewManager.test.js index 339ac874..fe6f5170 100644 --- a/src/main/views/viewManager.test.js +++ b/src/main/views/viewManager.test.js @@ -18,6 +18,7 @@ import {ViewManager} from './viewManager'; jest.mock('electron', () => ({ app: { getAppPath: () => '/path/to/app', + getPath: jest.fn(() => '/valid/downloads/path'), }, dialog: { showErrorBox: jest.fn(),