From ea247ab1bea3381596cf7cd60ef333ab5932b7fe Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Thu, 6 Jun 2019 14:30:34 -0700 Subject: [PATCH] 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 Signed-off-by: Terran McCanna Signed-off-by: Galen Charlton --- .../OpenILS/Application/Circ/CircCommon.pm | 54 +++++++++++++++ .../lib/OpenILS/Application/Circ/Money.pm | 66 +++---------------- 2 files changed, 64 insertions(+), 56 deletions(-) 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 73b79a3856..6a7044c769 100644 --- 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 46f067135a..4163cf0218 100644 --- 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}; } } -- 2.43.2