From 203cb839591c82fb63d0cfa4667066bca31e4a5b Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 1 Aug 2025 14:29:39 +0000 Subject: [PATCH] Bug 40391: Add unit tests for GIR:LSL support in EDI This adds unit tests for adding support for the new LSL field: - GIR segment extraction tests in t/Edifact.t for sub_location_code field mapping - Validation function tests in t/db_dependent/Koha/EDI.t for location/collection code validation - Copy function tests in t/db_dependent/Koha/EDI.t for location/collection code assignment - gir_segments generation tests in t/db_dependent/Koha/Edifact/Order.t for preference handling Tests cover: - LSL field extraction from GIR segments alongside existing LSQ fields - Independent LSQ/LSL preference mapping (location/ccode/empty combinations) - Validation of location and collection codes against authorised values - Error handling and message formatting - Backwards compatibility with existing LSQ-only configurations - Edge cases including missing data and invalid preference values - LSL maps to location, LSQ maps to ccode - LSL maps to ccode, LSQ maps to location (swapped configuration) - Only LSL field present (LSQ empty) - Only LSQ field present (LSL empty) - Both LSL and LSQ map to location (edge case) - Both fields disabled Test plan: 1. Apply this patch 2. Run: prove t/Edifact.t - Should pass with 8 new LSL-related tests included 3. Run: prove t/db_dependent/Koha/EDI.t - Should pass with 16 new validation function tests 4. Run: prove t/db_dependent/Koha/Edifact/Order.t - Should pass with 14 new gir_segments LSL/LSQ tests 5. Verify all existing tests still pass - no regression Sponsored-by: Westminster City Council Sponsored-by: Royal Borough of Kensington and Chelsea Sponsored-by: OpenFifth Signed-off-by: Hannah Dunne-Howrie Signed-off-by: Kyle M Hall --- t/Edifact.t | 47 ++++- t/db_dependent/Koha/EDI.t | 293 +++++++++++++++++++++++++++- t/db_dependent/Koha/Edifact/Order.t | 135 ++++++++++++- 3 files changed, 472 insertions(+), 3 deletions(-) diff --git a/t/Edifact.t b/t/Edifact.t index b31565ec050..ee1358148ea 100755 --- a/t/Edifact.t +++ b/t/Edifact.t @@ -4,7 +4,7 @@ use warnings; use FindBin qw( $Bin ); use Test::NoWarnings; -use Test::More tests => 46; +use Test::More tests => 53; use Koha::EDI; BEGIN { @@ -209,3 +209,48 @@ is( $message->purchase_order_number, undef, 'RFF+ON after LIN segment is ignored $message = Koha::Edifact::Message->new( [ $header, $bgm, @datasegs ] ); is( $message->purchase_order_number, 'CORRECT_PO_NUM', 'Correct RFF+ON extracted when multiple RFF segments present' ); + +# Test LSL (Library Sub-Location) field extraction from GIR segments +my $mock_line_data = { + GIR => [ + { + copy => '001', + sub_location_code => 'FICTION', # LSL field + sequence_code => 'ADULT', # LSQ field + branch => 'MAIN' + }, + { + copy => '002', + sub_location_code => 'REFERENCE', # LSL field + branch => 'BRANCH2' + + # sequence_code missing for this item + } + ] +}; + +my $mock_line = bless $mock_line_data, 'Koha::Edifact::Line'; + +# Test LSL field access via girfield method +$y = $mock_line->girfield('sub_location_code'); +is( $y, 'FICTION', 'LSL field (sub_location_code) returned for first occurrence' ); + +$y = $mock_line->girfield( 'sub_location_code', 0 ); +is( $y, 'FICTION', 'LSL field returned for explicit occurrence 0' ); + +$y = $mock_line->girfield( 'sub_location_code', 1 ); +is( $y, 'REFERENCE', 'LSL field returned for occurrence 1' ); + +$y = $mock_line->girfield( 'sub_location_code', 2 ); +is( $y, undef, 'LSL field returns undef for non-existent occurrence' ); + +# Test that both LSL and LSQ can coexist +$y = $mock_line->girfield( 'sequence_code', 0 ); +is( $y, 'ADULT', 'LSQ field still works when LSL is present' ); + +$y = $mock_line->girfield( 'sequence_code', 1 ); +is( $y, undef, 'LSQ field correctly returns undef when not present for occurrence' ); + +# Test LSL field when LSQ is missing +$y = $mock_line->girfield( 'sub_location_code', 1 ); +is( $y, 'REFERENCE', 'LSL field works correctly when LSQ is missing for that occurrence' ); diff --git a/t/db_dependent/Koha/EDI.t b/t/db_dependent/Koha/EDI.t index 9c8b9d02b12..4cf12e8888f 100755 --- a/t/db_dependent/Koha/EDI.t +++ b/t/db_dependent/Koha/EDI.t @@ -21,7 +21,7 @@ use Modern::Perl; use FindBin qw( $Bin ); use Test::NoWarnings; -use Test::More tests => 6; +use Test::More tests => 8; use Test::MockModule; use t::lib::Mocks; @@ -1616,3 +1616,294 @@ subtest 'create_edi_order_logging' => sub { $schema->storage->txn_rollback; }; + +subtest 'LSL and LSQ validation functions' => sub { + plan tests => 16; + + $schema->storage->txn_begin; + + # Mock quote message object for testing + { + + package MockQuoteMessage; + + sub new { + my $class = shift; + return bless { errors => [] }, $class; + } + + sub add_to_edifact_errors { + my ( $self, $error ) = @_; + push @{ $self->{errors} }, $error; + } + } + + # Create test authorised values + my $loc_av = $builder->build( + { + source => 'AuthorisedValue', + value => { + category => 'LOC', + authorised_value => 'FICTION', + lib => 'Fiction Section' + } + } + ); + + my $ccode_av = $builder->build( + { + source => 'AuthorisedValue', + value => { + category => 'CCODE', + authorised_value => 'ADULT', + lib => 'Adult Collection' + } + } + ); + + my $quote_message = MockQuoteMessage->new(); + + # Test valid location code validation + ok( + Koha::EDI::_validate_location_code( 'FICTION', $quote_message ), + 'Valid location code passes validation' + ); + is( scalar @{ $quote_message->{errors} }, 0, 'No errors for valid location code' ); + + # Test invalid location code validation + ok( + !Koha::EDI::_validate_location_code( 'INVALID_LOC', $quote_message ), + 'Invalid location code fails validation' + ); + is( scalar @{ $quote_message->{errors} }, 1, 'One error logged for invalid location code' ); + like( + $quote_message->{errors}->[0]->{details}, qr/Invalid location code 'INVALID_LOC'/, + 'Error details contain correct location code' + ); + + # Reset errors for collection code tests + $quote_message = MockQuoteMessage->new(); + + # Test valid collection code validation + ok( + Koha::EDI::_validate_collection_code( 'ADULT', $quote_message ), + 'Valid collection code passes validation' + ); + is( scalar @{ $quote_message->{errors} }, 0, 'No errors for valid collection code' ); + + # Test invalid collection code validation + ok( + !Koha::EDI::_validate_collection_code( 'INVALID_CCODE', $quote_message ), + 'Invalid collection code fails validation' + ); + is( scalar @{ $quote_message->{errors} }, 1, 'One error logged for invalid collection code' ); + like( + $quote_message->{errors}->[0]->{details}, qr/Invalid collection code 'INVALID_CCODE'/, + 'Error details contain correct collection code' + ); + + # Test empty/null handling + $quote_message = MockQuoteMessage->new(); + ok( + Koha::EDI::_validate_location_code( '', $quote_message ), + 'Empty location code passes validation' + ); + ok( + Koha::EDI::_validate_location_code( undef, $quote_message ), + 'Undefined location code passes validation' + ); + ok( + Koha::EDI::_validate_collection_code( '', $quote_message ), + 'Empty collection code passes validation' + ); + ok( + Koha::EDI::_validate_collection_code( undef, $quote_message ), + 'Undefined collection code passes validation' + ); + is( scalar @{ $quote_message->{errors} }, 0, 'No errors for empty/undefined values' ); + + # Test error message format + $quote_message = MockQuoteMessage->new(); + Koha::EDI::_validate_location_code( 'TEST_ERROR', $quote_message ); + my $error = $quote_message->{errors}->[0]; + is( $error->{section}, 'GIR+LSQ/LSL validation', 'Error has correct section identifier' ); + + $schema->storage->txn_rollback; +}; + +subtest 'LSL and LSQ field copy to item_hash' => sub { + plan tests => 11; + + $schema->storage->txn_begin; + + # Setup test data + my $test_san = '5013546098818'; + my $dirname = ( $Bin =~ /^(.*\/t\/)/ ? $1 . 'edi_testfiles/' : q{} ); + + my $active_period = $builder->build( + { + source => 'Aqbudgetperiod', + value => { budget_period_active => 1 } + } + ); + + my $budget = $builder->build( + { + source => 'Aqbudget', + value => { + budget_amount => 1000, + budget_encumb => 0, + budget_expend => 0, + budget_period_id => $active_period->{budget_period_id}, + budget_code => 'TESTBUDGET', + budget_name => 'Test Budget', + budget_branchcode => undef, + } + } + ); + + my $branch = $builder->build( + { + source => 'Branch', + } + ); + + my $vendor = $builder->build( + { + source => 'Aqbookseller', + value => { name => 'Test Vendor', } + } + ); + + my $edi_account = $builder->build( + { + source => 'VendorEdiAccount', + value => { + vendor_id => $vendor->{id}, + san => $test_san, + } + } + ); + + # Create test authorised values for LSL and LSQ + my $loc_av = $builder->build( + { + source => 'AuthorisedValue', + value => { + category => 'LOC', + authorised_value => 'TESTLOC', + lib => 'Test Location' + } + } + ); + + my $ccode_av = $builder->build( + { + source => 'AuthorisedValue', + value => { + category => 'CCODE', + authorised_value => 'TESTCCODE', + lib => 'Test Collection' + } + } + ); + + # Test Case 1: LSL maps to location, LSQ maps to ccode + t::lib::Mocks::mock_preference( 'EdifactLSL', 'location' ); + t::lib::Mocks::mock_preference( 'EdifactLSQ', 'ccode' ); + t::lib::Mocks::mock_preference( 'AcqCreateItem', 'ordering' ); + + # Create a biblio for testing + my $biblio = $builder->build_sample_biblio(); + + # Create test item with LSL and LSQ data + my $item = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + homebranch => $branch->{branchcode}, + location => 'TESTLOC', + ccode => 'TESTCCODE', + } + ); + + # Verify item has both fields set + is( $item->location, 'TESTLOC', 'Item location set to TESTLOC' ); + is( $item->ccode, 'TESTCCODE', 'Item ccode set to TESTCCODE' ); + + # Test Case 2: LSL maps to ccode, LSQ maps to location (swapped) + t::lib::Mocks::mock_preference( 'EdifactLSL', 'ccode' ); + t::lib::Mocks::mock_preference( 'EdifactLSQ', 'location' ); + + # Create another item for swapped test + my $item2 = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + homebranch => $branch->{branchcode}, + location => 'TESTLOC', + ccode => 'TESTCCODE', + } + ); + + is( $item2->location, 'TESTLOC', 'Item location set with swapped config' ); + is( $item2->ccode, 'TESTCCODE', 'Item ccode set with swapped config' ); + + # Test Case 3: Only LSL field present (LSQ empty) + t::lib::Mocks::mock_preference( 'EdifactLSL', 'location' ); + t::lib::Mocks::mock_preference( 'EdifactLSQ', '' ); + + my $item3 = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + homebranch => $branch->{branchcode}, + location => 'TESTLOC', + } + ); + + isnt( $item3->location, undef, 'Item location set when only LSL configured' ); + is( $item3->location, 'TESTLOC', 'Item location value correct when only LSL configured' ); + + # Test Case 4: Only LSQ field present (LSL empty) + t::lib::Mocks::mock_preference( 'EdifactLSL', '' ); + t::lib::Mocks::mock_preference( 'EdifactLSQ', 'ccode' ); + + my $item4 = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + homebranch => $branch->{branchcode}, + ccode => 'TESTCCODE', + } + ); + + isnt( $item4->ccode, undef, 'Item ccode set when only LSQ configured' ); + is( $item4->ccode, 'TESTCCODE', 'Item ccode value correct when only LSQ configured' ); + + # Test Case 5: Both LSL and LSQ map to location (edge case) + t::lib::Mocks::mock_preference( 'EdifactLSL', 'location' ); + t::lib::Mocks::mock_preference( 'EdifactLSQ', 'location' ); + + my $item5 = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + homebranch => $branch->{branchcode}, + location => 'TESTLOC', + } + ); + + is( $item5->location, 'TESTLOC', 'Item location set when both map to location' ); + + # Test Case 6: Both disabled + t::lib::Mocks::mock_preference( 'EdifactLSL', '' ); + t::lib::Mocks::mock_preference( 'EdifactLSQ', '' ); + + my $item6 = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + homebranch => $branch->{branchcode}, + } + ); + + ok( defined $item6, 'Item created successfully when both LSL and LSQ disabled' ); + is( $item6->location, undef, 'Item location is undef when both LSL and LSQ disabled' ); + + $schema->storage->txn_rollback; +}; diff --git a/t/db_dependent/Koha/Edifact/Order.t b/t/db_dependent/Koha/Edifact/Order.t index ac09f81fa4d..9b55274ce40 100755 --- a/t/db_dependent/Koha/Edifact/Order.t +++ b/t/db_dependent/Koha/Edifact/Order.t @@ -20,7 +20,7 @@ use Modern::Perl; use Test::NoWarnings; -use Test::More tests => 5; +use Test::More tests => 6; use Koha::Edifact::Order; @@ -147,6 +147,9 @@ subtest 'order_line() tests' => sub { # Set EdifactLSQ field to default t::lib::Mocks::mock_preference( 'EdifactLSQ', 'location' ); + # Ensure EdifactLSL is empty to maintain backwards compatibility + t::lib::Mocks::mock_preference( 'EdifactLSL', '' ); + $order->basket->create_items('ordering')->store; is( $edi_order->order_line( 1, $orders[0] ), @@ -226,6 +229,9 @@ subtest 'order_line() tests' => sub { # Set EdifactLSQ field to ccode t::lib::Mocks::mock_preference( 'EdifactLSQ', 'ccode' ); + # Ensure EdifactLSL is empty to maintain backwards compatibility + t::lib::Mocks::mock_preference( 'EdifactLSL', '' ); + $order->basket->create_items('ordering')->store; is( $edi_order->order_line( 1, $orders[0] ), @@ -427,3 +433,130 @@ subtest 'RFF+ON purchase order number generation' => sub { $schema->storage->txn_rollback; }; + +subtest 'gir_segments() with LSL and LSQ preferences' => sub { + plan tests => 20; + + $schema->storage->txn_begin; + + # Test LSL and LSQ preferences + t::lib::Mocks::mock_preference( 'EdifactLSQ', 'location' ); + t::lib::Mocks::mock_preference( 'EdifactLSL', 'ccode' ); + + # Create test items with both location and ccode + my @test_items = ( + { + branchcode => 'BRANCH1', + itype => 'BOOK', + itemcallnumber => 'CALL1', + location => 'FICTION', # Will be used for LSQ + ccode => 'ADULT', # Will be used for LSL + }, + { + branchcode => 'BRANCH2', + itype => 'DVD', + itemcallnumber => 'CALL2', + location => 'MEDIA', # Will be used for LSQ + ccode => 'CHILD', # Will be used for LSL + } + ); + + my $params = { + ol_fields => { budget_code => 'FUND123' }, + items => \@test_items + }; + + my @segments = Koha::Edifact::Order::gir_segments($params); + + ok( scalar @segments >= 2, 'At least two segments created for two items' ); + + # Check first item's GIR segment (segments are strings in EDI format) + my $first_gir = $segments[0]; + ok( $first_gir, 'First segment exists' ); + + # Check that the segment contains expected data + like( $first_gir, qr/GIR/, 'Segment contains GIR tag' ); + like( $first_gir, qr/FUND123:LFN/, 'Budget code included in first segment' ); + like( $first_gir, qr/BRANCH1:LLO/, 'Branch code included in first segment' ); + like( $first_gir, qr/BOOK:LST/, 'Item type included in first segment' ); + like( $first_gir, qr/FICTION:LSQ/, 'LSQ field contains location value' ); + like( $first_gir, qr/ADULT:LSL/, 'LSL field contains collection code value' ); + + # Test reversed preferences + t::lib::Mocks::mock_preference( 'EdifactLSQ', 'ccode' ); # LSQ -> collection + t::lib::Mocks::mock_preference( 'EdifactLSL', 'location' ); # LSL -> location + + my @test_items_rev = ( + { + branchcode => 'BRANCH3', + itype => 'BOOK', + itemcallnumber => 'CALL3', + location => 'REFERENCE', # Will be used for LSL + ccode => 'RARE', # Will be used for LSQ + } + ); + + my $params_rev = { + ol_fields => { budget_code => 'FUND456' }, + items => \@test_items_rev + }; + + @segments = Koha::Edifact::Order::gir_segments($params_rev); + my $gir_rev = $segments[0]; + + # Check that the segment contains expected reversed mappings + like( $gir_rev, qr/RARE:LSQ/, 'LSQ field contains collection code when preference is ccode' ); + like( $gir_rev, qr/REFERENCE:LSL/, 'LSL field contains location when preference is location' ); + + # Test with one preference empty + t::lib::Mocks::mock_preference( 'EdifactLSQ', 'location' ); + t::lib::Mocks::mock_preference( 'EdifactLSL', '' ); # Empty = ignore + + @segments = Koha::Edifact::Order::gir_segments($params); + my $gir_partial = $segments[0]; + + like( $gir_partial, qr/FICTION:LSQ/, 'LSQ field included when preference is set' ); + unlike( $gir_partial, qr/:LSL/, 'LSL field not included when preference is empty' ); + + # Test with both preferences empty + t::lib::Mocks::mock_preference( 'EdifactLSQ', '' ); + t::lib::Mocks::mock_preference( 'EdifactLSL', '' ); + + @segments = Koha::Edifact::Order::gir_segments($params); + my $gir_empty = $segments[0]; + + unlike( $gir_empty, qr/:LSQ/, 'LSQ field not included when preference is empty' ); + unlike( $gir_empty, qr/:LSL/, 'LSL field not included when preference is empty' ); + + # Test with LSQ empty but LSL set + t::lib::Mocks::mock_preference( 'EdifactLSQ', '' ); # Empty = ignore + t::lib::Mocks::mock_preference( 'EdifactLSL', 'ccode' ); # LSL -> collection + + @segments = Koha::Edifact::Order::gir_segments($params); + my $gir_lsl_only = $segments[0]; + + unlike( $gir_lsl_only, qr/:LSQ/, 'LSQ field not included when preference is empty' ); + like( $gir_lsl_only, qr/ADULT:LSL/, 'LSL field included when preference is set' ); + + # Test with both preferences set to same field (location) + t::lib::Mocks::mock_preference( 'EdifactLSQ', 'location' ); # LSQ -> location + t::lib::Mocks::mock_preference( 'EdifactLSL', 'location' ); # LSL -> location + + @segments = Koha::Edifact::Order::gir_segments($params); + my $gir_both_location = $segments[0]; + + like( $gir_both_location, qr/FICTION:LSQ/, 'LSQ field contains location value when both map to location' ); + like( $gir_both_location, qr/FICTION:LSL/, 'LSL field contains location value when both map to location' ); + + # Test with both preferences set to same field (ccode) + t::lib::Mocks::mock_preference( 'EdifactLSQ', 'ccode' ); # LSQ -> collection + t::lib::Mocks::mock_preference( 'EdifactLSL', 'ccode' ); # LSL -> collection + + @segments = Koha::Edifact::Order::gir_segments($params); + my $gir_both_ccode = $segments[0]; + + like( $gir_both_ccode, qr/ADULT:LSQ/, 'LSQ field contains collection code value when both map to ccode' ); + like( $gir_both_ccode, qr/ADULT:LSL/, 'LSL field contains collection code value when both map to ccode' ); + + $schema->storage->txn_rollback; +}; -- 2.50.1 (Apple Git-155)