[MM-47970] + [MM-47754] Downloads fixes (#2326) (#2327)

* Use downloads location as default when clicking "Save as". Remove update from downloads after upgrade when application starts

* Fix issue where "addedAt" was extracted from undefined object

* Fix tests

(cherry picked from commit ca62814ce6)

Co-authored-by: Tasos Boulis <tboulis@hotmail.com>

src/common/JsonFileManager.ts

@@ -31,9 +31,9 @@ export default class JsonFileManager<T> {
         });
     }
 
-    async setJson(json: T): Promise<void> {
+    setJson(json: T): void {
         this.json = json;
-        await this.writeToFile();
+        this.writeToFile();
     }
 
     setValue(key: keyof T, value: T[keyof T]): void {

src/common/constants.ts

@@ -1,8 +1,22 @@
 // Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
+import {DownloadedItem} from 'types/downloads';
+
+import {DownloadItemTypeEnum} from 'main/downloadsManager';
+
 /**
  * This string includes special characters so that it's not confused with
  * a file that may have the same filename (eg APP_UPDATE)
  */
 export const APP_UPDATE_KEY = '#:(APP_UPDATE):#';
+
+export const UPDATE_DOWNLOAD_ITEM: Omit<DownloadedItem, 'filename' | 'state'> = {
+    type: 'update' as DownloadItemTypeEnum,
+    progress: 0,
+    location: '',
+    mimeType: null,
+    addedAt: 0,
+    receivedBytes: 0,
+    totalBytes: 0,
+};

src/main/autoUpdater.ts

@@ -58,6 +58,7 @@ export class UpdateManager {
     lastCheck?: NodeJS.Timeout;
     versionAvailable?: string;
     versionDownloaded?: string;
+    downloadedInfo?: UpdateInfo;
 
     constructor() {
         this.cancellationToken = new CancellationToken();
@@ -76,6 +77,7 @@ export class UpdateManager {
 
         autoUpdater.on('update-downloaded', (info: UpdateInfo) => {
             this.versionDownloaded = info.version;
+            this.downloadedInfo = info;
             ipcMain.emit(UPDATE_SHORTCUT_MENU);
             log.info(`[Mattermost] downloaded version ${info.version}`);
             this.notifyDownloaded();
@@ -115,7 +117,7 @@ export class UpdateManager {
     }
 
     notifyDownloaded = (): void => {
-        ipcMain.emit(UPDATE_DOWNLOADED, null, this.versionDownloaded);
+        ipcMain.emit(UPDATE_DOWNLOADED, null, this.downloadedInfo);
         displayRestartToUpgrade(this.versionDownloaded || 'unknown', this.handleUpdate);
     }
 
@@ -141,8 +143,8 @@ export class UpdateManager {
         }
     }
 
-    handleUpdate = async (): Promise<void> => {
+    handleUpdate = (): void => {
-        await downloadsManager.removeUpdateBeforeRestart();
+        downloadsManager.removeUpdateBeforeRestart();
         autoUpdater.quitAndInstall();
     }
 

src/main/downloadsManager.ts

@@ -5,8 +5,7 @@ import fs from 'fs';
 
 import {DownloadItem, Event, WebContents, FileFilter, ipcMain, dialog, shell, Menu, app} from 'electron';
 import log from 'electron-log';
-import {ProgressInfo} from 'electron-updater';
+import {ProgressInfo, UpdateInfo} from 'electron-updater';
-
 import {DownloadedItem, DownloadItemDoneEventState, DownloadedItems, DownloadItemState, DownloadItemUpdatedEventState} from 'types/downloads';
 
 import {
@@ -26,15 +25,15 @@ import {
     UPDATE_PROGRESS,
 } from 'common/communication';
 import Config from 'common/config';
+import JsonFileManager from 'common/JsonFileManager';
+import {APP_UPDATE_KEY, UPDATE_DOWNLOAD_ITEM} from 'common/constants';
+import {DOWNLOADS_DROPDOWN_AUTOCLOSE_TIMEOUT, DOWNLOADS_DROPDOWN_MAX_ITEMS} from 'common/utils/constants';
 import {localizeMessage} from 'main/i18nManager';
 import {displayDownloadCompleted} from 'main/notifications';
 import WindowManager from 'main/windows/windowManager';
 import {doubleSecToMs, getPercentage, isStringWithLength, readFilenameFromContentDispositionHeader, shouldIncrementFilename} from 'main/utils';
-import {DOWNLOADS_DROPDOWN_AUTOCLOSE_TIMEOUT, DOWNLOADS_DROPDOWN_MAX_ITEMS} from 'common/utils/constants';
-import JsonFileManager from 'common/JsonFileManager';
-
-import {APP_UPDATE_KEY} from 'common/constants';
 
+import appVersionManager from './AppVersionManager';
 import {downloadsJson} from './constants';
 import * as Validator from './Validator';
 
@@ -46,11 +45,9 @@ export enum DownloadItemTypeEnum {
 export class DownloadsManager extends JsonFileManager<DownloadedItems> {
     autoCloseTimeout: NodeJS.Timeout | null;
     open: boolean;
-
     fileSizes: Map<string, string>;
     progressingItems: Map<string, DownloadItem>;
     downloads: DownloadedItems;
-
     willDownloadURLs: Map<string, {filePath: string; bookmark?: string}>;
     bookmarks: Map<string, {originalPath: string; bookmark: string}>;
 
@@ -172,15 +169,20 @@ export class DownloadsManager extends JsonFileManager<DownloadedItems> {
 
     checkForDeletedFiles = () => {
         log.debug('DownloadsManager.checkForDeletedFiles');
-
         const downloads = this.downloads;
         let modified = false;
 
         for (const fileId in downloads) {
-            if (fileId === APP_UPDATE_KEY) {
-                continue;
-            }
             if (Object.prototype.hasOwnProperty.call(downloads, fileId)) {
+                // Remove update if app was updated and restarted
+                if (fileId === APP_UPDATE_KEY) {
+                    if (appVersionManager.lastAppVersion === downloads[APP_UPDATE_KEY].filename) {
+                        delete downloads[APP_UPDATE_KEY];
+                        modified = true;
+                    } else {
+                        continue;
+                    }
+                }
                 const file = downloads[fileId];
                 if (file.state === 'completed') {
                     if (!file.location || !fs.existsSync(file.location)) {
@@ -333,7 +335,6 @@ export class DownloadsManager extends JsonFileManager<DownloadedItems> {
 
     openDownloadsDropdown = () => {
         log.debug('DownloadsManager.openDownloadsDropdown');
-
         this.open = true;
         ipcMain.emit(OPEN_DOWNLOADS_DROPDOWN);
         WindowManager.sendToRenderer(HIDE_DOWNLOADS_DROPDOWN_BUTTON_BADGE);
@@ -343,10 +344,10 @@ export class DownloadsManager extends JsonFileManager<DownloadedItems> {
         return Boolean(this.downloads[APP_UPDATE_KEY]?.type === DownloadItemTypeEnum.UPDATE);
     };
 
-    removeUpdateBeforeRestart = async () => {
+    removeUpdateBeforeRestart = (): void => {
         const downloads = this.downloads;
         delete downloads[APP_UPDATE_KEY];
-        await this.saveAll(downloads);
+        this.saveAll(downloads);
     };
 
     private markFileAsDeleted = (item: DownloadedItem) => {
@@ -363,18 +364,17 @@ export class DownloadsManager extends JsonFileManager<DownloadedItems> {
         }
     };
 
-    private saveAll = async (downloads: DownloadedItems) => {
+    private saveAll = (downloads: DownloadedItems): void => {
         log.debug('DownloadsManager.saveAll');
 
         this.downloads = downloads;
-        await this.setJson(downloads);
+        this.setJson(downloads);
         ipcMain.emit(UPDATE_DOWNLOADS_DROPDOWN, true, this.downloads);
         WindowManager?.sendToRenderer(UPDATE_DOWNLOADS_DROPDOWN, this.downloads);
     };
 
     private save = (key: string, item: DownloadedItem) => {
         log.debug('DownloadsManager.save');
-
         this.downloads[key] = item;
         this.setValue(key, item);
         ipcMain.emit(UPDATE_DOWNLOADS_DROPDOWN, true, this.downloads);
@@ -395,7 +395,6 @@ export class DownloadsManager extends JsonFileManager<DownloadedItems> {
      */
     private shouldShowSaveDialog = (item: DownloadItem, downloadLocation?: string) => {
         log.debug('DownloadsManager.shouldShowSaveDialog', {downloadLocation});
-
         return !item.hasUserGesture() || !downloadLocation;
     };
 
@@ -406,7 +405,7 @@ export class DownloadsManager extends JsonFileManager<DownloadedItems> {
 
         return dialog.showSaveDialog({
             title: filename,
-            defaultPath: filename,
+            defaultPath: Config.downloadLocation ? path.join(Config.downloadLocation, filename) : filename,
             filters,
             securityScopedBookmarks: true,
         });
@@ -414,11 +413,9 @@ export class DownloadsManager extends JsonFileManager<DownloadedItems> {
 
     private closeDownloadsDropdown = () => {
         log.debug('DownloadsManager.closeDownloadsDropdown');
-
         this.open = false;
         ipcMain.emit(CLOSE_DOWNLOADS_DROPDOWN);
         ipcMain.emit(CLOSE_DOWNLOADS_DROPDOWN_MENU);
-
         this.clearAutoCloseTimeout();
     };
 
@@ -432,7 +429,6 @@ export class DownloadsManager extends JsonFileManager<DownloadedItems> {
     private upsertFileToDownloads = (item: DownloadItem, state: DownloadItemState, overridePath?: string) => {
         const fileId = this.getFileId(item);
         log.debug('DownloadsManager.upsertFileToDownloads', {fileId});
-
         const formattedItem = this.formatDownloadItem(item, state, overridePath);
         this.save(fileId, formattedItem);
         this.checkIfMaxFilesReached();
@@ -442,7 +438,7 @@ export class DownloadsManager extends JsonFileManager<DownloadedItems> {
         const downloads = this.downloads;
         if (Object.keys(downloads).length > DOWNLOADS_DROPDOWN_MAX_ITEMS) {
             const oldestFileId = Object.keys(downloads).reduce((prev, curr) => {
-                return downloads[prev].addedAt > downloads[curr].addedAt ? curr : prev;
+                return downloads[prev]?.addedAt > downloads[curr]?.addedAt ? curr : prev;
             });
             delete downloads[oldestFileId];
             this.saveAll(downloads);
@@ -501,7 +497,6 @@ export class DownloadsManager extends JsonFileManager<DownloadedItems> {
         }
 
         this.upsertFileToDownloads(item, state, bookmark?.originalPath);
-
         this.fileSizes.delete(item.getFilename());
         this.progressingItems.delete(this.getFileId(item));
         this.shouldAutoClose();
@@ -513,31 +508,28 @@ export class DownloadsManager extends JsonFileManager<DownloadedItems> {
      */
     private onUpdateAvailable = (event: Event, version = 'unknown') => {
         this.save(APP_UPDATE_KEY, {
-            type: DownloadItemTypeEnum.UPDATE,
+            ...UPDATE_DOWNLOAD_ITEM,
             filename: version,
             state: 'available',
-            progress: 0,
-            location: '',
-            mimeType: null,
-            addedAt: 0,
-            receivedBytes: 0,
-            totalBytes: 0,
         });
         this.openDownloadsDropdown();
     };
-    private onUpdateDownloaded = (event: Event, version = 'unknown') => {
+    private onUpdateDownloaded = (event: Event, info: UpdateInfo) => {
+        log.debug('DownloadsManager.onUpdateDownloaded', {info});
+
+        const {version} = info;
         const update = this.downloads[APP_UPDATE_KEY];
         update.state = 'completed';
         update.progress = 100;
         update.filename = version;
+
         this.save(APP_UPDATE_KEY, update);
         this.openDownloadsDropdown();
     };
     private onUpdateProgress = (event: Event, progress: ProgressInfo) => {
         log.debug('DownloadsManager.onUpdateProgress', {progress});
         const {total, transferred, percent} = progress;
-
+        const update = this.downloads[APP_UPDATE_KEY] || {...UPDATE_DOWNLOAD_ITEM};
-        const update = this.downloads[APP_UPDATE_KEY];
         if (typeof update.addedAt !== 'number' || update.addedAt === 0) {
             update.addedAt = Date.now();
         }

src/renderer/components/DownloadsDropdown/FileSizeAndStatus.tsx

@@ -15,7 +15,7 @@ type OwnProps = {
 const FileSizeAndStatus = ({item}: OwnProps) => {
     const translate = useIntl();
 
-    const {totalBytes, receivedBytes, addedAt} = item;
+    const {totalBytes, receivedBytes, addedAt} = item || {};
 
     const getRemainingTime = useCallback(() => {
         const elapsedMs = Date.now() - addedAt;

src/renderer/downloadsDropdown.tsx

@@ -58,7 +58,7 @@ class DownloadsDropdown extends React.PureComponent<Record<string, never>, State
                 } else if (b.type === 'update') {
                     return 1;
                 }
-                return b.addedAt - a.addedAt;
+                return b?.addedAt - a?.addedAt;
             });
             this.setState({
                 downloads: newDownloads,