From 52b8a9f1dcbed2ea4bcec6b403677e4ec7cbf67e Mon Sep 17 00:00:00 2001 From: Michael Hafen Date: Wed, 2 Mar 2022 17:10:27 -0700 Subject: [PATCH] Bug 18855: Don't create duplicate overdue fines if an issue is returned This includes Jonathan's followup If cronjobs/fines.pl is running during circulation hours, then an issue may be considered for having it's overdue fine updated after the issue has been returned and it's fine status flipped from 'UNRETURNED' to 'RETURNED'. In this case UpdateFine will create a duplicate fine because it can't find the specific accountline for the (formerly) overdue issue. This changes cronjobs/fines.pl to double check the issue before updating the fine. If the issue has changed between starting the script and updating the fine, then the script will skip it. There is a small amount of time between the check and calling UpdateFine where the issue can be changed and this problem can reoccure. The chance of that happening is so small that it's probably fine to leave as is. It is also possible that the fine won't be updated because the issue was returned. In this case the fine payed by the patron will be lower, but that is better then the patron finding later that there is more to a fine they thought they had paid all of. Test plan (by Caroline): 0. Preliminary settings 0.1. finesMode system preference must be set to Calculate and charge 0.2. There must be a circulation rule that will charge fines (beware of bug 32271) 0.3. In Tools > Calendar, today must not be a holiday 1. Make a lot of overdue checkouts - I used the batch checkout feature, but if your system already has a lot of overdue checkouts, you can skip to step 2 1.1. Enable batch checkouts 1.1.1. Go to Administration > Global system preferences 1.1.2. Search for BatchCheckouts 1.1.3. Set BatchCheckouts to Allow 1.1.4. Select all categories in BatchCheckoutsValidCategories 1.1.5. Click "Save all Circulation preferences" 1.2. Get a list of barcodes 1.2.1. Go to Reports 1.2.2. Click "Create from SQL" 1.2.3. Give the report a name 1.2.4. For the SQL query, enter SELECT barcode FROM items WHERE onloan IS NULL LIMIT 60; 1.2.5. Click "Save report" 1.2.6. Click "Run report" 1.2.7. Click "Download" > "Tab separated text" 1.3. Go to a patron's file 1.3.1. Go to Patrons 1.3.2. Click on "Search" 1.3.3. Click on a patron's name 1.4. Do a batch checkout with a due date in the past 1.4.1. Click on the "Batch check out" tab on the left 1.4.2. In "Use a file", click "Choose file" 1.4.3. Choose the file downloaded from the report 1.4.4. In "Hard due date", choose a date in the past 1.4.5. Click "Check out" 1.4.6. Click "Checkout or renew" 2. Find the last issue in the database 2.1. In the database (or in reports), type the following query SELECT issues.*, items.itype as itemtype, items.homebranch, items.barcode, items.itemlost, items.replacementprice, items.biblionumber FROM issues LEFT JOIN items USING (itemnumber) WHERE date_due < NOW() \G; 2.2. Copy the barcode from the last entry 3. Set up so that you can run fines.pl and check in the item at the same time (or very close to the same time) 3.1. In Koha, click the "Check in" option in the search bar at the top of the page 3.2. Paste the barcode in the search bar BUT DO NOT PRESS ENTER OR THE ARROW RIGHT AWAY 3.3. In a terminal, enter the fines.pl command ./misc/cronjobs/fines.pl 3.4. Execute the command and immediately click on the arrow in the staff interface to check in the item 4. Check the patron's fines 4.1. Click on the patron's name in the check in screen 4.2. Go to the Accounting tab on the left 4.3. In the search box just above the table, paste in the returned item's barcode --> Without the patch, there are two fines, one Fine (Accruing) and one Fine (Returned) for the same item at the same time --> With the patch, there is only one fine, Fine (Returned) Signed-off-by: Caroline Cyr La Rose --- misc/cronjobs/fines.pl | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/misc/cronjobs/fines.pl b/misc/cronjobs/fines.pl index b141c55723..05f73216e8 100755 --- a/misc/cronjobs/fines.pl +++ b/misc/cronjobs/fines.pl @@ -130,6 +130,14 @@ for my $overdue ( @{$overdues} ) { "ERROR in Getoverdues : issues.borrowernumber IS NULL. Repair 'issues' table now! Skipping record.\n"; next; } + + # if the issue changed before the script got to it, then pass on it. + my $issue = Koha::Checkouts->find({ issue_id => $overdue->{issue_id} }); + next if ( ! $issue or $issue->date_due ne $overdue->{date_due} ); + + my $datedue = dt_from_string( $overdue->{date_due} ); + next unless $issue->is_overdue( $datedue ); + my $patron = Koha::Patrons->find( $overdue->{borrowernumber} ); my $branchcode = ( $control eq 'ItemHomeLibrary' ) ? $overdue->{$branch_type} @@ -141,10 +149,6 @@ for my $overdue ( @{$overdues} ) { $is_holiday{$branchcode} = set_holiday( $branchcode, $today ); } - my $datedue = dt_from_string( $overdue->{date_due} ); - if ( DateTime->compare( $datedue, $today ) == 1 ) { - next; # not overdue - } ++$counted; my ( $amount, $unitcounttotal, $unitcount ) = @@ -160,6 +164,9 @@ for my $overdue ( @{$overdues} ) { && ( $amount && $amount > 0 ) ) { + # if the issue changed before the script got to it, then pass on it. + my $issue = Koha::Checkouts->find({ issue_id => $overdue->{issue_id} }); + next if ( ! $issue or $issue->date_due ne $overdue->{date_due} ); UpdateFine( { issue_id => $overdue->{issue_id}, -- 2.34.1