From 7fb56d34443e2e2b79d07d25475ba7e624973a9e Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 12 Feb 2026 18:39:04 +0000 Subject: [PATCH] Bug 30144: Add tests for EDIFACT servicing instruction handling This commit adds comprehensive tests for servicing instruction processing in both incoming quotes and outgoing orders: 1. Integration tests for incoming quote processing (Koha/EDI.pm) - Tests LVC (coded) and LVT (freetext) extraction - Tests multiple GIR occurrences with different servicing instructions - Tests all combinations: LVC only, LVT only, and LVC+LVT together 2. Test EDIFACT quote file (QUOTES_SERVICING.CEQ) - Contains 3 line items with 4 total orders - Demonstrates different servicing instruction patterns 3. Fix for multi-occurrence servicing instruction extraction - Modified quote_item() to extract servicing instructions per GIR occurrence instead of across all occurrences - Ensures each order gets only its own servicing instructions - Aligns with existing multi-item processing for funds and stock rotation Test coverage: - t/db_dependent/Koha/EDI.t: 29 tests for incoming quote processing - t/db_dependent/Koha/Edifact/Order.t: 18 tests for outgoing order generation (previously added) All tests pass successfully. Sponsored-by: Martin Renvoize --- Koha/EDI.pm | 80 ++++++----- t/db_dependent/Koha/EDI.t | 198 ++++++++++++++++++++++++++- t/edi_testfiles/QUOTES_SERVICING.CEQ | 1 + 3 files changed, 244 insertions(+), 35 deletions(-) create mode 100644 t/edi_testfiles/QUOTES_SERVICING.CEQ diff --git a/Koha/EDI.pm b/Koha/EDI.pm index f3023e7494b..870dc68073c 100644 --- a/Koha/EDI.pm +++ b/Koha/EDI.pm @@ -917,49 +917,52 @@ sub quote_item { $order_hash->{line_item_id} = $item->item_number_id; } - # Extract servicing instructions (LVT freetext and/or LVC coded) - # Build JSON array structure: [[{type,value},...]] - my @servicing_groups; - my @current_group; + # Helper function to extract servicing instructions for a specific GIR occurrence + my $extract_servicing_instructions = sub { + my ($occurrence) = @_; + my @group; - # Read LVC (coded) instructions - my $occ = 0; - my $lvc_val; - while ( $lvc_val = $item->girfield( 'coded_servicing_instruction', $occ ) ) { + # Read LVC (coded) instruction for this occurrence + my $lvc_val = $item->girfield( 'coded_servicing_instruction', $occurrence ); + if ($lvc_val) { - # Validate against EDIFACT_SI authorized values - my $av = Koha::AuthorisedValues->find( - { - category => 'EDIFACT_SI', - authorised_value => $lvc_val - } - ); - if ($av) { - push @current_group, { type => 'LVC', value => $lvc_val }; - } else { - $quote_message->add_to_edifact_errors( + # Validate against EDIFACT_SI authorized values + my $av = Koha::AuthorisedValues->find( { - section => "GIR+LVC validation", - details => - "Invalid servicing instruction code '$lvc_val' - not found in EDIFACT_SI authorized values" + category => 'EDIFACT_SI', + authorised_value => $lvc_val } ); - $logger->trace("Invalid servicing instruction code: $lvc_val"); + if ($av) { + push @group, { type => 'LVC', value => $lvc_val }; + } else { + $quote_message->add_to_edifact_errors( + { + section => "GIR+LVC validation", + details => + "Invalid servicing instruction code '$lvc_val' - not found in EDIFACT_SI authorized values" + } + ); + $logger->trace("Invalid servicing instruction code: $lvc_val"); + } } - ++$occ; - } - # Read LVT (freetext) instructions - $occ = 0; - my $lvt_val; - while ( $lvt_val = $item->girfield( 'servicing_instruction', $occ ) ) { - push @current_group, { type => 'LVT', value => $lvt_val }; - ++$occ; - } + # Read LVT (freetext) instruction for this occurrence + my $lvt_val = $item->girfield( 'servicing_instruction', $occurrence ); + if ($lvt_val) { + push @group, { type => 'LVT', value => $lvt_val }; + } + + return \@group; + }; + + # Extract servicing instructions for first order (occurrence 0) + my @servicing_groups; + my $current_group = $extract_servicing_instructions->(0); # Store as JSON if we have any instructions - if (@current_group) { - push @servicing_groups, \@current_group; + if (@$current_group) { + push @servicing_groups, $current_group; $order_hash->{servicing_instruction} = encode_json( \@servicing_groups ); } if ($order_note) { @@ -1090,6 +1093,15 @@ sub quote_item { $order_hash->{budget_id} = $budget->budget_id; + # Extract servicing instructions for this occurrence + my $si_group = $extract_servicing_instructions->($occurrence); + if (@$si_group) { + my @si_groups = ($si_group); + $order_hash->{servicing_instruction} = encode_json( \@si_groups ); + } else { + $order_hash->{servicing_instruction} = undef; + } + my $new_order = $schema->resultset('Aqorder')->create($order_hash); my $o = $new_order->ordernumber(); $logger->trace("Order created: $o"); diff --git a/t/db_dependent/Koha/EDI.t b/t/db_dependent/Koha/EDI.t index 4cf12e8888f..a751692b389 100755 --- a/t/db_dependent/Koha/EDI.t +++ b/t/db_dependent/Koha/EDI.t @@ -21,8 +21,9 @@ use Modern::Perl; use FindBin qw( $Bin ); use Test::NoWarnings; -use Test::More tests => 8; +use Test::More tests => 9; use Test::MockModule; +use JSON qw( decode_json ); use t::lib::Mocks; use t::lib::Mocks::Logger; @@ -1907,3 +1908,198 @@ subtest 'LSL and LSQ field copy to item_hash' => sub { $schema->storage->txn_rollback; }; + +subtest 'servicing_instructions_quote_processing' => sub { + plan tests => 29; + + $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 } + } + ); + + # Create file transport + my $file_transport = $builder->build( + { + source => 'FileTransport', + value => { + name => 'Test Servicing Transport', + transport => 'local', + download_directory => $dirname, + upload_directory => $dirname, + } + } + ); + + # Create vendor EDI account + my $account = $builder->build( + { + source => 'VendorEdiAccount', + value => { + description => 'servicing instruction vendor', + file_transport_id => $file_transport->{file_transport_id}, + plugin => '', + san => $test_san, + orders_enabled => 1, + auto_orders => 0, + } + } + ); + + my $ean = $builder->build( + { + source => 'EdifactEan', + value => { + description => 'test ean', + branchcode => undef, + ean => $test_san + } + } + ); + + # Setup branches used in test file + for my $code (qw(CPL MPL FPL)) { + local $SIG{__WARN__} = sub { }; # Suppress warnings + eval { + $builder->build( + { + source => 'Branch', + value => { branchcode => $code } + } + ); + }; # Ignore duplicate key errors + } + + # Setup funds used in test file + for my $code (qw(REF LOAN)) { + $builder->build( + { + source => 'Aqbudget', + value => { + budget_code => $code, + budget_period_id => $active_period->{budget_period_id} + } + } + ); + } + + # Add EDIFACT_SI authorized values for servicing instruction codes + for my $code ( [ 'BB', 'Barcode labelling' ], [ 'BI', 'Binding' ], [ 'BJ', 'Sleeving' ] ) { + $builder->build( + { + source => 'AuthorisedValue', + value => { + category => 'EDIFACT_SI', + authorised_value => $code->[0], + lib => $code->[1] + } + } + ); + } + + my $filename = 'QUOTES_SERVICING.CEQ'; + ok( -e $dirname . $filename, 'File QUOTES_SERVICING.CEQ found' ); + + my $trans = Koha::Edifact::Transport->new( $account->{id} ); + $trans->working_directory($dirname); + + my $mhash = $trans->message_hash(); + $mhash->{message_type} = 'QUOTE'; + $trans->ingest( $mhash, $filename ); + + my $quote = $schema->resultset('EdifactMessage')->find( { filename => $filename } ); + ok( $quote, 'Quote message created' ); + + t::lib::Mocks::mock_preference( 'AcqCreateItem', 'ordering' ); + + # Process quote + my $die; + eval { + process_quote($quote); + 1; + } or do { + $die = $@; + }; + ok( !$die, 'Quote with servicing instructions processed without dying' ); + + # Get the basket + my $baskets = Koha::Acquisition::Baskets->search( { booksellerid => $account->{vendor_id} } ); + is( $baskets->count, 1, 'One basket created' ); + + my $basket = $baskets->next; + my $orders = $basket->orders->search( {}, { order_by => 'ordernumber' } ); + is( $orders->count, 4, 'Four order lines created (1 + 2 + 1)' ); + + # Test Case 1: First order - both LVC (BB) and LVT in same group + my $order1 = $orders->next; + ok( $order1->servicing_instruction, 'First order has servicing_instruction' ); + + my $si1 = decode_json( $order1->servicing_instruction ); + is( ref($si1), 'ARRAY', 'servicing_instruction is an array' ); + is( scalar @$si1, 1, 'First order has 1 servicing instruction group' ); + + my $group1 = $si1->[0]; + is( ref($group1), 'ARRAY', 'First group is an array' ); + is( scalar @$group1, 2, 'First group has 2 instructions (LVC + LVT)' ); + + # Find LVC and LVT entries + my ($lvc1) = grep { $_->{type} eq 'LVC' } @$group1; + my ($lvt1) = grep { $_->{type} eq 'LVT' } @$group1; + + ok( $lvc1, 'First order has LVC instruction' ); + is( $lvc1->{value}, 'BB', 'LVC value is BB (Barcode labelling)' ); + + ok( $lvt1, 'First order has LVT instruction' ); + is( $lvt1->{value}, 'Spine label required', 'LVT value is correct freetext' ); + + # Test Case 2: Second order - only LVC (BI) + my $order2 = $orders->next; + ok( $order2->servicing_instruction, 'Second order has servicing_instruction' ); + + my $si2 = decode_json( $order2->servicing_instruction ); + is( scalar @$si2, 1, 'Second order has 1 servicing instruction group' ); + + my $group2 = $si2->[0]; + is( scalar @$group2, 1, 'Second group has 1 instruction (LVC only)' ); + + is( $group2->[0]->{type}, 'LVC', 'Second order has LVC type' ); + is( $group2->[0]->{value}, 'BI', 'LVC value is BI (Binding)' ); + + # Test Case 3: Third order - LVC (BJ) and LVT in same group + my $order3 = $orders->next; + ok( $order3->servicing_instruction, 'Third order has servicing_instruction' ); + + my $si3 = decode_json( $order3->servicing_instruction ); + is( scalar @$si3, 1, 'Third order has 1 servicing instruction group' ); + + my $group3 = $si3->[0]; + is( scalar @$group3, 2, 'Third group has 2 instructions (LVC + LVT)' ); + + my ($lvc3) = grep { $_->{type} eq 'LVC' } @$group3; + my ($lvt3) = grep { $_->{type} eq 'LVT' } @$group3; + + is( $lvc3->{value}, 'BJ', 'Third order LVC is BJ (Sleeving)' ); + is( $lvt3->{value}, 'Handle with care', 'Third order LVT has correct text' ); + + # Test Case 4: Fourth order - only LVT + my $order4 = $orders->next; + ok( $order4->servicing_instruction, 'Fourth order has servicing_instruction' ); + + my $si4 = decode_json( $order4->servicing_instruction ); + is( scalar @$si4, 1, 'Fourth order has 1 servicing instruction group' ); + + my $group4 = $si4->[0]; + is( scalar @$group4, 1, 'Fourth group has 1 instruction (LVT only)' ); + + is( $group4->[0]->{type}, 'LVT', 'Fourth order has LVT type' ); + is( $group4->[0]->{value}, 'Rush processing required', 'Fourth order LVT has correct text' ); + + $schema->storage->txn_rollback; +}; diff --git a/t/edi_testfiles/QUOTES_SERVICING.CEQ b/t/edi_testfiles/QUOTES_SERVICING.CEQ new file mode 100644 index 00000000000..b5e01ee3b3d --- /dev/null +++ b/t/edi_testfiles/QUOTES_SERVICING.CEQ @@ -0,0 +1 @@ +UNA:+.? 'UNB+UNOC:3+5013546027173+5013546098818+240123:0945+1044417+ASKEDI:+QUOTES++++'UNH+1044417001+QUOTES:D:96A:UN:EAN002'BGM+31C::28+WO0800132+9'DTM+137:20240123:102'RFF+ON:orders 24/1'CUX+2:GBP:12'NAD+BY+5013546098818::9'NAD+SU+5013546027173::9'LIN+1++9781234567890:EN'PIA+5+1234567890:IB'IMD+L+010+:::Smith'IMD+L+011+:::John'IMD+L+050+:::Test Book with Servicing'IMD+L+060+:::test description'IMD+L+120+:::Test Publisher'IMD+L+170+:::2024'IMD+L+180+:::200'IMD+L+220+:::Hbk'IMD+L+230+:::123.45'IMD+L+250+:::AN'QTY+1:1'GIR+001+BOOK:LST+ANF:LSQ+CPL:LLO+REF:LFN+123.45:LCL'GIR+001+BB:LVC+Spine label required:LVT'GIR+001+15.00:LCV'PRI+AAE:15.00'RFF+QLI:WO0800132000001'LIN+2++9781234567891:EN'PIA+5+1234567891:IB'IMD+L+010+:::Jones'IMD+L+011+:::Sarah'IMD+L+050+:::Another Test Book'IMD+L+060+:::second test'IMD+L+120+:::Another Publisher'IMD+L+170+:::2024'IMD+L+180+:::150'IMD+L+220+:::Pbk'IMD+L+230+:::456.78'IMD+L+250+:::AN'QTY+1:2'GIR+001+BOOK:LST+FIC:LSQ+MPL:LLO+LOAN:LFN+456.78:LCL'GIR+001+BI:LVC'GIR+001+20.00:LCV'GIR+002+BOOK:LST+FIC:LSQ+CPL:LLO+REF:LFN+456.78:LCL'GIR+002+BJ:LVC+Handle with care:LVT'GIR+002+20.00:LCV'PRI+AAE:20.00'RFF+QLI:WO0800132000002'LIN+3++9781234567892:EN'PIA+5+1234567892:IB'IMD+L+010+:::Brown'IMD+L+011+:::Emily'IMD+L+050+:::Third Test Book'IMD+L+060+:::third test'IMD+L+120+:::Third Publisher'IMD+L+170+:::2024'IMD+L+180+:::180'IMD+L+220+:::Hbk'IMD+L+230+:::789.00'IMD+L+250+:::AN'QTY+1:1'GIR+001+BOOK:LST+REF:LSQ+FPL:LLO+LOAN:LFN+789.00:LCL'GIR+001+Rush processing required:LVT'GIR+001+25.00:LCV'PRI+AAE:25.00'RFF+QLI:WO0800132000003'UNS+S'CNT+2:3'UNT+58+1044417001'UNZ+1+1044417' -- 2.53.0