Skip to content

Commit 81f5efe

Browse files
author
shrey
committed
refactor: apply code review changes
1 parent bd68989 commit 81f5efe

5 files changed

Lines changed: 38 additions & 24 deletions

File tree

datashuttle/tui/screens/modal_dialogs.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,13 @@ async def handle_transfer_and_update_ui_when_complete(self) -> None:
138138

139139

140140
class SearchingCentralForNextSubSesPopup(ModalScreen):
141+
"""
142+
A popup to show message and a loading indicator when awaiting search next sub/ses across
143+
the folders present in both local and central machines. This search happens in a separate
144+
thread so as to allow TUI to display the loading indicate without freezing.
145+
146+
Only displayed when the `include_central` flag is checked and the connection method is "ssh".
147+
"""
141148

142149
def __init__(self, sub_or_ses: Prefix) -> None:
143150
super().__init__()

datashuttle/tui/tabs/create_folders.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -305,11 +305,10 @@ def suggest_next_sub_ses(
305305
include_central
306306
and self.interface.project.cfg["connection_method"] == "ssh"
307307
):
308-
searching_central_popup = SearchingCentralForNextSubSesPopup(
309-
prefix
308+
self.searching_central_popup_widget = (
309+
SearchingCentralForNextSubSesPopup(prefix)
310310
)
311-
self.searching_central_popup_widget = searching_central_popup
312-
self.mainwindow.push_screen(searching_central_popup)
311+
self.mainwindow.push_screen(self.searching_central_popup_widget)
313312

314313
asyncio.create_task(
315314
self.fill_suggestion_and_dismiss_popup(

tests/test_utils.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,3 +695,8 @@ def get_task_by_name(name):
695695
None,
696696
)
697697
return target_task
698+
699+
700+
async def await_task_by_name_if_present(name: str) -> None:
701+
if task := get_task_by_name(name):
702+
await task

tests/tests_tui/test_tui_create_folders.py

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -445,10 +445,9 @@ async def test_name_template_next_sub_or_ses_and_validation(
445445
await self.double_click(
446446
pilot, "#create_folders_subject_input", control=False
447447
)
448-
if suggest_sub_task := test_utils.get_task_by_name(
448+
await test_utils.await_task_by_name_if_present(
449449
"suggest_next_sub_async_task"
450-
):
451-
await suggest_sub_task
450+
)
452451
assert (
453452
pilot.app.screen.query_one(
454453
"#create_folders_subject_input"
@@ -462,10 +461,9 @@ async def test_name_template_next_sub_or_ses_and_validation(
462461
await self.double_click(
463462
pilot, "#create_folders_session_input", control=False
464463
)
465-
if suggest_ses_task := test_utils.get_task_by_name(
464+
await test_utils.await_task_by_name_if_present(
466465
"suggest_next_ses_async_task"
467-
):
468-
await suggest_ses_task
466+
)
469467
assert (
470468
pilot.app.screen.query_one(
471469
"#create_folders_session_input"
@@ -512,10 +510,9 @@ async def test_get_next_sub_and_ses_no_template(self, setup_project_paths):
512510

513511
# Double click without CTRL modifier key.
514512
await self.double_click(pilot, "#create_folders_subject_input")
515-
if suggest_sub_task := test_utils.get_task_by_name(
513+
await test_utils.await_task_by_name_if_present(
516514
"suggest_next_sub_async_task"
517-
):
518-
await suggest_sub_task
515+
)
519516
assert (
520517
pilot.app.screen.query_one(
521518
"#create_folders_subject_input"
@@ -527,10 +524,9 @@ async def test_get_next_sub_and_ses_no_template(self, setup_project_paths):
527524
pilot, "#create_folders_subject_input", "sub-001"
528525
)
529526
await self.double_click(pilot, "#create_folders_session_input")
530-
if suggest_ses_task := test_utils.get_task_by_name(
527+
await test_utils.await_task_by_name_if_present(
531528
"suggest_next_ses_async_task"
532-
):
533-
await suggest_ses_task
529+
)
534530
assert (
535531
pilot.app.screen.query_one(
536532
"#create_folders_session_input"
@@ -544,6 +540,11 @@ async def test_get_next_sub_and_ses_no_template(self, setup_project_paths):
544540
async def test_get_next_sub_and_ses_central_no_template(
545541
self, setup_project_paths, mocker
546542
):
543+
"""
544+
Test getting the next subject / session with the include_central option. Check the
545+
checkbox widget that turns the setting on. Trigger a get next subject / session and mock
546+
the underlying datashuttle function to ensure include_central is properly called.
547+
"""
547548
tmp_config_path, tmp_path, project_name = setup_project_paths.values()
548549

549550
app = TuiApp()
@@ -571,13 +572,11 @@ async def test_get_next_sub_and_ses_central_no_template(
571572
pilot.app.screen.interface.project, "get_next_ses"
572573
)
573574

574-
# # Check subject suggestion called mocked function correctly
575+
# Check subject suggestion called mocked function correctly
575576
await self.double_click(pilot, "#create_folders_subject_input")
576-
577-
if suggest_sub_task := test_utils.get_task_by_name(
577+
await test_utils.await_task_by_name_if_present(
578578
"suggest_next_sub_async_task"
579-
):
580-
await suggest_sub_task
579+
)
581580

582581
spy_get_next_sub.assert_called_with(
583582
"rawdata", return_with_prefix=True, include_central=True
@@ -589,10 +588,9 @@ async def test_get_next_sub_and_ses_central_no_template(
589588
)
590589
await self.double_click(pilot, "#create_folders_session_input")
591590

592-
if suggest_ses_task := test_utils.get_task_by_name(
591+
await test_utils.await_task_by_name_if_present(
593592
"suggest_next_ses_async_task"
594-
):
595-
await suggest_ses_task
593+
)
596594

597595
spy_get_next_ses.assert_called_with(
598596
"rawdata",

tests/tests_tui/test_tui_widgets_and_defaults.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -931,6 +931,11 @@ async def check_top_folder_select(
931931
async def test_search_central_for_suggestion_settings(
932932
self, setup_project_paths
933933
):
934+
"""
935+
Check the settings for the checkbox that selects include_central when
936+
getting the next subject or session in the 'Create' tab and ensure that
937+
the underlying settings are changed.
938+
"""
934939
tmp_config_path, tmp_path, project_name = setup_project_paths.values()
935940

936941
app = TuiApp()

0 commit comments

Comments
 (0)