https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Jason Robb <jrobb@sekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jrobb@sekls.org --- Comment #1 from Jason Robb <jrobb@sekls.org> --- 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) -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Spyros Pierros <spyros.pierros@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |spyros.pierros@gmail.com --- Comment #3 from Spyros Pierros <spyros.pierros@gmail.com> --- 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 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 rachel.rappaport@montgomerycountymd.gov changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rachel.rappaport@montgomery | |countymd.gov -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 helm.consortium@nhs.net changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |helm.consortium@nhs.net -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #4 from David Cook <dcook@prosentient.com.au> --- 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. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Ray Delahunty <r.delahunty@arts.ac.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |r.delahunty@arts.ac.uk --- Comment #5 from Ray Delahunty <r.delahunty@arts.ac.uk> --- 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. -- 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=31427 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- 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". -- 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=31427 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Automatic renewal too_many |Automatic renewal errors |error is confusing in |should come before other |notices |renewal errors --- Comment #8 from David Cook <dcook@prosentient.com.au> --- 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" -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #9 from David Cook <dcook@prosentient.com.au> --- Created attachment 153696 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153696&action=edit 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) -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Automatic renewal errors |Automatic renewal errors |should come before other |should come before many |renewal errors |other renewal errors -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Sam Lau <samalau@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #153696|0 |1 is obsolete| | --- Comment #11 from Sam Lau <samalau@gmail.com> --- Created attachment 153732 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153732&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Katie Bliss <kebliss@dmpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kebliss@dmpl.org -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |major -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #14 from 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. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #16 from David Cook <dcook@prosentient.com.au> --- (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. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Georgia Newman <g.newman@arts.ac.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |g.newman@arts.ac.uk -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 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=31427 --- Comment #19 from Nick Clemens <nick@bywatersolutions.com> --- (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) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|martin.renvoize@ptfs-europe |koha-bugs@lists.koha-commun |.com |ity.org -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- 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 . -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #23 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- WARN t/db_dependent/Circulation.t WARN tidiness The file is less tidy than before (bad/messy lines before: 1438, now: 1449) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.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=31427 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=23415 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #156232|0 |1 is obsolete| | --- Comment #28 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 162239 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162239&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 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=31427 --- Comment #31 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- 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 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #32 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- 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. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #34 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- 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? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162239|0 |1 is obsolete| | --- Comment #35 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 162656 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162656&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #37 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 162658 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162658&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #39 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to stable |RESOLVED --- Comment #41 from Lucas Gass <lucas@bywatersolutions.com> --- Doesn't apply cleanly to 23.05.x, no backport. Please rebase if needed/wanted in 23.05. -- You are receiving this mail because: You are watching all bug changes.