From b28609b5d4c968ee28f1f02aa3bd9ecc53378f09 Mon Sep 17 00:00:00 2001 From: ollietulloch Date: Tue, 23 Jun 2026 08:58:31 +0100 Subject: [PATCH 1/4] Allow direct assignment of table_metadata in the table definition --- lib/ndr_import/table.rb | 6 +++--- lib/ndr_import/universal_importer_helper.rb | 3 ++- test/non_tabular/table_test.rb | 2 +- test/universal_importer_helper_test.rb | 18 ++++++++++++++++++ test/vcf/table_test.rb | 4 ++-- 5 files changed, 26 insertions(+), 7 deletions(-) diff --git a/lib/ndr_import/table.rb b/lib/ndr_import/table.rb index 27711d3..845c990 100644 --- a/lib/ndr_import/table.rb +++ b/lib/ndr_import/table.rb @@ -11,9 +11,9 @@ class Table include NdrImport::Mapper def self.all_valid_options - %w[canonical_name delimiter liberal_parsing filename_pattern file_password last_data_column - tablename_pattern header_lines footer_lines format klass columns slurp row_identifier - significant_mapped_fields] + %w[canonical_name delimiter liberal_parsing filename_pattern file_password + last_data_column tablename_pattern header_lines footer_lines + format klass columns slurp row_identifier significant_mapped_fields table_metadata] end def all_valid_options diff --git a/lib/ndr_import/universal_importer_helper.rb b/lib/ndr_import/universal_importer_helper.rb index f24a225..fcba1e9 100644 --- a/lib/ndr_import/universal_importer_helper.rb +++ b/lib/ndr_import/universal_importer_helper.rb @@ -66,6 +66,7 @@ def table_options_from(table_mapping) 'slurp' => table_mapping.try(:slurp), 'yield_xml_record' => table_mapping.try(:yield_xml_record), 'pattern_match_record_xpath' => table_mapping.try(:pattern_match_record_xpath), + 'file_metadata' => table_mapping.try(:file_metadata), 'xml_file_metadata' => table_mapping.try(:xml_file_metadata), 'vcf_file_metadata' => table_mapping.try(:vcf_file_metadata) } end @@ -80,7 +81,7 @@ def yield_tables_and_their_content(filename, tables, &block) next if mapping.nil? mapping.notifier = get_notifier(record_total(filename, table_content)) - mapping.table_metadata = file_metadata || {} + mapping.table_metadata ||= file_metadata || {} yield(mapping, table_content) end end diff --git a/test/non_tabular/table_test.rb b/test/non_tabular/table_test.rb index 5ca304d..c66bbd8 100644 --- a/test/non_tabular/table_test.rb +++ b/test/non_tabular/table_test.rb @@ -50,7 +50,7 @@ def test_all_valid_options valid_options = %w[ canonical_name capture_end_line capture_start_line columns end_in_a_record end_line_pattern filename_pattern file_password format klass remove_lines row_identifier - significant_mapped_fields start_in_a_record start_line_pattern + significant_mapped_fields start_in_a_record start_line_pattern table_metadata ] assert_equal valid_options.sort, NdrImport::NonTabular::Table.all_valid_options.sort diff --git a/test/universal_importer_helper_test.rb b/test/universal_importer_helper_test.rb index c1e7fcf..d1fd052 100644 --- a/test/universal_importer_helper_test.rb +++ b/test/universal_importer_helper_test.rb @@ -248,4 +248,22 @@ class TestImporterWithoutNotifier assert_equal({ record_count: '6349923' }, table_mapping.table_metadata) end + + test 'should allow assigning to table_metadata directly in table definition' do + table_mappings = [ + NdrImport::Table.new(filename_pattern: /\.xls\z/i, + header_lines: 1, + footer_lines: 0, + table_metadata: { 'hello' => 'world' }, + klass: 'SomeTestClass', + columns: [{ 'column' => '1a' }]) + ] + source_file = @permanent_test_files.join('sample_xls.xls') + @test_importer.stubs(:get_table_mapping).returns(table_mappings.first) + @test_importer.extract(source_file) do |table, _rows| + assert_instance_of NdrImport::Table, table + + assert_equal({ 'hello' => 'world' }, table.table_metadata) + end + end end diff --git a/test/vcf/table_test.rb b/test/vcf/table_test.rb index 5ba78a7..d089a40 100644 --- a/test/vcf/table_test.rb +++ b/test/vcf/table_test.rb @@ -12,8 +12,8 @@ def setup test 'test_all_valid_options' do valid_options = %w[canonical_name columns file_password filename_pattern format klass - last_data_column liberal_parsing row_identifier - significant_mapped_fields slurp tablename_pattern vcf_file_metadata] + last_data_column liberal_parsing row_identifier significant_mapped_fields + slurp table_metadata tablename_pattern vcf_file_metadata] assert_equal valid_options.sort, NdrImport::Vcf::Table.all_valid_options.sort end From f8fd9d24ae6a5bb060194ea2220f4e0899589dc4 Mon Sep 17 00:00:00 2001 From: ollietulloch Date: Tue, 23 Jun 2026 09:01:22 +0100 Subject: [PATCH 2/4] Boyscouting: Use anonymous block forwarding --- lib/ndr_import/universal_importer_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ndr_import/universal_importer_helper.rb b/lib/ndr_import/universal_importer_helper.rb index fcba1e9..24e7569 100644 --- a/lib/ndr_import/universal_importer_helper.rb +++ b/lib/ndr_import/universal_importer_helper.rb @@ -73,7 +73,7 @@ def table_options_from(table_mapping) # This method does the table row yielding for the extract method, setting the notifier # so that we can monitor progress - def yield_tables_and_their_content(filename, tables, &block) + def yield_tables_and_their_content(filename, tables, &) return enum_for(:yield_tables_and_their_content, filename, tables) unless block_given? tables.each do |tablename, table_content, file_metadata| From 199c8add44d468e97522e60ae70ac54bddf03fca Mon Sep 17 00:00:00 2001 From: ollietulloch Date: Tue, 23 Jun 2026 09:22:53 +0100 Subject: [PATCH 3/4] Do not overwrite any non-directly assigned table_metadata --- lib/ndr_import/universal_importer_helper.rb | 5 +++-- test/universal_importer_helper_test.rb | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/ndr_import/universal_importer_helper.rb b/lib/ndr_import/universal_importer_helper.rb index 24e7569..b6caecb 100644 --- a/lib/ndr_import/universal_importer_helper.rb +++ b/lib/ndr_import/universal_importer_helper.rb @@ -66,7 +66,6 @@ def table_options_from(table_mapping) 'slurp' => table_mapping.try(:slurp), 'yield_xml_record' => table_mapping.try(:yield_xml_record), 'pattern_match_record_xpath' => table_mapping.try(:pattern_match_record_xpath), - 'file_metadata' => table_mapping.try(:file_metadata), 'xml_file_metadata' => table_mapping.try(:xml_file_metadata), 'vcf_file_metadata' => table_mapping.try(:vcf_file_metadata) } end @@ -81,7 +80,9 @@ def yield_tables_and_their_content(filename, tables, &) next if mapping.nil? mapping.notifier = get_notifier(record_total(filename, table_content)) - mapping.table_metadata ||= file_metadata || {} + # Merge file_metadata (from VCF or XML) with directly assigned table_metadata + # to form `table_metadata` + mapping.table_metadata = (mapping.table_metadata || {}).merge(file_metadata || {}) yield(mapping, table_content) end end diff --git a/test/universal_importer_helper_test.rb b/test/universal_importer_helper_test.rb index d1fd052..2a6f186 100644 --- a/test/universal_importer_helper_test.rb +++ b/test/universal_importer_helper_test.rb @@ -266,4 +266,22 @@ class TestImporterWithoutNotifier assert_equal({ 'hello' => 'world' }, table.table_metadata) end end + + test 'should merge file metadata with directly assigned metadata' do + table_mapping = + NdrImport::Xml::Table.new(filename_pattern: /.xml/i, + yield_xml_record: true, + xml_record_xpath: 'BreastRecord', + format: 'xml_table', + xml_file_metadata: { record_count: '//COSD:RecordCount/@value' }, + table_metadata: { hello: 'world' }, + klass: 'SomeTestClass', + columns: {}) + + source_file = @permanent_test_files.join('complex_xml.xml') + @test_importer.stubs(:get_table_mapping).returns(table_mapping) + @test_importer.extract(source_file) { |table, rows| table.transform(rows) } + + assert_equal({ record_count: '6349923', hello: 'world' }, table_mapping.table_metadata) + end end From 9460cb3dad993dfa277fb21d2f5f529bf663680f Mon Sep 17 00:00:00 2001 From: ollietulloch Date: Tue, 23 Jun 2026 09:32:42 +0100 Subject: [PATCH 4/4] Changelog update --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5241edd..4efbca7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## [Unreleased] ### Fixed * Support Rails 8.1, Ruby 4.0. Drop support for Ruby 3.2 +* Allow direct assignment of `table.table_metadata` ## 11.4.1 / 2025-11-05 ### Fixed