http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Koha Team Lyon 3 <koha@univ-lyon3.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10290|0 |1 is obsolete| | --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 10308 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10308&action=edit Bug 8251 - Patrons get incorrectly debarred Patrons get incorrectly debarred because of the use of Datetime->delta_days wich happens to always return a positive number. Also changed the use of Datetime->truncate function so it follows the docs, using 'day' instead of 'days' (which is used in DateTime::Duration). This patch has been tested against master. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |julian.maurice@biblibre.com -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Ian Walls <koha.sekjal@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |koha.sekjal@gmail.com --- Comment #5 from Ian Walls <koha.sekjal@gmail.com> --- Correctly reimplements duration as signed instead of absolute value. Marking Passed QA. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 10418 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10418&action=edit Bug 8251 - Follow up, use 'day' instead of 'day' for Datetime::truncate As noted in comment #4 by Jonathan Druart, this should be fixed in every call to DateTie::truncate function. This patch does exactly that. julian_m tested that truncate( to => 'days' ) didn't actually do anything, so it is understandable that this 'fix' might introduce new bugs as we might have 'fixed' at a higher libs level this issue. 'minutes' is used in truncate function too, so fixing. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Koha Team Lyon 3 <koha@univ-lyon3.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10348|0 |1 is obsolete| | --- Comment #9 from Koha Team Lyon 3 <koha@univ-lyon3.fr> --- Created attachment 10597 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10597&action=edit Patrons get incorrectly debarred

From a duration calculated with "$duration= $datestart_temp - $dateend_temp" subtract method (used to exclude holidays) can't apply correctly because it only do subtraction to the day value and not the global amount of month and days. Depending on the amount of closed holidays, this may end with a negative value for the days : a patron may not be restricted even with much late. example : A patron is 1 month and 3 days late. There are 6 days closed for holidays. Duration hash will look like this : $VAR1 = bless( { 'seconds' => 0, 'minutes' => 0, 'end_of_month' => 'wrap', 'nanoseconds' => 0, 'days' => -3, 'months' => 1 }, 'DateTime::Duration' );

May be there is an adequate Datetime::Duration method to calculate correctly but I could'nt find it in Datetime jungle. So, I came back to my first idea : keeping the calculation of duration with delta_days method (with this one the duration value is set only in day units and so subtract method applies correctly), then checking for late and if not, make the delta number negative. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 --- Comment #11 from Koha Team Lyon 3 <koha@univ-lyon3.fr> --- I suppose that if you test a checkin from a loan in days "Patrons get incorrectly debarred" patch should be enough (truncate function used in days_between routine is corrected into it) but if you do it on an hours loan, you should have both patches applied. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10418|0 |1 is obsolete| | --- Comment #13 from Dobrica Pavlinusic <dpavlin@rot13.org> --- Created attachment 10767 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10767&action=edit Bug 8251 - Follow up, use 'day' instead of 'day' for Datetime::truncate As noted in comment #4 by Jonathan Druart, this should be fixed in every call to DateTie::truncate function. This patch does exactly that. julian_m tested that truncate( to => 'days' ) didn't actually do anything, so it is understandable that this 'fix' might introduce new bugs as we might have 'fixed' at a higher libs level this issue. 'minutes' is used in truncate function too, so fixing. Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dpavlin@rot13.org -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10767|Bug 8251 - Follow up, use |[SIGNED-OFF] Bug 8251 - description|'day' instead of 'day' for |Follow up, use 'day' |Datetime::truncate |instead of 'day' for | |Datetime::truncate -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 --- Comment #16 from Paul Poulain <paul.poulain@biblibre.com> --- (In reply to comment #15)

I signed off first patch because I would really love to see it in Koha ASAP :-)

QA comment for the 1st patch = I've checked that the API is singular, and not plural (day, minute,...) The 1st patch (http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10767) fixes only that, passed QA

Unfortunately, I can't really test properly debarred code, but I did re-base it on top of current master, so Zeno if you could sign it off, this would be great :-)

Zeno, i'll wait for your signoff before pushing the http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10768, but from QA pov, I don't see any reason not to pass QA -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Zeno Tajoli <tajoli@cilea.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #18 from Zeno Tajoli <tajoli@cilea.it> --- Hi to all, I tested this patch [last version, http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10768] with sandboxes. I confirm that now patrons are NOT systematically debarred at checkin. So I sign-off the patch -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com --- Comment #20 from Mason James <mtj@kohaaloha.com> --- (In reply to comment #18)

Hi to all, I tested this patch [last version, http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10768] with sandboxes. I confirm that now patrons are NOT systematically debarred at checkin.

So I sign-off the patch

Zeno, you need to add a sign-off message to that patch, before it is properly signed-off like this... Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> can you do that please? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 --- Comment #21 from Zeno Tajoli <tajoli@cilea.it> --- Hi to all, today with this commit, http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=9d4e241adadc9a3c... the problem [Patrons are systematically debarred at checkin] is not more present in master. I test it with sanboxes But I don't know what to do. Is it usesul to test the last Dobrica's patch with the present master ? I see comment #19 but is very difficult to test so complex situation. For comment #20: Yesterday I test the path with sanboxes so I don't sign the patch. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10768|0 |1 is obsolete| | --- Comment #23 from Mason James <mtj@kohaaloha.com> --- Created attachment 10914 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10914&action=edit Patrons get incorrectly debarred Patrons get incorrectly debarred because of the use of Datetime->delta_days wich happens to always return a positive number. Added a check to inverse the delta when patron is not late. Also changed the use of Datetime->truncate function so it follows the docs, using 'day' instead of 'days' (which is used in DateTime::Duration). http://bugs.koha-community.org/show_bug.cgi?id=8251 Signed-off-by: Tajoli Zeno <tajoli@cilea.it> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #25 from Mason James <mtj@kohaaloha.com> --- (In reply to comment #18)

Hi to all, I tested this patch [last version, http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10768] with sandboxes. I confirm that now patrons are NOT systematically debarred at checkin.

So I sign-off the patch

passing QA on this patch $ koha-qa.pl - 58cc95a Patrons get incorrectly debarred Koha/Calendar.pm - perlcritic-progressive tests... OK - perl -c syntax tests... OK - xt/tt_valid.t tests... OK - xt/author/vaild-template.t tests... OK -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 --- Comment #27 from Koha Team Lyon 3 <koha@univ-lyon3.fr> --- This patch should not be pushed because it causes false calculation of late delay when patron is more than one month late. Here's how you can test it : First you have to install patch of Bug 8348 (http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10619) to correct 2 errors on day closed calculation (if not, day_month_closed_days and single_holidays won't be subtracted in 'while( DateTime...' loop, so the test would be falsed). Another patch fixes these errors and could be simpler and prefered but I have not tested it (bug 8418 : http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10840). Then you must have Follow-up of this page (http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10418) installed too. In our install the finesCalendar syspref is set to 'not including days the library is closed' and the useDaysMode syspref to 'the calendar to push the due date to the next open day'. Go to checkout tab of a patron. paste a barcode in barcode textbox In "specify due date" textbox set a date in order that the patron should be more than one month late (say '2012-05-07'). checkout and confirm Go to Calendar Tool and set between date due and today a number of closed days slightly greater than the nth day (7) of date due (say 9). Checkin the barcode. You 'll see that patron is not restricted. Uninstall the previous "Patrons are systematically debarred at checkin" patch, Go to obsolete patches of this page and install this one (Patrons get incorrectly debarred, 2012-07-02) : http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10597 Redo the same test. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 --- Comment #29 from Paul Poulain <paul.poulain@biblibre.com> --- (In reply to comment #27)

This patch should not be pushed because it causes false calculation of late delay when patron is more than one month late.

mmm... could you look at http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7420#c29 I think there are interesting informations here. I spent 3 hours trying to understand what was happening :( -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #30 from Paul Poulain <paul.poulain@biblibre.com> --- (In reply to comment #28)

So far, none of the 3 patches are in 3.8.x

But the first 2 are in master.

So if none should be in master you will need to tell Paul that so he can remove them.

I am not pushing any to 3.8.x until this is working.

I hope we've solved the problem: * There was a badly wrong calculation in days_between, proven on bug 8486, and also fixed here by Colin. * The last patch attached here (http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10914) should not be pushed, as it tries to address (uncorrectly) the problem fixed on bug 8486 * The 2 other patches can be applied (c6f1f64cb71e9d8ec560a201a4a0bc16c1510f3f & 9d4e241adadc9a3c62d94a2d487a738e9490925a = obvious API mistake, see comment 16 and comment 17 ) Chris_c = team Lyon 3 is AFK, until something like Aug 20th. I think we should not wait until they come back to push those 2 patches. So please check I'm not wrong, but I don't think so Dobrica, Mason & Tomas, you're welcomed to jump in the discussion too ! -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10348|1 |0 is obsolete| | --- Comment #32 from Paul Poulain <paul.poulain@biblibre.com> --- Comment on attachment 10348 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10348 [SIGNED-OFF] Bug 8251 - Patrons get incorrectly debarred This patch has already been pushed, unobsoleting it just to have a clean bug status with the 2 patches applied visible -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 --- Comment #34 from Chris Cormack <chris@bigballofwax.co.nz> --- (In reply to comment #30)

(In reply to comment #28)

...

So far, none of the 3 patches are in 3.8.x

But the first 2 are in master.

So if none should be in master you will need to tell Paul that so he can remove them.

I am not pushing any to 3.8.x until this is working.

I hope we've solved the problem: * There was a badly wrong calculation in days_between, proven on bug 8486, and also fixed here by Colin. * The last patch attached here (http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10914) should not be pushed, as it tries to address (uncorrectly) the problem fixed on bug 8486

The patch on bug 8486 does not apply cleanly on 3.8.x Does it need to to be pushed after these two? Not pushing to 3.8.x until there is a clear plan and they all apply cleanly. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Magnus Enger <magnus@enger.priv.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |magnus@enger.priv.no --- Comment #36 from Magnus Enger <magnus@enger.priv.no> --- Sorry, but I'm still seeing this behaviour on both 3.8.4 (running off the official packages) and current master (3.09.00.032, dev-install): 1. Check out then return = OK 2. Add a circ rule that includes "Suspension in days" = 7 3. Check out then return = Patron is debarred until 18/03/2013 (March 18th next year, which seems like a very long and somewhat arbitrary period of time) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 --- Comment #38 from Chris Cormack <chris@bigballofwax.co.nz> --- (In reply to comment #37)

There is a basic flaw in the logic in that patron's finedays are being calculated on all returns not just overdues. The delta days is a positive element that is the documented interface. Its purpose is not to inform if an issue is overdue that is already indicated in the return from GetItemIssue

Ah yes that makes sense. Yeah, that is a flaw, we should only be working out the delta days on things that are overdue, instead of on all items. Now you have explained it, it makes perfect sense. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff --- Comment #40 from Colin Campbell <colin.campbell@ptfs-europe.com> --- switch status back to requires signoff for latest patch -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #43 from Paul Poulain <paul.poulain@biblibre.com> --- "Do not try to debar patrons if returns are not overdue" follow-up pushed -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8251 --- Comment #45 from Magnus Enger <magnus@enger.priv.no> --- Just checked that the problem is no longer present in 3.8.5. Yay! -- You are receiving this mail because: You are watching all bug changes.