From 73e4da7187a6142586f3fd6976c392d305051812 Mon Sep 17 00:00:00 2001 From: Jason Stephenson Date: Sun, 22 Sep 2013 10:52:37 -0400 Subject: [PATCH] "Finish" the code for Billing Enhancement 01. Add in the logical flow for checking the negative balance settings. Also create a helper function for checking intervals along the way. This code has not actually been tried, yet, so more changes are likely to come. I don't know if it is even valid perl at this point. Signed-off-by: Jason Stephenson --- .../perlmods/lib/OpenILS/Application/AppUtils.pm | 21 ++++ .../lib/OpenILS/Application/Circ/CircCommon.pm | 135 ++++++++++++++------- 2 files changed, 114 insertions(+), 42 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm index 5a6d38582d..d00aa78de8 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm @@ -2181,5 +2181,26 @@ sub check_open_xact { return undef; } +# Because floating point math has rounding issues, and Dyrcona gets +# tired of typing out the code to multiply floating point numbers +# before adding and subtracting them and then dividing the result by +# 100 each time, he wrote this little subroutine for subtracting +# floating point values. It can serve as a model for the other +# operations if you like. +# +# It takes a list of floating point values as arguments. The rest are +# all subtracted from the first and the result is returned. The +# values are all multiplied by 100 before being used, and the result +# is divided by 100 in order to avoid decimal rounding errors inherent +# in floating point math. +sub fpdiff { + my ($class, @args) = @_; + my $result = shift(@args) * 100; + while (my $arg = shift(@args)) { + $result -= $arg * 100; + } + retrun $result / 100; +} + 1; 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 0bba096d92..8bfe7b8516 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/CircCommon.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/CircCommon.pm @@ -283,14 +283,6 @@ sub can_close_circ { # is intended to catch payments for lost and/or long overdue bills so # that they will match up. # -# NOTE: The original bill amount is considered the original bill -# amount minus any amount of void payment linked to that bill. Void -# payment not applied to any particular billing are handled like -# regular payments. -# -# I wonder if it would not be better to just summarize the billings by -# type? -# # This function is heavily adapted from code written by Jeff Godin of # Traverse Area District Library and submitted on LaunchPad bug # #1009049. @@ -364,7 +356,7 @@ sub bill_payment_map_for_xact { my @voids = map {$_->void_payment()} grep {$_->payment_type() eq 'void_payment' && $_->void_payment()->billing() == $bill->id()} @$payments; if (@voids) { foreach my $void (@voids) { - my $new_amount = ($bill->amount() * 100 - $void->amount() * 100) / 100; + my $new_amount = $U->fpdiff($bill->amount(),$void->amount()); if ($new_amount >= 0) { push @{$entry->{voids}}, $void; $entry->{void_amount} += $void->amount(); @@ -418,7 +410,7 @@ sub bill_payment_map_for_xact { while ($bill->amount() > 0) { my $payment = shift @$payments; last unless $payment; - my $new_amount = ($bill->amount() * 100 - $payment->amount() * 100) / 100; + my $new_amount = $U->fpdiff($bill->amount(),$payment->amount()); if ($new_amount < 0) { # Clone the payment so we can say how much applied # to this bill. @@ -482,41 +474,49 @@ sub real_void_bills { # Flesh grocery bills and circulations so we don't have to # retrieve them later. my ($circ, $grocery, $copy); - my $isgrocery = ($mbt->grocery()) ? 1 : 0; - if ($isgrocery) { - # We don't actually use this, yet, but just in case. - $grocery = $mbt->grocery(); - } else { - $circ = $mbt->circulation(); - $copy = $circ->target_copy(); - } + $grocery = $mbt->grocery(); + $circ = $mbt->circulation(); + $copy = $circ->target_copy() if ($circ); # Retrieve settings based on transaction location and copy # location if we have a circulation. my ($neg_balance_default, $neg_balance_overdues, $neg_balance_lost, $neg_balance_interval_default, $neg_balance_interval_overdues, $neg_balance_interval_lost); - if (!$isgrocery) { + if ($circ) { # defaults and overdue settings come from transaction org unit. $neg_balance_default = $U->ou_ancestor_setting( - $circ->circ_lib(), 'circ.prohibit_negative_balance_default'); - $neg_balance_overdues = $U->ou_ancestor_setting( - $circ->circ_lib(), 'circ.prohibit_negative_balance_on_overdues'); + $circ->circ_lib(), 'bill.prohibit_negative_balance_default'); + $neg_balance_overdues = ( + $U->ou_ancestor_setting($circ->circ_lib(), 'bill.prohibit_negative_balance_on_overdues') + || + $U->ou_ancestor_setting($circ->circ_lib(), 'bill.prohibit_netgative_balance_default') + ); $neg_balance_interval_default = $U->ou_ancestor_setting( - $circ->circ_lib(), 'circ.negative_balance_interval_default'); - $neg_balance_interval_overdues = $U->ou_ancestor_setting( - $circ->circ_lib(), 'circ.negative_balance_interval_on_overdues'); + $circ->circ_lib(), 'bill.negative_balance_interval_default'); + $neg_balance_interval_overdues = ( + $U->ou_ancestor_setting($circ->circ_lib(), 'bill.negative_balance_interval_on_overdues') + || + $U->ou_ancestor_setting($circ->circ_lib(), 'bill.negative_balance_interval_default') + ); # settings for lost come from copy circlib. $neg_balance_lost = ( - $U->ou_ancestor_setting($copy->circ_lib(), 'circ.prohibit_negative_balance_on_lost') + $U->ou_ancestor_setting($copy->circ_lib(), 'bill.prohibit_negative_balance_on_lost') || - $U->ou_ancestor_setting($copy->circ_lib(), 'circ.prohibit_negative_balance_default') + $U->ou_ancestor_setting($copy->circ_lib(), 'bill.prohibit_negative_balance_default') ); $neg_balance_interval_lost = ( - $U->ou_ancestor_setting($copy->circ_lib(), 'circ.negative_balance_interval_on_lost') + $U->ou_ancestor_setting($copy->circ_lib(), 'bill.negative_balance_interval_on_lost') || - $U->ou_ancestor_setting($copy->circ_lib(), 'circ.negative_balance_interval_default') + $U->ou_ancestor_setting($copy->circ_lib(), 'bill.negative_balance_interval_default') ); + } else { + # We only care about defaults, and they come from the + # billing location. + $neg_balance_default = $U->ou_ancestor_setting( + $grocery->billing_location(), 'bill.prohibit_negative_balance_default'); + $neg_balance_interval_default = $U->ou_ancestor_setting( + $grocery->billing_location(), 'bill.negative_balance_interval_default'); } # Get the bill_payment_map for the transaction. @@ -535,26 +535,48 @@ sub real_void_bills { # Get the bill_payment_map entry for this bill: my ($bpentry) = grep {$_->{bill}->id() == $bill->id()} @$bpmap; + # From here on out, use the bill object from the bill + # payment map entry. + $bill = $bpentry->{bill}; + + # The amount to void is the non-voided balance on the + # bill. It should never be less than zero. + my $amount_to_void = $U->fpdiff($bpentry->{bill_amount},$bpentry->{void_amount}); + # Check if this bill is already voided. We don't allow # "double" voids regardless of settings. The old code # made it impossible to void an already voided bill, so # we're doing the same. - if ($bpentry->{void_amount} > 0 && $bpentry->{void_amount} == $bpentry->{bill_amount}) { + if ($amount_to_void <= 0) { $e->rollback; return OpenILS::Event->new('BILL_ALREADY_VOIDED', payload => $bill); } - # We'll use this variable to determine if we need a void - # and how much the void payment amount should be. - my $amount_to_void = 0; - - # None of our new settings apply to grocery bills, so - # we'll just void them, regardless of balances, etc. - if ($isgrocery) { - $amount_to_void = $bpentry->{bill}->amount(); + # If we're voiding a circulation-related bill we have + # stuff to check. + if ($circ) { + if ($amount_to_void > $xact_total) { + my $btype = $bill->btype(); + if ($btype == 1) { + # Overdues + $amount_to_void = $xact_total unless(_check_payment_interval($bpenty, $neg_balance_interval_overdues)); + $amount_to_void = $xact_total if ($U->is_true($neg_balance_overdues)); + } elsif ($btype == 3 || $btype == 10) { + # Lost or Long Overdue + $amount_to_void = $xact_total unless(_check_payment_interval($bpenty, $neg_balance_interval_lost)); + $amount_to_void = $xact_total if ($U->is_true($neg_balance_lost)); + } else { + # Any other bill that we're trying to void. + $amount_to_void = $xact_total unless(_check_payment_interval($bpenty, $neg_balance_interval_default)); + $amount_to_void = $xact_total if ($U->is_true($neg_balance_default)); + } + } } else { - # Hang on tight. It's about to get hairy. - + # Grocery bills are simple by comparison. + if ($amount_to_void > $xact_total) { + $amount_to_void = $xact_total unless(_check_payment_interval($bpentry, $neg_balance_interval_default)); + $amount_to_void = $xact_total if ($U->is_true($neg_balance_default)); + } } # Create the void payment if necessary: @@ -572,8 +594,8 @@ sub real_void_bills { $bpentry->{void_amount} += $amount_to_void; push @{$bpentry->{voids}}, $payobj; # Should come to zero: - my $new_bill_amount = ($bpentry->{bill}->amount() * 100 - $amount_to_void * 100) / 100; - $bpentry->{bill}->amount($new_bill_amount); + my $new_bill_amount = $U->fpdiff($bill->amount(),$amount_to_void); + $bill->amount($new_bill_amount); } } @@ -595,4 +617,33 @@ sub real_void_bills { return 1; } +# A helper function to check if the payments on a bill are within the +# range of a given interval. The first argument is the entry hash +# from the bill payment map for the bill to check and the second +# argument is the interval. It returns true (1) if any of the bills +# are within range of the interval, or false (0) otherwise. It also +# returns true if the interval argument is undefined or empty, or if +# the bill has no payments whatsoever. It will return false if the +# entry has no payments other than voids. +sub _check_payment_interval { + my ($entry, $interval) = @_; + my $result = ($interval ? 0 : 1); + + if ($interval && $entry && $entry->{payments} && @{$entry->{payments}}) { + my $interval_secs = interval_to_seconds($interval); + my @pay_dates = map {$_->payment_ts()} sort {$b->payment_ts() cmp $a->payment_ts()} grep {$_->payment_type() ne 'void_payment'} @{$entry->{payments}}; + if (@pay_dates) { + # Since we've sorted the payment dates from highest to + # lowest, we really only need to check the 0th one. + my $payment_date = DateTime::Format::ISO8601->parse_datetime(cleanse_ISO8601($pay_dates[0]))->epoch; + my $now = time; + $result = 1 if ($payment_date + $interval_secs >= $now); + } + } elsif ($interval && (!$entry->{payments} || !@{$entry->{payments}})) { + $result = 1; + } + + return $result; +} + 1; -- 2.11.0