Bugzilla – Attachment 193022 Details for
Bug 30144
Add support for EDI GIR:LVT and/or GIR:LVC, Servicing instructions, segment
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 30144: Add tests for EDIFACT servicing instruction handling
7fb56d3.patch (text/plain), 14.83 KB, created by
Martin Renvoize (ashimema)
on 2026-02-12 19:07:02 UTC
(
hide
)
Description:
Bug 30144: Add tests for EDIFACT servicing instruction handling
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-02-12 19:07:02 UTC
Size:
14.83 KB
patch
obsolete
>From 7fb56d34443e2e2b79d07d25475ba7e624973a9e Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >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 <martin.renvoize@gmail.com> >--- > 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 >
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 30144
:
193017
|
193018
|
193019
|
193020
|
193021
|
193022
|
193093
|
193094
|
193095
|
193096
|
193097
|
193098
|
193099