LP#1830089: close transaction and update copy status on payment or adjust to zero

When you make a payment that sets the balance owed to zero, Evergreen
closes the transaction and sets the item status to Lost & Paid (if
appropriate).  Adjust to Zero should do the same thing, but hitherto it
would not update the item status.  This commit refactors some code to
ensure that Evergreen gives the same result whether you make a payment
or adjust to zero.

Signed-off-by: Jeff Davis <jeff.davis@bc.libraries.coop>
Signed-off-by: Terran McCanna <tmccanna@georgialibraries.org>
Signed-off-by: Galen Charlton <gmc@equinoxOLI.org>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/CircCommon.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/CircCommon.pm
index 73b79a3..6a7044c 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/CircCommon.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/CircCommon.pm
@@ -359,6 +359,60 @@ sub can_close_circ {
     return $can_close;
 }
 
+sub maybe_close_xact {
+    my ($class, $e, $xact_id) = @_;
+
+    my $circ = $e->retrieve_action_circulation(
+        [
+            $xact_id,
+            {
+                flesh => 1,
+                flesh_fields => {circ => ['target_copy','billings']}
+            }
+        ]
+    ); # Flesh the copy, so we can monkey with the status if
+       # necessary.
+
+    # Whether or not we close the transaction. We definitely close if no
+    # circulation transaction is present, otherwise we check if the circulation
+    # is in a state that allows itself to be closed.
+    if (!$circ || can_close_circ($class, $e, $circ)) {
+        my $billable_xact = $e->retrieve_money_billable_transaction($xact_id);
+        $billable_xact->xact_finish("now");
+        if (!$e->update_money_billable_transaction($billable_xact)) {
+            return {
+                message => "update_money_billable_transaction() failed",
+                evt => $e->die_event
+            };
+        }
+
+        # If we have a circ, we need to check if the copy status is lost or
+        # long overdue.  If it is then we check org_unit_settings for the copy
+        # owning library and adjust and possibly adjust copy status to lost and
+        # paid.
+        if ($circ && ($circ->stop_fines eq 'LOST' || $circ->stop_fines eq 'LONGOVERDUE')) {
+            # We need the copy to check settings and to possibly
+            # change its status.
+            my $copy = $circ->target_copy();
+            # Library where we'll check settings.
+            my $check_lib = $copy->circ_lib();
+
+            # check the copy status
+            if (($copy->status() == OILS_COPY_STATUS_LOST || $copy->status() == OILS_COPY_STATUS_LONG_OVERDUE)
+                    && $U->is_true($U->ou_ancestor_setting_value($check_lib, 'circ.use_lost_paid_copy_status', $e))) {
+                $copy->status(OILS_COPY_STATUS_LOST_AND_PAID);
+                if (!$e->update_asset_copy($copy)) {
+                    return {
+                        message => "update_asset_copy_failed()",
+                        evt => $e->die_event
+                    };
+                }
+            }
+        }
+    }
+}
+
+
 sub seconds_to_interval_hash {
         my $interval = shift;
         my $limit = shift || 's';

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Money.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Money.pm
index 46f0671..4163cf0 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Money.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Money.pm
@@ -481,55 +481,14 @@ sub make_payments {
             # credit
             $cred = -$cred;
             $credit += $cred;
-            my $circ = $e->retrieve_action_circulation(
-                [
-                    $transid,
-                    {
-                        flesh => 1,
-                        flesh_fields => {circ => ['target_copy','billings']}
-                    }
-                ]
-            ); # Flesh the copy, so we can monkey with the status if
-               # necessary.
-
-            # Whether or not we close the transaction. We definitely
-            # close if no circulation transaction is present,
-            # otherwise we check if the circulation is in a state that
-            # allows itself to be closed.
-            if (!$circ || $CC->can_close_circ($e, $circ)) {
-                $trans = $e->retrieve_money_billable_transaction($transid);
-                $trans->xact_finish("now");
-                if (!$e->update_money_billable_transaction($trans)) {
-                    return _recording_failure(
-                        $e, "update_money_billable_transaction() failed",
-                        $payment, $cc_payload
-                    )
-                }
 
-                # If we have a circ, we need to check if the copy
-                # status is lost or long overdue.  If it is then we
-                # check org_unit_settings for the copy owning library
-                # and adjust and possibly adjust copy status to lost
-                # and paid.
-                if ($circ && ($circ->stop_fines eq 'LOST' || $circ->stop_fines eq 'LONGOVERDUE')) {
-                    # We need the copy to check settings and to possibly
-                    # change its status.
-                    my $copy = $circ->target_copy();
-                    # Library where we'll check settings.
-                    my $check_lib = $copy->circ_lib();
-
-                    # check the copy status
-                    if (($copy->status() == OILS_COPY_STATUS_LOST || $copy->status() == OILS_COPY_STATUS_LONG_OVERDUE)
-                            && $U->is_true($U->ou_ancestor_setting_value($check_lib, 'circ.use_lost_paid_copy_status', $e))) {
-                        $copy->status(OILS_COPY_STATUS_LOST_AND_PAID);
-                        if (!$e->update_asset_copy($copy)) {
-                            return _recording_failure(
-                                $e, "update_asset_copy_failed()",
-                                $payment, $cc_payload
-                            )
-                        }
-                    }
-                }
+            # Attempt to close the transaction.
+            my $close_xact_fail = $CC->maybe_close_xact($e, $transid);
+            if ($close_xact_fail) {
+                return _recording_failure(
+                    $e, $close_xact_fail->{message},
+                    $payment, $cc_payload
+                );
             }
         }
 
@@ -1066,14 +1025,9 @@ sub adjust_bills_to_zero_manual {
 
         # now we see if we can close the transaction
         # same logic as make_payments();
-        my $circ = $e->retrieve_action_circulation($xact_id);
-        if (!$circ or $CC->can_close_circ($e, $circ)) {
-            # we don't check to see if the xact is already closed.  since the
-            # xact had a negative balance, it should not have been closed, so
-            # assume 'now' is the correct close time regardless.
-            my $trans = $e->retrieve_money_billable_transaction($xact_id);
-            $trans->xact_finish("now");
-            $e->update_money_billable_transaction($trans) or return $e->die_event;
+        my $close_xact_fail = $CC->maybe_close_xact($e, $xact_id);
+        if ($close_xact_fail) {
+            return $close_xact_fail->{evt};
         }
     }