When a user is returning an item with overudue, the suspension is calculated in days according 'suspensions in day' it doesn't calculate the 'Fine charging interval' that I guess is used for € fines, but doesn't apply to suspensions in day. Not all libraries has a relation > 1 to days and suspensions, for example now it is not possible to define 7 days of suspension for every 14. It should be nice if Koha also consider the 'Fine charging interval' for suspensions in days and library could decide to change the suspension, every 1, 2 ... days
Created attachment 69783 [details] [review] Bug 19804: Add new DB column issuingrules.suspension_chargeperiod
Created attachment 69784 [details] [review] Bug 19804: DBIC Schema changes
Created attachment 69785 [details] [review] Bug 19804: Add a 'Fine charging interval' for suspension days We already have a chargeperiod (Fine charging interval) value which is taken into account for fine ($) for not for the suspension period. This patch adds a new column suspension_chargeperiod (Fine day charging interval) to add the same behaviour when a suspension is calculated. Test plan: Add overdue item and play with the circulation rules (and the calendar). The suspension period must be correctly calculated. Please provide the different tests you made.
Created attachment 69786 [details] [review] Bug 19804: Add the new column to the circ rules interface
Good morning I have insall the patch into kohadev, I have checked out and checked in several items with different configuration and it works.. I have created a rule of 1 suspension day for every 2 delay Case 1 Cumulative on Calendar without holidays--> works as espected Calendar with holiday --> work as expected Cumulative off Calendar without holidays --> works as expected Calendar with holidays --> works as expected I missed something to check, if not I will sign it asap
Created attachment 70404 [details] [review] Bug 19804: Add new DB column issuingrules.suspension_chargeperiod Signed-off-by: Hugo Agud <hagud@orex.es>
Created attachment 70405 [details] [review] Bug 19804: DBIC Schema changes Signed-off-by: Hugo Agud <hagud@orex.es>
Created attachment 70406 [details] [review] Bug 19804: Add a 'Fine charging interval' for suspension days We already have a chargeperiod (Fine charging interval) value which is taken into account for fine ($) for not for the suspension period. This patch adds a new column suspension_chargeperiod (Fine day charging interval) to add the same behaviour when a suspension is calculated. Test plan: Add overdue item and play with the circulation rules (and the calendar). The suspension period must be correctly calculated. Please provide the different tests you made. Signed-off-by: Hugo Agud <hagud@orex.es>
Created attachment 70407 [details] [review] Bug 19804: Add the new column to the circ rules interface Signed-off-by: Hugo Agud <hagud@orex.es>
Jonathan, the atomicupdate should set an initial value for existing rules, based on chargeperiod.
(In reply to Tomás Cohen Arazi from comment #10) > Jonathan, the atomicupdate should set an initial value for existing rules, > based on chargeperiod. It's done already, "DEFAULT "1"'
Some tiny things: - number of tests needs to be 114 for them to pass - the headings at the top and the bottom row of the circulation rules table don't match up - Maybe rename "Fine day charging interval" to "Suspension charging interval" to match a bit more with the other related columns? - Adding a see also for bug 19204 - it's obvious here too, that calendar is ignored. Can you provide a quick follow up?
Created attachment 73665 [details] [review] Bug 19804: Add a 'Fine charging interval' for suspension days We already have a chargeperiod (Fine charging interval) value which is taken into account for fine ($) for not for the suspension period. This patch adds a new column suspension_chargeperiod (Fine day charging interval) to add the same behaviour when a suspension is calculated. Test plan: Add overdue item and play with the circulation rules (and the calendar). The suspension period must be correctly calculated. Please provide the different tests you made. Signed-off-by: Hugo Agud <hagud@orex.es>
Created attachment 73666 [details] [review] Bug 19804: Add the new column to the circ rules interface Signed-off-by: Hugo Agud <hagud@orex.es>
Created attachment 73667 [details] [review] Bug 19804: (follow-up) Fix wording and columns order
(In reply to Katrin Fischer from comment #12) > Some tiny things: > > - number of tests needs to be 114 for them to pass Patch amended. > - the headings at the top and the bottom row of the circulation rules table > don't match up > - Maybe rename "Fine day charging interval" to "Suspension charging > interval" to match a bit more with the other related columns? Done, thanks!
Hi Joubu, running out of time for testing this morning, but have 2 questions I want to note: 1) For the fine charging interval we multiply: Days overdue / fine charging interval * fine The equivalence would then be: Days overdue / suspension charging interval * suspension in days But that's not quite what this does, is it? 2) I have backdated an item on checkout to be immediately overdue, but on return no restriction was created. Can you add a more detailed test plan?
(In reply to Katrin Fischer from comment #17) > Hi Joubu, > > running out of time for testing this morning, but have 2 questions I want to > note: > > 1) For the fine charging interval we multiply: > Days overdue / fine charging interval * fine > > The equivalence would then be: > Days overdue / suspension charging interval * suspension in days > > But that's not quite what this does, is it? Yes it is what it does, take a look at the tests: 1795 # We want to charge 2 days every day, without grace 1796 # With 5 days of overdue: 5 * Z (Z is a typo for 2 I guess..) 1814 # We want to charge 2 days every 2 days, without grace 1815 # With 5 days of overdue: (5 * 2) / 2 1836 # We want to charge 2 days every 3 days, with 1 day of grace 1837 # With 5 days of overdue: ((5-1) / 3 ) * 2 > 2) I have backdated an item on checkout to be immediately overdue, but on > return no restriction was created. Can you add a more detailed test plan? Today is April 6th, I set: Suspension in days (day): 3 Suspension charging interval: 2 Which means: charge 3 days every 2 days. Check an item in with a due date on April 2nd (4 days of overdue) We are expecting: 3*4/2 = 6 days of suspension And I get: "Restricted: Patron's account is restricted until 12/04/2018" \o/
It was me, the devbox had the wrong time and I made a mistake in the circulation conditions... *sigh* - But the good news is the patch working :)
Created attachment 73830 [details] [review] Bug 19804: Add new DB column issuingrules.suspension_chargeperiod Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 73831 [details] [review] Bug 19804: DBIC Schema changes Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 73832 [details] [review] Bug 19804: Add a 'Fine charging interval' for suspension days We already have a chargeperiod (Fine charging interval) value which is taken into account for fine ($) for not for the suspension period. This patch adds a new column suspension_chargeperiod (Fine day charging interval) to add the same behaviour when a suspension is calculated. Test plan: Add overdue item and play with the circulation rules (and the calendar). The suspension period must be correctly calculated. Please provide the different tests you made. Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 73833 [details] [review] Bug 19804: Add the new column to the circ rules interface Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 73834 [details] [review] Bug 19804: (follow-up) Fix wording and columns order Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 18.05, thanks to everybody involved!
Created attachment 73894 [details] [review] Bug 19804: Fix test in GetHardDueDate.t
Created attachment 73895 [details] [review] Bug 19804: Fix test in maxsuspensiondays.t We do not want a random value of suspension_chargeperiod from TestBuilder
Last two patches pushed to master.