It was my understanding that checking the dropbox mode option on the check in screen made the due date effectively the last day the library was open and as such wiped out fines for the extra day. It appears that is not the case. Fines for the item being late remain on the patron's account. The only way around this is to 'forgive fines' when using dropbox mode, but that forgives all fines and really we only want to forgive one day of fines.
Has anyone else noticed this issue? I'm hoping it's an easy fix. Nicole
Just a little nudge again :)
I can take a look at this, but my understanding is that a fines rewrite is in progress, right? Might be better to fix once we have a better foundation, unless this is extremely crucial and that's going to take a long time.
Speaking of that, there is stuff ready for initial testing http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6427
This bug is kind of a big deal. Libraries need to be able to use dropbox mode to prevent the fine calculation from happening in the morning when they are checking in books that were put in the dropbox the night before. Right now in 3.8, they are not able to backdate the checkin date, so fines are being incorrectly charged on all books that come in from the dropbox. That's a lot of extra fines to remove manually every single day! Can we fix just the checkin date part of this without waiting on the entire Accountlines Rewrite?
Hmm there are 2 different things here Melia. Backdating the checkin date, and wiping fines. Backdating the checkin date always used to work, and was working with this bug was logged. What wasn't working was that if the fines had already run things were not wiped. If you are saying now that you can't even backdate the checkin date that is a different problem and something broken recently. Is that the case? IF so, fixing the backdating will still not remove the fines, but if the books are checked in before the fines script runs they will not get fines added to them. So .. is setting the backdate broken now too? Or can you still do that and is the problem that you still don't wipe fines?
Ah ok. Thanks for the explanation. I'm saying that backdating the checkin date is not working. (Maybe this needs a new bug report?) The library runs the fines script at noon. Before they upgraded to 3.8, they used to checkin the books from the dropbox in the morning and backdate the checkin date, so no fines were added for that day when the fines script ran at noon. Now they cannot backdate the checkin date, so fines get charged on those books.
Right that is a separate issue from the one nicole reported, which was an ehancement request to wipe fines already charged. Two choices, rename the description of this to be, dropbox mode doesnt set the correct date and leave it critical, and make a new one for the wiping fines, an ehancement, that is blocked by this. Or vice versa. I'm looking into the dropbox not setting the date issue now.
Thank you, Chris!
The problem is in Koha::Calendar if ( $add_duration->is_negative() ){ $day_dur->inverse(); } This does not do what we expect, namely it does not invert the value. Working on a fix now
Created attachment 10571 [details] [review] Bug 5644 : Fixing dropbox mode calculation
+----------------+------------+---------------------+------------+---------------+---------------------+-----------------+--------+----------+---------------------+---------------------+ | borrowernumber | itemnumber | date_due | branchcode | issuingbranch | returndate | lastreneweddate | return | renewals | timestamp | issuedate | +----------------+------------+---------------------+------------+---------------+---------------------+-----------------+--------+----------+---------------------+---------------------+ | 15861 | 23035 | 2012-07-21 23:59:00 | ME | NULL | 2012-06-29 09:10:00 | NULL | NULL | NULL | 2012-06-30 09:10:43 | 2012-06-30 09:09:00 | +----------------+------------+---------------------+------------+---------------+---------------------+-----------------+--------+----------+---------------------+---------------------+ Looks to be working now.
Just looking at the diff, I'm wondering if this patch only fixes it if the library has only been closed for one day, and not for multiple days at a time. Nicole's description of her understanding of the feature is that "checking the dropbox mode option on the check in screen made the due date effectively the last day the library was open". What do you think?
Hi Kyle, can't say anything about the code, but you described it right I think. That's how the feature has been documented and how I would expect it to work. So if people give something back over the weekend and I check it in Monday, the date should probably be Friday?
Yes, that's what should happen. Dropbox means that the patrons used the drop box for book/materials outside of the library. Many libraries don't penalize patrons who drop materials off while the library is closed so when the library opens again they take everything out of the box, and want to check the items in as if they were returned on a day the library was open. The intended side effect of this is that the patrons are not charged fines for the days the library was closed (if the library uses this feature). For libraries who don't have this policy they just don't use the drop box mode. Nicole
*** Bug 8087 has been marked as a duplicate of this bug. ***
(In reply to comment #13) > Just looking at the diff, I'm wondering if this patch only fixes it if the > library has only been closed for one day, and not for multiple days at a > time. Nicole's description of her understanding of the feature is that > "checking the dropbox mode option on the check in screen made the due date > effectively the last day the library was open". What do you think? This is why just looking at a diff is an entirely ineffective way to test a patch. This bug now is to do with the dropbox not setting the correct date. Anything to do with fines is on another bug. So please talk about that issue there. Bug 8338 But if you look at the whole of the code, or better yet run it. You will see all the patch changes is setting the day_duration to be negative. It then, (depending on the mode the calendar is in, or if its hours does different things) EG my $days = abs $add_duration->in_units('days'); while ($days) { $base_date->add_duration($day_dur); if ( $self->is_holiday($base_date) ) { next; } else { --$days; } } If the library is closed, it will keep subtracting (in this case because $day_dur) is negative. This is how this had always worked, up until 3.8.0 and so that regression is now fixed. Please, looking at diffs is not really a valid test of anything.
Created attachment 10622 [details] [review] Bug 5644 : Fixing dropbox mode calculation Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
mmm... I need a test plan, before pushing this patch:I tried -without the patch- to check-in a book, I checked "dropbox mode" I made a check-out of a book, return date july 31th Then I went to check-in, checked "Dropbox mode" button, made the checkin, and, in the database, I see: borrowernumber: 7852 itemnumber: 23170 date_due: 2012-07-31 23:59:00 branchcode: MAURES issuingbranch: NULL returndate: 2012-07-09 15:37:00 <<<<< YESTERDAY, dropbox mode OK lastreneweddate: NULL return: NULL renewals: NULL timestamp: 2012-07-10 15:37:45 issuedate: 2012-07-10 14:46:00 things seems OK. Any hint to understand what is fixed and I'll push it ! In the meantime = tiny patch, passed QA
Hi Paul, I think the way to test this involves the holiday calendar. It should always use the last opening day as return date - so the question is probably if it does work correctly with single, weekly and repeated holidays between between the last opening day and the checkin day.
Yep, it depends on what calendar mode you have set, whether it works or not. The bug only manifests if you have days mode set to calendar (not date due).
(In reply to comment #21) > Yep, it depends on what calendar mode you have set, whether it works or not. > > The bug only manifests if you have days mode set to calendar (not date due). Thanks, Testing = * set useDaysMode to "calendar" * reach return.pl page, click "dropbox mode, the display is "Dropbox Mode. (Effective checkin date is 14/07/2012 11:20 ).", which is wrong. * applying patch * reach return again, the display is "Dropbox Mode. (Effective checkin date is 11/07/2012 11:25 )." 12th is marked as closed => it's OK ! patch pushed !
Failed test 'Negative call to addDate' # at t/Kalendar.t line 62. # '2011-06-15' # cmp # '2011-06-15' # Looks like you failed 1 test of 9. Weird thing happening here, that looks ok to me
Created attachment 10835 [details] [review] Bug 5644 : Fixing the test
Fix for the test suite
Patch fixes the test, signed off & passed QA & push in one go
Pushed,will be in 3.8.3