Bug 19336 - Add option to fines.pl to apply maximal fine amount after delay
Summary: Add option to fines.pl to apply maximal fine amount after delay
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Charles Farmer
QA Contact: Alex Arnaud
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-18 17:31 UTC by David Bourgault
Modified: 2023-07-14 20:15 UTC (History)
9 users (show)

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


Attachments
Bug 19336 - Add --maxfinesday option to fines.pl (2.27 KB, patch)
2017-09-18 17:33 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 19336 - Correct DateTime and Koha calls (1.90 KB, patch)
2017-10-20 13:22 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 19336 - Add --maxfinesday option to fines.pl (2.75 KB, patch)
2017-10-20 23:10 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 19336 - Correct DateTime and Koha calls (1.98 KB, patch)
2017-10-20 23:11 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 19336: Add --maxfinesday option to fines.pl (2.74 KB, patch)
2018-05-16 17:20 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19336: (follow-up) Correct DateTime and Koha calls (1.99 KB, patch)
2018-05-16 17:20 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19336: (QA follow-up) Initialize $unitcounttotal in case a report is written (1011 bytes, patch)
2018-05-16 17:20 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19336: Add --maxfinesday option to fines.pl (2.82 KB, patch)
2019-03-01 21:04 UTC, Mikaël Olangcay Brisebois
Details | Diff | Splinter Review
Bug 19336: (follow-up) Correct DateTime and Koha calls (2.00 KB, patch)
2019-03-01 21:04 UTC, Mikaël Olangcay Brisebois
Details | Diff | Splinter Review
Bug 19336: (QA follow-up) Initialize $unitcounttotal in case a report is written (1018 bytes, patch)
2019-03-01 21:04 UTC, Mikaël Olangcay Brisebois
Details | Diff | Splinter Review
Bug 19336: Add --maxfinesday option to fines.pl (2.82 KB, patch)
2019-03-01 21:08 UTC, Mikaël Olangcay Brisebois
Details | Diff | Splinter Review
Bug 19336: (follow-up) Correct DateTime and Koha calls (2.00 KB, patch)
2019-03-01 21:08 UTC, Mikaël Olangcay Brisebois
Details | Diff | Splinter Review
Bug 19336: (QA follow-up) Initialize $unitcounttotal in case a report is written (1018 bytes, patch)
2019-03-01 21:08 UTC, Mikaël Olangcay Brisebois
Details | Diff | Splinter Review
Bug 19336: Add --maxfinesday option to fines.pl (2.82 KB, patch)
2019-03-01 21:08 UTC, Mikaël Olangcay Brisebois
Details | Diff | Splinter Review
Bug 19336: (follow-up) Correct DateTime and Koha calls (2.00 KB, patch)
2019-03-01 21:08 UTC, Mikaël Olangcay Brisebois
Details | Diff | Splinter Review
Bug 19336: (QA follow-up) Initialize $unitcounttotal in case a report is written (1018 bytes, patch)
2019-03-01 21:09 UTC, Mikaël Olangcay Brisebois
Details | Diff | Splinter Review
Bug 19336: (follow-up) Initialize $unitcount instead of $unitcounttotal after changes on master (1.10 KB, patch)
2019-03-01 21:09 UTC, Mikaël Olangcay Brisebois
Details | Diff | Splinter Review
Bug 19336: Add --maxfinesday option to fines.pl (2.80 KB, patch)
2021-08-27 21:00 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19336: (follow-up) Correct DateTime and Koha calls (2.07 KB, patch)
2021-08-27 21:00 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19336: (QA follow-up) Initialize $unitcounttotal in case a report is written (1018 bytes, patch)
2021-08-27 21:00 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19336: (follow-up) Initialize $unitcount instead of $unitcounttotal after changes on master (1.10 KB, patch)
2021-08-27 21:00 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19336: (QA follow-up) Using CirculationRules instead of IssuingRules (2.18 KB, patch)
2021-08-27 21:00 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19336: Add --maxfinesday option to fines.pl (2.86 KB, patch)
2023-07-14 20:15 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 19336: (follow-up) Correct DateTime and Koha calls (2.07 KB, patch)
2023-07-14 20:15 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 19336: (QA follow-up) Initialize $unitcounttotal in case a report is written (1018 bytes, patch)
2023-07-14 20:15 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 19336: (follow-up) Initialize $unitcount instead of $unitcounttotal after changes on master (1.10 KB, patch)
2023-07-14 20:15 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 19336: (QA follow-up) Using CirculationRules instead of IssuingRules (2.18 KB, patch)
2023-07-14 20:15 UTC, Hammat wele
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Bourgault 2017-09-18 17:31:04 UTC

    
Comment 1 David Bourgault 2017-09-18 17:33:18 UTC
Created attachment 67191 [details] [review]
Bug 19336 - Add --maxfinesday option to fines.pl

This is a functionality we have for some of our clients we want to push
to the community.

Adds the --maxfinesday argument to fines.pl which creates or updates a
fine with the maximum amount set in the circulation rules after the
delay has passed.

e.g.:
./fines.pl --maxfinesdays 30
All item over 30 days late will have the maximal fine applied.
Comment 2 Alex Buckley 2017-10-03 21:25:58 UTC
Hi David

After checking out an item out with a due date set to be over 30 days ago and then running this cronjob I have found that the following error is thrown in the cronjob output:

kohadev-koha@kohadevbox:/home/vagrant/kohaclone/misc/cronjobs$ ./fines.pl --maxfinesdays 30
DateTime::Duration does not overload comparison.  See the documentation on the compare() method for details. at ./fines.pl line 129.

And no maximum fine is added to the accountlines table
Comment 3 David Bourgault 2017-10-20 13:22:28 UTC
Created attachment 68318 [details] [review]
Bug 19336 - Correct DateTime and Koha calls

This fixes the 'DateTime does not overload' error.
It also fixes the next error, which was that C4::GetIssuingRules is deprecated.
Finally it now handles no overduefinescap being define (does nothing).
Comment 4 Alex Buckley 2017-10-20 23:10:19 UTC
Created attachment 68335 [details] [review]
Bug 19336 - Add --maxfinesday option to fines.pl

This is a functionality we have for some of our clients we want to push
to the community.

Adds the --maxfinesday argument to fines.pl which creates or updates a
fine with the maximum amount set in the circulation rules after the
delay has passed.

e.g.:
./fines.pl --maxfinesdays 30
All item over 30 days late will have the maximal fine applied.

Here is a test plan I followed based on above.
Test plan:
1. Create circulation rule with Overdue fines cap set

2. Set finesMode systempreference to 'Calculate and charge'

3. Check out an item with the due date set 30 days ago

4. In Koha shell run ./fines.pl --maxfinesdays 30

5. There should now be a new fine in the accountlines table of the
amount set in overdue fines cap

Followed test plan, patch worked as intended

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 5 Alex Buckley 2017-10-20 23:11:23 UTC
Created attachment 68336 [details] [review]
Bug 19336 - Correct DateTime and Koha calls

This fixes the 'DateTime does not overload' error.
It also fixes the next error, which was that C4::GetIssuingRules is deprecated.
Finally it now handles no overduefinescap being define (does nothing).

Passes QA test tool

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 6 Alex Arnaud 2018-02-07 15:37:45 UTC
Hello David,

What about $type and $unitcounttotal variables? If maxfinesdays is defined and amount != 0, they are empty.

Indeed, $type is always empty since chargename is never populated. But this could be fixed in another BZ...
Comment 7 Katrin Fischer 2018-02-17 09:20:30 UTC
Moving to FQA to trigger reaction from developter to question on comment#6
Comment 8 Charles Farmer 2018-05-07 14:14:03 UTC
Despite the 'uninitialized value in join...' , I feel like $unitcounttotal shouldn't take any _numerical_ values.

The concept of chargeunits stops making sense once you arbritraly charge the overduefinescap after X given days, which is exactly what maxfinesdays does. It's not bothered by closing hours, nor does it care about grace periods, it bluntly maxes out the fine. This seems like a reason not to consider any information derived from $issuing_rule->lengthunit.

Now, as I hinted at the beginning, I'm definitely open to initializing $unitcounttotal. Since its only use is to be concanated with other fields before being outputed to a file, it could make sense to inform the fines report's reader that maxfinesdays was applied.

Is this approach sound in your opinion?
Comment 9 Charles Farmer 2018-05-16 17:20:38 UTC
Created attachment 75379 [details] [review]
Bug 19336: Add --maxfinesday option to fines.pl

This is a functionality we have for some of our clients we want to push
to the community.

Adds the --maxfinesday argument to fines.pl which creates or updates a
fine with the maximum amount set in the circulation rules after the
delay has passed.

e.g.:
./fines.pl --maxfinesdays 30
All item over 30 days late will have the maximal fine applied.

Here is a test plan I followed based on above.
Test plan:
1. Create circulation rule with Overdue fines cap set

2. Set finesMode systempreference to 'Calculate and charge'

3. Check out an item with the due date set 30 days ago

4. In Koha shell run ./fines.pl --maxfinesdays 30

5. There should now be a new fine in the accountlines table of the
amount set in overdue fines cap

Followed test plan, patch worked as intended

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 10 Charles Farmer 2018-05-16 17:20:45 UTC
Created attachment 75380 [details] [review]
Bug 19336: (follow-up) Correct DateTime and Koha calls

This fixes the 'DateTime does not overload' error.
It also fixes the next error, which was that C4::GetIssuingRules is deprecated.
Finally it now handles no overduefinescap being define (does nothing).

Passes QA test tool

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 11 Charles Farmer 2018-05-16 17:20:50 UTC
Created attachment 75381 [details] [review]
Bug 19336: (QA follow-up) Initialize $unitcounttotal in case a report is written
Comment 12 Charles Farmer 2018-05-16 17:25:06 UTC
Rewrote the patch titles to abide by the standardized notation.

Provided a way out of the 'In discussion' status by initializing $unitcounttotal to 'n/a', the common abbreviation used in a report for a field that does not apply.

Back in Needs Signoff.
Comment 13 Mikaël Olangcay Brisebois 2019-03-01 21:04:38 UTC Comment hidden (obsolete)
Comment 14 Mikaël Olangcay Brisebois 2019-03-01 21:04:44 UTC Comment hidden (obsolete)
Comment 15 Mikaël Olangcay Brisebois 2019-03-01 21:04:52 UTC Comment hidden (obsolete)
Comment 16 Mikaël Olangcay Brisebois 2019-03-01 21:08:12 UTC Comment hidden (obsolete)
Comment 17 Mikaël Olangcay Brisebois 2019-03-01 21:08:16 UTC Comment hidden (obsolete)
Comment 18 Mikaël Olangcay Brisebois 2019-03-01 21:08:21 UTC Comment hidden (obsolete)
Comment 19 Mikaël Olangcay Brisebois 2019-03-01 21:08:53 UTC
Created attachment 85920 [details] [review]
Bug 19336: Add --maxfinesday option to fines.pl

This is a functionality we have for some of our clients we want to push
to the community.

Adds the --maxfinesday argument to fines.pl which creates or updates a
fine with the maximum amount set in the circulation rules after the
delay has passed.

e.g.:
./fines.pl --maxfinesdays 30
All item over 30 days late will have the maximal fine applied.

Here is a test plan I followed based on above.
Test plan:
1. Create circulation rule with Overdue fines cap set

2. Set finesMode systempreference to 'Calculate and charge'

3. Check out an item with the due date set 30 days ago

4. In Koha shell run ./fines.pl --maxfinesdays 30

5. There should now be a new fine in the accountlines table of the
amount set in overdue fines cap

Followed test plan, patch worked as intended

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 20 Mikaël Olangcay Brisebois 2019-03-01 21:08:57 UTC
Created attachment 85921 [details] [review]
Bug 19336: (follow-up) Correct DateTime and Koha calls

This fixes the 'DateTime does not overload' error.
It also fixes the next error, which was that C4::GetIssuingRules is deprecated.
Finally it now handles no overduefinescap being define (does nothing).

Passes QA test tool

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 21 Mikaël Olangcay Brisebois 2019-03-01 21:09:02 UTC
Created attachment 85922 [details] [review]
Bug 19336: (QA follow-up) Initialize $unitcounttotal in case a report is written
Comment 22 Mikaël Olangcay Brisebois 2019-03-01 21:09:06 UTC
Created attachment 85923 [details] [review]
Bug 19336: (follow-up) Initialize $unitcount  instead of $unitcounttotal after changes on master
Comment 23 Mazen Khallaf 2021-01-19 22:37:59 UTC
Couldn't apply the patch

$ git bz apply 19336
y
Bug 19336 - Add option to fines.pl to apply maximal fine amount after delay

85920 - Bug 19336: Add --maxfinesday option to fines.pl
85921 - Bug 19336: (follow-up) Correct DateTime and Koha calls
85922 - Bug 19336: (QA follow-up) Initialize $unitcounttotal in case a report is written
85923 - Bug 19336: (follow-up) Initialize $unitcount  instead of $unitcounttotal after changes on master

Apply? [(y)es, (n)o, (i)nteractive] Applying: Bug 19336: Add --maxfinesday option to fines.pl
Using index info to reconstruct a base tree...
M	misc/cronjobs/fines.pl
Falling back to patching base and 3-way merge...
Auto-merging misc/cronjobs/fines.pl
CONFLICT (content): Merge conflict in misc/cronjobs/fines.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 19336: Add --maxfinesday option to fines.pl
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-19336-Add---maxfinesday-option-to-finespl-d37mQ0.patch
Comment 24 Alexis Ripetti 2021-08-27 21:00:12 UTC
Created attachment 124197 [details] [review]
Bug 19336: Add --maxfinesday option to fines.pl

This is a functionality we have for some of our clients we want to push
to the community.

Adds the --maxfinesday argument to fines.pl which creates or updates a
fine with the maximum amount set in the circulation rules after the
delay has passed.

e.g.:
./fines.pl --maxfinesdays 30
All item over 30 days late will have the maximal fine applied.

Here is a test plan I followed based on above.
Test plan:
1. Create circulation rule with Overdue fines cap set

2. Set finesMode systempreference to 'Calculate and charge'

3. Check out an item with the due date set 30 days ago

4. In Koha shell run ./fines.pl --maxfinesdays 30

5. There should now be a new fine in the accountlines table of the
amount set in overdue fines cap

Followed test plan, patch worked as intended
Comment 25 Alexis Ripetti 2021-08-27 21:00:16 UTC
Created attachment 124198 [details] [review]
Bug 19336: (follow-up) Correct DateTime and Koha calls

This fixes the 'DateTime does not overload' error.
It also fixes the next error, which was that C4::GetIssuingRules is deprecated.
Finally it now handles no overduefinescap being define (does nothing).

Passes QA test tool

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 26 Alexis Ripetti 2021-08-27 21:00:20 UTC
Created attachment 124199 [details] [review]
Bug 19336: (QA follow-up) Initialize $unitcounttotal in case a report is written
Comment 27 Alexis Ripetti 2021-08-27 21:00:25 UTC
Created attachment 124200 [details] [review]
Bug 19336: (follow-up) Initialize $unitcount instead of $unitcounttotal after changes on master
Comment 28 Alexis Ripetti 2021-08-27 21:00:29 UTC
Created attachment 124201 [details] [review]
Bug 19336: (QA follow-up) Using CirculationRules instead of IssuingRules
Comment 29 Hammat wele 2023-07-14 20:15:40 UTC
Created attachment 153504 [details] [review]
Bug 19336: Add --maxfinesday option to fines.pl

This is a functionality we have for some of our clients we want to push
to the community.

Adds the --maxfinesday argument to fines.pl which creates or updates a
fine with the maximum amount set in the circulation rules after the
delay has passed.

e.g.:
./fines.pl --maxfinesdays 30
All item over 30 days late will have the maximal fine applied.

Here is a test plan I followed based on above.
Test plan:
1. Create circulation rule with Overdue fines cap set

2. Set finesMode systempreference to 'Calculate and charge'

3. Check out an item with the due date set 30 days ago

4. In Koha shell run ./fines.pl --maxfinesdays 30

5. There should now be a new fine in the accountlines table of the
amount set in overdue fines cap

Followed test plan, patch worked as intended

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 30 Hammat wele 2023-07-14 20:15:44 UTC
Created attachment 153505 [details] [review]
Bug 19336: (follow-up) Correct DateTime and Koha calls

This fixes the 'DateTime does not overload' error.
It also fixes the next error, which was that C4::GetIssuingRules is deprecated.
Finally it now handles no overduefinescap being define (does nothing).

Passes QA test tool

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 31 Hammat wele 2023-07-14 20:15:48 UTC
Created attachment 153506 [details] [review]
Bug 19336: (QA follow-up) Initialize $unitcounttotal in case a report is written
Comment 32 Hammat wele 2023-07-14 20:15:52 UTC
Created attachment 153507 [details] [review]
Bug 19336: (follow-up) Initialize $unitcount instead of $unitcounttotal after changes on master
Comment 33 Hammat wele 2023-07-14 20:15:56 UTC
Created attachment 153508 [details] [review]
Bug 19336: (QA follow-up) Using CirculationRules instead of IssuingRules