[open-ils-commits] [GIT] Evergreen ILS branch master updated. 3eb7f405e80743167163f94036e0ef9f38f945d7

Evergreen Git git at git.evergreen-ils.org
Thu Apr 18 13:35:34 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, master has been updated
       via  3eb7f405e80743167163f94036e0ef9f38f945d7 (commit)
      from  144dc798cafb4773201c1a3b625618f1822e23d0 (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 3eb7f405e80743167163f94036e0ef9f38f945d7
Author: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
Date:   Wed Feb 20 09:41:03 2013 -0500

    Acq: re-use more code for two ways of creating invoices (EDI and manual)
    
    This solves two problems.
    
     1) With EDI invoices, we had been failing to disencumber fund debits
        related to the invoiced lineitems, although that worked for manual
        invoices.
     2) With manual invoices, we would not automatically uncancel copies
        when the user decided to invoice them despite their canceled status.
        This was already working in EDI invoices though.  This is especially
        important since our schema lumps "backordered" in with "canceled,"
        and in theory backordered things do show up eventually.
    
    There were earlier version of this commit out there with bugs that
    prevented the EDI workflow from working correctly (the manual invoice
    flow worked and still should).
    
    Signed-off-by: Lebbeous Fogle-Weekley <lebbeous at esilibrary.com>
    Signed-off-by: Mike Rylander <mrylander at gmail.com>

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 4d8edcf..c46b504 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/EDI.pm
@@ -11,6 +11,7 @@ use OpenSRF::Utils::Logger qw(:logger);
 use OpenSRF::Utils::JSON;
 
 use OpenILS::Application::Acq::Lineitem;
+use OpenILS::Application::Acq::Invoice;
 use OpenILS::Utils::RemoteAccount;
 use OpenILS::Utils::CStoreEditor q/new_editor/;
 use OpenILS::Utils::Fieldmapper;
@@ -113,7 +114,7 @@ sub retrieve_core {
 
         my @files    = ($server->ls({remote_file => ($account->in_dir || './')}));
         my @ok_files = grep {$_ !~ /\/\.?\.$/ and $_ ne '0'} @files;
-        $logger->info(sprintf "%s of %s files at %s/%s", scalar(@ok_files), scalar(@files), $account->host, $account->in_dir);   
+        $logger->info(sprintf "%s of %s files at %s/%s", scalar(@ok_files), scalar(@files), $account->host, $account->in_dir || "");   
 
         foreach my $remote_file (@ok_files) {
             my $description = sprintf "%s/%s", $account->host, $remote_file;
@@ -871,6 +872,7 @@ sub create_acq_invoice_from_edi {
     }
 
     my $eg_inv = Fieldmapper::acq::invoice->new;
+    $eg_inv->isnew(1);
 
     # Some troubleshooting aids.  Yeah we should have made appropriate links
     # for this in the schema, but this is better than nothing.  Probably
@@ -911,7 +913,6 @@ sub create_acq_invoice_from_edi {
     }
 
     my @eg_inv_entries;
-    my @eg_inv_cancel_lis;
 
     $message->purchase_order($msg_data->{purchase_order});
 
@@ -942,6 +943,8 @@ sub create_acq_invoice_from_edi {
         # 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
@@ -949,6 +952,7 @@ sub create_acq_invoice_from_edi {
             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
@@ -968,9 +972,6 @@ sub create_acq_invoice_from_edi {
         $eg_inv_entry->amount_paid($lineitem_price);
 
         push @eg_inv_entries, $eg_inv_entry;
-        push @eg_inv_cancel_lis, 
-            {lineitem => $li, quantity => $quantity} 
-            if $li->cancel_reason;
 
         # 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
@@ -988,6 +989,7 @@ sub create_acq_invoice_from_edi {
 
     for my $charge (@{$msg_data->{misc_charges}}, @{$msg_data->{taxes}}) {
         my $eg_inv_item = Fieldmapper::acq::invoice_item->new;
+        $eg_inv_item->isnew(1);
 
         my $amount = $charge->{amount};
 
@@ -999,10 +1001,8 @@ sub create_acq_invoice_from_edi {
         my $map = $charge_type_map{$charge->{type}};
 
         if (!$map) {
-            $map = [
-                'PRO',
-                'Unknown charge type ' .  $charge->{type}
-            ];
+            $map = ['PRO', 'Misc / unspecified'];
+            $eg_inv_item->note($charge->{type});
         }
 
         $eg_inv_item->inv_item_type($$map[0]);
@@ -1024,92 +1024,14 @@ sub create_acq_invoice_from_edi {
         die($log_prefix . "couldn't update edi_message " . $message->id);
     }
 
-    # create EG invoice
-    if (not $e->create_acq_invoice($eg_inv)) {
-        die($log_prefix . "couldn't create invoice: " . $e->event);
-    }
-
-    # Now we have a pkey for our EG invoice, so set the invoice field on all
-    # our entries according and create those too.
-    my $eg_inv_id = $e->data->id;
-    foreach (@eg_inv_entries) {
-        $_->invoice($eg_inv_id);
-        if (not $e->create_acq_invoice_entry($_)) {
-            die(
-                $log_prefix . "couldn't create entry against lineitem " .
-                $_->lineitem . ": " . $e->event
-            );
-        }
-    }
-
-    # Create any invoice items (taxes)
-    foreach (@eg_inv_items) {
-        $_->invoice($eg_inv_id);
-        if (not $e->create_acq_invoice_item($_)) {
-            die($log_prefix . "couldn't create inv item: " . $e->event);
-        }
-    }
-
-    # if an invoiced lineitem is marked as cancelled 
-    # (e.g. back-order), invoicing the lineitem implies 
-    # we need to un-cancel it
-    for my $li_chunk (@eg_inv_cancel_lis) {
-        my $li = $li_chunk->{lineitem};
-        my $quantity = $li_chunk->{quantity};
-
-        $logger->info($log_prefix . 
-            "un-cancelling invoiced lineitem ". $li->id);
-         
-        # collect the LIDs, starting with those that are
-        # not cancelled (should not happen), followed by
-        # those that have keep-debits cancel_reasons, 
-        # followed by non-keep-debit cancel reasons.
-
-        my $lid_ids = $e->json_query({
-            select => {acqlid => ['id']},
-            from => {
-                acqlid => {
-                    acqcr => {type => 'left'},
-                    acqfdeb => {type => 'left'}
-                }
-            },
-            where => {
-                '+acqlid' => {lineitem => $li->id},
-                # not-yet invoiced copies
-                '+acqfdeb' => {encumbrance => 't'}
-            },
-            order_by => [{
-                class => 'acqcr',
-                field => 'keep_debits',
-                direction => 'desc'
-            }],
-            limit => $quantity
-        });
-
-        for my $lid_id (map {$_->{id}} @$lid_ids) {
-            my $lid = $e->retrieve_acq_lineitem_detail($lid_id);
-            next unless $lid->cancel_reason;
-
-            $lid->clear_cancel_reason;
-            unless ($e->update_acq_lineitem_detail($lid)) {
-                die($log_prefix .
-                    "couldn't clear lid cancel reason: ". $e->die_event
-                );
-            }
-        }
-
-        $li->clear_cancel_reason;
-        $li->state("on-order");
-        $li->edit_time('now'); 
+    my $result = OpenILS::Application::Acq::Invoice::build_invoice_impl(
+        $e, $eg_inv, \@eg_inv_entries, \@eg_inv_items, 0   # don't commit yet
+    );
 
-        unless ($e->update_acq_lineitem($li)) {
-            die($log_prefix .
-                "couldn't clear li cancel reason: ". $e->die_event
-            );
-        }
+    if ($U->event_code($result)) {
+        die($log_prefix. "build_invoice_impl() failed: " . $result->{textcode});
     }
 
-
     $e->xact_commit;
     return 1;
 }
diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/Invoice.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/Invoice.pm
index 7563e88..2bcce55 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/Invoice.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/Invoice.pm
@@ -10,16 +10,22 @@ use OpenILS::Event;
 my $U = 'OpenILS::Application::AppUtils';
 
 
+# return nothing on success, event on failure
 sub _prepare_fund_debit_for_inv_item {
     my ($debit, $item, $e) = @_;
+
     $debit->fund($item->fund);
     $debit->amount($item->amount_paid);
     $debit->origin_amount($item->amount_paid);
-    $debit->origin_currency_type(
-        $e->retrieve_acq_fund($item->fund)->currency_type
-    ); # future: cache funds locally
+
+    # future: cache funds locally
+    my $fund = $e->retrieve_acq_fund($item->fund) or return $e->die_event;
+
+    $debit->origin_currency_type($fund->currency_type);
     $debit->encumbrance('f');
     $debit->debit_type('direct_charge');
+
+    return;
 }
 
 __PACKAGE__->register_method(
@@ -37,55 +43,49 @@ __PACKAGE__->register_method(
     }
 );
 
-sub build_invoice_api {
-    my($self, $conn, $auth, $invoice, $entries, $items) = @_;
 
-    my $e = new_editor(xact => 1, authtoken=>$auth);
-    return $e->die_event unless $e->checkauth;
-    my $evt;
+sub build_invoice_impl {
+    my ($e, $invoice, $entries, $items, $do_commit) = @_;
 
-    if(ref $invoice) {
-        if($invoice->isnew) {
-            $invoice->receiver($e->requestor->ws_ou) unless $invoice->receiver;
-            $invoice->recv_method('PPR') unless $invoice->recv_method;
-            $invoice->recv_date('now') unless $invoice->recv_date;
-            $e->create_acq_invoice($invoice) or return $e->die_event;
-        } elsif($invoice->isdeleted) {
-            i$e->delete_acq_invoice($invoice) or return $e->die_event;
-        } else {
-            $e->update_acq_invoice($invoice) or return $e->die_event;
-        }
+    if ($invoice->isnew) {
+        $invoice->recv_method('PPR') unless $invoice->recv_method;
+        $invoice->recv_date('now') unless $invoice->recv_date;
+        $e->create_acq_invoice($invoice) or return $e->die_event;
+    } elsif ($invoice->isdeleted) {
+        $e->delete_acq_invoice($invoice) or return $e->die_event;
     } else {
-        # caller only provided the ID
-        $invoice = $e->retrieve_acq_invoice($invoice) or return $e->die_event;
+        $e->update_acq_invoice($invoice) or return $e->die_event;
     }
 
-    return $e->die_event unless $e->allowed('CREATE_INVOICE', $invoice->receiver);
+    my $evt;
 
-    if($entries) {
+    if ($entries) {
         for my $entry (@$entries) {
             $entry->invoice($invoice->id);
 
-            if($entry->isnew) {
-
+            if ($entry->isnew) {
                 $e->create_acq_invoice_entry($entry) or return $e->die_event;
+                return $evt if $evt = uncancel_copies_as_needed($e, $entry);
                 return $evt if $evt = update_entry_debits($e, $entry);
-
-            } elsif($entry->isdeleted) {
-
-                return $evt if $evt = rollback_entry_debits($e, $entry); 
+            } elsif ($entry->isdeleted) {
+                # XXX Deleting entries does not recancel anything previously
+                # uncanceled.
+                return $evt if $evt = rollback_entry_debits($e, $entry);
                 $e->delete_acq_invoice_entry($entry) or return $e->die_event;
+            } elsif ($entry->ischanged) {
+                my $orig_entry = $e->retrieve_acq_invoice_entry($entry->id) or
+                    return $e->die_event;
 
-            } elsif($entry->ischanged) {
+                if ($orig_entry->amount_paid != $entry->amount_paid or
+                    $entry->phys_item_count != $orig_entry->phys_item_count) {
+                    return $evt if $evt = rollback_entry_debits($e,$orig_entry);
 
-                my $orig_entry = $e->retrieve_acq_invoice_entry($entry->id) or return $e->die_event;
+                    # XXX Updates can only uncancel more LIDs when
+                    # phys_item_count goes up, but cannot recancel them when
+                    # phys_item_count goes down.
+                    return $evt if $evt = uncancel_copies_as_needed($e, $entry);
 
-                if($orig_entry->amount_paid != $entry->amount_paid or 
-                        $entry->phys_item_count != $orig_entry->phys_item_count) {
-
-                    return $evt if $evt = rollback_entry_debits($e, $orig_entry); 
                     return $evt if $evt = update_entry_debits($e, $entry);
-
                 }
 
                 $e->update_acq_invoice_entry($entry) or return $e->die_event;
@@ -93,60 +93,77 @@ sub build_invoice_api {
         }
     }
 
-    if($items) {
+    if ($items) {
         for my $item (@$items) {
             $item->invoice($invoice->id);
 
-            if($item->isnew) {
-
+            if ($item->isnew) {
                 $e->create_acq_invoice_item($item) or return $e->die_event;
 
                 # future: cache item types
                 my $item_type = $e->retrieve_acq_invoice_item_type(
                     $item->inv_item_type) or return $e->die_event;
 
-                # prorated items are handled separately
-                unless($U->is_true($item_type->prorate)) {
+                # This following complex conditional statement effecively means:
+                #   1) Items with item_types that are prorate are handled
+                #       differently.
+                #   2) Only items with a po_item, or which are linked to a fund
+                #       already, or which belong to invoices which we're trying
+                #       to *close* will actually go through this fund_debit
+                #       creation process.  In other cases, we'll consider it
+                #       ok for an item to remain sans fund_debit for the time
+                #       being.
+
+                if (not $U->is_true($item_type->prorate) and
+                    ($item->po_item or $item->fund or
+                        $U->is_true($invoice->complete))) {
+
                     my $debit;
-                    if($item->po_item) {
-                        my $po_item = $e->retrieve_acq_po_item($item->po_item) or return $e->die_event;
-                        $debit = $e->retrieve_acq_fund_debit($po_item->fund_debit) or return $e->die_event;
+                    if ($item->po_item) {
+                        my $po_item = $e->retrieve_acq_po_item($item->po_item)
+                            or return $e->die_event;
+                        $debit = $e->retrieve_acq_fund_debit($po_item->fund_debit)
+                            or return $e->die_event;
                     } else {
                         $debit = Fieldmapper::acq::fund_debit->new;
                         $debit->isnew(1);
                     }
-                    _prepare_fund_debit_for_inv_item($debit, $item, $e);
 
-                    if($debit->isnew) {
-                        $e->create_acq_fund_debit($debit) or return $e->die_event;
+                    return $evt if
+                        $evt = _prepare_fund_debit_for_inv_item($debit, $item, $e);
+
+                    if ($debit->isnew) {
+                        $e->create_acq_fund_debit($debit)
+                            or return $e->die_event;
                     } else {
-                        $e->update_acq_fund_debit($debit) or return $e->die_event;
+                        $e->update_acq_fund_debit($debit)
+                            or return $e->die_event;
                     }
 
                     $item->fund_debit($debit->id);
                     $e->update_acq_invoice_item($item) or return $e->die_event;
                 }
-
-            } elsif($item->isdeleted) {
-
+            } elsif ($item->isdeleted) {
                 $e->delete_acq_invoice_item($item) or return $e->die_event;
 
-                if($item->po_item and $e->retrieve_acq_po_item($item->po_item)->fund_debit == $item->fund_debit) {
-                    # the debit is attached to the po_item.  instead of deleting it, roll it back 
-                    # to being an encumbrance.  Note: a prorated invoice_item that points to a po_item 
-                    # could point to a different fund_debit.  We can't go back in time to collect all the
-                    # prorated invoice_items (nor is the caller asking us too), so when that happens, 
-                    # just delete the extraneous debit (in the else block).
+                if ($item->po_item and
+                    $e->retrieve_acq_po_item($item->po_item)->fund_debit == $item->fund_debit) {
+                    # the debit is attached to the po_item. instead of
+                    # deleting it, roll it back to being an encumbrance.
+                    # Note: a prorated invoice_item that points to a
+                    # po_item could point to a different fund_debit. We
+                    # can't go back in time to collect all the prorated
+                    # invoice_items (nor is the caller asking us too),
+                    # so when that happens, just delete the extraneous
+                    # debit (in the else block).
                     my $debit = $e->retrieve_acq_fund_debit($item->fund_debit);
                     $debit->encumbrance('t');
                     $e->update_acq_fund_debit($debit) or return $e->die_event;
-                } elsif($item->fund_debit) {
+                } elsif ($item->fund_debit) {
                     $e->delete_acq_fund_debit($e->retrieve_acq_fund_debit($item->fund_debit))
                         or return $e->die_event;
                 }
-
-
-            } elsif($item->ischanged) {
+            } elsif ($item->ischanged) {
                 my $debit;
 
                 if (!$item->fund_debit) {
@@ -154,7 +171,8 @@ sub build_invoice_api {
                     $debit = Fieldmapper::acq::fund_debit->new;
                     $debit->isnew(1);
 
-                    _prepare_fund_debit_for_inv_item($debit, $item, $e);
+                    return $evt if
+                        $evt = _prepare_fund_debit_for_inv_item($debit, $item, $e);
                 } else {
                     $debit = $e->retrieve_acq_fund_debit($item->fund_debit) or
                         return $e->die_event;
@@ -177,11 +195,34 @@ sub build_invoice_api {
     }
 
     $invoice = fetch_invoice_impl($e, $invoice->id);
-    $e->commit;
+    if ($do_commit) {
+        $e->commit or return $e->die_event;
+    }
 
     return $invoice;
 }
 
+sub build_invoice_api {
+    my($self, $conn, $auth, $invoice, $entries, $items) = @_;
+
+    my $e = new_editor(xact => 1, authtoken=>$auth);
+    return $e->die_event unless $e->checkauth;
+
+    if (not ref $invoice) {
+        # caller only provided the ID
+        $invoice = $e->retrieve_acq_invoice($invoice) or return $e->die_event;
+    }
+
+    if (not $invoice->receiver and $invoice->isnew) {
+        $invoice->receiver($e->requestor->ws_ou);
+    }
+
+    return $e->die_event unless
+        $e->allowed('CREATE_INVOICE', $invoice->receiver);
+
+    return build_invoice_impl($e, $invoice, $entries, $items, 1);
+}
+
 
 sub rollback_entry_debits {
     my($e, $entry) = @_;
@@ -231,6 +272,70 @@ sub update_entry_debits {
     return undef;
 }
 
+# This was originally done only for EDI invoices, but needs added to the
+# manual invoice-entering process for consistency's sake.
+sub uncancel_copies_as_needed {
+    my ($e, $entry) = @_;
+
+    return unless $entry->lineitem and $entry->phys_item_count;
+
+    my $li = $e->retrieve_acq_lineitem($entry->lineitem) or
+        return $e->die_event;
+
+    # if an invoiced lineitem is marked as cancelled
+    # (e.g. back-order), invoicing the lineitem implies
+    # we need to un-cancel it
+
+    # collect the LIDs, starting with those that are
+    # not cancelled, followed by those that have keep-debits cancel_reasons,
+    # followed by non-keep-debit cancel reasons.
+
+    my $lid_ids = $e->json_query({
+        select => {acqlid => ['id']},
+        from => {
+            acqlid => {
+                acqcr => {type => 'left'},
+                acqfdeb => {type => 'left'}
+            }
+        },
+        where => {
+            '+acqlid' => {lineitem => $li->id},
+            '+acqfdeb' => {encumbrance => 't'}  # not-yet invoiced copies
+        },
+        order_by => [{
+            class => 'acqcr',
+            field => 'keep_debits',
+            direction => 'desc'
+        }],
+        limit => $entry->phys_item_count    # crucial
+    });
+
+    for my $lid_id (map {$_->{id}} @$lid_ids) {
+        my $lid = $e->retrieve_acq_lineitem_detail($lid_id);
+        next unless $lid->cancel_reason;
+
+        $logger->info(
+            "un-cancelling invoice lineitem " . $li->id .
+            " lineitem_detail " . $lid_id
+        );
+        $lid->clear_cancel_reason;
+        return $e->die_event unless $e->update_acq_lineitem_detail($lid);
+    }
+
+    $li->clear_cancel_reason;
+    $li->state("on-order") if $li->state eq "cancelled";    # sic
+    $li->edit_time("now");
+
+    unless ($e->update_acq_lineitem($li)) {
+        my $evt = $e->die_event;
+        $logger->error("couldn't clear li cancel reason: ". $evt->{textcode});
+        return $evt;
+    }
+
+    return;
+}
+
+
 # update the linked copy to reflect the amount paid for the item
 # returns true on success, false on error
 sub update_copy_cost {
@@ -243,8 +348,15 @@ sub update_copy_cost {
 
     if($lid and my $copy = $lid->eg_copy_id) {
         defined $amount and $copy->cost($amount) or $copy->clear_cost;
-        $copy->editor($e->requestor->id);
-        $copy->edit_date('now');
+
+        # XXX It would be nice to have a way to record that a copy was
+        # updated by a non-user mechanism, like EDI, but we don't have
+        # a clear way to do that here.
+        if ($e->requestor) {
+            $copy->editor($e->requestor->id);
+            $copy->edit_date('now');
+        }
+
         $e->update_asset_copy($copy) or return 0;
     }
 

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

Summary of changes:
 .../perlmods/lib/OpenILS/Application/Acq/EDI.pm    |  106 ++--------
 .../lib/OpenILS/Application/Acq/Invoice.pm         |  242 ++++++++++++++------
 2 files changed, 191 insertions(+), 157 deletions(-)


hooks/post-receive
-- 
Evergreen ILS


More information about the open-ils-commits mailing list