Bug 13958

Summary: Add a suspensionsCalendar syspref
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: CirculationAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: black23, gmcharlt, jonathan.druart, katrin.fischer, kyle.m.hall, lucas, martin.renvoize, niamh.walker-headon
Version: master   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Before 18.05, suspension expiry date calculation didn't take the calendar into account. This behaviour changed with bug 19204, and some libraries miss the old behaviour. These patches decouple overdue days calculation configuration (`finesCalendar`) from how the expiry date is calculated for the suspension through a new system preference: `SuspensionsCalendar`, that has the exact same options but only applies to suspensions. On upgrade, the new preference is populated with the value from `finesCalendar`, thus respecting the current configuration.
Version(s) released in:
19.11.00
Bug Depends on: 13909, 19204    
Bug Blocks:    
Attachments: Bug 13958: Add SuspensionsCalendar syspref
Bug 13958: Use SuspensionsCalendar syspref for more granularity
Bug 13958: Add SuspensionsCalendar syspref
Bug 13958: Use SuspensionsCalendar syspref for more granularity

Description Tomás Cohen Arazi 2015-04-06 13:54:17 UTC
We need a way to provide more granularity to the circulation/fines/suspensions rules. Specifically we would benefit from decoupling fines and suspensions regarding how overdue days are calculated.

For that purpose we need to have a suspensionsCalendar syspref that does the same finesCalendar does for fines.
Comment 1 Tomás Cohen Arazi 2019-05-27 13:56:39 UTC
Bug 19204 introduced a behavior change making the suspension calculation follow finesCalendar. While this is desirable, libraries have their own rules and this change requires more flexibility so they can revert the 'fix' and follow their established rules.
Comment 2 Tomás Cohen Arazi 2019-05-27 15:20:51 UTC
Created attachment 90118 [details] [review]
Bug 13958: Add SuspensionsCalendar syspref

Sponsored-by: Universidad Nacional de Córdoba
Comment 3 Tomás Cohen Arazi 2019-05-27 15:20:56 UTC
Created attachment 90119 [details] [review]
Bug 13958: Use SuspensionsCalendar syspref for more granularity

This patch makes the calculation of suspension expiration configuration
detached from the finesCalendar syspref. This way libraries have more
flexibility when setting circ rules.

I wrote this path as a bugfix, because the recent change pushed by bug 19204
changed this behavior and libraries have been complaining since they got
upgraded.

It is a minor change, just adding the new syspref, and replacing the
single place in which it is used for debarment calculation for the new
one.

The tests are updated similarly, just adding the new mocked syspref as
it was with the old one.

To test:
- Apply this patches
- updatedatabase
- Run:
  $ kshell
 k$ prove t/db_dependent/Circulation.t
=> SUCCESS: Tests pass!
- Sign off :-D

Sponsored-by: Universidad Nacional de Córdoba
Comment 4 Michal Denar 2019-06-03 20:35:13 UTC
Hi Tomás,

test passed with Zebra, but fails with ES. Can you take a look? Thank You.

Result:
t/db_dependent/Circulation.t .. 104/130     # No tests run!
t/db_dependent/Circulation.t .. 108/130
#   Failed test 'No tests run for subtest "CanBookBeIssued & AllowReturnToBranch"'
#   at t/db_dependent/Circulation.t line 1330.
Can't call method "leader" on an undefined value at /home/vagrant/kohaclone/Koha/SearchEngine/Elasticsearch.pm line 429.
# Looks like your test exited with 255 just after 108.
t/db_dependent/Circulation.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 23/130 subtests

Test Summary Report
-------------------
t/db_dependent/Circulation.t (Wstat: 65280 Tests: 108 Failed: 1)
  Failed test:  108
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 130 tests but ran 108.
Files=1, Tests=108,  8 wallclock secs ( 0.04 usr  0.00 sys +  5.32 cusr  0.66 csys =  6.02 CPU)
Result: FAIL
Comment 5 Martin Renvoize 2019-11-07 09:02:53 UTC
Created attachment 95131 [details] [review]
Bug 13958: Add SuspensionsCalendar syspref

Sponsored-by: Universidad Nacional de Córdoba
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2019-11-07 09:02:56 UTC
Created attachment 95132 [details] [review]
Bug 13958: Use SuspensionsCalendar syspref for more granularity

This patch makes the calculation of suspension expiration configuration
detached from the finesCalendar syspref. This way libraries have more
flexibility when setting circ rules.

I wrote this path as a bugfix, because the recent change pushed by bug 19204
changed this behavior and libraries have been complaining since they got
upgraded.

It is a minor change, just adding the new syspref, and replacing the
single place in which it is used for debarment calculation for the new
one.

The tests are updated similarly, just adding the new mocked syspref as
it was with the old one.

To test:
- Apply this patches
- updatedatabase
- Run:
  $ kshell
 k$ prove t/db_dependent/Circulation.t
=> SUCCESS: Tests pass!
- Sign off :-D

Sponsored-by: Universidad Nacional de Córdoba
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2019-11-07 09:06:20 UTC
This works as expected for me, and I'm pleased that the DB update takes the original settings into account.

Trivial patch, passing QA.
Comment 8 Martin Renvoize 2019-11-07 09:12:41 UTC
Nice work!

Pushed to master for 19.11.00
Comment 9 Lucas Gass 2019-12-10 23:13:51 UTC
backported to 19.05.x for 19.05.06