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>
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Martin Renvoize
Pedro Amorim
parent
b02cd5e1fe
commit
711bb2dd76
52
Koha/EDI.pm
52
Koha/EDI.pm
|
|
@ -959,15 +959,14 @@ 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(
|
||||
|
|
@ -977,7 +976,7 @@ sub quote_item {
|
|||
}
|
||||
);
|
||||
if ($av) {
|
||||
push @current_group, { type => 'LVC', value => $lvc_val };
|
||||
push @group, { type => 'LVC', value => $lvc_val };
|
||||
} else {
|
||||
$quote_message->add_to_edifact_errors(
|
||||
{
|
||||
|
|
@ -986,23 +985,29 @@ sub quote_item {
|
|||
"Invalid servicing instruction code '$lvc_val' - not found in EDIFACT_SI authorized values"
|
||||
}
|
||||
);
|
||||
$logger->trace("Invalid servicing instruction code: $lvc_val");
|
||||
$logger->warn("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 );
|
||||
} else {
|
||||
$order_hash->{servicing_instruction} = undef;
|
||||
}
|
||||
if ($order_note) {
|
||||
$order_hash->{order_vendornote} = $order_note;
|
||||
|
|
@ -1132,6 +1137,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");
|
||||
|
|
|
|||
|
|
@ -22,8 +22,9 @@ use FindBin qw( $Bin );
|
|||
|
||||
use Test::Exception;
|
||||
use Test::NoWarnings;
|
||||
use Test::More tests => 9;
|
||||
use Test::More tests => 11;
|
||||
use Test::MockModule;
|
||||
use JSON qw( decode_json );
|
||||
|
||||
use t::lib::Mocks;
|
||||
use t::lib::Mocks::Logger;
|
||||
|
|
@ -2125,6 +2126,330 @@ subtest 'process_ordrsp' => sub {
|
|||
'Item deletion failure recorded in the EDI log'
|
||||
);
|
||||
};
|
||||
$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.
|
||||
# These normally already exist as installer defaults; only build them
|
||||
# here as a fallback for environments where those defaults are missing.
|
||||
for my $code ( [ 'BB', 'Barcode labelling' ], [ 'BI', 'Binding' ], [ 'BJ', 'Sleeving' ] ) {
|
||||
local $SIG{__WARN__} = sub { }; # Suppress warnings
|
||||
eval {
|
||||
$builder->build(
|
||||
{
|
||||
source => 'AuthorisedValue',
|
||||
value => {
|
||||
category => 'EDIFACT_SI',
|
||||
authorised_value => $code->[0],
|
||||
lib => $code->[1]
|
||||
}
|
||||
}
|
||||
);
|
||||
}; # Ignore duplicate key errors
|
||||
}
|
||||
|
||||
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;
|
||||
};
|
||||
|
||||
subtest 'servicing_instructions_invalid_lvc_code' => sub {
|
||||
plan tests => 8;
|
||||
|
||||
$schema->storage->txn_begin;
|
||||
|
||||
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 $file_transport = $builder->build(
|
||||
{
|
||||
source => 'FileTransport',
|
||||
value => {
|
||||
name => 'Test Invalid LVC Transport',
|
||||
transport => 'local',
|
||||
download_directory => $dirname,
|
||||
upload_directory => $dirname,
|
||||
}
|
||||
}
|
||||
);
|
||||
|
||||
my $account = $builder->build(
|
||||
{
|
||||
source => 'VendorEdiAccount',
|
||||
value => {
|
||||
description => 'invalid lvc code vendor',
|
||||
file_transport_id => $file_transport->{file_transport_id},
|
||||
plugin => '',
|
||||
san => $test_san,
|
||||
orders_enabled => 1,
|
||||
auto_orders => 0,
|
||||
}
|
||||
}
|
||||
);
|
||||
|
||||
$builder->build(
|
||||
{
|
||||
source => 'EdifactEan',
|
||||
value => {
|
||||
description => 'test ean',
|
||||
branchcode => undef,
|
||||
ean => $test_san
|
||||
}
|
||||
}
|
||||
);
|
||||
|
||||
for my $code (qw(CPL)) {
|
||||
local $SIG{__WARN__} = sub { };
|
||||
eval {
|
||||
$builder->build(
|
||||
{
|
||||
source => 'Branch',
|
||||
value => { branchcode => $code }
|
||||
}
|
||||
);
|
||||
};
|
||||
}
|
||||
|
||||
$builder->build(
|
||||
{
|
||||
source => 'Aqbudget',
|
||||
value => {
|
||||
budget_code => 'LOAN',
|
||||
budget_period_id => $active_period->{budget_period_id}
|
||||
}
|
||||
}
|
||||
);
|
||||
|
||||
# Deliberately do NOT create an EDIFACT_SI authorised value for 'ZZ' - it
|
||||
# must be treated as invalid.
|
||||
|
||||
my $filename = 'QUOTES_SERVICING_INVALID_LVC.CEQ';
|
||||
ok( -e $dirname . $filename, 'File QUOTES_SERVICING_INVALID_LVC.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 } );
|
||||
|
||||
t::lib::Mocks::mock_preference( 'AcqCreateItem', 'ordering' );
|
||||
|
||||
$logger->clear;
|
||||
my $die;
|
||||
eval {
|
||||
process_quote($quote);
|
||||
1;
|
||||
} or do {
|
||||
$die = $@;
|
||||
};
|
||||
ok( !$die, 'Quote with an invalid servicing instruction code processed without dying' );
|
||||
|
||||
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, 1, 'One order line created' );
|
||||
|
||||
my $order = $orders->next;
|
||||
|
||||
# The invalid LVC is skipped, but the LVT in the same group is still kept
|
||||
ok( $order->servicing_instruction, 'Order still has a servicing_instruction (LVT survives)' );
|
||||
my $si = decode_json( $order->servicing_instruction );
|
||||
my $group = $si->[0];
|
||||
is( scalar @$group, 1, 'Group has only the LVT instruction, the invalid LVC was dropped' );
|
||||
is( $group->[0]->{type}, 'LVT', 'Remaining instruction is the LVT one' );
|
||||
|
||||
$logger->warn_like(
|
||||
qr/Invalid servicing instruction code/,
|
||||
'A warning is logged for the invalid LVC code'
|
||||
);
|
||||
|
||||
$schema->storage->txn_rollback;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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'
|
||||
|
|
@ -0,0 +1 @@
|
|||
UNA:+.? 'UNB+UNOC:3+5013546027173+5013546098818+240123:0945+1044418+ASKEDI:+QUOTES++++'UNH+1044418001+QUOTES:D:96A:UN:EAN002'BGM+31C::28+WO0800133+9'DTM+137:20240123:102'RFF+ON:orders24/2'CUX+2:GBP:12'NAD+BY+5013546098818::9'NAD+SU+5013546027173::9'LIN+1++9781234567899:EN'PIA+5+1234567899:IB'IMD+L+010+:::Doe'IMD+L+011+:::Jane'IMD+L+050+:::Invalid Code Test Book'IMD+L+060+:::invalid code test'IMD+L+120+:::Test Publisher'IMD+L+170+:::2024'IMD+L+180+:::100'IMD+L+220+:::Hbk'IMD+L+230+:::99.99'IMD+L+250+:::AN'QTY+1:1'GIR+001+BOOK:LST+REF:LSQ+CPL:LLO+LOAN:LFN+99.99:LCL'GIR+001+ZZ:LVC+Freetext despite invalid code:LVT'GIR+001+15.00:LCV'PRI+AAE:15.00'RFF+QLI:WO0800133000001'UNS+S'CNT+2:1'UNT+28+1044418001'UNZ+1+1044418'
|
||||
Loading…
Reference in New Issue