Merge pull request #464 from kdmukai/settingsqr_status_update

[bugfix] Prevent SettingsQR from trying to enable Persistent Settings when SD card is not inserted

src/seedsigner/gui/screens/scan_screens.py

@@ -174,36 +174,3 @@ def _run(self):
                     self.camera.stop_video_stream_mode()
                     break
 
-
-
-@dataclass
-class SettingsUpdatedScreen(ButtonListScreen):
-    config_name: str = None
-    title: str = "Settings QR"
-    is_bottom_list: bool = True
-
-    def __post_init__(self):
-        # Customize defaults
-        self.button_data = ["Home"]
-        self.show_back_button = False
-
-        super().__post_init__()
-
-        start_y = self.top_nav.height + 20
-        if self.config_name:
-            self.config_name_textarea = TextArea(
-                text=f'"{self.config_name}"',
-                is_text_centered=True,
-                auto_line_break=True,
-                screen_y=start_y
-            )
-            self.components.append(self.config_name_textarea)
-            start_y = self.config_name_textarea.screen_y + 50
-
-        self.components.append(TextArea(
-            text="Settings imported successfully!",
-            is_text_centered=True,
-            auto_line_break=True,
-            screen_y=start_y
-        ))
-

src/seedsigner/gui/screens/settings_screens.py

@@ -296,3 +296,36 @@ def __post_init__(self):
             supersampling_factor=1,
             screen_y=self.components[-1].screen_y + self.components[-1].height + GUIConstants.COMPONENT_PADDING
         ))
+
+
+
+@dataclass
+class SettingsQRConfirmationScreen(ButtonListScreen):
+    config_name: str = None
+    title: str = "Settings QR"
+    status_message: str = "Settings updated..."
+    is_bottom_list: bool = True
+
+    def __post_init__(self):
+        # Customize defaults
+        self.button_data = ["Home"]
+        self.show_back_button = False
+        super().__post_init__()
+
+        start_y = self.top_nav.height + 20
+        if self.config_name:
+            self.config_name_textarea = TextArea(
+                text=f'"{self.config_name}"',
+                is_text_centered=True,
+                auto_line_break=True,
+                screen_y=start_y
+            )
+            self.components.append(self.config_name_textarea)
+            start_y = self.config_name_textarea.screen_y + 50
+
+        self.components.append(TextArea(
+            text=self.status_message,
+            is_text_centered=True,
+            auto_line_break=True,
+            screen_y=start_y
+        ))

src/seedsigner/models/settings.py

@@ -87,6 +87,12 @@ def parse_settingsqr(cls, data: str) -> tuple[str, dict]:
                 values = value
             for v in values:
                 if v not in [opt[0] for opt in settings_entry.selection_options]:
+                    if settings_entry.attr_name == SettingsConstants.SETTING__PERSISTENT_SETTINGS and v == SettingsConstants.OPTION__ENABLED:
+                        # Special case: trying to enable Persistent Settings when 
+                        # DISABLED is the only option allowed (because the SD card is not
+                        # inserted. Explicitly set to DISABLED.
+                        value = SettingsConstants.OPTION__DISABLED
+                        break
                     raise InvalidSettingsQRData(f"""{abbreviated_name} = '{v}' is not valid""")
 
             updated_settings[settings_entry.attr_name] = value

src/seedsigner/views/settings_views.py

@@ -1,5 +1,6 @@
 import logging
 from seedsigner.gui.components import SeedSignerIconConstants
+from seedsigner.hardware.microsd import MicroSD
 
 from .view import View, Destination, MainMenuView
 
@@ -195,14 +196,21 @@ def __init__(self, data: str):
         # May raise an Exception which will bubble up to the Controller to display to the
         # user.
         self.config_name, settings_update_dict = Settings.parse_settingsqr(data)
+
         self.settings.update(settings_update_dict)
-
+
+        if MicroSD.get_instance().is_inserted and self.settings.get_value(SettingsConstants.SETTING__PERSISTENT_SETTINGS) == SettingsConstants.OPTION__ENABLED:
+            self.status_message = "Persistent Settings enabled. Settings saved to SD card."
+        else:
+            self.status_message = "Settings updated in temporary memory"
+
 
     def run(self):
-        from seedsigner.gui.screens.scan_screens import SettingsUpdatedScreen
+        from seedsigner.gui.screens.settings_screens import SettingsQRConfirmationScreen
         self.run_screen(
-            SettingsUpdatedScreen,
-            config_name=self.config_name
+            SettingsQRConfirmationScreen,
+            config_name=self.config_name,
+            status_message=self.status_message,
         )
 
         # Only one exit point

tests/base.py

@@ -1,6 +1,6 @@
 import sys
 from dataclasses import dataclass
-from mock import MagicMock, patch
+from mock import MagicMock, Mock, patch
 from typing import Callable
 
 # Prevent importing modules w/Raspi hardware dependencies.
@@ -11,17 +11,28 @@
 sys.modules['seedsigner.views.screensaver'] = MagicMock()
 sys.modules['seedsigner.hardware.buttons'] = MagicMock()
 sys.modules['seedsigner.hardware.camera'] = MagicMock()
-sys.modules['seedsigner.hardware.microsd'] = MagicMock()
 
 from seedsigner.controller import Controller, FlowBasedTestException, StopFlowBasedTest
 from seedsigner.gui.screens.screen import RET_CODE__BACK_BUTTON, RET_CODE__POWER_BUTTON
+from seedsigner.hardware.microsd import MicroSD
 from seedsigner.models.settings import Settings
 from seedsigner.views.view import Destination, MainMenuView, View
 
 
 
+
 class BaseTest:
 
+    class MockMicroSD(Mock):
+        """
+        A test suite-friendly replacement for `MicroSD` that gives a test explicit
+        control over the reported state of the SD card.
+        """
+        # Tests are free to directly manipulate this attribute as needed (it's reset to
+        # True before each test in `BaseTest.setup_method()`).
+        is_inserted: bool = True
+
+
     @classmethod
     def setup_class(cls):
         # Ensure there are no on-disk artifacts after running tests.
@@ -30,6 +41,13 @@ def setup_class(cls):
         # Mock out the loading screen so it can't spawn. View classes must import locally!
         patch('seedsigner.gui.screens.screen.LoadingScreenThread').start()
 
+        # Instantiate the mocked MicroSD; hold on to the instance so tests can manipulate
+        # it later.
+        cls.mock_microsd = BaseTest.MockMicroSD()
+
+        # And mock it over `MicroSD`'s instance
+        MicroSD.get_instance = Mock(return_value=cls.mock_microsd)
+
 
     @classmethod
     def teardown_class(cls):
@@ -62,11 +80,12 @@ def reset_controller(cls):
 
 
     def setup_method(self):
-        """ Guarantee a clean/default Controller and Settings state for each test case """
+        """ Guarantee a clean/default Controller, Settings, & MicroSD state for each test case """
         BaseTest.reset_controller()
         BaseTest.reset_settings()
         self.controller = Controller.get_instance()
         self.settings = Settings.get_instance()
+        self.mock_microsd.is_inserted = True
 
 
     def teardown_method(self):

tests/screenshot_generator/generator.py

@@ -110,10 +110,11 @@ def test_generate_screenshots(target_locale):
             continue
 
         settings_views_list.append((settings_views.SettingsEntryUpdateSelectionView, dict(attr_name=settings_entry.attr_name), f"SettingsEntryUpdateSelectionView_{settings_entry.attr_name}"))
-    settings_views_list.append(settings_views.IOTestView)
-    settings_views_list.append(settings_views.DonateView)
 
 
+    settingsqr_data_persistent = "settings::v1 name=Total_noob_mode persistent=E coords=spa,spd denom=thr network=M qr_density=M xpub_export=E sigs=ss scripts=nat xpub_details=E passphrase=E camera=0 compact_seedqr=E bip85=D priv_warn=E dire_warn=E partners=E"
+    settingsqr_data_not_persistent = "settings::v1 name=Ephemeral_noob_mode persistent=D coords=spa,spd denom=thr network=M qr_density=M xpub_export=E sigs=ss scripts=nat xpub_details=E passphrase=E camera=0 compact_seedqr=E bip85=D priv_warn=E dire_warn=E partners=E"
+
     screenshot_sections = {
         "Main Menu Views": [
             MainMenuView,
@@ -123,7 +124,6 @@ def test_generate_screenshots(target_locale):
             PowerOptionsView,
             RestartView,
             PowerOffView,
-            (settings_views.SettingsIngestSettingsQRView, dict(data="settings::v1 name=Uncle_Jim's_noob_mode")),
         ],
         "Seed Views": [
             seed_views.SeedsMenuView,
@@ -215,7 +215,12 @@ def test_generate_screenshots(target_locale):
             tools_views.ToolsAddressExplorerAddressListView,
             #tools_views.ToolsAddressExplorerAddressView,
         ],
-        "Settings Views": settings_views_list,
+        "Settings Views": settings_views_list + [
+            settings_views.IOTestView,
+            settings_views.DonateView,
+            (settings_views.SettingsIngestSettingsQRView, dict(data=settingsqr_data_persistent), "SettingsIngestSettingsQRView_persistent"),
+            (settings_views.SettingsIngestSettingsQRView, dict(data=settingsqr_data_not_persistent), "SettingsIngestSettingsQRView_not_persistent"),
+        ],
         "Misc Error Views": [
             NotYetImplementedView,
             (UnhandledExceptionView, dict(error=UnhandledExceptionViewFood)),
@@ -227,7 +232,7 @@ def test_generate_screenshots(target_locale):
                 text="QRCode is invalid or is a data format not yet supported.",
                 button_text="Back",
             )),
-        ],
+        ]
     }
 
     readme = f"""# SeedSigner Screenshots\n"""

tests/test_flows_settings.py

@@ -1,18 +1,21 @@
 import os
+from typing import Callable
+
+from mock import PropertyMock, patch
 
 # Must import test base before the Controller
 from base import FlowTest, FlowStep
 
 from seedsigner.models.settings import Settings
 from seedsigner.models.settings_definition import SettingsDefinition, SettingsConstants
 from seedsigner.gui.screens.screen import RET_CODE__BACK_BUTTON
+from seedsigner.hardware.microsd import MicroSD
 from seedsigner.views.view import MainMenuView
-from seedsigner.views import settings_views
+from seedsigner.views import scan_views, settings_views
 
 
 
 class TestSettingsFlows(FlowTest):
-
     def test_persistent_settings(self):
         """ Basic flow from MainMenuView to enable/disable persistent settings """
         # Which option are we testing?
@@ -67,3 +70,69 @@ def test_donate(self):
             FlowStep(settings_views.DonateView),
             FlowStep(settings_views.SettingsMenuView),
         ])
+
+
+    def test_settingsqr(self):
+        """ 
+        Scanning a SettingsQR should present the success screen and then return to
+        MainMenuView.
+        """
+        def load_persistent_settingsqr_into_decoder(view: scan_views.ScanView):
+            settingsqr_data_persistent: str = "settings::v1 name=Total_noob_mode persistent=E coords=spa,spd denom=thr network=M qr_density=M xpub_export=E sigs=ss scripts=nat xpub_details=E passphrase=E camera=0 compact_seedqr=E bip85=D priv_warn=E dire_warn=E partners=E"
+            view.decoder.add_data(settingsqr_data_persistent)
+
+        def load_not_persistent_settingsqr_into_decoder(view: scan_views.ScanView):
+            settingsqr_data_not_persistent: str = "settings::v1 name=Ephemeral_noob_mode persistent=D coords=spa,spd denom=thr network=M qr_density=M xpub_export=E sigs=ss scripts=nat xpub_details=E passphrase=E camera=0 compact_seedqr=E bip85=D priv_warn=E dire_warn=E partners=E"
+            view.decoder.add_data(settingsqr_data_not_persistent)
+
+        def _run_test(initial_setting_state: str, load_settingsqr_into_decoder: Callable, expected_setting_state: str):
+            self.settings.set_value(SettingsConstants.SETTING__PERSISTENT_SETTINGS, initial_setting_state)
+            self.run_sequence([
+                FlowStep(MainMenuView, button_data_selection=MainMenuView.SCAN),
+                FlowStep(scan_views.ScanView, before_run=load_settingsqr_into_decoder),  # simulate read message QR; ret val is ignored
+                FlowStep(settings_views.SettingsIngestSettingsQRView),   # ret val is ignored
+                FlowStep(MainMenuView),
+            ])
+
+            assert self.settings.get_value(SettingsConstants.SETTING__PERSISTENT_SETTINGS) == expected_setting_state
+
+
+        # First load a SettingsQR that enables persistent settings
+        self.mock_microsd.is_inserted = True
+        assert MicroSD.get_instance().is_inserted is True
+
+        _run_test(
+            initial_setting_state=SettingsConstants.OPTION__DISABLED,
+            load_settingsqr_into_decoder=load_persistent_settingsqr_into_decoder,
+            expected_setting_state=SettingsConstants.OPTION__ENABLED
+        )
+
+        # Then one that disables it
+        _run_test(
+            initial_setting_state=SettingsConstants.OPTION__ENABLED,
+            load_settingsqr_into_decoder=load_not_persistent_settingsqr_into_decoder,
+            expected_setting_state=SettingsConstants.OPTION__DISABLED
+        )
+
+        # Now try to enable persistent settings when the SD card is not inserted
+        self.mock_microsd.is_inserted = False
+        assert MicroSD.get_instance().is_inserted is False
+
+        # Have to jump through some hoops to completely simulate the SD card being
+        # removed; we need Settings to restrict Persistent Settings to only allow
+        # DISABLED.
+        with patch('seedsigner.models.settings.Settings.HOSTNAME', new_callable=PropertyMock) as mock_hostname:
+            # Must identify itself as SeedSigner OS to trigger the SD card removal logic
+            mock_hostname.return_value = Settings.SEEDSIGNER_OS
+            Settings.handle_microsd_state_change(MicroSD.ACTION__REMOVED)
+
+        selection_options = SettingsDefinition.get_settings_entry(SettingsConstants.SETTING__PERSISTENT_SETTINGS).selection_options
+        assert len(selection_options) == 1
+        assert selection_options[0][0] == SettingsConstants.OPTION__DISABLED
+        assert self.settings.get_value(SettingsConstants.SETTING__PERSISTENT_SETTINGS) == SettingsConstants.OPTION__DISABLED
+
+        _run_test(
+            initial_setting_state=SettingsConstants.OPTION__DISABLED,
+            load_settingsqr_into_decoder=load_persistent_settingsqr_into_decoder,
+            expected_setting_state=SettingsConstants.OPTION__DISABLED
+        )

tests/test_settings.py

@@ -8,6 +8,7 @@
 class TestSettings(BaseTest):
     @classmethod
     def setup_class(cls):
+        super().setup_class()
         cls.settings = Settings.get_instance()