From da4be6592fafe7c5a72c3fcd1d24bedcb9d8ac20 Mon Sep 17 00:00:00 2001
From: Samuel Maddock <samuel.maddock@gmail.com>
Date: Fri, 22 Nov 2024 19:10:10 -0500
Subject: [PATCH] fix: dialog clobbered on multiple requests

When multiple dialogs are requested to be shown, they now wait for the previous dialog to be closed and cleaned up.
---
 src/renderer/state.ts        | 31 ++++++++++++++++++++---
 tests/renderer/state-spec.ts | 49 +++++++++++++++++++++++++++++++++++-
 2 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/src/renderer/state.ts b/src/renderer/state.ts
index da3bc1996f..96044a5c68 100644
--- a/src/renderer/state.ts
+++ b/src/renderer/state.ts
@@ -4,6 +4,7 @@ import {
   computed,
   makeObservable,
   observable,
+  runInAction,
   when,
 } from 'mobx';
 
@@ -975,12 +976,34 @@ export class AppState {
   public async showGenericDialog(
     opts: GenericDialogOptions,
   ): Promise<{ confirm: boolean; input: string }> {
-    this.genericDialogLastResult = null;
-    this.genericDialogOptions = opts;
-    this.isGenericDialogShowing = true;
+    // Wait for any existing dialog to be closed and cleaned up.
+    await when(() => {
+      if (
+        !this.isGenericDialogShowing &&
+        this.genericDialogLastResult === null
+      ) {
+        // Set dialog immediately to prevent any other queued dialogs from
+        // showing.
+        runInAction(() => {
+          this.genericDialogOptions = opts;
+          this.isGenericDialogShowing = true;
+        });
+        return true;
+      }
+      return false;
+    });
+
+    // Wait for dialog to be closed.
     await when(() => !this.isGenericDialogShowing);
+
+    // Cleanup to allow queued dialog to show.
+    const result = this.genericDialogLastResult;
+    runInAction(() => {
+      this.genericDialogLastResult = null;
+    });
+
     return {
-      confirm: Boolean(this.genericDialogLastResult),
+      confirm: Boolean(result),
       input: this.genericDialogLastInput || opts.defaultInput || '',
     };
   }
diff --git a/tests/renderer/state-spec.ts b/tests/renderer/state-spec.ts
index d9856900b3..e2b18630d6 100644
--- a/tests/renderer/state-spec.ts
+++ b/tests/renderer/state-spec.ts
@@ -1,5 +1,5 @@
 import { mocked } from 'jest-mock';
-import { IReactionDisposer, reaction } from 'mobx';
+import { IReactionDisposer, reaction, runInAction, when } from 'mobx';
 
 import {
   AppStateBroadcastMessageType,
@@ -585,6 +585,53 @@ describe('AppState', () => {
         const result = await appState.showGenericDialog(Opts);
         expect(result).toHaveProperty('input', '');
       });
+
+      it('queues dialogs', async () => {
+        const optsA = {
+          label: 'A',
+          ok: 'Close',
+          type: GenericDialogType.warning,
+          wantsInput: false,
+        } as const;
+
+        const optsB = {
+          label: 'B',
+          ok: 'Close',
+          type: GenericDialogType.warning,
+          wantsInput: false,
+        } as const;
+
+        // Show Dialog A
+        const dialogPromiseA = appState.showGenericDialog(optsA);
+        await when(() => appState.genericDialogOptions?.label === optsA.label);
+
+        // Queue Dialog B
+        const dialogPromiseB = appState.showGenericDialog(optsB);
+        expect(appState).toHaveProperty('isGenericDialogShowing', true);
+        expect(appState.genericDialogOptions?.label).toEqual(optsA.label);
+
+        // Close Dialog A
+        runInAction(() => {
+          appState.genericDialogLastResult = true;
+          appState.isGenericDialogShowing = false;
+        });
+        expect(appState.genericDialogOptions?.label).toEqual(optsA.label);
+        await dialogPromiseA;
+
+        // Expect dialog set to Dialog B
+        expect(appState).toHaveProperty('isGenericDialogShowing', true);
+        expect(appState.genericDialogOptions?.label).toEqual(optsB.label);
+
+        // Close Dialog B
+        runInAction(() => {
+          appState.genericDialogLastResult = true;
+          appState.isGenericDialogShowing = false;
+        });
+        await dialogPromiseB;
+
+        // No remaining dialogs queued
+        expect(appState).toHaveProperty('isGenericDialogShowing', false);
+      });
     });
 
     describe('showInputDialog', () => {