Bugzilla – Attachment 189562 Details for
Bug 40391
EDI: Add support for GIR:LSL field
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40391: Add unit tests for GIR:LSL support in EDI
Bug-40391-Add-unit-tests-for-GIRLSL-support-in-EDI.patch (text/plain), 20.83 KB, created by
Martin Renvoize (ashimema)
on 2025-11-13 14:56:31 UTC
(
hide
)
Description:
Bug 40391: Add unit tests for GIR:LSL support in EDI
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-11-13 14:56:31 UTC
Size:
20.83 KB
patch
obsolete
>From 77d8bb167f7bc75fad7254cbca24feb91536834f Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >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 <https://www.westminster.gov.uk/> >Sponsored-by: Royal Borough of Kensington and Chelsea <https://www.rbkc.gov.uk/> >Sponsored-by: OpenFifth <https://openfifth.co.uk/> >Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk> >--- > 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.51.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40391
:
185031
|
185032
|
185233
|
185234
|
187174
|
187175
|
189562
|
189563
|
189573
|
189574