File import: Empty column fix (#813)

* fix for remiving empty headers * adding tests * fixing typo * fixing flaky test
phac-nml · Oct 15, 2024 · 6f4d62a · 6f4d62a
1 parent 633ca84
commit 6f4d62a
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 1 deletion.
diff --git a/app/services/samples/metadata/file_import_service.rb b/app/services/samples/metadata/file_import_service.rb
@@ -89,7 +89,7 @@ def validate_file
                          Roo::Spreadsheet.open(@file)
                        end
 
-        @headers = @spreadsheet.row(1)
+        @headers = @spreadsheet.row(1).compact
 
         validate_file_headers
 

diff --git a/test/fixtures/files/metadata/contains_empty_columns.csv b/test/fixtures/files/metadata/contains_empty_columns.csv
@@ -0,0 +1,3 @@
+sample_name,metadatafield1,,,metadatafield3
+Project 1 Sample 1,10,,,30
+Project 1 Sample 2,15,,,35
diff --git a/test/fixtures/files/metadata/contains_empty_header.csv b/test/fixtures/files/metadata/contains_empty_header.csv
@@ -0,0 +1,3 @@
+sample_name,metadatafield1,,metadatafield3
+Project 1 Sample 1,10,20,30
+Project 1 Sample 2,15,25,35
diff --git a/test/services/samples/metadata/file_import_service_test.rb b/test/services/samples/metadata/file_import_service_test.rb
@@ -226,6 +226,36 @@ def setup
                      I18n.t('services.samples.metadata.import_file.missing_metadata_row'))
       end
 
+      test 'import sample metadata with an empty header' do
+        assert_equal({}, @sample1.metadata)
+        assert_equal({}, @sample2.metadata)
+        params = { file: File.new('test/fixtures/files/metadata/contains_empty_header.csv', 'r'),
+                   sample_id_column: 'sample_name' }
+        response = Samples::Metadata::FileImportService.new(@project.namespace, @john_doe,
+                                                            params).execute
+        assert_equal({ @sample1.name => { added: %w[metadatafield1 metadatafield3],
+                                          updated: [], deleted: [], not_updated: [], unchanged: [] },
+                       @sample2.name => { added: %w[metadatafield1 metadatafield3],
+                                          updated: [], deleted: [], not_updated: [], unchanged: [] } }, response)
+        assert_equal({ 'metadatafield1' => '10', 'metadatafield3' => '30' }, @sample1.reload.metadata)
+        assert_equal({ 'metadatafield1' => '15', 'metadatafield3' => '35' }, @sample2.reload.metadata)
+      end
+
+      test 'import sample metadata with multiple empty columns' do
+        assert_equal({}, @sample1.metadata)
+        assert_equal({}, @sample2.metadata)
+        params = { file: File.new('test/fixtures/files/metadata/contains_empty_columns.csv', 'r'),
+                   sample_id_column: 'sample_name' }
+        response = Samples::Metadata::FileImportService.new(@project.namespace, @john_doe,
+                                                            params).execute
+        assert_equal({ @sample1.name => { added: %w[metadatafield1 metadatafield3],
+                                          updated: [], deleted: [], not_updated: [], unchanged: [] },
+                       @sample2.name => { added: %w[metadatafield1 metadatafield3],
+                                          updated: [], deleted: [], not_updated: [], unchanged: [] } }, response)
+        assert_equal({ 'metadatafield1' => '10', 'metadatafield3' => '30' }, @sample1.reload.metadata)
+        assert_equal({ 'metadatafield1' => '15', 'metadatafield3' => '35' }, @sample2.reload.metadata)
+      end
+
       test 'import sample metadata with empty values set to true' do
         sample32 = samples(:sample32)
         project29 = projects(:project29)

diff --git a/test/system/projects/samples_test.rb b/test/system/projects/samples_test.rb
@@ -518,8 +518,18 @@ class SamplesTest < ApplicationSystemTestCase
 
       Project.reset_counters(project2.id, :samples_count)
       visit namespace_project_samples_url(namespace1, project2)
+
       assert_text strip_tags(I18n.t(:'viral.pagy.limit_component.summary', from: 1, to: 20, count: 220,
                                                                            locale: @user.locale))
+
+      click_button I18n.t(:'projects.samples.index.select_all_button')
+
+      within 'tfoot' do
+        sample_counts = all('strong')
+        total_samples = sample_counts[0].text.to_i
+        selected_samples = sample_counts[1].text.to_i
+        assert selected_samples <= total_samples
+      end
     end
 
     test 'empty state of transfer sample project selection' do