[Backport main] Automatically remove macro from toolbar on uninstall

AddonManagerTest/app/test_uninstaller.py

@@ -22,6 +22,7 @@
 """Contains the unit test class for addonmanager_uninstaller.py non-GUI functionality."""
 
 import functools
+import json
 import os
 from stat import S_IREAD, S_IRGRP, S_IROTH, S_IWUSR
 import tempfile
@@ -364,6 +365,42 @@ def test_remove_macro_with_files(self):
             self.assertIn("success", self.signals_caught)
             self.assertIn("finished", self.signals_caught)
 
+    def test_remove_macro_removes_generated_toolbar_icon(self):
+        with tempfile.TemporaryDirectory() as temp_dir:
+            self.test_object.installation_location = temp_dir
+            self.mock_addon.macro.icon = "mock_icon_test.svg"
+            self.mock_addon.macro.install(temp_dir)
+            toolbar_icon = os.path.join(temp_dir, "MockMacro_icon.svg")
+            with open(toolbar_icon, "wb") as f:
+                f.write(b"Fake icon data generated by the toolbar button installer")
+            self.test_object.run()
+            self.assertFalse(
+                os.path.exists(toolbar_icon),
+                "Expected the generated toolbar icon to be removed, and it was not",
+            )
+            self.assertNotIn("failure", self.signals_caught)
+            self.assertIn("success", self.signals_caught)
+
+    def test_remove_macro_with_manifest_removes_generated_toolbar_icon(self):
+        with tempfile.TemporaryDirectory() as temp_dir:
+            self.test_object.installation_location = temp_dir
+            self.mock_addon.macro.xpm = "/*Fake XPM data*/"
+            self.mock_addon.macro.install(temp_dir)
+            macro_file = os.path.join(temp_dir, self.mock_addon.macro.filename)
+            manifest_file = macro_file + ".manifest"
+            with open(manifest_file, "w", encoding="utf-8") as f:
+                f.write(json.dumps([macro_file]))
+            toolbar_icon = os.path.join(temp_dir, "MockMacro_icon.xpm")
+            self.assertTrue(os.path.exists(toolbar_icon))
+            self.test_object.run()
+            self.assertFalse(
+                os.path.exists(toolbar_icon),
+                "Expected the generated toolbar icon to be removed, and it was not",
+            )
+            self.assertFalse(os.path.exists(manifest_file))
+            self.assertNotIn("failure", self.signals_caught)
+            self.assertIn("success", self.signals_caught)
+
     def test_remove_nonexistent_macro(self):
         with tempfile.TemporaryDirectory() as temp_dir:
             self.test_object.installation_location = temp_dir

AddonManagerTest/gui/test_uninstaller_gui.py

@@ -21,6 +21,7 @@
 
 import functools
 import unittest
+from unittest.mock import MagicMock, patch
 
 try:
     from PySide import QtCore, QtWidgets
@@ -38,7 +39,7 @@
     FakeWorker,
     MockThread,
 )
-from AddonManagerTest.app.mocks import MockAddon
+from AddonManagerTest.app.mocks import MockAddon, MockMacro
 
 from addonmanager_uninstaller_gui import AddonUninstallerGUI
 
@@ -132,6 +133,43 @@ def test_failure_dialog(self):
         self.assertTrue(dialog_watcher.dialog_found, "Failed to find the expected dialog box")
         self.assertTrue(dialog_watcher.button_found, "Failed to find the expected button")
 
+    def test_toolbar_button_removed_for_macro(self):
+        macro_addon = MockAddon()
+        macro_addon.macro = MockMacro()
+        uninstaller_gui = AddonUninstallerGUI(macro_addon)
+        with patch("addonmanager_uninstaller_gui.fci.FreeCADGui", MagicMock()):
+            with patch("addonmanager_uninstaller_gui.ToolbarAdapter") as toolbar_adapter:
+                uninstaller_gui._remove_toolbar_button()
+        toolbar_adapter.return_value.remove_custom_toolbar_button.assert_called_once_with(
+            macro_addon.macro.filename
+        )
+
+    def test_toolbar_button_not_removed_for_non_macro(self):
+        with patch("addonmanager_uninstaller_gui.fci.FreeCADGui", MagicMock()):
+            with patch("addonmanager_uninstaller_gui.ToolbarAdapter") as toolbar_adapter:
+                self.uninstaller_gui._remove_toolbar_button()
+        toolbar_adapter.assert_not_called()
+
+    def test_toolbar_button_not_removed_without_gui(self):
+        macro_addon = MockAddon()
+        macro_addon.macro = MockMacro()
+        uninstaller_gui = AddonUninstallerGUI(macro_addon)
+        with patch("addonmanager_uninstaller_gui.fci.FreeCADGui", None):
+            with patch("addonmanager_uninstaller_gui.ToolbarAdapter") as toolbar_adapter:
+                uninstaller_gui._remove_toolbar_button()
+        toolbar_adapter.assert_not_called()
+
+    def test_toolbar_button_removal_failure_is_not_fatal(self):
+        macro_addon = MockAddon()
+        macro_addon.macro = MockMacro()
+        uninstaller_gui = AddonUninstallerGUI(macro_addon)
+        with patch("addonmanager_uninstaller_gui.fci.FreeCADGui", MagicMock()):
+            with patch(
+                "addonmanager_uninstaller_gui.ToolbarAdapter",
+                side_effect=RuntimeError("Unit test failure"),
+            ):
+                uninstaller_gui._remove_toolbar_button()  # Should not raise
+
     def test_finalize(self):
         self.uninstaller_gui.finished.connect(functools.partial(self.catch_signal, "finished"))
         self.uninstaller_gui.worker_thread = MockThread()

addonmanager_uninstaller.py

@@ -281,15 +281,29 @@ def _get_files_to_remove(self) -> List[str]:
                 manifest_data = f.read()
                 manifest = json.loads(manifest_data)
                 manifest.append(manifest_file)  # Remove the manifest itself as well
-                return manifest
+                return manifest + self._get_toolbar_icon_files()
         files_to_remove = [self.addon_to_remove.macro.filename]
         if self.addon_to_remove.macro.icon:
             files_to_remove.append(self.addon_to_remove.macro.icon)
         if self.addon_to_remove.macro.xpm:
             files_to_remove.append(self.addon_to_remove.macro.name.replace(" ", "_") + "_icon.xpm")
         for f in self.addon_to_remove.macro.other_files:
             files_to_remove.append(f)
-        return files_to_remove
+        return files_to_remove + self._get_toolbar_icon_files()
+
+    def _get_toolbar_icon_files(self) -> List[str]:
+        """Get the names of the icon files that the toolbar button installer may have created for
+        this macro. Those files are created after the installation manifest is written, so they are
+        not listed in it."""
+        macro = self.addon_to_remove.macro
+        icon_files = []
+        if macro.icon:
+            _, ext = os.path.splitext(macro.icon)
+            extension = ext[1:].lower() if ext else "png"
+            icon_files.append(f"{macro.name}_icon.{extension}")
+        if macro.xpm:
+            icon_files.append(f"{macro.name}_icon.xpm")
+        return icon_files
 
     @staticmethod
     def _cleanup_directories(directories):

addonmanager_uninstaller_gui.py

@@ -33,6 +33,7 @@
     except ImportError:
         from PySide2 import QtCore, QtWidgets  # Fall back to Qt5
 
+from addonmanager_toolbar_adapter import ToolbarAdapter
 from addonmanager_uninstaller import AddonUninstaller, MacroUninstaller
 import addonmanager_utilities as utils
 
@@ -117,6 +118,7 @@ def _succeeded(self, addon):
         self.dialog_timer.stop()
         if self.progress_dialog:
             self.progress_dialog.hide()
+        self._remove_toolbar_button()
         MessageDialog.show_modal(
             MessageDialog.DialogType.INFO,
             "AddonManager_UninstallCompleteDialog",
@@ -126,6 +128,27 @@ def _succeeded(self, addon):
         )
         self._finalize()
 
+    def _remove_toolbar_button(self):
+        """Remove the custom toolbar button that the Addon Manager created for a macro, if there
+        is one. Does nothing for addons that are not macros, and does nothing when the FreeCAD GUI
+        is not running."""
+        if fci.FreeCADGui is None:
+            return
+        macro = getattr(self.addon_to_remove, "macro", None)
+        if macro is None or not getattr(macro, "filename", ""):
+            return
+        # pylint: disable=broad-exception-caught
+        try:
+            ToolbarAdapter().remove_custom_toolbar_button(macro.filename)
+        except Exception as e:
+            fci.Console.PrintWarning(
+                translate(
+                    "AddonsInstaller",
+                    "Failed to remove the toolbar button for macro {}",
+                ).format(self.addon_to_remove.display_name)
+                + f": {e}\n"
+            )
+
     def _failed(self, addon, message):
         """Callback for failed or partially failed removal"""
         self.dialog_timer.stop()