Acq invoices - Use build_invoice_impl() instead of creating directly

In this way we use the same logic in both creating EDI invoices and
creating them from user input to do two things in particular:

1) always disencumber fund debits (used to be done in manual process only
and not EDI)

2) automatically uncancel the necessary canceled copies if you're
invoicing them anyway, since they must have shown up after all
(especiallly important since "cancelled" [sic] in our schema includes
copies merely back-ordered) (used to be done in EDI only and
not the manual process)

Signed-off-by: Lebbeous Fogle-Weekley <lebbeous@esilibrary.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..7ff0df8 100644 (file)
--- 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};
 
@@ -1024,92 +1026,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 708c7df..044823d 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/Invoice.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/Invoice.pm
@@ -49,9 +49,11 @@ sub build_invoice_impl {
 
             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) {
+                # 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) {
@@ -60,8 +62,13 @@ sub build_invoice_impl {
 
                 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);
+
+                    # 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);
+
                     return $evt if $evt = update_entry_debits($e, $entry);
                 }
 
@@ -241,6 +248,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 {