Default to "all" subjects or sessions for custom transfer if input left blank

[ishan372or]

Description

• Bug fix
• Addition of a new feature
• Other

Why is this PR needed?
Currently, leaving the subject or session fields blank in the "Custom Transfer" TUI causes the transfer to fail silently (it attempts to transfer but selects nothing). This confusing behavior was identified in Issue #619. This PR ensures that blank inputs are interpreted as a wildcard ("all"), improving user experience.

What does this PR do?
I updated the logic in datashuttle/tui/tabs/transfer.py to check for blank inputs. Specifically, since the TUI returns [""] for empty input fields, the code now explicitly catches this case and defaults sub_names and ses_names to "all".

References

fixes #619

How has this PR been tested?

Manual Verification:

1.Launched the TUI using datashuttle launch.

2.Navigated to the Transfer tab and selected Custom mode.

3.Left Subject(s) and Session(s) blank and clicked Transfer.

4.Verified in the logs and terminal that the system correctly defaulted to "all" and attempted the transfer, rather than failing silently.

Is this a breaking change?

No. This change is backward-compatible. It only modifies the behavior when the subject or session fields are strictly empty in the TUI, which previously resulted in a failed operation anyway.

Does this PR require an update to the documentation?

No. This is a fix to align the TUI behavior with expected user assumptions (blank = all).

Checklist:

• The code has been tested locally
• Tests have been added to cover all new functionality
• The documentation has been updated to reflect any changes
• The code has been formatted with pre-commit

[ishan372or]

@JoeZiminski, I implemented the "default to all" behavior as this seemed to be the preferred solution in the issue discussion.

However, if you think it would be safer to raise an explicit error message (forcing the user to type "all" manually), please let me know! I am happy to switch the logic if you prefer that approach for safety.

[JoeZiminski]

Hi @ishan372or, apologies for the delay in response. I'll review this next week, thanks for your patience!

[JoeZiminski]

Great thanks for this @ishan372or, this looks like it achieves the desired behavior. I think we can also add to the suggestion to make it clear the default behavior is "all".

When the inputs are created, they have a placeholder argument:

ClickableInput(
    self.mainwindow,
    id="transfer_session_input",
    placeholder="e.g. ses-001",
),

We can change this to e.g. 'ses-001 (default: "all")' if you agree (same for the ses input.

The only other thing will be to test this. I have an idea for a test but it is quite complicated and interacts with some existing machinery. I think I'll push it directly to your fork if that's okay with you? I'll explain it afterwards, otherwise if you would like to implement it I'd be happy to walk you through!

[ishan372or]

Thanks for the review @JoeZiminski, I agree on updating the placeholders to display (default: "all") for both subject and session inputs. And yes, I’m completely fine with you pushing the test directly to my fork. Please go ahead—an explanation afterward would be great so I can understand the approach and learn from it. I will make the changes as soon as possible.

[ishan372or]

@JoeZiminski ,The CI failure is due to the updated placeholder text (e.g. sub-001 (default: "all")) — an existing TUI test asserts the old placeholder value.

I’m happy to update the test on my side to match the new behavior, or if you prefer to include it in the test you mentioned pushing to my fork, I can leave it to you. Just let me know what you’d prefer.

[JoeZiminski]

HI @ishan372or thanks for this, please feel free to update the test too and push again so the CI passes, then I will update the other test. Thanks!

[ishan372or]

@JoeZiminski

The remaining CI failure is in test_ssh_suggest_next_sub_ses, which appears unrelated to the placeholder change or the custom transfer logic in this PR.

[JoeZiminski]

Thanks @ishan372or, yes that's been an ongoing issue, hopefully fixed with #674 but unrelated to this PR. Will update the test next week, then I think this will be good to go! thanks a lot

[ishan372or]

Thanks for confirming @JoeZiminski. Sounds good — happy to wait for the test update. Thanks for taking a look and for the clarification.