Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nextflow Samplesheet: File Selector #879

Merged
merged 38 commits into from
Jan 13, 2025
Merged

Conversation

ChrisHuynh333
Copy link
Collaborator

@ChrisHuynh333 ChrisHuynh333 commented Dec 30, 2024

What does this PR do and why?

This PR is for issue STRY0016505, which requested the nextflow samplesheet be populated with the most recent uploaded file, and to add the uploaded time of each file to its respective listing.

Having filename + created date within the original dropdown selection looked very cumbersome and so, for a better user experience, we decided to create a file selector so more file data could be displayed to the user.

Screenshots or screen recordings

Samplesheet (attachments selection no longer a dropdown)
image

File Selector:
image

How to set up and validate locally

Test selecting paired-end files:

  1. Use or create a sample, and upload multiple paired-end attachments to the sample. Note the file ordering based on upload time.
  2. Select the above sample and launch a workflow (any version)
  3. Verify the most recent files are selected by default.
  4. Within either the fastq_1 or fastq_2 fields, click the file to open the file selector.
  5. Within the file selector, from top down, verify the file ordering is from most recent to oldest upload time.
  6. Verify the correct attachment was displayed as selected.
  7. Select any other file than what was currently selected and submit
  8. Verify the selected field now has the selected attachment, and the paired-end field was also updated with the associated attachment.
  9. Repeat the above with the other fastq_# field,
  10. Submit the workflow execution and verify it contains the expected files.

Test selecting single-end files, default paired-end selection and No file selection:

  1. Use a seeded sample (with its seeded attachments), and upload non-paired fastq attachments to the sample (ex: only 1 of the 2 attachments of a paired-end).
  2. Select the above sample and launch a workflow (any version)
  3. Verify the seeded paired end files are selected by default even though they are not the most recent upload (paired-end selection is preferred over single end).
  4. Click the fastq_1 field, and verify all the uploaded attachments are included, and that there is no No file option (since fastq_1 is required).
  5. Select any of the attachments, and verify the fastq_1 field updates with the selection and fastq_2 changed to No File.
  6. Click the fastq_2 field, and verify none of the uploaded attachments are included, and that there is a No file option.
  7. Submit the workflow execution and verify the attachments (fastq_1 contains the selected attachment and fastq_2 contains no attachment)

Test samplesheet validation of selected files and file selector empty state:

  1. Create a sample with no attachments
  2. Select the above sample and launch a workflow (any version)
  3. Verify both fastq_1 and fastq_2 attachment fields have no file selected.
  4. Submit the workflow
  5. Verify an error message occurs within the samplesheet dialog, and that only the fastq_1 field is highlighted in red, as fastq_2 is not required.
  6. Click either the fastq_1 or fastq_2 file field. Verify a no attachments message within the file selector, with no submit button.

Test file selector filtering:
1 Selected a seeded sample and launch a workflow (any version)
2. Click either the fastq_1 or fastq_2 attachment fields.
3. Verify reference.fasta is not displayed as it's not a fastq file.

PR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Sorry, something went wrong.

@ChrisHuynh333 ChrisHuynh333 force-pushed the nextflow-samplesheet/file-selector branch 2 times, most recently from 012975c to 6f9cc82 Compare January 7, 2025 22:21
@ChrisHuynh333 ChrisHuynh333 self-assigned this Jan 8, 2025

This comment has been minimized.

This comment has been minimized.

@ChrisHuynh333 ChrisHuynh333 marked this pull request as ready for review January 8, 2025 19:46
@ChrisHuynh333 ChrisHuynh333 added the ready for review Pull request is ready for review label Jan 8, 2025

This comment has been minimized.

@ChrisHuynh333 ChrisHuynh333 force-pushed the nextflow-samplesheet/file-selector branch from cbb7f96 to 26efb01 Compare January 9, 2025 21:02

This comment has been minimized.

This comment has been minimized.

Copy link
Member

@ericenns ericenns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed a few issues while testing this out.

  1. No way of seeing if a file is single ended or pair end. (Maybe we need to add format and type columns)
  2. If I select a single end file for fastq_1 it does not set the input for fastq_2 to none which was the previous behaviour

@ChrisHuynh333
Copy link
Collaborator Author

I noticed a few issues while testing this out.

  1. No way of seeing if a file is single ended or pair end. (Maybe we need to add format and type columns)
  2. If I select a single end file for fastq_1 it does not set the input for fastq_2 to none which was the previous behaviour

Thanks for catching these. I've updated with associated test in 8179e7d

This comment has been minimized.

@ChrisHuynh333 ChrisHuynh333 force-pushed the nextflow-samplesheet/file-selector branch from 4d9c5a8 to 1863c8c Compare January 10, 2025 18:06

This comment has been minimized.

@ChrisHuynh333 ChrisHuynh333 force-pushed the nextflow-samplesheet/file-selector branch from 1863c8c to bb784ff Compare January 13, 2025 16:18
Copy link
Contributor

@joshsadam joshsadam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me

Copy link
Member

@ericenns ericenns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@ericenns ericenns merged commit ef00f5a into main Jan 13, 2025
4 checks passed
@ericenns ericenns deleted the nextflow-samplesheet/file-selector branch January 13, 2025 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review Pull request is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants