From 1d1dae0c7ec305f5b605df8b0efdf8b90013eb48 Mon Sep 17 00:00:00 2001
From: Veronica Berglyd Olsen <1619840+vkbo@users.noreply.github.com>
Date: Wed, 29 Nov 2023 16:03:48 +0100
Subject: [PATCH] Remove caching of the alert object in the shared instance

---
 novelwriter/shared.py                       | 66 +++++++++++++--------
 tests/test_base/test_base_shared.py         | 38 +++---------
 tests/test_core/test_core_project.py        | 18 ++----
 tests/test_gui/test_gui_mainmenu.py         |  3 +-
 tests/test_tools/test_tools_dictionaries.py | 10 ++--
 5 files changed, 60 insertions(+), 75 deletions(-)

diff --git a/novelwriter/shared.py b/novelwriter/shared.py
index 9bad600a..5383765a 100644
--- a/novelwriter/shared.py
+++ b/novelwriter/shared.py
@@ -45,7 +45,7 @@ logger = logging.getLogger(__name__)
 class SharedData(QObject):
 
     __slots__ = (
-        "_gui", "_theme", "_project", "_spelling", "_lockedBy", "_alert",
+        "_gui", "_theme", "_project", "_spelling", "_lockedBy", "_lastAlert",
         "_idleTime", "_idleRefTime",
     )
 
@@ -68,7 +68,7 @@ class SharedData(QObject):
 
         # Settings
         self._lockedBy = None
-        self._alert = None
+        self._lastAlert = ""
         self._idleTime = 0.0
         self._idleRefTime = time()
 
@@ -122,9 +122,9 @@ class SharedData(QObject):
         return self._idleTime
 
     @property
-    def alert(self) -> _GuiAlert | None:
-        """Return a pointer to the last alert box."""
-        return self._alert
+    def lastAlert(self) -> str:
+        """Return the last alert message."""
+        return self._lastAlert
 
     ##
     #  Methods
@@ -238,44 +238,53 @@ class SharedData(QObject):
 
     def info(self, text: str, info: str = "", details: str = "", log: bool = True) -> None:
         """Open an information alert box."""
-        self._alert = _GuiAlert(self.mainGui, self.theme)
-        self._alert.setMessage(text, info, details)
-        self._alert.setAlertType(_GuiAlert.INFO, False)
+        alert = _GuiAlert(self.mainGui, self.theme)
+        alert.setMessage(text, info, details)
+        alert.setAlertType(_GuiAlert.INFO, False)
+        self._lastAlert = alert.logMessage
         if log:
-            logger.info(self._alert.logMessage, stacklevel=2)
-        self._alert.exec_()
+            logger.info(self._lastAlert, stacklevel=2)
+        alert.exec_()
+        alert.deleteLater()
         return
 
     def warn(self, text: str, info: str = "", details: str = "", log: bool = True) -> None:
         """Open a warning alert box."""
-        self._alert = _GuiAlert(self.mainGui, self.theme)
-        self._alert.setMessage(text, info, details)
-        self._alert.setAlertType(_GuiAlert.WARN, False)
+        alert = _GuiAlert(self.mainGui, self.theme)
+        alert.setMessage(text, info, details)
+        alert.setAlertType(_GuiAlert.WARN, False)
+        self._lastAlert = alert.logMessage
         if log:
-            logger.warning(self._alert.logMessage, stacklevel=2)
-        self._alert.exec_()
+            logger.warning(self._lastAlert, stacklevel=2)
+        alert.exec_()
+        alert.deleteLater()
         return
 
     def error(self, text: str, info: str = "", details: str = "", log: bool = True,
               exc: Exception | None = None) -> None:
         """Open an error alert box."""
-        self._alert = _GuiAlert(self.mainGui, self.theme)
-        self._alert.setMessage(text, info, details)
-        self._alert.setAlertType(_GuiAlert.ERROR, False)
+        alert = _GuiAlert(self.mainGui, self.theme)
+        alert.setMessage(text, info, details)
+        alert.setAlertType(_GuiAlert.ERROR, False)
         if exc:
-            self._alert.setException(exc)
+            alert.setException(exc)
+        self._lastAlert = alert.logMessage
         if log:
-            logger.error(self._alert.logMessage, stacklevel=2)
-        self._alert.exec_()
+            logger.error(self._lastAlert, stacklevel=2)
+        alert.exec_()
+        alert.deleteLater()
         return
 
     def question(self, text: str, info: str = "", details: str = "", warn: bool = False) -> bool:
         """Open a question box."""
-        self._alert = _GuiAlert(self.mainGui, self.theme)
-        self._alert.setMessage(text, info, details)
-        self._alert.setAlertType(_GuiAlert.WARN if warn else _GuiAlert.ASK, True)
-        self._alert.exec_()
-        return self._alert.result() == QMessageBox.Yes
+        alert = _GuiAlert(self.mainGui, self.theme)
+        alert.setMessage(text, info, details)
+        alert.setAlertType(_GuiAlert.WARN if warn else _GuiAlert.ASK, True)
+        self._lastAlert = alert.logMessage
+        alert.exec_()
+        isYes = alert.result() == QMessageBox.StandardButton.Yes
+        alert.deleteLater()
+        return isYes
 
     ##
     #  Internal Functions
@@ -312,6 +321,11 @@ class _GuiAlert(QMessageBox):
         super().__init__(parent=parent)
         self._theme = theme
         self._message = ""
+        logger.debug("Ready: _GuiAlert")
+        return
+
+    def __del__(self) -> None:  # pragma: no cover
+        logger.debug("Delete: _GuiAlert")
         return
 
     @property
diff --git a/tests/test_base/test_base_shared.py b/tests/test_base/test_base_shared.py
index 4e3de75a..0f105284 100644
--- a/tests/test_base/test_base_shared.py
+++ b/tests/test_base/test_base_shared.py
@@ -22,13 +22,13 @@ from __future__ import annotations
 
 import pytest
 
+from tools import buildTestProject
 from mocked import MockGuiMain, MockTheme
 
 from PyQt5.QtWidgets import QMessageBox
 
+from novelwriter.shared import SharedData
 from novelwriter.core.project import NWProject
-from novelwriter.shared import SharedData, _GuiAlert
-from tests.tools import buildTestProject
 
 
 @pytest.mark.base
@@ -62,8 +62,6 @@ def testBaseSharedData_Init():
     assert shared.projectIdleTime == 0.0
     assert shared.projectLock is None
 
-    assert shared.alert is None
-
 # END Test testBaseSharedData_Init
 
 
@@ -124,7 +122,7 @@ def testBaseSharedData_Projects(fncPath, caplog):
 
 
 @pytest.mark.base
-def testBaseSharedData_Alerts(monkeypatch, caplog):
+def testBaseSharedData_Alerts(qtbot, monkeypatch, caplog):
     """Test SharedData class alert helper functions."""
     monkeypatch.setattr(QMessageBox, "exec_", lambda *a: None)
     monkeypatch.setattr(QMessageBox, "result", lambda *a: QMessageBox.Yes)
@@ -135,56 +133,38 @@ def testBaseSharedData_Alerts(monkeypatch, caplog):
     mockTheme = MockTheme()
     shared.initSharedData(mockGui, mockTheme)  # type: ignore
 
-    assert shared.alert is None
+    assert shared.lastAlert == ""
 
     # Info box
     caplog.clear()
     shared.info("Hello World", info="foo", details="bar")
-    assert isinstance(shared.alert, _GuiAlert)
-    assert shared.alert.text() == "Hello World"
-    assert shared.alert.informativeText() == "foo"
-    assert shared.alert.detailedText() == "bar"
+    assert shared.lastAlert == "Hello World foo bar"
     assert caplog.text.strip().startswith("INFO")
     assert caplog.text.strip().endswith("Hello World foo bar")
-    shared._alert = None
 
     # Warning box
     caplog.clear()
     shared.warn("Oops!", info="foo", details="bar")
-    assert isinstance(shared.alert, _GuiAlert)
-    assert shared.alert.text() == "Oops!"
-    assert shared.alert.informativeText() == "foo"
-    assert shared.alert.detailedText() == "bar"
+    assert shared.lastAlert == "Oops! foo bar"
     assert caplog.text.strip().startswith("WARNING")
     assert caplog.text.strip().endswith("Oops! foo bar")
-    shared._alert = None
 
     # Error box
     caplog.clear()
     shared.error("Oh noes!", info="foo", details="bar")
-    assert isinstance(shared.alert, _GuiAlert)
-    assert shared.alert.text() == "Oh noes!"
-    assert shared.alert.informativeText() == "foo"
-    assert shared.alert.detailedText() == "bar"
+    assert shared.lastAlert == "Oh noes! foo bar"
     assert caplog.text.strip().startswith("ERROR")
     assert caplog.text.strip().endswith("Oh noes! foo bar")
-    shared._alert = None
 
     # Error box with exception
     caplog.clear()
     shared.error("Oh noes!", info="foo", details="bar", exc=Exception("Boom!"))
-    assert isinstance(shared.alert, _GuiAlert)
-    assert shared.alert.text() == "Oh noes!"
-    assert shared.alert.informativeText() == "foo<br><b>Exception</b>: Boom!"
-    assert shared.alert.detailedText() == "bar"
+    assert shared.lastAlert == "Oh noes! foo bar"
     assert caplog.text.strip().startswith("ERROR")
     assert caplog.text.strip().endswith("Oh noes! foo bar")
-    shared._alert = None
 
     # Question box
     assert shared.question("Why?") is True
-    assert isinstance(shared.alert, _GuiAlert)
-    assert shared.alert.text() == "Why?"
-    shared._alert = None
+    assert shared.lastAlert == "Why?"
 
 # END Test testBaseSharedData_Alerts
diff --git a/tests/test_core/test_core_project.py b/tests/test_core/test_core_project.py
index 475c9ce1..6cf3b04a 100644
--- a/tests/test_core/test_core_project.py
+++ b/tests/test_core/test_core_project.py
@@ -206,40 +206,35 @@ def testCoreProject_Open(monkeypatch, caplog, mockGUI, fncPath, mockRnd):
         mp.setattr(ProjectXMLReader, "read", lambda *a: False)
         mp.setattr(ProjectXMLReader, "state", property(lambda *a: XMLReadState.NOT_NWX_FILE))
         assert theProject.openProject(fncPath) is False
-        lastMsg = SHARED.alert.logMessage if SHARED.alert else ""
-        assert "Project file does not appear" in lastMsg
+        assert "Project file does not appear" in SHARED.lastAlert
 
     # Unknown project file version
     with monkeypatch.context() as mp:
         mp.setattr(ProjectXMLReader, "read", lambda *a: False)
         mp.setattr(ProjectXMLReader, "state", property(lambda *a: XMLReadState.UNKNOWN_VERSION))
         assert theProject.openProject(fncPath) is False
-        lastMsg = SHARED.alert.logMessage if SHARED.alert else ""
-        assert "Unknown or unsupported novelWriter project file" in lastMsg
+        assert "Unknown or unsupported novelWriter project file" in SHARED.lastAlert
 
     # Other parse error
     with monkeypatch.context() as mp:
         mp.setattr(ProjectXMLReader, "read", lambda *a: False)
         mp.setattr(ProjectXMLReader, "state", property(lambda *a: XMLReadState.CANNOT_PARSE))
         assert theProject.openProject(fncPath) is False
-        lastMsg = SHARED.alert.logMessage if SHARED.alert else ""
-        assert "Failed to parse project xml" in lastMsg
+        assert "Failed to parse project xml" in SHARED.lastAlert
 
     # Won't convert legacy file
     with monkeypatch.context() as mp:
         mp.setattr(ProjectXMLReader, "state", property(lambda *a: XMLReadState.WAS_LEGACY))
         mp.setattr(QMessageBox, "result", lambda *a: QMessageBox.No)
         assert theProject.openProject(fncPath) is False
-        lastMsg = SHARED.alert.logMessage if SHARED.alert else ""
-        assert "The file format of your project is about to be" in lastMsg
+        assert "The file format of your project is about to be" in SHARED.lastAlert
 
     # Won't open project from newer version
     with monkeypatch.context() as mp:
         mp.setattr(ProjectXMLReader, "hexVersion", property(lambda *a: 0x99999999))
         mp.setattr(QMessageBox, "result", lambda *a: QMessageBox.No)
         assert theProject.openProject(fncPath) is False
-        lastMsg = SHARED.alert.logMessage if SHARED.alert else ""
-        assert "This project was saved by a newer version" in lastMsg
+        assert "This project was saved by a newer version" in SHARED.lastAlert
 
     # Fail checking items should still pass
     with monkeypatch.context() as mp:
@@ -254,8 +249,7 @@ def testCoreProject_Open(monkeypatch, caplog, mockGUI, fncPath, mockRnd):
         mp.setattr("novelwriter.core.index.NWIndex.loadIndex", lambda *a: True)
         theProject.index._indexBroken = True
         assert theProject.openProject(fncPath) is True
-        lastMsg = SHARED.alert.logMessage if SHARED.alert else ""
-        assert "The file format of your project is about to be" in lastMsg
+        assert "The file format of your project is about to be" in SHARED.lastAlert
         assert theProject.index._indexBroken is False
 
     theProject.closeProject()
diff --git a/tests/test_gui/test_gui_mainmenu.py b/tests/test_gui/test_gui_mainmenu.py
index 721cc130..91c1ac13 100644
--- a/tests/test_gui/test_gui_mainmenu.py
+++ b/tests/test_gui/test_gui_mainmenu.py
@@ -656,8 +656,7 @@ def testGuiMenu_Insert(qtbot, monkeypatch, nwGUI, fncPath, projPath, mockRnd):
 
     nwGUI.mainMenu.aFileDetails.activate(QAction.Trigger)
     path = str(projPath / "content" / "000000000000f.nwd")
-    logMsg = SHARED.alert.logMessage if SHARED.alert else ""
-    assert logMsg.endswith(f"File Location: {path}")
+    assert SHARED.lastAlert.endswith(f"File Location: {path}")
 
     # qtbot.stop()
 
diff --git a/tests/test_tools/test_tools_dictionaries.py b/tests/test_tools/test_tools_dictionaries.py
index eee0f0f7..02cc5dc0 100644
--- a/tests/test_tools/test_tools_dictionaries.py
+++ b/tests/test_tools/test_tools_dictionaries.py
@@ -44,8 +44,7 @@ def testToolDictionaries_Main(qtbot, monkeypatch, nwGUI, fncPath):
     with monkeypatch.context() as mp:
         mp.setattr(enchant, "get_user_config_dir", lambda *a: causeException)
         nwGUI.showDictionariesDialog()
-        assert SHARED.alert is not None
-        assert SHARED.alert.logMessage == "Could not initialise the dialog."
+        assert SHARED.lastAlert == "Could not initialise the dialog."
 
     # Open the tool
     nwGUI.showDictionariesDialog()
@@ -57,17 +56,16 @@ def testToolDictionaries_Main(qtbot, monkeypatch, nwGUI, fncPath):
     assert nwDicts.inPath.text() == str(fncPath)
 
     # Allow Open Dir
-    SHARED._alert = None
+    SHARED._lastAlert = ""
     with monkeypatch.context() as mp:
         mp.setattr(QDesktopServices, "openUrl", lambda *a: None)
         nwDicts._doOpenInstallLocation()
-        assert SHARED.alert is None
+        assert SHARED.lastAlert == ""
 
     # Fail Open Dir
     nwDicts.inPath.setText("/foo/bar")
     nwDicts._doOpenInstallLocation()
-    assert SHARED.alert is not None
-    assert SHARED.alert.logMessage == "Path not found."
+    assert SHARED.lastAlert == "Path not found."
     nwDicts.inPath.setText(str(fncPath))
 
     # Create Mock Dicts