[open-ils-commits] [GIT] Evergreen ILS branch rel_2_3 updated. 9717addcd5221e594df9c9025e9c3e51475b6db3

Evergreen Git git at git.evergreen-ils.org
Tue Aug 6 23:00:04 EDT 2013


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Evergreen ILS".

The branch, rel_2_3 has been updated
       via  9717addcd5221e594df9c9025e9c3e51475b6db3 (commit)
       via  96a1fb4e9a148af329a0b024534ab4279963c791 (commit)
      from  ed48cbdedf0b6e4be292c4fa2ad9d6741a1ee225 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 9717addcd5221e594df9c9025e9c3e51475b6db3
Author: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
Date:   Mon Jun 24 11:47:40 2013 -0400

    Acq: When processing EDI invoices, skip unknown line item references
    
    See LP #1172373.
    
    In their electronic invoices, vendors sometimes include a mix of line
    items that your ILS knows about, because you ordered them through it,
    and line items of which your ILS knows nothing. We should not fail
    altogether at processing invoices, but instead process what line items
    we can.
    
    Signed-off-by: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
    Signed-off-by: Ben Shum <bshum at biblio.org>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm
index 71c6881..aa2e67d 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm
@@ -863,6 +863,88 @@ sub get_kludges {
     return map { $_ => 1 } @kludges;
 }
 
+sub invoice_lineitem_to_invoice_entry {
+    my ($li, $quantity, $price) = @_;
+
+    my $eg_inv_entry = Fieldmapper::acq::invoice_entry->new;
+    $eg_inv_entry->isnew(1);
+    $eg_inv_entry->inv_item_count($quantity);
+
+    # amount staff agree to pay for
+    $eg_inv_entry->phys_item_count($quantity);
+
+    # XXX Validate by making sure the LI is on-order and belongs to
+    # the right provider and ordering agency and all that.
+    $eg_inv_entry->lineitem($li->id);
+
+    # XXX Do we actually need to link to PO directly here?
+    $eg_inv_entry->purchase_order($li->purchase_order);
+
+    # This is the total price for all units billed, not per-unit.
+    $eg_inv_entry->cost_billed($price);
+
+    # amount staff agree to pay
+    $eg_inv_entry->amount_paid($price);
+
+    # The EDIReader class does detect certain per-lineitem
+    # taxes, but we'll ignore them for now, as the only sample
+    # invoices I've yet seen containing them also had a final
+    # cumulative tax at the end.
+
+    return $eg_inv_entry;
+}
+
+# Return an arrayref containing acqie objects, an another of unknown lineitem
+# references from the electronic invoice.
+# @param    $message            An acqedim object
+# @param    $invoice_lineitems  An arrayref from part of EDIReader output
+sub process_invoice_lineitems {
+    my ($e, $msg_kludges, $log_prefix, $message, $invoice_lineitems) = @_;
+
+    my (@entries, @unknowns);
+
+    foreach my $lineitem (@$invoice_lineitems) {
+        if (!$lineitem->{id}) {
+            $logger->warn($log_prefix . "no lineitem ID");
+            next;
+        }
+
+        my ($quant) = grep {$_->{code} eq '47'} @{$lineitem->{quantities}};
+        my $quantity = ($quant) ? $quant->{quantity} : 0;
+
+        if (!$quantity) {
+            $logger->warn($log_prefix . "no invoice quantity " .
+                "specified for invoice LI $lineitem->{id}");
+            next;
+        }
+
+        # NOTE: if needed, we also have $lineitem->{net_unit_price}
+        # and $lineitem->{gross_unit_price}
+        my $price = $lineitem->{amount_billed};
+
+        # XXX Should we set acqie.billed_per_item=t in this case
+        # instead? Not sure whether that actually works everywhere
+        # it needs to. LFW
+        $price *= $quantity if $msg_kludges->{amount_billed_is_per_unit};
+
+        my $li = $e->retrieve_acq_lineitem($lineitem->{id});
+
+        if ($li) {
+            # If the top-level PO value is unset, get it from the first LI
+            $message->purchase_order($li->purchase_order)
+                unless $message->purchase_order;
+
+            push @entries, invoice_lineitem_to_invoice_entry(
+                $li, $quantity, $price
+            );
+        } else {
+            push @unknowns, $lineitem->{id};
+        }
+    }
+
+    return \@entries, \@unknowns;
+}
+
 # create_acq_invoice_from_edi() does what it sounds like it does for INVOIC
 # messages.  For similar operation on ORDRSP messages, see the guts of
 # process_jedi().
@@ -926,74 +1008,30 @@ sub create_acq_invoice_from_edi {
         die($log_prefix . "no invoice ID # in INVOIC message; " . shift);
     }
 
-    my @eg_inv_entries;
-
     $message->purchase_order($msg_data->{purchase_order});
 
-    for my $lineitem (@{$msg_data->{lineitems}}) {
-        my $li_id = $lineitem->{id};
-
-        if (!$li_id) {
-            $logger->warn($log_prefix . "no lineitem ID");
-            next;
-        }
-
-        my $li = $e->retrieve_acq_lineitem($li_id);
-
-        if (!$li) {
-            die($log_prefix . "no LI found with ID: $li_id : " . $e->event);
-        }
-
-        my ($quant) = grep {$_->{code} eq '47'} @{$lineitem->{quantities}};
-        my $quantity = ($quant) ? $quant->{quantity} : 0;
-        
-        if (!$quantity) {
-            $logger->warn($log_prefix . 
-                "no invoice quantity specified for LI $li_id");
-            next;
-        }
-
-        # NOTE: if needed, we also have $lineitem->{net_unit_price}
-        # and $lineitem->{gross_unit_price}
-        my $lineitem_price = $lineitem->{amount_billed};
-
-        # XXX Should we set acqie.billed_per_item=t in this case instead?  Not
-        # sure whether that actually works everywhere it needs to. LFW
-        $lineitem_price *= $quantity if $msg_kludges{amount_billed_is_per_unit};
-
-        # if the top-level PO value is unset, get it from the first LI
-        $message->purchase_order($li->purchase_order)
-            unless $message->purchase_order;
-
-        my $eg_inv_entry = Fieldmapper::acq::invoice_entry->new;
-        $eg_inv_entry->isnew(1);
-        $eg_inv_entry->inv_item_count($quantity);
-
-        # amount staff agree to pay for
-        $eg_inv_entry->phys_item_count($quantity);
-
-        # XXX Validate by making sure the LI is on-order and belongs to
-        # the right provider and ordering agency and all that.
-        $eg_inv_entry->lineitem($li_id);
-
-        # XXX Do we actually need to link to PO directly here?
-        $eg_inv_entry->purchase_order($li->purchase_order);
-
-        # This is the total price for all units billed, not per-unit.
-        $eg_inv_entry->cost_billed($lineitem_price);
-
-        # amount staff agree to pay
-        $eg_inv_entry->amount_paid($lineitem_price);
-
-        push @eg_inv_entries, $eg_inv_entry;
+    # Invoice lineitems should generally link to Evergreen lineitems
+    # (with acq.invoice_entry rows), except when they don't refer to any
+    # Evergreen lineitems by their known number. In that case, they're
+    # probably things ordered not through the ILS. We don't have an
+    # appropriate table for storing that kind of information right now,
+    # so we skip those. No, we don't have enough information to create
+    # Evergreen lineitems on the fly and create acqie rows linking to
+    # those.
+    my ($eg_inv_entries, $unknowns) = process_invoice_lineitems(
+        $e, \%msg_kludges, $log_prefix, $message, $msg_data->{lineitems}
+    );
 
-        # The EDIReader class does detect certain per-lineitem taxes, but
-        # we'll ignore them for now, as the only sample invoices I've yet seen
-        # containing them also had a final cumulative tax at the end.
+    if (@$unknowns) {
+        $logger->warn(
+            $log_prefix . sprintf(
+                "skipped %d unknown lineitem reference(s) from EDI invoice: %s",
+                scalar(@$unknowns),
+                join("; ", map { "'$_'" } @$unknowns)
+            )
+        );
     }
 
-    my @eg_inv_items;
-
     my %charge_type_map = (
         'TX' => ['TAX', 'Tax from electronic invoice'],
         'CA' => ['PRO', 'Cataloging services'], 
@@ -1001,6 +1039,8 @@ sub create_acq_invoice_from_edi {
         'GST' => ['TAX', 'Goods and services tax']
     ); # XXX i18n, somehow
 
+    my $eg_inv_items = [];
+
     for my $charge (@{$msg_data->{misc_charges}}, @{$msg_data->{taxes}}) {
         my $eg_inv_item = Fieldmapper::acq::invoice_item->new;
         $eg_inv_item->isnew(1);
@@ -1024,12 +1064,12 @@ sub create_acq_invoice_from_edi {
         $eg_inv_item->cost_billed($amount);
         $eg_inv_item->amount_paid($amount);
 
-        push @eg_inv_items, $eg_inv_item;
+        push @$eg_inv_items, $eg_inv_item;
     }
 
     $logger->info($log_prefix . 
         sprintf("creating invoice with %d entries and %d items.",
-            scalar(@eg_inv_entries), scalar(@eg_inv_items)));
+            scalar(@$eg_inv_entries), scalar(@$eg_inv_items)));
 
     $e->xact_begin;
 
@@ -1039,7 +1079,7 @@ sub create_acq_invoice_from_edi {
     }
 
     my $result = OpenILS::Application::Acq::Invoice::build_invoice_impl(
-        $e, $eg_inv, \@eg_inv_entries, \@eg_inv_items, 0   # don't commit yet
+        $e, $eg_inv, $eg_inv_entries, $eg_inv_items, 0   # don't commit yet
     );
 
     if ($U->event_code($result)) {

commit 96a1fb4e9a148af329a0b024534ab4279963c791
Author: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
Date:   Tue Apr 16 15:18:17 2013 -0400

    Acq: When building invoices from EDI messages, avoid bad data
    
    From some vendors, these EDI messages contain strings (useless ones,
    like just the name of the vendor) where we had been expecting numeric
    identifiers.
    
    Signed-off-by: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
    Signed-off-by: Ben Shum <bshum at biblio.org>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm
index 924787a..71c6881 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm
@@ -218,11 +218,24 @@ sub process_retrieval {
         $incoming->translate_time('NOW');
 
         if ($msg_hash->{purchase_order}) {
-            $logger->info("EDI: processing message for PO " . $msg_hash->{purchase_order});
-            $incoming->purchase_order($msg_hash->{purchase_order});
-            unless ($e->retrieve_acq_purchase_order($incoming->purchase_order)) {
-                $logger->warn("EDI: received order response for nonexistent PO.  Skipping...");
-                next;
+            # Some vendors just put their name where there ought to be a number,
+            # and others put alphanumeric strings that mean nothing to us, so
+            # we sure won't match a PO in the system this way. We can pick
+            # up PO number later from the lineitems themselves if necessary.
+
+            if ($msg_hash->{purchase_order} !~ /^\d+$/) {
+                $logger->warn("EDI: PO identifier is non-numeric. Continuing.");
+                # No "next" here; we'll process this and just not link to acqpo.
+            } else {
+                $logger->info("EDI: processing message for PO " .
+                    $msg_hash->{purchase_order});
+                $incoming->purchase_order($msg_hash->{purchase_order});
+                unless ($e->retrieve_acq_purchase_order(
+                        $incoming->purchase_order)) {
+                    $logger->warn("EDI: received order response for " .
+                        "nonexistent PO.  Skipping...");
+                    next;
+                }
             }
         }
 

-----------------------------------------------------------------------

Summary of changes:
 .../perlmods/lib/OpenILS/Application/Acq/EDI.pm    |  195 +++++++++++++-------
 1 files changed, 124 insertions(+), 71 deletions(-)


hooks/post-receive
-- 
Evergreen ILS


More information about the open-ils-commits mailing list