Moved SettingVisibilityPreset loading to it's own class

Since there was so much debate regarding the unit testing of the visiblity presets, i had another look at it.
The old version was almost untestable because all functionalities were mushed together into a single class.

CURA-5734

cura/CuraApplication.py

@@ -701,10 +701,8 @@ class CuraApplication(QtApplication):
         self._print_information = PrintInformation.PrintInformation(self)
         self._cura_actions = CuraActions.CuraActions(self)
 
-        # Initialize setting visibility presets model
+        # Initialize setting visibility presets model.
         self._setting_visibility_presets_model = SettingVisibilityPresetsModel(self)
-        default_visibility_profile = self._setting_visibility_presets_model.getItem(0)
-        self.getPreferences().setDefault("general/visible_settings", ";".join(default_visibility_profile["settings"]))
 
         # Detect in which mode to run and execute that mode
         if self._is_headless:

cura/Machines/Models/SettingVisibilityPresetsModel.py

@@ -1,12 +1,13 @@
 # Copyright (c) 2018 Ultimaker B.V.
 # Cura is released under the terms of the LGPLv3 or higher.
 
-from typing import Optional
+from typing import Optional, List
 import os
 import urllib.parse
 from configparser import ConfigParser
 
-from PyQt5.QtCore import pyqtProperty, Qt, pyqtSignal, pyqtSlot
+from PyQt5.QtCore import pyqtProperty, Qt, pyqtSignal, pyqtSlot, QObject
+
 
 from UM.Application import Application
 from UM.Logger import Logger
@@ -15,121 +16,101 @@ from UM.Resources import Resources
 from UM.MimeTypeDatabase import MimeTypeDatabase, MimeTypeNotFoundError
 
 from UM.i18n import i18nCatalog
+from cura.Settings.SettingVisibilityPreset import SettingVisibilityPreset
+
 catalog = i18nCatalog("cura")
 
 
-class SettingVisibilityPresetsModel(ListModel):
-    IdRole = Qt.UserRole + 1
-    NameRole = Qt.UserRole + 2
-    SettingsRole = Qt.UserRole + 3
+class SettingVisibilityPresetsModel(QObject):
+    onItemsChanged = pyqtSignal()
+    activePresetChanged = pyqtSignal()
 
     def __init__(self, parent = None):
         super().__init__(parent)
-        self.addRoleName(self.IdRole, "id")
-        self.addRoleName(self.NameRole, "name")
-        self.addRoleName(self.SettingsRole, "settings")
 
+        self._items = []  # type: List[SettingVisibilityPreset]
         self._populate()
-        basic_item = self.items[1]
-        basic_visibile_settings = ";".join(basic_item["settings"])
+
+        basic_item = self._getVisibilityPresetById("basic")
+        basic_visibile_settings = ";".join(basic_item.settings)
 
         self._preferences = Application.getInstance().getPreferences()
+
         # Preference to store which preset is currently selected
         self._preferences.addPreference("cura/active_setting_visibility_preset", "basic")
+
         # Preference that stores the "custom" set so it can always be restored (even after a restart)
         self._preferences.addPreference("cura/custom_visible_settings", basic_visibile_settings)
         self._preferences.preferenceChanged.connect(self._onPreferencesChanged)
 
-        self._active_preset_item = self._getItem(self._preferences.getValue("cura/active_setting_visibility_preset"))
+        self._active_preset_item = self._getVisibilityPresetById(self._preferences.getValue("cura/active_setting_visibility_preset"))
+
         # Initialize visible settings if it is not done yet
         visible_settings = self._preferences.getValue("general/visible_settings")
         if not visible_settings:
-            self._preferences.setValue("general/visible_settings", ";".join(self._active_preset_item["settings"]))
+            self._preferences.setValue("general/visible_settings", ";".join(self._active_preset_item.settings))
+
         else:
             self._onPreferencesChanged("general/visible_settings")
 
         self.activePresetChanged.emit()
 
-    def _getItem(self, item_id: str) -> Optional[dict]:
+    def _getVisibilityPresetById(self, item_id: str) -> Optional[SettingVisibilityPreset]:
         result = None
-        for item in self.items:
-            if item["id"] == item_id:
+        for item in self._items:
+            if item.id == item_id:
                 result = item
                 break
         return result
 
     def _populate(self) -> None:
         from cura.CuraApplication import CuraApplication
-        items = []
-        for file_path in Resources.getAllResourcesOfType(CuraApplication.ResourceTypes.SettingVisibilityPreset):
-            try:
-                mime_type = MimeTypeDatabase.getMimeTypeForFile(file_path)
-            except MimeTypeNotFoundError:
-                Logger.log("e", "Could not determine mime type of file %s", file_path)
-                continue
+        items = []  # type: List[SettingVisibilityPreset]
 
-            item_id = urllib.parse.unquote_plus(mime_type.stripExtension(os.path.basename(file_path)))
-            if not os.path.isfile(file_path):
-                Logger.log("e", "[%s] is not a file", file_path)
-                continue
-
-            parser = ConfigParser(allow_no_value = True)  # accept options without any value,
+        custom_preset = SettingVisibilityPreset(id = "custom", name = "Custom selection", weight = -100)
+        items.append(custom_preset)
+        for file_path in Resources.getAllResourcesOfType(CuraApplication.ResourceTypes.SettingVisibilityPreset):
+            setting_visibility_preset = SettingVisibilityPreset()
             try:
-                parser.read([file_path])
-                if not parser.has_option("general", "name") or not parser.has_option("general", "weight"):
-                    continue
-
-                settings = []
-                for section in parser.sections():
-                    if section == 'general':
-                        continue
-
-                    settings.append(section)
-                    for option in parser[section].keys():
-                        settings.append(option)
-
-                items.append({
-                    "id": item_id,
-                    "name": catalog.i18nc("@action:inmenu", parser["general"]["name"]),
-                    "weight": parser["general"]["weight"],
-                    "settings": settings,
-                })
-
+                setting_visibility_preset.loadFromFile(file_path)
             except Exception:
                 Logger.logException("e", "Failed to load setting preset %s", file_path)
 
-        items.sort(key = lambda k: (int(k["weight"]), k["id"]))
-        # Put "custom" at the top
-        items.insert(0, {"id": "custom",
-                         "name": "Custom selection",
-                         "weight": -100,
-                         "settings": []})
+            items.append(setting_visibility_preset)
+
+        # Sort them on weight (and if that fails, use ID)
+        items.sort(key = lambda k: (int(k.weight), k.id))
 
         self.setItems(items)
 
+    @pyqtProperty("QVariantList", notify = onItemsChanged)
+    def items(self):
+        return self._items
+
+    def setItems(self, items: List[SettingVisibilityPreset]) -> None:
+        if self._items != items:
+            self._items = items
+            self.onItemsChanged.emit()
+
     @pyqtSlot(str)
-    def setActivePreset(self, preset_id: str):
-        if preset_id == self._active_preset_item["id"]:
+    def setActivePreset(self, preset_id: str) -> None:
+        if preset_id == self._active_preset_item.id:
             Logger.log("d", "Same setting visibility preset [%s] selected, do nothing.", preset_id)
             return
 
-        preset_item = None
-        for item in self.items:
-            if item["id"] == preset_id:
-                preset_item = item
-                break
+        preset_item = self._getVisibilityPresetById(preset_id)
         if preset_item is None:
             Logger.log("w", "Tried to set active preset to unknown id [%s]", preset_id)
             return
 
-        need_to_save_to_custom = self._active_preset_item["id"] == "custom" and preset_id != "custom"
+        need_to_save_to_custom = self._active_preset_item.id == "custom" and preset_id != "custom"
         if need_to_save_to_custom:
             # Save the current visibility settings to custom
             current_visibility_string = self._preferences.getValue("general/visible_settings")
             if current_visibility_string:
                 self._preferences.setValue("cura/custom_visible_settings", current_visibility_string)
 
-        new_visibility_string = ";".join(preset_item["settings"])
+        new_visibility_string = ";".join(preset_item.settings)
         if preset_id == "custom":
             # Get settings from the stored custom data
             new_visibility_string = self._preferences.getValue("cura/custom_visible_settings")
@@ -141,11 +122,9 @@ class SettingVisibilityPresetsModel(ListModel):
         self._active_preset_item = preset_item
         self.activePresetChanged.emit()
 
-    activePresetChanged = pyqtSignal()
-
     @pyqtProperty(str, notify = activePresetChanged)
     def activePreset(self) -> str:
-        return self._active_preset_item["id"]
+        return self._active_preset_item.id
 
     def _onPreferencesChanged(self, name: str) -> None:
         if name != "general/visible_settings":
@@ -158,25 +137,26 @@ class SettingVisibilityPresetsModel(ListModel):
 
         visibility_set = set(visibility_string.split(";"))
         matching_preset_item = None
-        for item in self.items:
-            if item["id"] == "custom":
+        for item in self._items:
+            if item.id == "custom":
                 continue
-            if set(item["settings"]) == visibility_set:
+            if set(item.settings) == visibility_set:
                 matching_preset_item = item
                 break
 
         item_to_set = self._active_preset_item
         if matching_preset_item is None:
             # The new visibility setup is "custom" should be custom
-            if self._active_preset_item["id"] == "custom":
+            if self._active_preset_item.id == "custom":
                 # We are already in custom, just save the settings
                 self._preferences.setValue("cura/custom_visible_settings", visibility_string)
             else:
-                item_to_set = self.items[0]  # 0 is custom
+                # We need to move to custom preset.
+                item_to_set = self._getVisibilityPresetById("custom")
         else:
             item_to_set = matching_preset_item
 
-        if self._active_preset_item is None or self._active_preset_item["id"] != item_to_set["id"]:
+        if self._active_preset_item is None or self._active_preset_item.id != item_to_set.id:
             self._active_preset_item = item_to_set
-            self._preferences.setValue("cura/active_setting_visibility_preset", self._active_preset_item["id"])
+            self._preferences.setValue("cura/active_setting_visibility_preset", self._active_preset_item.id)
             self.activePresetChanged.emit()

cura/Settings/SettingVisibilityPreset.py

@@ -0,0 +1,87 @@
+import os
+import urllib.parse
+from configparser import ConfigParser
+from typing import List
+
+from PyQt5.QtCore import pyqtProperty, QObject, pyqtSignal
+
+from UM.Logger import Logger
+from UM.MimeTypeDatabase import MimeTypeDatabase, MimeTypeNotFoundError
+
+
+class SettingVisibilityPreset(QObject):
+    onSettingsChanged = pyqtSignal()
+    onNameChanged = pyqtSignal()
+    onWeightChanged = pyqtSignal()
+    onIdChanged = pyqtSignal()
+
+    def __init__(self, id: str = "", name: str = "" , weight: int = 0, parent = None) -> None:
+        super().__init__(parent)
+        self._settings = []  # type: List[str]
+        self._id = id
+        self._weight = weight
+        self._name = name
+
+    @pyqtProperty("QStringList", notify = onSettingsChanged)
+    def settings(self) -> List[str]:
+        return self._settings
+
+    @pyqtProperty(str, notify=onIdChanged)
+    def id(self) -> str:
+        return self._id
+
+    @pyqtProperty(int, notify=onWeightChanged)
+    def weight(self) -> int:
+        return self._weight
+
+    @pyqtProperty(str, notify=onNameChanged)
+    def name(self) -> str:
+        return self._name
+
+    def setName(self, name: str) -> None:
+        if name != self._name:
+            self._name = name
+            self.onNameChanged.emit()
+
+    def setId(self, id: int) -> None:
+        if id != self._id:
+            self._id = id
+            self.onIdChanged.emit()
+
+    def setWeight(self, weight: str) -> None:
+        if weight != self._weight:
+            self._weight = weight
+            self.onWeightChanged.emit()
+
+    def setSettings(self, settings: List[str]) -> None:
+        if settings != self._settings:
+            self._settings = settings
+            self.onSettingsChanged.emit()
+
+    def loadFromFile(self, file_path: str) -> None:
+        mime_type = MimeTypeDatabase.getMimeTypeForFile(file_path)
+
+        item_id = urllib.parse.unquote_plus(mime_type.stripExtension(os.path.basename(file_path)))
+        if not os.path.isfile(file_path):
+            Logger.log("e", "[%s] is not a file", file_path)
+            return None
+
+        parser = ConfigParser(allow_no_value=True)  # Accept options without any value,
+
+        parser.read([file_path])
+        if not parser.has_option("general", "name") or not parser.has_option("general", "weight"):
+            return None
+
+        settings = []  # type: List[str]
+        for section in parser.sections():
+            if section == "general":
+                continue
+
+            settings.append(section)
+            for option in parser[section].keys():
+                settings.append(option)
+        self.setSettings(settings)
+        self.setId(item_id)
+        self.setName(parser["general"]["name"])
+        self.setWeight(parser["general"]["weight"])
+

resources/qml/Menus/SettingVisibilityPresetsMenu.qml

@@ -18,17 +18,17 @@ Menu
 
     Instantiator
     {
-        model: settingVisibilityPresetsModel
+        model: settingVisibilityPresetsModel.items
 
         MenuItem
         {
-            text: model.name
+            text: modelData.name
             checkable: true
-            checked: model.id == settingVisibilityPresetsModel.activePreset
+            checked: modelData.id == settingVisibilityPresetsModel.activePreset
             exclusiveGroup: group
             onTriggered:
             {
-                settingVisibilityPresetsModel.setActivePreset(model.id);
+                settingVisibilityPresetsModel.setActivePreset(modelData.id);
             }
         }
 

resources/qml/Preferences/SettingVisibilityPage.qml

@@ -110,24 +110,23 @@ UM.PreferencesPage
                 right: parent.right
             }
 
-            model: settingVisibilityPresetsModel
+            model: settingVisibilityPresetsModel.items
             textRole: "name"
 
             currentIndex:
             {
-                // Load previously selected preset.
-                var index = settingVisibilityPresetsModel.find("id", settingVisibilityPresetsModel.activePreset)
-                if (index == -1)
-                {
-                    return 0
+                for(var i = 0; i < settingVisibilityPresetsModel.items.length; ++i) {
+                    if(settingVisibilityPresetsModel.items[i].id == settingVisibilityPresetsModel.activePreset) {
+                        currentIndex = i;
+                        return;
+                    }
                 }
-
-                return index
+                return -1
             }
 
             onActivated:
             {
-                var preset_id = settingVisibilityPresetsModel.getItem(index).id;
+                var preset_id = settingVisibilityPresetsModel.items[index].id;
                 settingVisibilityPresetsModel.setActivePreset(preset_id);
             }
         }