Bug 6858 - Adds staticfines.pl for static fines processing
Summary: Adds staticfines.pl for static fines processing
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low normal (vote)
Assignee: Matthias Meusburger
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-08 12:43 UTC by Matthias Meusburger
Modified: 2014-10-01 09:54 UTC (History)
10 users (show)

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


Attachments
Bug 6858: Adds staticfines.pl for static fines processing (9.45 KB, patch)
2011-09-09 08:46 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 6858: Adds staticfines.pl for static fines processing (9.45 KB, patch)
2011-11-04 10:59 UTC, claire.hernandez@biblibre.com
Details | Diff | Splinter Review
Bug 6858: Adds staticfines.pl for static fines processing (9.81 KB, patch)
2012-03-22 13:42 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 6858: Adds staticfines.pl for static fines processing (9.88 KB, patch)
2012-03-22 14:45 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug-6858-Adds-staticfinespl-for-static-fines-proce.patch (9.88 KB, patch)
2012-03-22 14:50 UTC, delaye
Details | Diff | Splinter Review
Bug 6858: Adds staticfines.pl for static fines processing (7.93 KB, patch)
2012-05-25 09:08 UTC, Matthias Meusburger
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Meusburger 2011-09-08 12:43:01 UTC
Add a tool to calculate static fine. For example, 7 days left = 1€ fixed fine
Comment 1 Matthias Meusburger 2011-09-09 08:46:18 UTC Comment hidden (obsolete)
Comment 2 Paul Poulain 2011-10-24 11:38:22 UTC
Updating Version : This ENH will be for Koha 3.8
Comment 3 Paul Poulain 2011-10-25 15:06:00 UTC
Bug versionned for master. entries will be made against rel_3_8 once the patch has been applied (see thread about that on koha-devel yesterday)
Comment 4 claire.hernandez@biblibre.com 2011-11-04 10:59:29 UTC Comment hidden (obsolete)
Comment 5 claire.hernandez@biblibre.com 2011-12-02 13:47:13 UTC
$ git bz apply 6858
Bug 6858 - Adds staticfines.pl for static fines processing

Bug 6858: Adds staticfines.pl for static fines processing
Apply? [yn] y

Applying: Bug 6858: Adds staticfines.pl for static fines processing
/home/claire/dev/community_repo/koha/.git/rebase-apply/patch:175: trailing whitespace.
    
error: misc/cronjobs/staticfines.pl: already exists in index
Patch failed at 0001 Bug 6858: Adds staticfines.pl for static fines processing
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Patch left in /tmp/Bug-6858-Adds-staticfinespl-for-static-fines-proce-LGi2ve.patch
Comment 6 claire.hernandez@biblibre.com 2011-12-02 14:53:01 UTC
Forget it, applies well with a 3-way merge in git bz

(In reply to comment #5)
Comment 7 Katrin Fischer 2011-12-21 07:20:58 UTC
Patches both apply - which patch is obsolete?

Can you explain a bit more about how this works? How often should this be run?
Comment 8 Matthias Meusburger 2012-02-03 13:17:29 UTC
Hi Katrin,

The two attached patches are the same, only the "from" md5 differs, so I guess we can safely get rid of one of them.

 
This patch allows to apply only one fine for a given borrower, no matter how many items are late, based on a given delay, which applies to the most late item.

The script bypasses overdue rules already defined in koha.

Example : 

staticfines.pl --category CAT1, 10 --category CAT2, 20 --library LIB1 --library LIB2 --delay 20

This will charge a 10€ fine for borrowers of category CAT1 and a 20€ fine for borrowers of category CAT2 for libraries LIB1 and LIB2, after a 20 days overdue delay for their most late item.

This script is typically used on a daily cronjob basis.
Comment 9 delaye 2012-03-09 16:38:12 UTC
the 9 march 2012
in the sandbox test1

I apply the patch

I test the command :

perl staticfines.pl -category J14,10 -library MAURES -delay 2

for the borrwernumber 5103

borrowernumber | categorycode | date_due   |
+----------------+--------------+------------+
|           5103 | J14          | 2012-03-07 |

but nothing in the borrower account

i test with all the option in the system preference finmode...but nothing....

mistake with finemode ?
Comment 10 Koha Team University Lyon 3 2012-03-19 16:14:22 UTC
I don't really understand how to test this patch... Can you propose a test plan ?
Comment 11 MathildeF 2012-03-20 13:33:35 UTC
OK

testing with :

syspref : finesMode on "calculate and bill" ("calculer et facturer" in french)
and 
run "misc/cronjobs/fines.pl"
Comment 12 Matthias Meusburger 2012-03-20 13:41:18 UTC
For a user (of a given category and library) with several overdues, launch the script :

staticfines.pl --category CAT,AMOUNT --library LIB --delay DELAY

Then, check that the user has been charged of AMOUNT if the due date of the most late item plus the delay is *before* today.

One day later, re-execute the script with the same parameters and check that the fine has not been charged twice.

Eventually, try to execute the script with various combinations of categories, amounts, libraries and delays, and check that it applies to the right patrons.

You may edit the $debug and $bigdebug variables in the script to make it more verbose, and check what it does during its execution.

Do you need more informations ?
Comment 13 Matthias Meusburger 2012-03-20 13:47:30 UTC
MathildeF, the script to execute is staticfines.pl (fines.pl is the "standard" way of applying fines)
Comment 14 Paul Poulain 2012-03-22 04:47:05 UTC
QA comment:
* good test plan in comment 12, thanks

* not PERLCRITIC compliant:
"require" statement with library name as string at line 38, column 12.  Use a bareword instead.  (Severity: 5)
Bareword file handle opened at line 143, column 1.  See pages 202,204 of PBP.  (Severity: 5)
Two-argument "open" used at line 143, column 1.  See page 207 of PBP.  (Severity: 5)

(I think the 1st one can't be avoided, but the 2 others should)

* this script is completely new, and:
  - will have to be documented/advertised
  - can't break anything if not used, so will pass QA once the perlcritic stuff is OK

* Mathilde, I feel you haven't successfully tested that it work. Am I wrong ? If i'm not, it mean the patch is not signed off for now
Comment 15 MathildeF 2012-03-22 08:37:39 UTC
(In reply to comment #14)
 
> * Mathilde, I feel you haven't successfully tested that it work. Am I wrong
> ? If i'm not, it mean the patch is not signed off for now

I make the test before the test plan. (In reply to comment #14)

I took the test before the test-plan.
the result was good,
but with the sandbox, I have not been able to test again the next day.
Comment 16 delaye 2012-03-22 10:51:41 UTC
i test and it's good th escrit add record in accountlines

Note : the scrip don't work if the date in caldenar is closed.
Comment 17 Matthias Meusburger 2012-03-22 13:42:49 UTC Comment hidden (obsolete)
Comment 18 Matthias Meusburger 2012-03-22 13:47:44 UTC
This new patch :

  - updates documentation (about fines not applied on holidays)
  - corrects typos in documentation (option --use-borrower-library)
  - adds a message (when debug mode is enabled) to tell that the fine will not be applied if we're currently on a holiday
  - fixes two deprecated "use hashref as a reference"
  - fixes the two fixable errors raised by perlcritic


This patch does not modify the script's behaviour, but might be re-tested though.
Comment 19 Paul Poulain 2012-03-22 14:45:59 UTC Comment hidden (obsolete)
Comment 20 delaye 2012-03-22 14:50:00 UTC Comment hidden (obsolete)
Comment 21 Paul Poulain 2012-03-28 16:10:07 UTC
QA comment:
* new and isolated script, that can't harm anything outside from the script itself
* passed perlcritic
* a few tabs, i've replaced by spaces
* if run with --help, I get the documentation, it's OK
* if run without any parameters, the script seems to do some work, start writing a report file (in /tmp), that, at the end is empty. An improvement would be to show doc & quit when the script is run without any parameters, but, if I understand well, it's not supposed to be, so won't fail QA for that

overall = passed QA, and if you want to provide a follow-up to die when no parameters are given, it will be welcomed !
Comment 22 Paul Poulain 2012-04-04 09:08:45 UTC
Warning = we have found some (complex) cases where this script make wrong calculations. A follow-up will be sent in a few days
Comment 23 Matthias Meusburger 2012-05-25 09:08:52 UTC
Created attachment 9772 [details] [review]
Bug 6858: Adds staticfines.pl for static fines processing

Add a tool to calculate static fine. For example, 7 days left = 1€ fixed fine

Signed-off-by: Delaye Stephane <stephane.delaye@biblibre.com>
Comment 24 Matthias Meusburger 2012-05-25 09:15:30 UTC
This new patch :

  - Fixes calculation bugs
  - Removes -o option for logs (which wasn't working anyway)
  - Adds a --borrower <borrowernumber> option for applying the fine only to one given borrower
Comment 25 Paul Poulain 2012-05-28 14:34:46 UTC
QA comments:
 * requires +use Modern::Perl;, good point
 * passes perlcricit
 * independant script that can't harm the rest of Koha if it's bugguy
 * clean some code

passes QA

SIDE COMMENT: Matthias, I see that you compare dues dates. Have you seen Jonathan comment on koha-devel about the due_date now being a date AND time column in the database (to handle hourly loans) ? Have you tested there's no possible side effect ? (I don't think so, but sharing just to be sure you haven't missed a test case)
Comment 26 Paul Poulain 2012-05-28 14:35:34 UTC
This patch has been pushed on 3.10, and should be backported on 3.8 because it fixes a problem on the script available in 3.8
Comment 27 Chris Cormack 2012-05-29 04:25:47 UTC
Paul is right, this script exists in 3.8.0, (and 3.8.1) so will push this bugfix to it for 3.8.2
Comment 28 Chris Cormack 2012-05-29 04:30:17 UTC
Side note, I have updated the commit message to be a little more informative, Matthias you wrote really good comments on the bug, if you put that in the commit message that would have been even more awesome :)

http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=484f4a999827d9fad8320b876921052310be299e
Comment 29 Viktor Sarge 2012-06-18 12:17:23 UTC
This one sounds just like what I will need for migrating a library I have coming up. But is it possible already to separate so that overdue loans get charged right away if there are holds on them - while loans with no holds get charged after say a week? 

And if that can be done - can I also set separate rules for rental media? 

I know this question is a bit OT with all the testing discussions going on, but I´d love to know how much can be done with this patch and what we´d have to expand the functionality.
Comment 30 Matthias Meusburger 2012-08-17 09:35:34 UTC
Hi Viktor,

Perhaps you could create another ticket for the enhancements you'd like to have, so we can close this ticket, as the patch has already been pushed ?