Bug 12895 - Dropbox mode doesn't work correctly
Summary: Dropbox mode doesn't work correctly
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P1 - high major (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-09 13:39 UTC by Koha Team Lyon 3
Modified: 2016-06-21 21:37 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 12895 repair dropbox mode (3.87 KB, patch)
2014-12-05 09:43 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
[SIGNED OFF]Bug 12895 repair dropbox mode (3.94 KB, patch)
2014-12-26 22:27 UTC, Nick Clemens
Details | Diff | Splinter Review
[PASSED QA] Bug 12895 repair dropbox mode (5.29 KB, patch)
2014-12-27 14:22 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Koha Team Lyon 3 2014-09-09 13:39:49 UTC
When dropbox mode is enabled, if you check in a document with a duedate planned on previous opening day, the patron is restricted nevertheless.
Seems that the reason why is that the comparison of dates is made fom current date instead of  last opening date.
Circulation.pm/AddReturn, ligne 1793 :
$issue->{'overdue'} = DateTime->compare($issue->{'date_due'}, $today ) == -1 ? 1 : 0;
Comment 1 Koha Team Lyon 3 2014-12-05 09:43:35 UTC Comment hidden (obsolete)
Comment 2 Koha Team Lyon 3 2014-12-05 09:48:15 UTC
I put the importance level to P1 because I thing this problem may be source of litigation with patrons
Comment 3 Katrin Fischer 2014-12-06 09:10:31 UTC
Is the patch intended for 3.12 or is it also intended for master/3.18?
Comment 4 Koha Team Lyon 3 2014-12-09 15:18:37 UTC
Yes it's also for 3.18
Comment 5 Koha Team Rennes 2 2014-12-11 10:10:29 UTC
Test failed. The borrower is never restricted when checking the item with dropbox mode on, whatever the duedate is. We tried with one day late, two days late, 5 days late.

It should be useful to precise that the circulation rules must be the same and to check these rules before testing.

Kohateam Rennes 2
Comment 6 Koha Team Lyon 3 2014-12-11 13:27:29 UTC
Very sorry. my fault. My test plan was missing some most important thing.
Here's the reason why that doesn't work :
you must affect suspension days parameter in the circulation rules (Administration/Circulation and fine rules) matching both borrower and document type you choose for the test.

Test plan :
1) choose a borrower and an item 
2) Set suspension in the suspension days parameter in the circulation rules (Administration/Circulation and fine rules) matching both borrower and document type
3) Checkout the item selecting the previous opening date in the Specify-due-date box.
4) click on Circulation in the upper menu, then on Checkin and check the Book drop mode. The Book drop date showed should be the previous opening date.
5) Check in the item : you can see that the patron is restricted
6) apply the patch
7) redo 1 to 5 : Now, you can see that the patron is not restricted.
8) If you redo the test with two day late, you will see that the patron
is not restricted : that's ok because his restriction of one day is already finished.
9) If you redo the test with more than two day late, you see that the patron restriction is, as expected, one day shorter than it were if the item had been returned without dropbox mode.
Comment 7 Koha Team Rennes 2 2014-12-11 13:43:26 UTC
(In reply to Koha Team Rennes 2 from comment #5)
> Test failed. The borrower is never restricted when checking the item with
> dropbox mode on, whatever the duedate is. We tried with one day late, two
> days late, 5 days late.
> 
> It should be useful to specify that the circulation rules must be the same
> and to check these rules before testing.
> 
> Kohateam Rennes 2
Comment 8 Koha Team Lyon 3 2014-12-12 14:54:44 UTC
NEW TEST PLAN again (hope that will be the good one) :

1)Check in the calendar (Tools/Calendar), that the previous days you are about to use as date due are realy entered as opening day (never know).
2) Add a suspension in the suspension days parameter of the circulation rules (Administration/Circulation and fine rules) to the MOST specific category of borrower and MOST specific type of document among the existing rules of the LOGGED IN Site(cf explications in the circ-rules page).
3) Choose a borrower using the search by category and an item through the advanced search using the limit by type.
4) Checkout the item selecting the previous opening date in the Specify-due-date box.
5) Click on Circulation in the upper menu, then on Checkin and check the Book drop mode. The Book drop date showed should be the previous opening date.
6) Check in the item : you can see that the patron is restricted
7) apply the patch
8) Redo 1 to 5 : Now, you can see that the patron is not restricted.
9) If you redo the test with two day late, you will see that the patron
is not restricted : that's ok because his restriction of one day is already finished.
10) If you redo the test with more than two day late, you see that the patron restriction is, as expected, one day shorter than it were if the item had been returned without dropbox mode.

Olivier Crouzet
Comment 9 Nick Clemens 2014-12-26 22:27:09 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2014-12-27 14:15:03 UTC
I understand this is a bad bug for libraries using fine days, so I am going to pass QA.

 But I also feel we need more unit tests for AddReturns - it would be great to see a regression test for this added on a separate bug report. I tried t/db_dependent/Circulation/Returns.t, but it currently fails on master for me. t/db_dependent/Circulation_issue.t (see bug 13494)
Comment 11 Katrin Fischer 2014-12-27 14:22:07 UTC
Created attachment 34716 [details] [review]
[PASSED QA] Bug 12895 repair dropbox mode

One day late patrons were restricted even with dropbox mode activated

1) Check in the calendar (Tools/Calendar), that the
   previous days you are about to use as date due are
   really entered as opening day (never know).
2) Add a suspension in the suspension days parameter
   of the circulation rules (Administration/Circulation
   and fine rules) to the MOST specific category of
   borrower and MOST specific type of document among the
   existing rules of the LOGGED IN Site(cf explications
   in the circ-rules page).
3) Choose a borrower using the search by category and an
   item through the advanced search using the limit by type.
4) Checkout the item selecting the previous opening date
   in the Specify-due-date box.
5) Click on Circulation in the upper menu, then on Checkin
   and check the Book drop mode. The Book drop date showed
   should be the previous opening date.
6) Check in the item : you can see that the patron is restricted
7) apply the patch
8) Redo 1 to 5 : Now, you can see that the patron is not restricted.
9) If you redo the test with two day late, you will see that
   the patron is not restricted : that's ok because his
   restriction of one day is already finished.
10) If you redo the test with more than two day late, you see
    that the patron restriction is, as expected, one day shorter
    than it were if the item had been returned without dropbox mode.

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Tomás Cohen Arazi 2014-12-28 23:51:41 UTC
Patch pushed to master.

Thanks Koha Team Lyon 3!
Comment 13 Chris Cormack 2014-12-30 21:41:32 UTC
Pushed to 3.18.x will be in 3.18.3
Comment 14 Mason James 2015-01-16 20:36:26 UTC
Pushed to 3.16.x, will be in 3.16.7