Handle bad project names (#527)

* Do not allow empty project name.

* Handle project names that begin with number.

* Add empty project name tests.

* Add tests.

* Fix test.

* Apply review feedback.

Author: JoeZiminski

datashuttle/datashuttle_class.py

@@ -1364,6 +1364,10 @@ def _error_on_base_project_name(self, project_name):
                 "The project name must contain alphanumeric characters only.",
                 ValueError,
             )
+        if project_name == "":
+            utils.log_and_raise_error(
+                "The project name cannot be empty.", NeuroBlueprintError
+            )
 
     def _log_successful_config_change(self, message: bool = False) -> None:
         """Log the entire config at the time of config change.

datashuttle/tui/screens/project_selector.py

@@ -50,15 +50,18 @@ def compose(self) -> ComposeResult:
         yield Header(id="project_select_header")
         yield Button("Main Menu", id="all_main_menu_buttons")
         yield Container(
-            *[Button(name, id=name) for name in self.project_names],
+            *[
+                Button(name, id=self.name_to_id(name))
+                for name in self.project_names
+            ],
             id="project_select_top_container",
         )
 
     def on_button_pressed(self, event: Button.Pressed) -> None:
         """Handle a button press on ProjectSelectorScreen."""
-        if event.button.id in self.project_names:
-            project_name = event.button.id
+        project_name = self.id_to_name(event.button.id)
 
+        if project_name in self.project_names:
             interface = Interface()
             success, output = interface.select_existing_project(project_name)
 
@@ -69,3 +72,17 @@ def on_button_pressed(self, event: Button.Pressed) -> None:
 
         elif event.button.id == "all_main_menu_buttons":
             self.dismiss(False)
+
+    @staticmethod
+    def name_to_id(name: str):
+        """Convert the project name to a textual ID.
+
+        Textual ids cannot start with a number, so ensure
+        all ids are prefixed with text instead of the project name.
+        """
+        return f"safety_prefix_{name}"
+
+    @staticmethod
+    def id_to_name(id: str):
+        """See `name_to_id()`."""
+        return id[len("safety_prefix_") :]

tests/tests_integration/test_configs.py

@@ -6,7 +6,10 @@
 
 from datashuttle import DataShuttle
 from datashuttle.utils import getters
-from datashuttle.utils.custom_exceptions import ConfigError
+from datashuttle.utils.custom_exceptions import (
+    ConfigError,
+    NeuroBlueprintError,
+)
 
 
 class TestConfigs(BaseTest):
@@ -45,6 +48,17 @@ def test_warning_on_startup(self, no_cfg_project):
             "Use make_config_file() to setup before continuing."
         )
 
+    def test_empty_project_name(self, tmp_path):
+        """
+        Empty project names ("") are not allowed.
+        """
+        os.chdir(tmp_path)  # avoids messy backup folder creation
+
+        with pytest.raises(NeuroBlueprintError) as e:
+            DataShuttle("")
+
+        assert str(e.value) == "The project name cannot be empty."
+
     @pytest.mark.parametrize(
         "bad_pattern",
         [

tests/tests_tui/test_tui_configs.py

@@ -360,6 +360,43 @@ async def test_bad_configs_screen_input(self, empty_project_paths):
             )
             await pilot.pause()
 
+    # -------------------------------------------------------------------------
+    # Test project name is number
+    # -------------------------------------------------------------------------
+
+    @pytest.mark.asyncio
+    async def test_project_name_is_number(self, empty_project_paths):
+        """
+        Make a project that has a number name, and check the project screen
+        can be loaded.
+        """
+        app = TuiApp()
+        async with app.run_test(size=self.tui_size()) as pilot:
+            # Set up a project with a numerical project name
+            project_name = "123"
+
+            await self.scroll_to_click_pause(
+                pilot, "#mainwindow_new_project_button"
+            )
+
+            await self.fill_input(pilot, "#configs_name_input", project_name)
+            await self.fill_input(pilot, "#configs_local_path_input", "a")
+            await self.fill_input(pilot, "#configs_central_path_input", "b")
+            await self.scroll_to_click_pause(
+                pilot, "#configs_save_configs_button"
+            )
+
+            # Go back to main menu and load the project screen
+            await self.close_messagebox(pilot)
+
+            await self.scroll_to_click_pause(pilot, "#all_main_menu_buttons")
+
+            await self.check_and_click_onto_existing_project(
+                pilot, project_name
+            )
+
+            await pilot.pause()
+
     # -------------------------------------------------------------------------
     # Helpers
     # -------------------------------------------------------------------------

tests/tests_tui/tui_base.py

@@ -166,7 +166,7 @@ async def check_and_click_onto_existing_project(self, pilot, project_name):
         assert len(pilot.app.screen.project_names) == 1
         assert project_name in pilot.app.screen.project_names
 
-        await pilot.click(f"#{project_name}")
+        await pilot.click(f"#safety_prefix_{project_name}")
         await pilot.pause()
 
         assert isinstance(pilot.app.screen, ProjectManagerScreen)