From b54f2c52e294360c50df5110add64ec8e773cd77 Mon Sep 17 00:00:00 2001 From: Wendy Yang Date: Wed, 14 Jan 2026 09:12:55 +0000 Subject: [PATCH 1/8] add orange color --- config/sample_manifest_excel/columns.yml | 5 +++-- config/sample_manifest_excel/conditional_formattings.yml | 9 +++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/config/sample_manifest_excel/columns.yml b/config/sample_manifest_excel/columns.yml index 58be3e02e2..e71a2d0f50 100644 --- a/config/sample_manifest_excel/columns.yml +++ b/config/sample_manifest_excel/columns.yml @@ -39,7 +39,7 @@ i7: promptTitle: "i7" prompt: "Input i7." conditional_formattings: - empty_cell: + empty_mandatory_cell_2: i5: heading: i5 TAG SEQUENCE unlocked: true @@ -50,10 +50,11 @@ i5: formula1: "255" allowBlank: false showInputMessage: true + errorStyle: :warning promptTitle: "i5" prompt: "Input i5." conditional_formattings: - empty_cell: + empty_mandatory_cell_2: tag_group: heading: TAG GROUP unlocked: true diff --git a/config/sample_manifest_excel/conditional_formattings.yml b/config/sample_manifest_excel/conditional_formattings.yml index 1554673165..5dc77ad9d6 100644 --- a/config/sample_manifest_excel/conditional_formattings.yml +++ b/config/sample_manifest_excel/conditional_formattings.yml @@ -16,6 +16,15 @@ empty_mandatory_cell: formula: "FALSE" operator: :equal priority: 1 +empty_mandatory_cell_2: + style: + bg_color: "FF991C" + type: :dxf + options: + type: :cellIs + formula: "FALSE" + operator: :equal + priority: 1 len: style: bg_color: "FF0000" From 86cbf2eeea80b54691977c1f94a91cede5e7548a Mon Sep 17 00:00:00 2001 From: Wendy Yang Date: Wed, 21 Jan 2026 10:53:32 +0000 Subject: [PATCH 2/8] add validation of i7, i5 column --- .../sample_manifest_excel/upload/row.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/app/sample_manifest_excel/sample_manifest_excel/upload/row.rb b/app/sample_manifest_excel/sample_manifest_excel/upload/row.rb index ccce068b63..f608f8ca4b 100644 --- a/app/sample_manifest_excel/sample_manifest_excel/upload/row.rb +++ b/app/sample_manifest_excel/sample_manifest_excel/upload/row.rb @@ -22,6 +22,22 @@ class Row # rubocop:todo Metrics/ClassLength validate :country_of_origin_has_correct_case, if: -> { data.present? && columns.present? && columns.names.include?('country_of_origin') } + validate :i7_present + # Ensure i7 column is not blank if it exists in the manifest + def i7_present + return unless columns.names.include?('i7') && value('i7').blank? + + errors.add(:base, "#{row_title} i7 can't be blank") + end + + validate :i5_present + # Ensure i5 column is not blank if it exists in the manifest + def i5_present + return unless columns.names.include?('i5') && value('i5').blank? + + errors.add(:base, "#{row_title} i5 can't be blank, putting “n/a” in i5 if only needs one set of tags") + end + delegate :present?, to: :sample, prefix: true delegate :aliquots, :asset, to: :manifest_asset From cc016ff531eb9df6743bccb56084bc33321febf4 Mon Sep 17 00:00:00 2001 From: Wendy Yang Date: Wed, 21 Jan 2026 15:04:04 +0000 Subject: [PATCH 3/8] add unit test --- .../sample_manifest_excel/upload/row.rb | 4 +- spec/sample_manifest_excel/upload/row_spec.rb | 49 ++++++++++++++++++- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/app/sample_manifest_excel/sample_manifest_excel/upload/row.rb b/app/sample_manifest_excel/sample_manifest_excel/upload/row.rb index f608f8ca4b..c466503b6d 100644 --- a/app/sample_manifest_excel/sample_manifest_excel/upload/row.rb +++ b/app/sample_manifest_excel/sample_manifest_excel/upload/row.rb @@ -25,7 +25,7 @@ class Row # rubocop:todo Metrics/ClassLength validate :i7_present # Ensure i7 column is not blank if it exists in the manifest def i7_present - return unless columns.names.include?('i7') && value('i7').blank? + return unless columns.present? && data.present? && columns.names.include?('i7') && value('i7').blank? errors.add(:base, "#{row_title} i7 can't be blank") end @@ -33,7 +33,7 @@ def i7_present validate :i5_present # Ensure i5 column is not blank if it exists in the manifest def i5_present - return unless columns.names.include?('i5') && value('i5').blank? + return unless columns.present? && data.present? && columns.names.include?('i5') && value('i5').blank? errors.add(:base, "#{row_title} i5 can't be blank, putting “n/a” in i5 if only needs one set of tags") end diff --git a/spec/sample_manifest_excel/upload/row_spec.rb b/spec/sample_manifest_excel/upload/row_spec.rb index 96a1b7299c..24b464653e 100644 --- a/spec/sample_manifest_excel/upload/row_spec.rb +++ b/spec/sample_manifest_excel/upload/row_spec.rb @@ -24,7 +24,7 @@ tube.human_barcode, sample_manifest.sample_manifest_assets.first.sanger_sample_id, 'AA', - '', + 'CC', 'My reference genome', 'My New Library Type', 200, @@ -101,6 +101,47 @@ 'Unknown' ] end + let(:data_without_i7_i5) do + [ + tube.human_barcode, + sample_manifest.sample_manifest_assets.first.sanger_sample_id, + '', + '', + 'My reference genome', + 'My New Library Type', + 200, + 1500, + 'SCG--1222_A01', + '', + 1, + 1, + 'Unknown', + '', + '', + '', + 'Cell Line', + 'Nov-16', + 'Nov-16', + '', + 'No', + '', + 'OTHER', + '', + '', + '', + '', + '', + 'SCG--1222_A01', + 9606, + 'Homo sapiens', + '', + '', + '', + '', + 11, + 'Unknown' + ] + end it 'is not valid without row number' do expect(described_class.new(number: 'one', data: data, columns: columns)).not_to be_valid @@ -115,6 +156,10 @@ expect(described_class.new(number: 1, data: data)).not_to be_valid end + it 'is not valid without i7/i5 if i7/i5 column exists' do + expect(described_class.new(number: 1, data: data_without_i7_i5, columns: columns)).not_to be_valid + end + it '#value returns value for specified key' do expect(described_class.new(number: 1, data: data, columns: columns).value(:sanger_sample_id)).to eq( sample_manifest.labware.first.sample_manifest_assets.first.sanger_sample_id @@ -184,7 +229,7 @@ aliquot = row.aliquots.first expect(Sample.count - sample_count).to eq(1) expect(aliquot.tag.oligo).to eq('AA') - expect(aliquot.tag2).to be_nil + expect(aliquot.tag2.oligo).to eq('CC') expect(aliquot.insert_size_from).to eq(200) expect(aliquot.insert_size_to).to eq(1500) end From 6092baa98063fd52221ba2af5e0d60960fa60282 Mon Sep 17 00:00:00 2001 From: Wendy Yang Date: Wed, 21 Jan 2026 20:26:14 +0000 Subject: [PATCH 4/8] add i7/i5 to the test fiile --- spec/factories/sample_manifest_excel/test_download_tubes.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/factories/sample_manifest_excel/test_download_tubes.rb b/spec/factories/sample_manifest_excel/test_download_tubes.rb index 3241bd7f3b..da95f58694 100644 --- a/spec/factories/sample_manifest_excel/test_download_tubes.rb +++ b/spec/factories/sample_manifest_excel/test_download_tubes.rb @@ -31,6 +31,8 @@ sample_taxon_id: 9606, sample_common_name: 'Homo sapiens', donor_id: 'id', + 'i7' => 'ATTACTCG', + 'i5' => 'CC', phenotype: 'Unknown' }.with_indifferent_access end From 772bc944e7126daf2b8cab2323e10f6c5bff455c Mon Sep 17 00:00:00 2001 From: Wendy Yang Date: Thu, 22 Jan 2026 11:27:45 +0000 Subject: [PATCH 5/8] update the test --- spec/factories/sample_manifest_excel/test_download_tubes.rb | 2 -- spec/sample_manifest_excel/upload/rows_spec.rb | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/spec/factories/sample_manifest_excel/test_download_tubes.rb b/spec/factories/sample_manifest_excel/test_download_tubes.rb index da95f58694..3241bd7f3b 100644 --- a/spec/factories/sample_manifest_excel/test_download_tubes.rb +++ b/spec/factories/sample_manifest_excel/test_download_tubes.rb @@ -31,8 +31,6 @@ sample_taxon_id: 9606, sample_common_name: 'Homo sapiens', donor_id: 'id', - 'i7' => 'ATTACTCG', - 'i5' => 'CC', phenotype: 'Unknown' }.with_indifferent_access end diff --git a/spec/sample_manifest_excel/upload/rows_spec.rb b/spec/sample_manifest_excel/upload/rows_spec.rb index 19ecff54b4..d5e1ff63c6 100644 --- a/spec/sample_manifest_excel/upload/rows_spec.rb +++ b/spec/sample_manifest_excel/upload/rows_spec.rb @@ -39,7 +39,7 @@ end it 'is valid if some rows are empty' do - download = build(:test_download_tubes_partial, columns:) + download = build(:test_download_tubes_partial, columns:, manifest_type: 'tube_library_with_tag_sequences') download.save(test_file_name) expect(described_class.new(SampleManifestExcel::Upload::Data.new(test_file), columns)).to be_valid end From 3a4574eaa63f465c7984b0c1dd58514ff6fc0d8f Mon Sep 17 00:00:00 2001 From: Wendy Yang Date: Thu, 22 Jan 2026 11:33:13 +0000 Subject: [PATCH 6/8] fix linting --- spec/sample_manifest_excel/upload/rows_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/sample_manifest_excel/upload/rows_spec.rb b/spec/sample_manifest_excel/upload/rows_spec.rb index d5e1ff63c6..8efd3e9186 100644 --- a/spec/sample_manifest_excel/upload/rows_spec.rb +++ b/spec/sample_manifest_excel/upload/rows_spec.rb @@ -39,7 +39,7 @@ end it 'is valid if some rows are empty' do - download = build(:test_download_tubes_partial, columns:, manifest_type: 'tube_library_with_tag_sequences') + download = build(:test_download_tubes_partial, columns: columns, manifest_type: 'tube_library_with_tag_sequences') download.save(test_file_name) expect(described_class.new(SampleManifestExcel::Upload::Data.new(test_file), columns)).to be_valid end From d6489dc6d07dc943af11c4e14801a093ad64da1a Mon Sep 17 00:00:00 2001 From: Wendy Yang Date: Thu, 22 Jan 2026 12:03:33 +0000 Subject: [PATCH 7/8] fix the test --- .../uploader_for_manifests_with_tag_sequences_spec.rb | 5 +++-- spec/sample_manifest_excel/upload/upload_spec.rb | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/spec/features/sample_manifests/uploader_for_manifests_with_tag_sequences_spec.rb b/spec/features/sample_manifests/uploader_for_manifests_with_tag_sequences_spec.rb index a62990cec1..4000cb56b4 100644 --- a/spec/features/sample_manifests/uploader_for_manifests_with_tag_sequences_spec.rb +++ b/spec/features/sample_manifests/uploader_for_manifests_with_tag_sequences_spec.rb @@ -26,6 +26,7 @@ let!(:user) { create(:admin) } let(:columns) { SampleManifestExcel.configuration.columns.tube_library_with_tag_sequences.dup } let(:test_file) { 'test_file.xlsx' } + let(:manifest_type) { 'tube_library_with_tag_sequences' } before do download.save(test_file) @@ -33,7 +34,7 @@ context 'valid' do context 'standard' do - let(:download) { build(:test_download_tubes, columns:) } + let(:download) { build(:test_download_tubes, columns:, manifest_type:) } it 'upload' do login_user(user) @@ -81,7 +82,7 @@ end context 'cgap foreign barcodes' do - let(:download) { build(:test_download_tubes_cgap, columns:) } + let(:download) { build(:test_download_tubes_cgap, columns:, manifest_type:) } it 'upload' do login_user(user) diff --git a/spec/sample_manifest_excel/upload/upload_spec.rb b/spec/sample_manifest_excel/upload/upload_spec.rb index e45611d999..fb7456dae6 100644 --- a/spec/sample_manifest_excel/upload/upload_spec.rb +++ b/spec/sample_manifest_excel/upload/upload_spec.rb @@ -28,7 +28,8 @@ download = build( :test_download_tubes, - columns: SampleManifestExcel.configuration.columns.tube_library_with_tag_sequences.dup + columns: SampleManifestExcel.configuration.columns.tube_library_with_tag_sequences.dup, + manifest_type: 'tube_library_with_tag_sequences' ) download.save(test_file_name) upload = SampleManifestExcel::Upload::Base.new(file: test_file, column_list: columns, start_row: 9) From 177f438d9df155319eb66791d696b98ec51b2399 Mon Sep 17 00:00:00 2001 From: Wendy Yang Date: Wed, 4 Feb 2026 16:28:20 +0000 Subject: [PATCH 8/8] add more columns to be orange --- .../sample_manifest_excel/upload/row.rb | 32 +++++++++++++++++++ config/sample_manifest_excel/columns.yml | 25 ++++++++++----- spec/data/sample_manifest_excel/columns.yml | 8 ++--- 3 files changed, 53 insertions(+), 12 deletions(-) diff --git a/app/sample_manifest_excel/sample_manifest_excel/upload/row.rb b/app/sample_manifest_excel/sample_manifest_excel/upload/row.rb index c466503b6d..34aa49c03b 100644 --- a/app/sample_manifest_excel/sample_manifest_excel/upload/row.rb +++ b/app/sample_manifest_excel/sample_manifest_excel/upload/row.rb @@ -38,6 +38,38 @@ def i5_present errors.add(:base, "#{row_title} i5 can't be blank, putting “n/a” in i5 if only needs one set of tags") end + validate :chromium_tag_group + # Ensure chromium_tag_group column is not blank if it exists in the manifest + def chromium_tag_group + return unless columns.present? && data.present? && columns.names.include?('chromium_tag_group') && value('chromium_tag_group').blank? + + errors.add(:base, "#{row_title} chromium_tag_group can't be blank") + end + + validate :chromium_tag_well + # Ensure chromium_tag_well column is not blank if it exists in the manifest + def chromium_tag_well + return unless columns.present? && data.present? && columns.names.include?('chromium_tag_well') && value('chromium_tag_well').blank? + + errors.add(:base, "#{row_title} chromium_tag_well can't be blank") + end + + validate :dual_index_tag_set + # Ensure dual_index_tag_set column is not blank if it exists in the manifest + def dual_index_tag_set + return unless columns.present? && data.present? && columns.names.include?('dual_index_tag_set') && value('dual_index_tag_set').blank? + + errors.add(:base, "#{row_title} dual_index_tag_set can't be blank") + end + + validate :dual_index_tag_well + # Ensure dual_index_tag_well column is not blank if it exists in the manifest + def dual_index_tag_well + return unless columns.present? && data.present? && columns.names.include?('dual_index_tag_well') && value('dual_index_tag_well').blank? + + errors.add(:base, "#{row_title} dual_index_tag_well can't be blank") + end + delegate :present?, to: :sample, prefix: true delegate :aliquots, :asset, to: :manifest_asset diff --git a/config/sample_manifest_excel/columns.yml b/config/sample_manifest_excel/columns.yml index e71a2d0f50..9096fd1dca 100644 --- a/config/sample_manifest_excel/columns.yml +++ b/config/sample_manifest_excel/columns.yml @@ -32,12 +32,17 @@ i7: validation: options: type: :textLength - operator: :lessThanOrEqual - formula1: "255" + operator: :between + formula1: "1" + formula2: "255" allowBlank: false showInputMessage: true + showErrorMessage: true promptTitle: "i7" prompt: "Input i7." + errorStyle: :warning + errorTitle: "i7" + error: "i7 cannot be blank" conditional_formattings: empty_mandatory_cell_2: i5: @@ -46,11 +51,15 @@ i5: validation: options: type: :textLength - operator: :lessThanOrEqual - formula1: "255" + operator: :between + formula1: "1" + formula2: "255" allowBlank: false showInputMessage: true + showErrorMessage: true errorStyle: :warning + errorTitle: "i5" + error: "i5 cannot be blank" promptTitle: "i5" prompt: "Input i5." conditional_formattings: @@ -138,7 +147,7 @@ chromium_tag_group: prompt: "Input the name of a valid tag set. All samples in a library need to be tagged with the same tag set." range_name: :chromium_tag_groups conditional_formattings: - empty_cell: + empty_mandatory_cell_2: chromium_tag_well: heading: CHROMIUM TAG WELL unlocked: true @@ -156,7 +165,7 @@ chromium_tag_well: errorTitle: "Tag well" error: "Tag Index must be a well." conditional_formattings: - empty_cell: + empty_mandatory_cell_2: is_number: len: formula: @@ -236,7 +245,7 @@ dual_index_tag_set: prompt: "Input the name of a valid dual index tag plate." range_name: :dual_index_tag_sets conditional_formattings: - empty_cell: + empty_mandatory_cell_2: dual_index_tag_well: heading: DUAL INDEX TAG WELL unlocked: true @@ -254,7 +263,7 @@ dual_index_tag_well: errorTitle: "Dual index tag well" error: "Dual Index Tag must be a well." conditional_formattings: - empty_cell: + empty_mandatory_cell_2: is_number: len: formula: diff --git a/spec/data/sample_manifest_excel/columns.yml b/spec/data/sample_manifest_excel/columns.yml index 48a29008c3..f6727872f4 100644 --- a/spec/data/sample_manifest_excel/columns.yml +++ b/spec/data/sample_manifest_excel/columns.yml @@ -127,7 +127,7 @@ chromium_tag_group: prompt: "Input the name of a valid tag set. All samples in a library need to be tagged with the same tag set." range_name: :chromium_tag_groups conditional_formattings: - empty_cell: + empty_mandatory_cell_2: chromium_tag_well: heading: CHROMIUM TAG WELL unlocked: true @@ -145,7 +145,7 @@ chromium_tag_well: errorTitle: "Tag well" error: "Tag Index must be a well." conditional_formattings: - empty_cell: + empty_mandatory_cell_2: is_number: len: formula: @@ -184,7 +184,7 @@ dual_index_tag_set: prompt: "Input the name of a valid dual index tag plate." range_name: :dual_index_tag_sets conditional_formattings: - empty_cell: + empty_mandatory_cell_2: dual_index_tag_well: heading: DUAL INDEX TAG WELL unlocked: true @@ -202,7 +202,7 @@ dual_index_tag_well: errorTitle: "Dual index tag well" error: "Dual Index Tag must be a well." conditional_formattings: - empty_cell: + empty_mandatory_cell_2: is_number: len: formula: