[Bug 23051] New: Optionally renew fine accruing items when all fines on item are paid off
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Bug ID: 23051 Summary: Optionally renew fine accruing items when all fines on item are paid off Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: andrew.isherwood@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com Target Milestone: --- 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. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |andrew.isherwood@ptfs-europ |ity.org |e.com -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #1 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 90559 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90559&action=edit Bug 23051: Add RenewAccruingItemWhenPaid syspref This patch adds the new syspref "RenewAccruingItemWhenPaid" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #2 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 90560 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90560&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #3 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 90561 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90561&action=edit Bug 23051: Add unit tests This patch adds unit tests for all modules affected by this bug -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff --- Comment #4 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90559|0 |1 is obsolete| | --- Comment #5 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 91058 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91058&action=edit Bug 23051: Add RenewAccruingItemWhenPaid syspref This patch adds the new syspref "RenewAccruingItemWhenPaid" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90560|0 |1 is obsolete| | --- Comment #6 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 91059 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91059&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90561|0 |1 is obsolete| | --- Comment #7 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 91060 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91060&action=edit Bug 23051: Add unit tests This patch adds unit tests for all modules affected by this bug -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Janet McGowan <janet.mcgowan@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |janet.mcgowan@ptfs-europe.c | |om -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Alex Buckley <alexbuckley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |alexbuckley@catalyst.net.nz --- Comment #8 from Alex Buckley <alexbuckley@catalyst.net.nz> --- 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91059|0 |1 is obsolete| | --- Comment #9 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 91792 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91792&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91058|0 |1 is obsolete| | --- Comment #10 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 91793 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91793&action=edit Bug 23051: Add RenewAccruingItemWhenPaid syspref This patch adds the new syspref "RenewAccruingItemWhenPaid" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91060|0 |1 is obsolete| | --- Comment #11 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 91794 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91794&action=edit Bug 23051: Add unit tests This patch adds unit tests for all modules affected by this bug -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91792|0 |1 is obsolete| | --- Comment #12 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 91795 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91795&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #13 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Thanks for the heads up Alex. This should be fine now :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Holly <hc@interleaf.ie> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hc@interleaf.ie -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91793|0 |1 is obsolete| | --- Comment #14 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 92684 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92684&action=edit Bug 23051: Add RenewAccruingItemWhenPaid syspref This patch adds the new syspref "RenewAccruingItemWhenPaid" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91794|0 |1 is obsolete| | --- Comment #15 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 92685 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92685&action=edit Bug 23051: Add unit tests This patch adds unit tests for all modules affected by this bug -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91795|0 |1 is obsolete| | --- Comment #16 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 92686 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92686&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 PTFS Europe Sandboxes <sandboxes@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92684|0 |1 is obsolete| | --- Comment #17 from PTFS Europe Sandboxes <sandboxes@ptfs-europe.com> --- Created attachment 92687 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92687&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 PTFS Europe Sandboxes <sandboxes@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92685|0 |1 is obsolete| | --- Comment #18 from PTFS Europe Sandboxes <sandboxes@ptfs-europe.com> --- Created attachment 92688 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92688&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 PTFS Europe Sandboxes <sandboxes@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92686|0 |1 is obsolete| | --- Comment #19 from PTFS Europe Sandboxes <sandboxes@ptfs-europe.com> --- Created attachment 92689 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92689&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Keywords| |rel_19_11_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Change sponsored?|--- |Sponsored -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |nick@bywatersolutions.com --- Comment #20 from Nick Clemens <nick@bywatersolutions.com> --- Hi Andrew, Tests need a minor rebase to pass. I have a few questions from reading the code: 1 - You duplicate the code in Account->pay and Line->apply, could that be a subroutine that can be shared and tested? It's minor but would be nice to avoid duplication 2 - I don't like the lack of feedback for the renewal/failure. At the very least maybe some feedback on the interface that the items may be renewed 3 - In the case where a renewal doesn't make the book current (i.e. the book is overdue longer than the renewal period) I think this would give strange results. Possibly you could consider this a configuration issue, but it should be warned somewhere at the least -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Sally <sally.healey@cheshiresharedservices.gov.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sally.healey@cheshireshared | |services.gov.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92687|0 |1 is obsolete| | --- Comment #21 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 93922 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93922&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92688|0 |1 is obsolete| | --- Comment #22 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 93923 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93923&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92689|0 |1 is obsolete| | --- Comment #23 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 93924 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93924&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #24 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 93925 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93925&action=edit Bug 23051: (follow-up) Refactor renewal code As per Nick's first point in comment #20, the code that tests for renewability and renews items has been refactored into it's own function. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #25 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Hi Nick Thanks for taking a look at this.
Tests need a minor rebase to pass. Fixed
You duplicate the code in Account->pay and Line->apply [..] Indeed. I've now refactored the duplicated code into a couple of helper functions in Koha::Account.
I don't like the lack of feedback for the renewal/failure [..] This is a very tricky issue, and one I side-stepped during dev. Firstly, any feedback would only be available in the UI. Since there is more than one way of paying off fines that don't involve the Koha UI (e.g. SIP), those methods would not receive the feedback. Also, since the renewal is happening as a side effect of paying off a fine, anything that was saying "pay off the fine on this item" will have to be expecting something entirely unrelated, i.e. "this item couldn't renew" back, which feels wrong. I do agree that no feedback at all is bad though. I'll have a think and see if I can come up with.
In the case where a renewal doesn't make the book current [..] Same as the previous one really.
I'll do some pondering... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andreas.hedstrom.mace@sub.s | |u.se --- Comment #26 from Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se> --- I don't understand Nick's third point. Any attempted renewal should base that renewal on today's date and calculate the new due date based on the circulation rule for the item from that? Not the original due date. (At least that is what happens if I renew in our version of Koha, 18.11.) So it could never happen that the item is not "current" after a renewal? Or am I reading that wrong? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #27 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Hi Andreas I believe this is determined by the "RenewalPeriodBase" syspref, if set to "the old due date of the checkout" then the scenario Nick describes could happpen. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #28 from Andreas Hedström Mace <andreas.hedstrom.mace@sub.su.se> --- Ah, thanks! I had forgotten about that one. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93922|0 |1 is obsolete| | --- Comment #29 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94020 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94020&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93923|0 |1 is obsolete| | --- Comment #30 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94021 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94021&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93924|0 |1 is obsolete| | --- Comment #31 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94022 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94022&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93925|0 |1 is obsolete| | --- Comment #32 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94023 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94023&action=edit Bug 23051: (follow-up) Refactor renewal code As per Nick's first point in comment #20, the code that tests for renewability and renews items has been refactored into it's own function. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #33 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94024 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94024&action=edit Bug 23051: (follow-up) Provide feedback For renewals that fail when a fine is being paid off, this patch causes any errors to be passed back to the template for display. Addresses the second point in Nick's comment #20 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #34 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Hi Nick Further to the discussion on your third point. Personally I think this is outside of the scope of this work. In the situation where the "RenewalPeriodBase" syspref is set to "the old due date of the checkout" and an outstanding fine on it is paid off, the item would be renewed but, as you say, it may still be overdue if the old due date is further in the past than the renewal period. However, this is also the case if a regular renewal is carried out in the Circulation module, no warning is given in this situation, so an item can be renewed but still be overdue. I agree that this is not an ideal situation, but I feel that if such a warning is to be added, it should be done as a separate bug which addresses the (probably numerous) places where such a warning would need to be displayed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |nick@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #35 from Nick Clemens <nick@bywatersolutions.com> --- t/db_dependent/Koha/Account.t - test count needs adjustment t/db_dependent/Koha/Account/Lines.t - tests are failing The feedback is improved, it would be nice to get feedback on what was successfully renewed as well. Ideally SIP would get feedback in a screen message, and patrons would get a notice of renewal on the opac. I am also not sure about the code path if a patron paid fines on the opac (via paypal etc.) but renewals are not allowed on the opac. In general I think I would prefer to see 'ok' and 'not_ok' returned as a single 'renewal_results' array, then it can be parsed once and success and failure returned. Our trainers would very much like there to be feedback for any renewal, otherwise it feels a bit magic and unpredictable There is no listing of errors if I use the 'pay' button on an individual fine, but there is when I use 'pay amount' or 'pay selected' I don't really like that the functions are internal functions and then exported - fine_paid_renewable could be an accountline method? I think maybe_renew could also be an accountline method? it could act on the item attached to fine, and maybe names like: renewable renew_items For the last point, I think the pref description should highlight that if 'RenewalPeriodBase' is set to due date, there may be doubled charges I do really like the idea here, but I think it is a big change and needs some refining -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #36 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94402 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94402&action=edit Bug 23051: (follow-up) Fix unit tests As raised by Nick in comment #35 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #37 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94434 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94434&action=edit Bug 23051: (follow-up) Fix/improve feedback This follow up patch addresses the following parts of Nick's feedback in comment #35: - it would be nice to get feedback on what was successfully renewed as well - In general I think I would prefer to see 'ok' and 'not_ok' returned as a single 'renewal_results' array - There is no listing of errors if I use the 'pay' button on an individual fine -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #38 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94453 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94453&action=edit Bug 23051: (follow-up) Refactor methods This follow up patch addresses the following parts of Nick's feedback in comment #35: - I don't really like that the functions are internal functions and then exported - I think the pref description should highlight that if 'RenewalPeriodBase' is set to due date, there may be doubled charges -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94453|0 |1 is obsolete| | --- Comment #39 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94525 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94525&action=edit Bug 23051: (follow-up) Refactor methods This follow up patch addresses the following parts of Nick's feedback in comment #35: - I don't really like that the functions are internal functions and then exported - I think the pref description should highlight that if 'RenewalPeriodBase' is set to due date, there may be doubled charges -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #40 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Hi Nick
I am also not sure about the code path if a patron paid fines on the opac (via paypal etc.) but renewals are not allowed on the opac.
Looking at the code, the OPAC PayPal payment would cause items to be renewed as part of the fine payment. The same applies if the site is using the WPM or Capita payment plugins, I'm not sure of any others though I suspect the same would also be true for any payment plugin, essentially anything that pays the fines using the Koha::Account::pay function. I'm unsure what to do here. There seems to be a convention of passing a "note" parameter when calling the "pay" function. With the OPAC PayPal payment, the note is passed as "PayPal", with Capita it's "Pay360 Payment" etc. So, we could have a blacklist (not sure which would be best), which prevented the renewal from happening if the payment was passed a note containing one of these values, though that seems a bit fragile. If we are going to prevent renewal for payments made via the OPAC, we should probably highlight that in the syspref. Any thoughts? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #41 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- As the maintainer of the Pay360 and WPM plugins, I'd be happy to remove the logic that attempts to renew items from within those plugins themselves and have that action taken from 'koha proper' as per sysprefs... Just my two cents. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #42 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Hi Martin - The renewal is carried out from within Koha::Account::pay if the syspref is turned on and the issue meets the required conditions, so even if the plugins are not explicitly calling the renewal, it will still happen anyway. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #43 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Andrew Isherwood from comment #42)
Hi Martin - The renewal is carried out from within Koha::Account::pay if the syspref is turned on and the issue meets the required conditions, so even if the plugins are not explicitly calling the renewal, it will still happen anyway.
The logic could be moved higher? Or a switch in Koha::Account::pay to prevent the renewal code? I also think it could be that this pref always renews, that just needs to be made clear to the end user - it isn't technically an 'opac renewal' if done by the accounts job. I would be more interested in making sure the patron knows if the renewal happened or not after the payment then preventing renewal in edge cases -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94020|0 |1 is obsolete| | --- Comment #44 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94604 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94604&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94021|0 |1 is obsolete| | --- Comment #45 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94605 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94605&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94022|0 |1 is obsolete| | --- Comment #46 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94606 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94606&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94023|0 |1 is obsolete| | --- Comment #47 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94607 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94607&action=edit Bug 23051: (follow-up) Refactor renewal code As per Nick's first point in comment #20, the code that tests for renewability and renews items has been refactored into it's own function. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94024|0 |1 is obsolete| | --- Comment #48 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94608 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94608&action=edit Bug 23051: (follow-up) Provide feedback For renewals that fail when a fine is being paid off, this patch causes any errors to be passed back to the template for display. Addresses the second point in Nick's comment #20 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94402|0 |1 is obsolete| | --- Comment #49 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94609 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94609&action=edit Bug 23051: (follow-up) Fix unit tests As raised by Nick in comment #35 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94434|0 |1 is obsolete| | --- Comment #50 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94610 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94610&action=edit Bug 23051: (follow-up) Fix/improve feedback This follow up patch addresses the following parts of Nick's feedback in comment #35: - it would be nice to get feedback on what was successfully renewed as well - In general I think I would prefer to see 'ok' and 'not_ok' returned as a single 'renewal_results' array - There is no listing of errors if I use the 'pay' button on an individual fine -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94525|0 |1 is obsolete| | --- Comment #51 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94611 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94611&action=edit Bug 23051: (follow-up) Refactor methods This follow up patch addresses the following parts of Nick's feedback in comment #35: - I don't really like that the functions are internal functions and then exported - I think the pref description should highlight that if 'RenewalPeriodBase' is set to due date, there may be doubled charges -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #52 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94629 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94629&action=edit Bug 23051: (follow-up) Add SIP summary This follow up patch addresses the following parts of Nick's feedback in comment #35: - Ideally SIP would get feedback in a screen message -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #53 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 94649 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94649&action=edit Bug 23051: (follow-up) Renewing in OPAC This follow up patch addresses the following parts of Nick's feedback in comment #35: - I am also not sure about the code path if a patron paid fines on the opac (via paypal etc.) but renewals are not allowed on the opac. We've introduced the syspref RenewAccruingItemInOpac (default is off) which, when enabled, will cause items attached to fines that are paid off in the OPAC (via payment plugins), to be automatically renewed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #54 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Revised 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 STAFF RENEWALS: - 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" - TEST: Observe that in the payment result pay, details of the item having been renewed are displayed - Go to the patron's checkouts list - TEST: Observe that the item has been renewed as part of the fine payment process - Add some restriction that would prevent the item being renewed, maybe modify the number of renewals (issues.renewals in the DB) it has had such that it exceeds the allowed limit - Backdate the due date of the item 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" - TEST: Observe that in the payment result pay, the fact that the item was not renewed is summarised, with the reason for the renewal failure SIP RENEWALS: - Modify the item's renewal count back so that they are allowed - Backdate the due date to the past so the loan will accrue fines - Run the fines.pl cronjob (misc/cronjobs/fines.pl) - Use sip_cli_emulator.pl to send a fee_paid message for the full amount oweing, for example: ./sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su <selfcheck_user> -sp <selfcheck_password> --patron <patron_username> --password <patron_password> -l <location_code> -m fee_paid --fee-amount <fee_amount> - TEST: Observe that the response message contains a summary message with details of renewal successes and failures OPAC RENEWALS: Note: Currently the WPM payment plugin and Capita payment plugin pass the required parameter that will allow this development to know that it is being called from the OPAC and should act or not depending on the new syspref "RenewAccruingItemInOpac". This portion of the development can only be tested with one of these plugins. - In Administration, search for the new syspref "RenewAccruingItemInOpac" - TEST: Observe that the new syspref exists and is turned off by default - Ensure the syspref is turned off - Backdate the due date to the past so the loan will accrue fines - Run the fines.pl cronjob (misc/cronjobs/fines.pl) - In the OPAC, pay the item in full - TEST: Observe that the item is not renewed - In Administration, search for the new syspref "RenewAccruingItemInOpac" - TEST: Observe that the new syspref exists and is turned off by default - Turn on the syspref - Backdate the due date to the past so the loan will accrue fines - Run the fines.pl cronjob (misc/cronjobs/fines.pl) - In the OPAC, pay the item in full - TEST: Observe that the item is renewed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_19_11_candidate |rel_20_05_target -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94604|0 |1 is obsolete| | --- Comment #55 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 95557 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95557&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94605|0 |1 is obsolete| | --- Comment #56 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 95558 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95558&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94606|0 |1 is obsolete| | --- Comment #57 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 95559 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95559&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #58 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 95560 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95560&action=edit Bug 23051: Squash of sign off follow-ups Rebasing was a nightmare, so I'm squashing the sign off follow-ups to ease the pain with any future rebases Includes: Bug 23051: (follow-up) Refactor renewal code As per Nick's first point in comment #20, the code that tests for renewability and renews items has been refactored into it's own function. Bug 23051: (follow-up) Provide feedback For renewals that fail when a fine is being paid off, this patch causes any errors to be passed back to the template for display. Addresses the second point in Nick's comment #20 Bug 23051: (follow-up) Fix unit tests As raised by Nick in comment #35 Bug 23051: (follow-up) Fix/improve feedback This follow up patch addresses the following parts of Nick's feedback in comment #35: - it would be nice to get feedback on what was successfully renewed as well - In general I think I would prefer to see 'ok' and 'not_ok' returned as a single 'renewal_results' array - There is no listing of errors if I use the 'pay' button on an individual fine Bug 23051: (follow-up) Refactor methods This follow up patch addresses the following parts of Nick's feedback in comment #35: - I don't really like that the functions are internal functions and then exported - I think the pref description should highlight that if 'RenewalPeriodBase' is set to due date, there may be doubled charges Bug 23051: (follow-up) Add SIP summary This follow up patch addresses the following parts of Nick's feedback in comment #35: - Ideally SIP would get feedback in a screen message Bug 23051: (follow-up) Renewing in OPAC This follow up patch addresses the following parts of Nick's feedback in comment #35: - I am also not sure about the code path if a patron paid fines on the opac (via paypal etc.) but renewals are not allowed on the opac. We've introduced the syspref RenewAccruingItemInOpac (default is off) which, when enabled, will cause items attached to fines that are paid off in the OPAC (via payment plugins), to be automatically renewed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94649|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94629|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94611|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94610|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94609|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94608|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #94607|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #95557|0 |1 is obsolete| | --- Comment #59 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 100127 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100127&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #95558|0 |1 is obsolete| | --- Comment #60 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 100128 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100128&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #95559|0 |1 is obsolete| | --- Comment #61 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 100129 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100129&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #95560|0 |1 is obsolete| | --- Comment #62 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 100130 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100130&action=edit Bug 23051: Squash of sign off follow-ups Rebasing was a nightmare, so I'm squashing the sign off follow-ups to ease the pain with any future rebases Includes: Bug 23051: (follow-up) Refactor renewal code As per Nick's first point in comment #20, the code that tests for renewability and renews items has been refactored into it's own function. Bug 23051: (follow-up) Provide feedback For renewals that fail when a fine is being paid off, this patch causes any errors to be passed back to the template for display. Addresses the second point in Nick's comment #20 Bug 23051: (follow-up) Fix unit tests As raised by Nick in comment #35 Bug 23051: (follow-up) Fix/improve feedback This follow up patch addresses the following parts of Nick's feedback in comment #35: - it would be nice to get feedback on what was successfully renewed as well - In general I think I would prefer to see 'ok' and 'not_ok' returned as a single 'renewal_results' array - There is no listing of errors if I use the 'pay' button on an individual fine Bug 23051: (follow-up) Refactor methods This follow up patch addresses the following parts of Nick's feedback in comment #35: - I don't really like that the functions are internal functions and then exported - I think the pref description should highlight that if 'RenewalPeriodBase' is set to due date, there may be doubled charges Bug 23051: (follow-up) Add SIP summary This follow up patch addresses the following parts of Nick's feedback in comment #35: - Ideally SIP would get feedback in a screen message Bug 23051: (follow-up) Renewing in OPAC This follow up patch addresses the following parts of Nick's feedback in comment #35: - I am also not sure about the code path if a patron paid fines on the opac (via paypal etc.) but renewals are not allowed on the opac. We've introduced the syspref RenewAccruingItemInOpac (default is off) which, when enabled, will cause items attached to fines that are paid off in the OPAC (via payment plugins), to be automatically renewed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #63 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Another monster rebase done. Good lord I hope this one gets into master soon. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100127|0 |1 is obsolete| | Attachment #100128|0 |1 is obsolete| | Attachment #100129|0 |1 is obsolete| | Attachment #100130|0 |1 is obsolete| | --- Comment #64 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 100131 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100131&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #65 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 100132 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100132&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #66 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 100133 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100133&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #67 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 100134 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100134&action=edit Bug 23051: Squash of sign off follow-up Rebasing was a nightmare, so I'm squashing the sign off follow-ups to ease the pain with any future rebases Includes: Bug 23051: (follow-up) Refactor renewal code As per Nick's first point in comment #20, the code that tests for renewability and renews items has been refactored into it's own function. Bug 23051: (follow-up) Provide feedback For renewals that fail when a fine is being paid off, this patch causes any errors to be passed back to the template for display. Addresses the second point in Nick's comment #20 Bug 23051: (follow-up) Fix unit tests As raised by Nick in comment #35 Bug 23051: (follow-up) Fix/improve feedback This follow up patch addresses the following parts of Nick's feedback in comment #35: - it would be nice to get feedback on what was successfully renewed as well - In general I think I would prefer to see 'ok' and 'not_ok' returned as a single 'renewal_results' array - There is no listing of errors if I use the 'pay' button on an individual fine Bug 23051: (follow-up) Refactor methods This follow up patch addresses the following parts of Nick's feedback in comment #35: - I don't really like that the functions are internal functions and then exported - I think the pref description should highlight that if 'RenewalPeriodBase' is set to due date, there may be doubled charges Bug 23051: (follow-up) Add SIP summary This follow up patch addresses the following parts of Nick's feedback in comment #35: - Ideally SIP would get feedback in a screen message Bug 23051: (follow-up) Renewing in OPAC This follow up patch addresses the following parts of Nick's feedback in comment #35: - I am also not sure about the code path if a patron paid fines on the opac (via paypal etc.) but renewals are not allowed on the opac. We've introduced the syspref RenewAccruingItemInOpac (default is off) which, when enabled, will cause items attached to fines that are paid off in the OPAC (via payment plugins), to be automatically renewed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #68 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 100135 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100135&action=edit Bug 23051: (QA follow-up) Missing curly and tabs and fix test Some rebase issues, accounttype no longer exists, circ rules make CanBookBeRenewed fail, so we mock that too interface must be passed as a hashref -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #69 from Nick Clemens <nick@bywatersolutions.com> --- SIP gives nice feedback, staff client does unless I click 'pay' next to the individual fine, can you check that one? I do wonder about write-offs and whether they should also trigger renewal, but am calling that scope creep I don't know about the call to 'GetFine' in the renew_item sub, seems an extra DB hit. I am not sure either - how do you end up with more than one OVERDUE fine on an item? if from an old checkout that was returned, do we care in this instance? I may be overthinking - not a blocker, but curious about the reasoning Please check my work on rebasing and cleaning in the last patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #70 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 100138 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100138&action=edit Bug 23051: (follow-up) Fix renew summary display As mentioned in comment #69:
staff client does unless I click 'pay' next to the individual fine
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #71 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- This looks really great Nick, thanks very much for looking at this. It looks like we were both rebasing the same bug at the same time! :-) To respond to your comments:
I don't know about the call to 'GetFine' in the renew_item sub, seems an extra DB hit. I am not sure either - how do you end up with more than one OVERDUE fine on an item?
It was a while ago, so I'm just speculating, but I suspect I did the call to GetFine and checked the fine to ensure that the fine had been fully paid off and no outstanding amount remained. Does that make sense? Anyway, your rebasing work looks great! Many thanks again for your work on this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #72 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- Created attachment 100172 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100172&action=edit Bug 23051: (follow-up) Remove call to GetFines As mentioned by Nick in comment #69 and subsequently discussed on Slack, the call to GetFines in renew_item was unnececcesary. It has now been removed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Medium patch Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100131|0 |1 is obsolete| | --- Comment #73 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 100182 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100182&action=edit 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 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100132|0 |1 is obsolete| | --- Comment #74 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 100183 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100183&action=edit 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 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100133|0 |1 is obsolete| | --- Comment #75 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 100184 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100184&action=edit 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 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #76 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 100185 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100185&action=edit Bug 23051: (follow-up) Add renewal feedback and move code to subroutines and test Rebasing was a nightmare, so I'm squashing the sign off follow-ups to ease the pain with any future rebases Includes: Bug 23051: (follow-up) Refactor renewal code As per Nick's first point in comment #20, the code that tests for renewability and renews items has been refactored into it's own function. Bug 23051: (follow-up) Provide feedback For renewals that fail when a fine is being paid off, this patch causes any errors to be passed back to the template for display. Addresses the second point in Nick's comment #20 Bug 23051: (follow-up) Fix unit tests As raised by Nick in comment #35 Bug 23051: (follow-up) Fix/improve feedback This follow up patch addresses the following parts of Nick's feedback in comment #35: - it would be nice to get feedback on what was successfully renewed as well - In general I think I would prefer to see 'ok' and 'not_ok' returned as a single 'renewal_results' array - There is no listing of errors if I use the 'pay' button on an individual fine Bug 23051: (follow-up) Refactor methods This follow up patch addresses the following parts of Nick's feedback in comment #35: - I don't really like that the functions are internal functions and then exported - I think the pref description should highlight that if 'RenewalPeriodBase' is set to due date, there may be doubled charges Bug 23051: (follow-up) Add SIP summary This follow up patch addresses the following parts of Nick's feedback in comment #35: - Ideally SIP would get feedback in a screen message Bug 23051: (follow-up) Renewing in OPAC This follow up patch addresses the following parts of Nick's feedback in comment #35: - I am also not sure about the code path if a patron paid fines on the opac (via paypal etc.) but renewals are not allowed on the opac. We've introduced the syspref RenewAccruingItemInOpac (default is off) which, when enabled, will cause items attached to fines that are paid off in the OPAC (via payment plugins), to be automatically renewed. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100135|0 |1 is obsolete| | --- Comment #77 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 100186 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100186&action=edit Bug 23051: (QA follow-up) Missing curly and tabs and fix test Some rebase issues, accounttype no longer exists, circ rules make CanBookBeRenewed fail, so we mock that too interface must be passed as a hashref Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100138|0 |1 is obsolete| | --- Comment #78 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 100187 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100187&action=edit Bug 23051: (follow-up) Fix renew summary display As mentioned in comment #69:
staff client does unless I click 'pay' next to the individual fine
Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100172|0 |1 is obsolete| | --- Comment #79 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 100188 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100188&action=edit Bug 23051: (follow-up) Remove call to GetFines As mentioned by Nick in comment #69 and subsequently discussed on Slack, the call to GetFines in renew_item was unnececcesary. It has now been removed. Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #80 from Nick Clemens <nick@bywatersolutions.com> --- OPAC renewals not tested (i don't have the plugins) but are syspref controlled and should work similarly - passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100134|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #81 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #82 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 100271 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100271&action=edit Bug 23051: (RM follow-up) Fix rebase error Looks like at some point during reworking/rebasing the signature for Koha::Account::Line->apply changed and was then reverted but one case of the reversion got lost. The error was highlighted in the patrons_accounts credit api tests. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 --- Comment #83 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |With the addition of a new release notes| |`RenewAccruingItemWhenPaid` | |system preference, we gain | |the ability to | |automatically renew items | |with accruing fines at the | |point of payment of those | |fines. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #84 from Joy Nelson <joy@bywatersolutions.com> --- enhancement not backported to 19.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_20_05_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |25508 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25508 [Bug 25508] Confusing renewal message when paying accruing fine -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Optionally renew fine |Add ability to optionally |accruing items when all |renew fine accruing items |fines on item are paid off |when all fines on item are | |paid off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|With the addition of a new |With the addition of a new release notes|`RenewAccruingItemWhenPaid` |`RenewAccruingItemWhenPaid` |system preference, we gain |system preference, we gain |the ability to |the ability to |automatically renew items |automatically renew items |with accruing fines at the |with accruing fines at the |point of payment of those |point of payment of those |fines. |fines. | | | |**New system | |preference**: | |`RenewAccruingItemWhenPaid` | |defaults to disabled. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|With the addition of a new |With the addition of a new release notes|`RenewAccruingItemWhenPaid` |`RenewAccruingItemWhenPaid` |system preference, we gain |system preference, we gain |the ability to |the ability to |automatically renew items |automatically renew items |with accruing fines at the |with accruing fines at the |point of payment of those |point of payment of those |fines. |fines. | | |**New system |**New system |preference**: |preferences**: |`RenewAccruingItemWhenPaid` |`RenewAccruingItemWhenPaid` |defaults to disabled. |and | |`RenewAccruingItemInOpac` | |both default to disabled. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |25717 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25717 [Bug 25717] Improve messages for automatic renewal errors -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |26915 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26915 [Bug 26915] Koha explodes when writing off a fee with FinePaymentAutoPopup -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to master |RESOLVED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |27927 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27927 [Bug 27927] longoverdue cronjob renews items before marking lost when both RenewAccruingItemWhenPaid and WhenLostForgiveFine are enabled -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|27927 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27927 [Bug 27927] longoverdue cronjob renews items before marking lost when both RenewAccruingItemWhenPaid and WhenLostForgiveFine are enabled -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28156 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28156 [Bug 28156] Koha::Account::Line->renewable must be named is_renewable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23051 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |36676 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36676 [Bug 36676] SIP2 drops connection when using unknown patron id in fee paid message -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org