Bugzilla – Attachment 176530 Details for
Bug 38689
EDIFACT errors should log to a staff accessible location
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 38689: Record errors on quote processing
Bug-38689-Record-errors-on-quote-processing.patch (text/plain), 11.52 KB, created by
Martin Renvoize (ashimema)
on 2025-01-14 17:17:09 UTC
(
hide
)
Description:
Bug 38689: Record errors on quote processing
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-01-14 17:17:09 UTC
Size:
11.52 KB
patch
obsolete
>From 0361363149cdfbf488552f6b2aff72a32169efd5 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Fri, 3 Jan 2025 13:53:47 +0000 >Subject: [PATCH] Bug 38689: Record errors on quote processing > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > Koha/EDI.pm | 121 +++++++++++++++++++++++++++++++++++----------------- > 1 file changed, 83 insertions(+), 38 deletions(-) > >diff --git a/Koha/EDI.pm b/Koha/EDI.pm >index d3e71918ef2..bf8627e810f 100644 >--- a/Koha/EDI.pm >+++ b/Koha/EDI.pm >@@ -614,52 +614,54 @@ sub transfer_items { > } > > sub process_quote { >- my $quote = shift; >+ my $quote_message = shift; > >- $quote->status('processing'); >- $quote->update; >+ $quote_message->status('processing'); >+ $quote_message->update; > >- my $edi = Koha::Edifact->new( { transmission => $quote->raw_msg, } ); >+ my $edi = Koha::Edifact->new( { transmission => $quote_message->raw_msg, } ); > > my $messages = $edi->message_array(); > my $process_errors = 0; > my $logger = Koha::Logger->get( { interface => 'edi' } ); > my $schema = Koha::Database->new()->schema(); > my $message_count = 0; >+ my $split_message; > my @added_baskets; # if auto & multiple baskets need to order all > >- if ( @{$messages} && $quote->vendor_id ) { >+ if ( @{$messages} && $quote_message->vendor_id ) { > foreach my $msg ( @{$messages} ) { > ++$message_count; >- my $basketno = >- NewBasket( $quote->vendor_id, 0, $quote->filename, q{}, >- q{} . q{} ); >+ my $basketno = NewBasket( >+ $quote_message->vendor_id, 0, $quote_message->filename, q{}, >+ q{} . q{} >+ ); > push @added_baskets, $basketno; > if ( $message_count > 1 ) { >- my $m_filename = $quote->filename; >+ my $m_filename = $quote_message->filename; > $m_filename .= "_$message_count"; >- $schema->resultset('EdifactMessage')->create( >+ $split_message = $schema->resultset('EdifactMessage')->create( > { >- message_type => $quote->message_type, >- transfer_date => $quote->transfer_date, >- vendor_id => $quote->vendor_id, >- edi_acct => $quote->edi_acct, >+ message_type => $quote_message->message_type, >+ transfer_date => $quote_message->transfer_date, >+ vendor_id => $quote_message->vendor_id, >+ edi_acct => $quote_message->edi_acct, > status => 'recmsg', > basketno => $basketno, > raw_msg => q{}, > filename => $m_filename, > } > ); >- } >- else { >- $quote->basketno($basketno); >+ } else { >+ $quote_message->basketno($basketno); > } > $logger->trace("Created basket :$basketno"); > my $items = $msg->lineitems(); > my $refnum = $msg->message_refno; > > for my $item ( @{$items} ) { >- if ( !quote_item( $item, $quote, $basketno ) ) { >+ my $message = $split_message // $quote_message; >+ if ( !quote_item( $item, $message, $basketno ) ) { > ++$process_errors; > } > } >@@ -670,12 +672,12 @@ sub process_quote { > $status = 'error'; > } > >- $quote->status($status); >- $quote->update; # status and basketno link >- # Do we automatically generate orders for this vendor >+ $quote_message->status($status); >+ $quote_message->update; # status and basketno link >+ # Do we automatically generate orders for this vendor > my $v = $schema->resultset('VendorEdiAccount')->search( > { >- vendor_id => $quote->vendor_id, >+ vendor_id => $quote_message->vendor_id, > } > )->single; > if ( $v->auto_orders ) { >@@ -687,25 +689,25 @@ sub process_quote { > } > ); > Koha::Acquisition::Baskets->find($b)->close; >+ > # Log the approval >- if (C4::Context->preference("AcquisitionLog")) { >- my $approved = Koha::Acquisition::Baskets->find( $b ); >+ if ( C4::Context->preference("AcquisitionLog") ) { >+ my $approved = Koha::Acquisition::Baskets->find($b); > logaction( > 'ACQUISITIONS', > 'APPROVE_BASKET', > $b, >- to_json($approved->unblessed) >+ to_json( $approved->unblessed ) > ); > } > } > } > >- > return; > } > > sub quote_item { >- my ( $item, $quote, $basketno ) = @_; >+ my ( $item, $quote_message, $basketno ) = @_; > > my $schema = Koha::Database->new()->schema(); > my $logger = Koha::Logger->get( { interface => 'edi' } ); >@@ -714,6 +716,12 @@ sub quote_item { > # So this call should not fail unless that has > my $basket = Koha::Acquisition::Baskets->find( $basketno ); > unless ( $basket ) { >+ $quote_message->add_to_edifact_errors( >+ { >+ section => "", >+ details => "Failed to create basket" >+ } >+ ); > $logger->error('Skipping order creation no valid basketno'); > return; > } >@@ -721,7 +729,7 @@ sub quote_item { > my $bib = _check_for_existing_bib( $item->item_number_id() ); > if ( !defined $bib ) { > $bib = {}; >- my $bib_record = _create_bib_from_quote( $item, $quote ); >+ my $bib_record = _create_bib_from_quote( $item, $quote_message ); > > # Check for and add default 008 as this is a mandatory field > $bib_record = _handle_008_field($bib_record); >@@ -742,6 +750,12 @@ sub quote_item { > $order_quantity ||= 1; # quantity not necessarily present > if ( $gir_count > 1 ) { > if ( $gir_count != $order_quantity ) { >+ $quote_message->add_to_edifact_errors( >+ { >+ section => "GIR", >+ details => "Order for $order_quantity items, $gir_count segments present" >+ } >+ ); > $logger->error( > "Order for $order_quantity items, $gir_count segments present"); > } >@@ -752,7 +766,7 @@ sub quote_item { > if (!$price) { > $price = $item->price_gross; > } >- my $vendor = Koha::Acquisition::Booksellers->find( $quote->vendor_id ); >+ my $vendor = Koha::Acquisition::Booksellers->find( $quote_message->vendor_id ); > > # NB quote will not include tax info it only contains the list price > my $ecost = _discounted_price( $vendor->discount, $price, $item->price_info_inclusive ); >@@ -822,12 +836,21 @@ sub quote_item { > my $skip = '0'; > if ( !$budget ) { > if ( $item->quantity > 1 ) { >- carp 'Skipping line with no budget info'; >+ $quote_message->add_to_edifact_errors( >+ { >+ section => "GIR", >+ details => "Skipped GIR line with invalid budget: " . $item->girfield('fund_allocation') >+ } >+ ); > $logger->trace('girfield skipped for invalid budget'); > $skip++; >- } >- else { >- carp 'Skipping line with no budget info'; >+ } else { >+ $quote_message->add_to_edifact_errors( >+ { >+ section => "GIR", >+ details => "Skipped orderline line with invalid budget: " . $item->girfield('fund_allocation') >+ } >+ ); > $logger->trace('orderline skipped for invalid budget'); > return; > } >@@ -854,7 +877,7 @@ sub quote_item { > $ordernumber{ $budget->budget_id } = $o; > > if ( C4::Context->preference('AcqCreateItem') eq 'ordering' ) { >- $item_hash = _create_item_from_quote( $item, $quote ); >+ $item_hash = _create_item_from_quote( $item, $quote_message ); > > my $created = 0; > while ( $created < $order_quantity ) { >@@ -881,6 +904,12 @@ sub quote_item { > $rota->add_item($itemnumber); > $logger->trace("Item added to rota $rota->id"); > } else { >+ $quote_message->add_to_edifact_errors( >+ { >+ section => "LRP", >+ details => "No rota found for passed $lrp in orderline" >+ } >+ ); > $logger->error("No rota found matching $lrp in orderline"); > } > } >@@ -899,7 +928,12 @@ sub quote_item { > if ( !$budget ) { > my $bad_budget = > $item->girfield( 'fund_allocation', $occurrence ); >- carp 'Skipping line with no budget info'; >+ $quote_message->add_to_edifact_errors( >+ { >+ section => "GIR", >+ details => "Skipped GIR line with invalid budget: $bad_budget" >+ } >+ ); > $logger->trace( > "girfield skipped for invalid budget:$bad_budget"); > ++$occurrence; ## lets look at the next one not this one again >@@ -931,7 +965,7 @@ sub quote_item { > > if ( C4::Context->preference('AcqCreateItem') eq 'ordering' ) { > if ( !defined $item_hash ) { >- $item_hash = _create_item_from_quote( $item, $quote ); >+ $item_hash = _create_item_from_quote( $item, $quote_message ); > } > my $new_item = { > itype => >@@ -986,9 +1020,15 @@ sub quote_item { > { key => 'stockrotationrotas_title' } ); > if ($rota) { > $rota->add_item($itemnumber); >- $logger->trace("Item added to rota $rota->id"); >+ $logger->trace("Item added to rota " .$rota->id); > } > else { >+ $quote_message->add_to_edifact_errors( >+ { >+ section => "LRP", >+ details => "No rota found for passed $lrp in orderline" >+ } >+ ); > $logger->error( > "No rota found matching $lrp in orderline"); > } >@@ -1059,6 +1099,12 @@ sub quote_item { > $logger->trace("Item added to rota $rota->id"); > } > else { >+ $quote_message->add_to_edifact_errors( >+ { >+ section => "LRP", >+ details => "No rota found for passed $lrp in orderline" >+ } >+ ); > $logger->error( > "No rota found matching $lrp in orderline"); > } >@@ -1070,7 +1116,6 @@ sub quote_item { > } > } > return 1; >- > } > > sub get_edifact_ean { >-- >2.47.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 38689
:
175788
|
175789
|
175790
|
175793
|
175794
|
175795
|
175796
|
175797
|
175809
|
175810
|
175811
|
175812
|
175813
|
175814
|
175815
|
175816
|
175817
|
176127
|
176128
|
176129
|
176130
|
176131
|
176132
|
176133
|
176134
|
176173
|
176174
|
176175
|
176176
|
176177
|
176178
|
176179
|
176180
|
176181
|
176182
|
176183
|
176373
|
176374
|
176375
|
176376
|
176377
|
176378
|
176379
|
176380
|
176381
|
176382
|
176524
|
176525
|
176526
|
176527
|
176528
|
176529
|
176530
|
176531
|
176532
|
176533
|
176605
|
176606
|
176607
|
176608
|
176609
|
176610
|
176611
|
176612
|
176613
|
176614