Bug 23051 - Optionally renew fine accruing items when all fines on item are paid off
Summary: Optionally renew fine accruing items when all fines on item are paid off
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Andrew Isherwood
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-06-05 13:13 UTC by Andrew Isherwood
Modified: 2019-09-10 11:13 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 23051: Add RenewAccruingItemWhenPaid syspref (3.54 KB, patch)
2019-06-13 07:44 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23051: Renew items when fines paid off (9.33 KB, patch)
2019-06-13 07:44 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23051: Add unit tests (7.97 KB, patch)
2019-06-13 07:44 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23051: Add RenewAccruingItemWhenPaid syspref (3.54 KB, patch)
2019-06-28 08:10 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23051: Renew items when fines paid off (9.33 KB, patch)
2019-06-28 08:10 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23051: Add unit tests (8.04 KB, patch)
2019-06-28 08:10 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23051: Renew items when fines paid off (9.20 KB, patch)
2019-07-26 07:31 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23051: Add RenewAccruingItemWhenPaid syspref (3.54 KB, patch)
2019-07-26 07:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23051: Add unit tests (8.04 KB, patch)
2019-07-26 07:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23051: Renew items when fines paid off (9.20 KB, patch)
2019-07-26 07:32 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23051: Add RenewAccruingItemWhenPaid syspref (3.54 KB, patch)
2019-09-10 11:07 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23051: Add unit tests (7.96 KB, patch)
2019-09-10 11:07 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23051: Renew items when fines paid off (9.20 KB, patch)
2019-09-10 11:08 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 23051: Add RenewAccruingItemWhenPaid syspref (3.63 KB, patch)
2019-09-10 11:13 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 23051: Add unit tests (8.06 KB, patch)
2019-09-10 11:13 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 23051: Renew items when fines paid off (9.29 KB, patch)
2019-09-10 11:13 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Isherwood 2019-06-05 13:13:41 UTC
Currently, if a user pays an accruing fine on an overdue item but forgets to renew the item fines start accruing again. 

If a user pays off the amount outstanding on an item with an accruing fine (accounttype "OVERDUE" & status "UNRETURNED" in 19.05) via the OPAC or self-service, or a member of staff takes payment in the staff client, attempt to automatically renew the item according to circulation rules.

This behaviour should be controlled by a new syspref "RenewAccruingItemWhenPaid" which is initially turned off.
Comment 1 Andrew Isherwood 2019-06-13 07:44:29 UTC
Created attachment 90559 [details] [review]
Bug 23051: Add RenewAccruingItemWhenPaid syspref

This patch adds the new syspref "RenewAccruingItemWhenPaid"
Comment 2 Andrew Isherwood 2019-06-13 07:44:32 UTC
Created attachment 90560 [details] [review]
Bug 23051: Renew items when fines paid off

When the RenewAccruingItemWhenPaid syspref is enabled and all the fines
on an item that is accruing fines are paid, we automatically renew that
item to prevent it from starting to accrue fines again.

This patch adds an additional argument to C4::Circulation::AddRenewal
which allows us to skip the calculation of fines upon renewal, which we
don't want to do if the fines on that item have just been paid. Existing
calls to AddRenewal have not been amended because there seems to be a
convention of only passing undef when adding arguments that require
their positioning to be maintained. Since the new argument is the last
one, this is not the case with any existing call.
Comment 3 Andrew Isherwood 2019-06-13 07:44:34 UTC
Created attachment 90561 [details] [review]
Bug 23051: Add unit tests

This patch adds unit tests for all modules affected by this bug
Comment 4 Andrew Isherwood 2019-06-13 07:55:43 UTC
Test plan:

- In Administration, search for the new syspref "RenewAccruingItemWhenPaid"
- TEST: Observe that the new syspref exists and is turned off by default
- Turn on the syspref
- Loan an item to a patron, ensure you choose a patron, branch and item combination that will incur overdue fines
- Backdate the due date to the past so the loan will accrue fines
- Run the fines.pl cronjob (misc/cronjobs/fines.pl)
- Go to the patron's "Accounting" page and choose "Pay" against the item that has the fine
- Enter an amount to pay that will fully pay off the item and click "Confirm"
- Go to the patron's checkouts list
- TEST: Observe that the item has been renewed as part of the fine payment process
Comment 5 Andrew Isherwood 2019-06-28 08:10:44 UTC
Created attachment 91058 [details] [review]
Bug 23051: Add RenewAccruingItemWhenPaid syspref

This patch adds the new syspref "RenewAccruingItemWhenPaid"
Comment 6 Andrew Isherwood 2019-06-28 08:10:47 UTC
Created attachment 91059 [details] [review]
Bug 23051: Renew items when fines paid off

When the RenewAccruingItemWhenPaid syspref is enabled and all the fines
on an item that is accruing fines are paid, we automatically renew that
item to prevent it from starting to accrue fines again.

This patch adds an additional argument to C4::Circulation::AddRenewal
which allows us to skip the calculation of fines upon renewal, which we
don't want to do if the fines on that item have just been paid. Existing
calls to AddRenewal have not been amended because there seems to be a
convention of only passing undef when adding arguments that require
their positioning to be maintained. Since the new argument is the last
one, this is not the case with any existing call.
Comment 7 Andrew Isherwood 2019-06-28 08:10:50 UTC
Created attachment 91060 [details] [review]
Bug 23051: Add unit tests

This patch adds unit tests for all modules affected by this bug
Comment 8 Alex Buckley 2019-07-26 00:53:38 UTC
Hi there,

I am sorry this patch does not apply cleanly. 

vagrant@kohadevbox:kohaclone((b558895...))$ git bz apply 23051
Bug 23051 - Optionally renew fine accruing items when all fines on item are paid off

91058 - Bug 23051: Add RenewAccruingItemWhenPaid syspref
91059 - Bug 23051: Renew items when fines paid off
91060 - Bug 23051: Add unit tests

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 23051: Add RenewAccruingItemWhenPaid syspref
Applying: Bug 23051: Renew items when fines paid off
Using index info to reconstruct a base tree...
M	C4/Circulation.pm
M	Koha/Account.pm
M	Koha/Account/Line.pm
Falling back to patching base and 3-way merge...
Auto-merging Koha/Account/Line.pm
Auto-merging Koha/Account.pm
CONFLICT (content): Merge conflict in Koha/Account.pm
Auto-merging C4/Circulation.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 23051: Renew items when fines paid off
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.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-23051-Renew-items-when-fines-paid-off-YTUZjH.patch

Can you please rebase it and re-attach it to this bug report, thanks for your contribution :)

Cheers,
Alex
Comment 9 Andrew Isherwood 2019-07-26 07:31:32 UTC
Created attachment 91792 [details] [review]
Bug 23051: Renew items when fines paid off

When the RenewAccruingItemWhenPaid syspref is enabled and all the fines
on an item that is accruing fines are paid, we automatically renew that
item to prevent it from starting to accrue fines again.

This patch adds an additional argument to C4::Circulation::AddRenewal
which allows us to skip the calculation of fines upon renewal, which we
don't want to do if the fines on that item have just been paid. Existing
calls to AddRenewal have not been amended because there seems to be a
convention of only passing undef when adding arguments that require
their positioning to be maintained. Since the new argument is the last
one, this is not the case with any existing call.
Comment 10 Andrew Isherwood 2019-07-26 07:32:09 UTC
Created attachment 91793 [details] [review]
Bug 23051: Add RenewAccruingItemWhenPaid syspref

This patch adds the new syspref "RenewAccruingItemWhenPaid"
Comment 11 Andrew Isherwood 2019-07-26 07:32:12 UTC
Created attachment 91794 [details] [review]
Bug 23051: Add unit tests

This patch adds unit tests for all modules affected by this bug
Comment 12 Andrew Isherwood 2019-07-26 07:32:15 UTC
Created attachment 91795 [details] [review]
Bug 23051: Renew items when fines paid off

When the RenewAccruingItemWhenPaid syspref is enabled and all the fines
on an item that is accruing fines are paid, we automatically renew that
item to prevent it from starting to accrue fines again.

This patch adds an additional argument to C4::Circulation::AddRenewal
which allows us to skip the calculation of fines upon renewal, which we
don't want to do if the fines on that item have just been paid. Existing
calls to AddRenewal have not been amended because there seems to be a
convention of only passing undef when adding arguments that require
their positioning to be maintained. Since the new argument is the last
one, this is not the case with any existing call.
Comment 13 Andrew Isherwood 2019-07-26 07:33:14 UTC
Thanks for the heads up Alex. This should be fine now :)
Comment 14 Andrew Isherwood 2019-09-10 11:07:54 UTC
Created attachment 92684 [details] [review]
Bug 23051: Add RenewAccruingItemWhenPaid syspref

This patch adds the new syspref "RenewAccruingItemWhenPaid"
Comment 15 Andrew Isherwood 2019-09-10 11:07:58 UTC
Created attachment 92685 [details] [review]
Bug 23051: Add unit tests

This patch adds unit tests for all modules affected by this bug
Comment 16 Andrew Isherwood 2019-09-10 11:08:03 UTC
Created attachment 92686 [details] [review]
Bug 23051: Renew items when fines paid off

When the RenewAccruingItemWhenPaid syspref is enabled and all the fines
on an item that is accruing fines are paid, we automatically renew that
item to prevent it from starting to accrue fines again.

This patch adds an additional argument to C4::Circulation::AddRenewal
which allows us to skip the calculation of fines upon renewal, which we
don't want to do if the fines on that item have just been paid. Existing
calls to AddRenewal have not been amended because there seems to be a
convention of only passing undef when adding arguments that require
their positioning to be maintained. Since the new argument is the last
one, this is not the case with any existing call.
Comment 17 PTFS Europe Sandboxes 2019-09-10 11:13:30 UTC
Created attachment 92687 [details] [review]
Bug 23051: Add RenewAccruingItemWhenPaid syspref

This patch adds the new syspref "RenewAccruingItemWhenPaid"

Signed-off-by: Lucy Harrison <L.M.Harrison@lboro.ac.uk>
Sponsored-by: Loughborough University
Comment 18 PTFS Europe Sandboxes 2019-09-10 11:13:34 UTC
Created attachment 92688 [details] [review]
Bug 23051: Add unit tests

This patch adds unit tests for all modules affected by this bug

Signed-off-by: Lucy Harrison <L.M.Harrison@lboro.ac.uk>
Sponsored-by: Loughborough University
Comment 19 PTFS Europe Sandboxes 2019-09-10 11:13:39 UTC
Created attachment 92689 [details] [review]
Bug 23051: Renew items when fines paid off

When the RenewAccruingItemWhenPaid syspref is enabled and all the fines
on an item that is accruing fines are paid, we automatically renew that
item to prevent it from starting to accrue fines again.

This patch adds an additional argument to C4::Circulation::AddRenewal
which allows us to skip the calculation of fines upon renewal, which we
don't want to do if the fines on that item have just been paid. Existing
calls to AddRenewal have not been amended because there seems to be a
convention of only passing undef when adding arguments that require
their positioning to be maintained. Since the new argument is the last
one, this is not the case with any existing call.

Signed-off-by: Lucy Harrison <L.M.Harrison@lboro.ac.uk>
Sponsored-by: Loughborough University