Add waiting screen to validate and centralise CentralWaitingScreen (#693)

* Fix: Centralise waiting screen into CentralWaitingScreen, add validate waiting screen (Fixes #604)

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fix: mypy type errors in validate_content.py

* Fix: save asyncio task reference and fix Worker return type

* Refactor: replace setattr call_from_thread with _update_logs_label method

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Small updates.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fix css.

* Fix linting.

* Fix waiting.

* Remove unused log label.

* Store asyncio to prevent (unlikely? impossible?) garbage collection.

* Fix tests.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: JoeZiminski <joseph.j.ziminski@gmail.com>

Author: 3 people

datashuttle/tui/css/tui_menu.tcss

@@ -551,7 +551,7 @@ ConfirmAndAwaitTransferPopup {
 
 /* Suggest next subject / session loading pop up --------------------------------------------------- */
 
-SearchingCentralForNextSubSesPopup {
+CentralWaitingScreen {
     align: center middle;
 }
 

datashuttle/tui/screens/modal_dialogs.py

@@ -11,7 +11,7 @@
     from textual.worker import Worker
 
     from datashuttle.tui.app import TuiApp
-    from datashuttle.utils.custom_types import InterfaceOutput, Prefix
+    from datashuttle.utils.custom_types import InterfaceOutput
 
 import platform
 from pathlib import Path
@@ -281,24 +281,27 @@ async def handle_transfer_and_update_ui_when_complete(self) -> None:
             self.app.show_modal_error_dialog(str(e))
 
 
-class SearchingCentralForNextSubSesPopup(ModalScreen):
-    """Show message and a loading indicator for suggesting the next subject or session.
+class CentralWaitingScreen(ModalScreen):
+    """Show a message and loading indicator while awaiting a slow central connection.
 
-    Used to await searching next sub/ses across including folders
-    present on the central machine. This search happens in a separate
-    thread to allow TUI to display the loading indicate without freezing.
-
-    Only displayed when the `include_central` flag is checked and the
-    connection method is "ssh".
+    Used when operations require network access (ssh, aws, gdrive), such as
+    suggesting the next sub/ses or validating the central project.
     """
 
-    def __init__(self, sub_or_ses: Prefix) -> None:
-        """Initialise SearchingCentralForNextSubSesPopup."""
+    def __init__(self, message: str) -> None:
+        """Initialise CentralWaitingScreen.
+
+        Parameters
+        ----------
+        message
+            Message to display while waiting.
+
+        """
         super().__init__()
-        self.message = f"Searching central for next {sub_or_ses}"
+        self.message = message
 
     def compose(self) -> ComposeResult:
-        """Add widgets to SearchingCentralForNextSubSesPopup."""
+        """Add widgets to CentralWaitingScreen."""
         yield Container(
             Label(self.message, id="searching_message_label"),
             LoadingIndicator(id="searching_animated_indicator"),

datashuttle/tui/shared/validate_content.py

@@ -1,16 +1,18 @@
 from __future__ import annotations
 
 import platform
-from typing import TYPE_CHECKING, Optional, Union, cast
+from typing import TYPE_CHECKING, Optional, Union
 
 if TYPE_CHECKING:
     from textual.app import ComposeResult
+    from textual.worker import Worker
 
     from datashuttle.tui.interface import Interface
     from datashuttle.tui.screens.project_manager import ProjectManagerScreen
 
 from pathlib import Path
 
+from textual import work
 from textual.containers import Container, Horizontal
 from textual.widgets import (
     Button,
@@ -64,6 +66,9 @@ def __init__(
 
         self.parent_class = parent_class
         self.interface = interface
+        self.validating_central_popup: (
+            modal_dialogs.CentralWaitingScreen | None
+        ) = None
 
     def compose(self) -> ComposeResult:
         """Set up the widgets for the container."""
@@ -108,7 +113,6 @@ def compose(self) -> ComposeResult:
                 id="validate_arguments_horizontal",
             ),
             RichLog(highlight=True, markup=True, id="validate_richlog"),
-            Label("", id="validate_logs_label"),
             Button("Validate", id="validate_validate_button"),
         ]
 
@@ -150,6 +154,7 @@ def on_button_pressed(self, event: Button.Pressed) -> None:
             )
 
         elif event.button.id == "validate_validate_button":
+            # Get settings from widgets
             select_value = self.query_one(
                 "#validate_top_level_folder_select"
             ).value
@@ -165,28 +170,33 @@ def on_button_pressed(self, event: Button.Pressed) -> None:
             ).value
 
             if self.interface:
+                # If we are in a project, and it has a central storage we
+                # will connect and, if it's a slow connection, show a waiting screen
                 if self.interface.project.is_local_project():
                     include_central = False
                 else:
                     include_central = self.query_one(
                         "#validate_include_central_checkbox"
                     ).value
 
-                success, output = self.interface.validate_project(
+                if include_central and self.interface.project.cfg[
+                    "connection_method"
+                ] in ["aws", "gdrive", "ssh"]:
+                    self.validating_central_popup = (
+                        modal_dialogs.CentralWaitingScreen(
+                            "Validating central project..."
+                        )
+                    )
+                    self.parent_class.mainwindow.push_screen(
+                        self.validating_central_popup
+                    )
+
+                self.run_validate_and_dismiss_popup(
                     top_level_folder=top_level_folder,
                     include_central=include_central,
                     strict_mode=strict_mode,
                     allow_letters_in_sub_ses_values=allow_letters_in_sub_ses_values,
                 )
-                if not success:
-                    self.parent_class.mainwindow.show_modal_error_dialog(
-                        cast("str", output)
-                    )
-                else:
-                    self.write_results_to_richlog(output)
-                    self.query_one(
-                        "#validate_logs_label"
-                    ).value = f"Logs output to: {self.interface.project.get_logging_path()}"
             else:
                 path_ = self.query_one("#validate_path_input").value
 
@@ -215,6 +225,65 @@ def set_select_path(self, path_):
         if path_:
             self.query_one("#validate_path_input").value = path_.as_posix()
 
+    @work(group="validate_async", exclusive=True)
+    async def run_validate_and_dismiss_popup(
+        self,
+        top_level_folder,
+        include_central,
+        strict_mode,
+        allow_letters_in_sub_ses_values,
+    ) -> None:
+        """Run validation in a worker thread and dismiss the waiting popup when done.
+
+        Decorated with ``@work`` so Textual owns the task lifecycle (no need to
+        hold an ``asyncio.Task`` reference to prevent premature GC), and so a
+        repeat button-press cancels the previous in-flight invocation via
+        ``exclusive=True``.
+        """
+        worker = self.validate_project_worker(
+            top_level_folder=top_level_folder,
+            include_central=include_central,
+            strict_mode=strict_mode,
+            allow_letters_in_sub_ses_values=allow_letters_in_sub_ses_values,
+        )
+        await worker.wait()
+        if self.validating_central_popup:
+            self._hide_validating_central_popup()
+
+    @work(exclusive=True, thread=True)
+    def validate_project_worker(
+        self,
+        top_level_folder,
+        include_central,
+        strict_mode,
+        allow_letters_in_sub_ses_values,
+    ) -> Worker[None]:
+        """Run validation in a separate thread to avoid freezing the TUI."""
+        assert self.interface is not None
+
+        success, output = self.interface.validate_project(
+            top_level_folder=top_level_folder,
+            include_central=include_central,
+            strict_mode=strict_mode,
+            allow_letters_in_sub_ses_values=allow_letters_in_sub_ses_values,
+        )
+
+        if not success:
+            if self.validating_central_popup:
+                self.app.call_from_thread(
+                    self._hide_validating_central_popup,
+                )
+            self.app.call_from_thread(
+                self.parent_class.mainwindow.show_modal_error_dialog,
+                output,
+            )
+        else:
+            self.app.call_from_thread(self.write_results_to_richlog, output)
+
+    def _hide_validating_central_popup(self):
+        self.validating_central_popup.dismiss()
+        self.validating_central_popup = None
+
     def write_results_to_richlog(self, results):
         """Display the validation results on the Rich Log widget."""
         text_log = self.query_one("#validate_richlog")

datashuttle/tui/tabs/create_folders.py

@@ -1,6 +1,5 @@
 from __future__ import annotations
 
-import asyncio
 from typing import TYPE_CHECKING, List, Optional
 
 if TYPE_CHECKING:
@@ -26,16 +25,14 @@
     CustomDirectoryTree,
     TreeAndInputTab,
 )
+from datashuttle.tui.screens import modal_dialogs
 from datashuttle.tui.screens.create_folder_settings import (
     CreateFoldersSettingsScreen,
 )
 from datashuttle.tui.screens.datatypes import (
     CreateDatatypeCheckboxes,
     DisplayedDatatypesScreen,
 )
-from datashuttle.tui.screens.modal_dialogs import (
-    SearchingCentralForNextSubSesPopup,
-)
 from datashuttle.tui.tooltips import get_tooltip
 from datashuttle.tui.utils.tui_decorators import (
     ClickInfo,
@@ -65,7 +62,7 @@ def __init__(self, mainwindow: TuiApp, interface: Interface) -> None:
         self.mainwindow = mainwindow
         self.interface = interface
         self.searching_central_popup_widget: (
-            SearchingCentralForNextSubSesPopup | None
+            modal_dialogs.CentralWaitingScreen | None
         ) = None
 
         self.click_info = ClickInfo()
@@ -318,7 +315,7 @@ def suggest_next_sub_ses(
         Shows a pop up screen in cases when searching for next sub/ses takes
         time such as searching central in SSH connection method.
 
-        Creates an asyncio task which handles the suggestion logic and
+        Spawns a Textual worker which handles the suggestion logic and
         dismissing the pop up.
 
         Parameters
@@ -342,17 +339,17 @@ def suggest_next_sub_ses(
             "connection_method"
         ] in ["aws", "gdrive", "ssh"]:
             self.searching_central_popup_widget = (
-                SearchingCentralForNextSubSesPopup(prefix)
+                modal_dialogs.CentralWaitingScreen(
+                    f"Searching central for next {prefix}"
+                )
             )
             self.mainwindow.push_screen(self.searching_central_popup_widget)
 
-        asyncio.create_task(
-            self.fill_suggestion_and_dismiss_popup(
-                prefix, input_id, include_central
-            ),
-            name=f"suggest_next_{prefix}_async_task",
+        self.fill_suggestion_and_dismiss_popup(
+            prefix, input_id, include_central
         )
 
+    @work(group="suggest_next_async", exclusive=True)
     async def fill_suggestion_and_dismiss_popup(
         self, prefix, input_id, include_central
     ) -> None:
@@ -364,6 +361,11 @@ async def fill_suggestion_and_dismiss_popup(
         Else, if the worker successfully exits, this function handles dismissal
         of the popup.
 
+        Decorated with ``@work`` so Textual owns the task lifecycle (no need to
+        hold an ``asyncio.Task`` reference to prevent premature GC), and so a
+        repeat invocation cancels the previous in-flight one via
+        ``exclusive=True``.
+
         see `suggest_next_sub_ses()` for parameters.
         """
         worker = self.fill_input_with_next_sub_or_ses_template(

tests/tests_tui/test_tui_validate.py

@@ -54,6 +54,7 @@ async def test_validate_on_project_manager_output(
             await self.scroll_to_click_pause(
                 pilot, "#validate_validate_button"
             )
+            await pilot.pause(10)
 
             written_lines = [
                 ele.text
@@ -91,6 +92,7 @@ async def test_validate_on_project_manager_kwargs(
             await self.scroll_to_click_pause(
                 pilot, "#validate_validate_button"
             )
+            await pilot.pause(10)
 
             args_, kwargs_ = spy_validate.call_args_list[0]
 
@@ -138,6 +140,7 @@ async def test_validate_on_project_manager_kwargs(
             await self.scroll_to_click_pause(
                 pilot, "#validate_validate_button"
             )
+            await pilot.pause(10)
 
             args_, kwargs_ = spy_validate.call_args_list[1]
 
@@ -196,6 +199,7 @@ async def test_validate_at_path_kwargs(self, setup_project_paths, mocker):
             await self.scroll_to_click_pause(
                 pilot, "#validate_validate_button"
             )
+            await pilot.pause(10)
             warnings.filterwarnings("default")
 
             # Check args are passed through to function as expected