I am not sure if this error, too_many, should be removed or just handled differently. It is confusing for patrons, here is why: Day 1, a patron gets an email telling them an item has been successfully renewed. It is auto renewal 5/5, the item will no longer renew. Day 2: The next a patron receives a new auto renewal message telling them the same item, from yesterday, cannot be renewed. Patrons are confused.
Agree that this is confusing and misleading to the patron. We're seeing a consistent duplication of messages whenever something auto renews, both with digests and one-off messaging thanks to the odd too_many behavior. The too_many error is also getting sent while a title has renewals available. For example, we had a scenario where Book A and Book B both had one renew available. Checkout period was 14 days. No renewal before was set to 0 so that the auto-renew happened on the day the item came due. Book A was due 7/1. Book B was due 7/7. The digested messaging went like this: 7/1 - Book A was renewed, Book B was not renewed (too_many error) 7/2 - Both titles were not renewed (too_many error) 7/7 - Book B was renewed, Book A was not renewed (too_many error) 7/8 - Both titles were not renewed (too_many error) The messages on 7/1 and 7/2 gave Book B the too_many error so it appears like no renewals were available, proven untrue when the title auto-renewed on its due date of 7/7. Ideally the 'too_many' error would only pop when all renewals have been exhausted. Titles that have renewals available but aren't getting renewed yet because they aren't due yet should be handled separately so they can be explained/ignored in the templates. The goal for digested messaging in the above scenario would be something like: 7/1 - Book A was renewed, Book B is not yet eligible for auto renewal 7/7 - Book B was renewed, Book A is no longer eligible for auto renewal (too_many) 7/15 - Book A and Book B are no longer eligible for auto renewal (too_many) 7/21 - Book A and Book B are no longer eligible for auto renewal (too_many)
Our library has the same problem. Our patrons get confused with the back to back emails for the same item telling them the item was renewed / not renewed.
In our case the loan period can't be shorten for any reason. The renewal routine seems that it examines every checkout of a patron and then the notice Auto renewals (Digest) reports that some items didn't renew, which is true, but this occurs because there is not any reason to be renewed or returned (they have been renewed yesterday for example), and this is so confusing. It should skip these items and to report only 1) the items that they have been renewed, and 2) the items that they haven't been renewed for a specific reason provided by the library rules. Furthermore, the selection of digest should stop the RENEWAL notice
I'm starting to think that automatic renewal errors shouldn't be sent out at all. That only automatic successes should be sent out. Although in this case I actually have a local customization where I skip renewal handling if it's too soon to renew. The current precedence of errors in automatic renewals is completely illogical.
We are testing auto-renewals in 22.11 (preparing to ditch our auto-renewal workaround which avoided various bugs like 19014 in place since 17.11). We have the ‘No renewal before’ set to zero and the renewal script running at 00:15 after the item was late. So far we have found two interesting issues. One is the renewal handling too soon problem that should be solved if we could have David Cook’s local customisation. Is the customisation TT code added to the AUTO_RENEWAL_DGST? I don’t want ‘Visual merchandising’ tacked on as an orphan on this digest: There were 1 items that were not renewed. There were 2 items that were renewed. Mastering wildlife photography (barcode: 54250053) was not renewed as it is reserved by another reader. Cool infographics (barcode 54197511) was renewed until 12/06/2023 The information design handbook (barcode 54240693) was renewed until 12/06/2023 Visual merchandising / : 40620336 The other issue was the digest not dealing well with leavers. We have added one renewal failure reason missing from the default digest shipped with 22.11: [% ELSIF checkout.auto_renew_error == 'auto_account_expired' %] could not be renewed as your account has expired. This new reason should work well with our renewals assessment taking place the day after the account expiry. But unexpectedly, if I have 2 items due for renewal assessment tonight and 2 more not due for some days, for leavers, Koha lumps all 4 items together and tells me all 4 items have failed to renew ([% error %]=4 not 2), and the two ones that aren’t eligible yet for renewal assessment are not left tacked onto the end of the renewed / failed renew list (so, no ‘Visual merchandising’ orphans for leavers). OK, it might seem odd that items could be due after expiry, but our data feed manipulates the borrower.dateexpiry and the scenario is (unfortunately) possible. Perhaps the customisation might fix this and see [% error %] set to two and the digest only listing the 2 items due now. OK, there would be a second digest for the other items due later, but that is OK.
(In reply to Ray Delahunty from comment #5) > We are testing auto-renewals in 22.11 (preparing to ditch our auto-renewal > workaround which avoided various bugs like 19014 in place since 17.11). We > have the ‘No renewal before’ set to zero and the renewal script running at > 00:15 after the item was late. So far we have found two interesting issues. > One is the renewal handling too soon problem that should be solved if we > could have David Cook’s local customisation. Is the customisation TT code > added to the AUTO_RENEWAL_DGST? The local customisation I'd done was in the code in the automatic_renewals.pl script. However, it hasn't been working correctly in 21.11 and 22.11. I haven't had any complaints about 22.11, so I'm not planning on looking at it again unless I get people complaining about confusion/unexpected behaviours locally. (In reply to Ray Delahunty from comment #5) > Is the customisation TT code > added to the AUTO_RENEWAL_DGST? I don’t want ‘Visual merchandising’ tacked > on as an orphan on this digest: > > There were 1 items that were not renewed. > There were 2 items that were renewed. > Mastering wildlife photography (barcode: 54250053) was not renewed as it is > reserved by another reader. > Cool infographics (barcode 54197511) was renewed until 12/06/2023 > The information design handbook (barcode 54240693) was renewed until > 12/06/2023 > Visual merchandising / : 40620336 It might be that you need to tweak your template as this does look a bit odd.
I'm looking at master and it looks like this would still be a problem. It seems to me that _CanBookBeAutoRenewed() and friends should be moved up in C4::Circulation::CanBookBeRenewed() to before the return for "too_many".
I'm going to change the title from "Automatic renewal too_many error is confusing in notices" to "Automatic renewal errors should come before other renewal errors"
Created attachment 153696 [details] [review] Bug 31427: Get auto renewal errors before other renewal errors This patch changes CanBookBeRenewed so that automatic renewal errors pop up before other renewal errors. This means that a book will be considered "auto_too_soon" before things like "too_many" or "restricted". (Otherwise, you'll get an email saying you can't renew a book the day after using your last auto renewal, even though the earliest renewal isn't available until later.) Test plan: 0. Apply patch 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Holds.t 3. prove t/db_dependent/Koha/Account/Line.t 4. prove t/db_dependent/Koha/Account.t Additional tests: 5. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=RestrictionBlockRenewing 6. Change to "block" 7. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=AutoRenewalNotices 8. Change to "according to patron messaging preferences" 9. Go to http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl 10. Set "Automatic renewal" to "Yes" and "No renewal before" to 4 11. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51 12. Checkout 39999000001310 with a due date 4 days in the future 13. Add a manual restriction 14. Run `perl ./misc/cronjobs/automatic_renewals.pl` 15. Note that it says something like the following: Issue id: 1237 for borrower: 51 and item: 73 would not be renewed. (auto_too_soon) Instead of something like the following: Issue id: 1237 for borrower: 51 and item: 73 would not be renewed. (restriction)
Note: You can test "too_many" error by changing the circ rule "Renewals allowed (count)" to 0. With the patch, it will say "auto_too_soon" until the the earliest renewal date is available. And then it will say "too_many". This should be in line with what Lucas would want in the original description. It's also what all my libraries have wanted over the years.
Created attachment 153732 [details] [review] Bug 31427: Get auto renewal errors before other renewal errors This patch changes CanBookBeRenewed so that automatic renewal errors pop up before other renewal errors. This means that a book will be considered "auto_too_soon" before things like "too_many" or "restricted". (Otherwise, you'll get an email saying you can't renew a book the day after using your last auto renewal, even though the earliest renewal isn't available until later.) Test plan: 0. Apply patch 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Holds.t 3. prove t/db_dependent/Koha/Account/Line.t 4. prove t/db_dependent/Koha/Account.t Additional tests: 5. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=RestrictionBlockRenewing 6. Change to "block" 7. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=AutoRenewalNotices 8. Change to "according to patron messaging preferences" 9. Go to http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl 10. Set "Automatic renewal" to "Yes" and "No renewal before" to 4 11. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51 12. Checkout 39999000001310 with a due date 4 days in the future 13. Add a manual restriction 14. Run `perl ./misc/cronjobs/automatic_renewals.pl` 15. Note that it says something like the following: Issue id: 1237 for borrower: 51 and item: 73 would not be renewed. (auto_too_soon) Instead of something like the following: Issue id: 1237 for borrower: 51 and item: 73 would not be renewed. (restriction) Signed-off-by: Sam Lau <samalau@gmail.com>
I would actually say this isn't an enhancement at all.. it's a regression fix. Bug 19014 mentions exactly this sort of issue as part of what it was attempting to resolve. However, we need to be very careful we don't cause an alternative regression.. I think the auto checks need to be conditional to on top of them being moved.
Created attachment 156232 [details] [review] Bug 31427: Get auto renewal errors before other renewal errors This patch changes CanBookBeRenewed so that automatic renewal errors pop up before other renewal errors. This means that a book will be considered "auto_too_soon" before things like "too_many" or "restricted". (Otherwise, you'll get an email saying you can't renew a book the day after using your last auto renewal, even though the earliest renewal isn't available until later.) Test plan: 0. Apply patch 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Holds.t 3. prove t/db_dependent/Koha/Account/Line.t 4. prove t/db_dependent/Koha/Account.t Additional tests: 5. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=RestrictionBlockRenewing 6. Change to "block" 7. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=AutoRenewalNotices 8. Change to "according to patron messaging preferences" 9. Go to http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl 10. Set "Automatic renewal" to "Yes" and "No renewal before" to 4 11. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51 12. Checkout 39999000001310 with a due date 4 days in the future 13. Add a manual restriction 14. Run `perl ./misc/cronjobs/automatic_renewals.pl` 15. Note that it says something like the following: Issue id: 1237 for borrower: 51 and item: 73 would not be renewed. (auto_too_soon) Instead of something like the following: Issue id: 1237 for borrower: 51 and item: 73 would not be renewed. (restriction) Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
I think this all works as I would expect.. however I think we need a unit test to try and prevent future regression again in this complex and frequently flip-flopping piece of code.
Sorry David.. if you find yourself struggling on unit tests I can try to find a moment.. I must admit, I've not looked at what already exists for these methods.
(In reply to Martin Renvoize from comment #14) > I think this all works as I would expect.. however I think we need a unit > test to try and prevent future regression again in this complex and > frequently flip-flopping piece of code. (In reply to Martin Renvoize from comment #15) > Sorry David.. if you find yourself struggling on unit tests I can try to > find a moment.. I must admit, I've not looked at what already exists for > these methods. I was thinking the 4 existing unit tests that cover CanBookBeRenewed would be enough, but I suppose there wasn't a unit test that revealed the problem, so clearly there is at least 1 unit test missing... This one is pretty low on my priority list, as I have a different (less optimal) workaround for this problem that I've been using locally for many years. I'll get to it eventually but if someone else wants to write that test first I wouldn't complain hehe.
Created attachment 157894 [details] [review] Bug 31427: (follow-up) Unit tests This patch adds a unit test for error precidence where autorenewals is involved. It is not comprehensive however, and I'm a little confused by the logic around cron vs non-cron handling...
I think I need some additional QA help here.. I've started writing a unit test to prevent future regressions, but it's raised more questions for me. When exactly should an auto-renewal error get passed up.. for 'too_soon' we only pass when called via the cron.. but for other autorenewal errors we pass regardless.. which seems a little odd... if it's a non-cron check of renewability.. should we not skip the auto-renew failure code for those other failures too?
(In reply to Martin Renvoize from comment #18) > I think I need some additional QA help here.. I've started writing a unit > test to prevent future regressions, but it's raised more questions for me. > > When exactly should an auto-renewal error get passed up.. for 'too_soon' we > only pass when called via the cron.. but for other autorenewal errors we > pass regardless.. which seems a little odd... if it's a non-cron check of > renewability.. should we not skip the auto-renew failure code for those > other failures too? I think part of the issue here is that before bug 25393 an issue being marked as 'auto_renew' in the issues table meant that it was not allowed to be renewed via staff/opac at all - so passing the auto_renew_error codes back was realy for information purposes. After that bug, I think you are right, the auto renew specific errors should not be returned when the check is not coming from the cron. Looking at the logic, I think we can actually move all of the auto_renew checks to the auto_renew checks to _CanBookBeAutoRenewed and have that routine call CanBookBeRenewed - rather than having to check if we are coming from the cron. So I think the order of blocks are: For all: no_item no_checkout onsite_checkout item_issued_to_other_patron item_denied_renewal For auto_renew cron: auto_account_expired auto_too_late auto_too_much_oweing auto_too_soon too_many too_unseen restriction overdue recalled on_reserve too_soon For staff: too_many too_unseen restriction overdue recalled on_reserve too_soon For opac: OPACFineNoRenewals - checked in script opac-user.pl BlockExpiredPatronActions - checked in script opac-renew.pl too_many too_unseen restriction overdue recalled on_reserve too_soon 'too_unseen' for staff actually feels wrong as well, staff side renewals will succeed, but passing this is useful in that it reminds staff they are supposed to 'see' the item? Bug 34924 will have an effect here too, and I think there are rabbit holes all around this because sometimes the 'error' is trying to give information, and sometimes actually blocking (like the too_unseen above)
It sounds from the last comment like this needs a little more work?
Yes and no.. the patch here, and accompanying unit test, does enough to fix the issue as it is.. but Nick's suggestion is a really valid one and I think it's the way forward. I just have no idea when I'll find a moment to spend on it .
No Assignee. Please always use this field, David :)
WARN t/db_dependent/Circulation.t WARN tidiness The file is less tidy than before (bad/messy lines before: 1438, now: 1449)
* Proving /usr/share/koha/t/db_dependent/Circulation.t KO! DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Incorrect datetime value: 'SCALAR(0x5627926767b8)' for column `koha_myclone`.`borrowers`.`updated_on` at row 1 at /usr/share/koha/Koha/Object.pm line 170 # No tests run! # Failed test 'No tests run for subtest "CanBookBeRenewed tests"' # at /usr/share/koha/t/db_dependent/Circulation.t line 1729. Invalid value passed, borrowers.updated_on=SCALAR(0x5627926767b8) expected type is datetime # Looks like your test exited with 11 just after 17. /usr/share/koha/t/db_dependent/Circulation.t ..
(In reply to Marcel de Rooy from comment #22) > No Assignee. Please always use this field, David :) If you look at the history, you'll see that I did back in July, Marcel :) Last week Martin changed it to him and then to the default. At the moment, I don't intend to work on this further. I've been trying to get this issue fixed for years, but autorenewals is a moving target. That is, it's constantly changing. Perhaps Nick's ideas in Comment 19 are a better way to go. Resetting assignee to default in any case.
The reset to default on my part I think was a page reload or bz conflict. I grabbed it from David knowing he'd said he didn't have time to work on it. I intend to come back here soon and work through Nick's suggestion.. we discussed it out if band and both agreed it needed a little more than we currently have here to ensure we don't end up regressing again.
Bug 34924 is now in Koha.. is it time to revisit this
Created attachment 162239 [details] [review] Bug 31427: Get auto renewal errors before other renewal errors This patch changes CanBookBeRenewed so that automatic renewal errors pop up before other renewal errors. This means that a book will be considered "auto_too_soon" before things like "too_many" or "restricted". (Otherwise, you'll get an email saying you can't renew a book the day after using your last auto renewal, even though the earliest renewal isn't available until later.) Test plan: 0. Apply patch 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Holds.t 3. prove t/db_dependent/Koha/Account/Line.t 4. prove t/db_dependent/Koha/Account.t Additional tests: 5. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=RestrictionBlockRenewing 6. Change to "block" 7. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=AutoRenewalNotices 8. Change to "according to patron messaging preferences" 9. Go to http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl 10. Set "Automatic renewal" to "Yes" and "No renewal before" to 4 11. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51 12. Checkout 39999000001310 with a due date 4 days in the future 13. Add a manual restriction 14. Run `perl ./misc/cronjobs/automatic_renewals.pl` 15. Note that it says something like the following: Issue id: 1237 for borrower: 51 and item: 73 would not be renewed. (auto_too_soon) Instead of something like the following: Issue id: 1237 for borrower: 51 and item: 73 would not be renewed. (restriction) Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 162240 [details] [review] Bug 31427: (follow-up) Unit tests This patch adds a unit test for error precidence where autorenewals is involved. It is not comprehensive however, and I'm a little confused by the logic around cron vs non-cron handling...
Rebased.. I still think this is important
Still having trouble with tests: Running tests (1) * Proving /usr/share/koha/t/db_dependent/Circulation.t KO! DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /usr/share/koha/Koha/Objects.pm line 96 DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /usr/share/koha/Koha/Objects.pm line 96 DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /usr/share/koha/Koha/Objects.pm line 96 DBIx::Class::Storage::DBI::select_single(): Query returned more than one row. SQL that returns multiple rows is DEPRECATED for ->find and ->single at /usr/share/koha/Koha/Objects.pm line 96 # Looks like you planned 9 tests but ran 3. # Failed test 'transferbook tests' # at /usr/share/koha/t/db_dependent/Circulation.t line 5754. Exception 'Koha::Exceptions::MissingParameter' thrown 'The to parameter is mandatory' # Looks like your test exited with 11 just after 57. /usr/share/koha/t/db_dependent/Circulation.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 12/68 subtests Test Summary Report ------------------- /usr/share/koha/t/db_dependent/Circulation.t (Wstat: 2816 Tests: 57 Failed: 1) Failed test: 57 Non-zero exit status: 11 Parse errors: Bad plan. You planned 68 tests but ran 57. Files=1, Tests=57, 41 wallclock secs ( 0.11 usr 0.01 sys + 32.32 cusr 4.83 csys = 37.27 CPU) Result: FAIL
Needs a little digging. Test fails without this patch too. And culprit is transferbook: my $item = Koha::Items->find( { barcode => $barcode } ); If barcode is empty and you have items without barcode, things go wrong. Solved it on bug 36212. Chose to not record as dependency.
Another related observation in CanBookBeRenewed: if ( $auto_renew eq 'auto_too_soon' ) { # If its cron, tell it it's too soon for a an auto renewal return ( 0, $auto_renew, { soonest_renew_date => $soonest } ) if $cron; # Check if it's too soon for a manual renewal my $soonestManual = GetSoonestRenewDate( $patron, $issue ); if ( $soonestManual > dt_from_string() ) { return ( 0, "too_soon", { soonest_renew_date => $soonestManual } ) unless $override_limit; } } $soonest = GetSoonestRenewDate($patron, $issue); if ( $soonest > dt_from_string() ){ return (0, "too_soon", { soonest_renew_date => $soonest } ) unless $override_limit; } When we have auto_too_soon, we might be calling GetSoonestRenewDate twice here?
Koha::Checkout sub attempt_auto_renew { my ( $self, $params ) = @_; my $confirm = $params->{confirm} // 0; # CanBookBeRenewed returns 'auto_renew' when the renewal should be done by this script my ( $ok, $error ) = C4::Circulation::CanBookBeRenewed( $self->patron, $self, undef, 1 ); After this call the $ok result is not even checked?! Could be 'fine' but does not look good in general. Related observation, going out of scope. In the same routine: my $updated = 0; if ( !$self->auto_renew_error || $error ne $self->auto_renew_error ) { $updated = 1 unless ( $self->auto_renew_error && ( $self->auto_renew_error eq 'auto_renew_final' && $error eq 'too_many' || $self->auto_renew_error eq 'auto_unseen_final' && $error eq 'too_unseen' ) ); $self->auto_renew_error($error)->store if $confirm; } return ( 0, $error, $updated ); This does not look good either. $updated is passed back but $confirm is not checked?
Created attachment 162656 [details] [review] Bug 31427: Get auto renewal errors before other renewal errors This patch changes CanBookBeRenewed so that automatic renewal errors pop up before other renewal errors. This means that a book will be considered "auto_too_soon" before things like "too_many" or "restricted". (Otherwise, you'll get an email saying you can't renew a book the day after using your last auto renewal, even though the earliest renewal isn't available until later.) Test plan: 0. Apply patch 1. prove t/db_dependent/Circulation.t 2. prove t/db_dependent/Holds.t 3. prove t/db_dependent/Koha/Account/Line.t 4. prove t/db_dependent/Koha/Account.t Additional tests: 5. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=RestrictionBlockRenewing 6. Change to "block" 7. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=AutoRenewalNotices 8. Change to "according to patron messaging preferences" 9. Go to http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl 10. Set "Automatic renewal" to "Yes" and "No renewal before" to 4 11. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51 12. Checkout 39999000001310 with a due date 4 days in the future 13. Add a manual restriction 14. Run `perl ./misc/cronjobs/automatic_renewals.pl` 15. Note that it says something like the following: Issue id: 1237 for borrower: 51 and item: 73 would not be renewed. (auto_too_soon) Instead of something like the following: Issue id: 1237 for borrower: 51 and item: 73 would not be renewed. (restriction) Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 162657 [details] [review] Bug 31427: (follow-up) Unit tests This patch adds a unit test for error precidence where autorenewals is involved. It is not comprehensive however, and I'm a little confused by the logic around cron vs non-cron handling... Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 162658 [details] [review] Bug 31427: Remove a few refs to CanBookBeRenewed Actually, the module is not even needed anymore here. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Looks good to me. Checked references to CanBookBeRenewed and ->attempt_auto_renew. [OK] prove t/db_dependent/Holds.t t/db_dependent/Koha/Account/Line.t t/db_dependent/Koha/Account.t Could be in test plan too: prove t/db_dependent/Koha/Checkouts.t t/db_dependent/Koha/Checkouts.t .. ok All tests successful. Files=1, Tests=12, 7 wallclock secs ( 0.03 usr 0.01 sys + 5.42 cusr 0.87 csys = 6.33 CPU) Result: PASS C4/SIP/ILS/Transaction/Renew.pm sub do_renew_for The auto_xxx codes from CanBookBeRenewed are not listed here, but seem to be passed through as-is. Added a follow-up where CanBookBeRenewed is no longer called.
Pushed for 24.05! Well done everyone, thank you!
Pushed to 23.11.x for 23.11.04
Doesn't apply cleanly to 23.05.x, no backport. Please rebase if needed/wanted in 23.05.