Bug 13909 - Suspension days calculation doesn't honour finesCalendar
Summary: Suspension days calculation doesn't honour finesCalendar
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 13958
  Show dependency treegraph
 
Reported: 2015-03-25 19:04 UTC by Tomás Cohen Arazi
Modified: 2016-06-21 21:38 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 13909: Suspension days calculation should respect finesCalendar (4.34 KB, patch)
2015-03-30 13:50 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13909: Suspension days calculation should respect finesCalendar (4.41 KB, patch)
2015-04-09 06:59 UTC, Josef Moravec
Details | Diff | Splinter Review
[PASSED QA] Bug 13909: Suspension days calculation should respect finesCalendar (4.46 KB, patch)
2015-04-10 12:55 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 13909: (QA followup) fix references to get_chargeable_units (1.58 KB, patch)
2015-04-12 14:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2015-03-25 19:04:26 UTC
The routine should be using C4::Overdues::_get_chargeable_units instead of an ad-hoc calculation of the overdue days to use as basis for the calculatio here:


    my $calendar = Koha::Calendar->new( branchcode => $branchcode );

    # $deltadays is a DateTime::Duration object
    my $deltadays = $calendar->days_between( $dt_due, $dt_today );

To reproduce:
- Set finesCalendar = "directly"
- Have a circulation rule stating:
  interval for calculating fines = 1
  suspension days = 3
- Have the calendar set for sunday and saturday as holidays.
- Checkout an item with a branch/itype/borrower category that matches the defined circ rule with a hand-writen due date to (say) last friday.
- Check the item in
=> FAIL: Notice that the user is debarred using the calendar (skipping saturday and sunday).
Comment 1 Tomás Cohen Arazi 2015-03-30 13:50:30 UTC Comment hidden (obsolete)
Comment 2 Josef Moravec 2015-04-09 06:59:36 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2015-04-10 12:55:51 UTC
Created attachment 37646 [details] [review]
[PASSED QA] Bug 13909: Suspension days calculation should respect finesCalendar

This patch make _debar_user_on_return respect the finesCalendar syspref.

It does so, by replacing the ad-hoc overdue days calculation in favor of
C4::Overdues::_get_chargeable_units (which is renamed C4::Overdues::get_chargeable_units
and exported). There's no behaviour change besides making the calculation simpler
and correct.

To test:
- Set finesCalendar = "directly"
- Have a circulation rule stating:
  interval for calculating fines = 1
  suspension days = 3
- Have the calendar set for sunday and saturday as holidays.
- Checkout an item with a branch/itype/borrower category that matches the defined circ rule with a hand-writen due date to (say) last friday.
- Check the item in
=> FAIL: Notice that the user is debarred using the calendar (skipping saturday and sunday).
- Apply the patch
- Repeat the previous steps
=> SUCCESS: calculation is correct (counting saturday and sunday as overdue days, i.e. 'directly').
- Set finesCalendar = "calendar"
- Repeat the test
=> SUCCESS: calculation is correct (skipping holidays).
- Sign off.

Sponsored-by: Universidad Nacional de Cordoba

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 4 Tomás Cohen Arazi 2015-04-12 03:56:18 UTC
Patch pushed to master.
Comment 5 Tomás Cohen Arazi 2015-04-12 14:10:51 UTC
Created attachment 37708 [details] [review]
Bug 13909: (QA followup) fix references to get_chargeable_units

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 6 Chris Cormack 2015-04-20 10:16:46 UTC
Pushed to 3.18.x will be in 3.18.6
Comment 7 MetaBiblioteca Colombia 2015-09-03 18:37:33 UTC
Hello in Koha 3.20 Suspentions fails when loan by hours

See http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14293

The solution:

Remove line 2157 in the /usr/share/koha/lib/C4/Circulation.pm

$finedays = $finedays * 24 if ( $unit eq 'hours' );