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

Batch sample import service #897

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

JeffreyThiessen
Copy link
Member

@JeffreyThiessen JeffreyThiessen commented Jan 14, 2025

What does this PR do and why?

Describe in detail what your merge request does and why.

  • Added BatchFileImportService for importing a spreadsheet of samples

Additional fixes/changes in this pr

  • Fixed bug where tsv files were not being read as csv files sometimes
  • refactored sample metadata import service to be a child of new parent class BaseSpreadsheetImportService
  • simplified spreadsheet parsing

Note: I decided to split this PR into 2 parts, this part which handles the sample file creation and refactor, and a separate PR for doing the metadata parsing.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other pull requests.

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

  1. seed your db
  2. Start irida next with command line enabled
  3. Sign into User3
  4. navigate to http://localhost:3000/borrelia/borrelia-burgdorferi/outbreak-2024/-/samples
  5. run the following in the terminal to import new samples.
u = User.all[3];n = Project.all[0].namespace;

# replace this path with the path to your irida next git directory
filepath = '/home/CSCScience.ca/jthiessen/git/irida-next-core/test/fixtures/files/batch_sample_import_valid.csv'

blob = ActiveStorage::Blob.create_and_upload!(io: File.open(filepath), filename: fname)

s = Samples::BatchFileImportService.new(n,u, blob.id, {sample_name_column: 'sample_name', project_puid_column: 'project_puid', sample_description_column: "description"} )

s.execute
  1. You should see 2 new samples added to the project you are viewing

PR acceptance checklist

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

@JeffreyThiessen JeffreyThiessen force-pushed the batch_sample_import_service branch from 6bb7ad9 to b034367 Compare January 15, 2025 20:52

This comment has been minimized.

@JeffreyThiessen JeffreyThiessen force-pushed the batch_sample_import_service branch from b034367 to 9a49b1c Compare February 3, 2025 23:51

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link

github-actions bot commented Feb 6, 2025

Code Metrics Report

Coverage Test Execution Time
92.1% 10m25s

Code coverage of files in pull request scope (56.3%)

Files Coverage
app/jobs/samples/batch_sample_import_job.rb 0.0%
app/services/base_spreadsheet_import_service.rb 96.0%
app/services/samples/batch_file_import_service.rb 0.0%
app/services/samples/metadata/file_import_service.rb 100.0%

Reported by octocov

Copy link
Contributor

@deepsidhu85 deepsidhu85 left a comment

Choose a reason for hiding this comment

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

Base of this looks great, just have one comment for now

@spreadsheet.each_with_index(parse_settings) do |data, index|
next unless index.positive?

# TODO: handle metadata
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking this can be done in the process_sample_row once the sample is persisted

Copy link
Member Author

Choose a reason for hiding this comment

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

That makes sense. The metadata headers should probably be processed here with the other headers though.

app/models/project.rb Outdated Show resolved Hide resolved
@JeffreyThiessen JeffreyThiessen force-pushed the batch_sample_import_service branch from b949234 to 0401de1 Compare February 10, 2025 15:52
@JeffreyThiessen JeffreyThiessen marked this pull request as ready for review February 10, 2025 22:32
@JeffreyThiessen JeffreyThiessen self-assigned this Feb 10, 2025
@JeffreyThiessen JeffreyThiessen added enhancement New feature or request ready for review Pull request is ready for review labels Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready for review Pull request is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants