[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Lucas Gass changed: What|Removed |Added Resolution|--- |FIXED Status|Pushed to stable|RESOLVED --- Comment #41 from Lucas Gass --- 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. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Fridolin Somers changed: What|Removed |Added Version(s)|24.05.00|24.05.00,23.11.04 released in|| Status|Pushed to master|Pushed to stable CC||fridolin.som...@biblibre.co ||m --- Comment #40 from Fridolin Somers --- Pushed to 23.11.x for 23.11.04 -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #39 from Katrin Fischer --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Katrin Fischer changed: What|Removed |Added Status|Passed QA |Pushed to master Version(s)||24.05.00 released in|| -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Marcel de Rooy changed: What|Removed |Added QA Contact|testo...@bugs.koha-communit |m.de.r...@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #38 from Marcel de Rooy --- 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. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #37 from Marcel de Rooy --- Created attachment 162658 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162658=edit Bug 31427: Remove a few refs to CanBookBeRenewed Actually, the module is not even needed anymore here. Signed-off-by: Marcel de Rooy -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Marcel de Rooy changed: What|Removed |Added Attachment #162240|0 |1 is obsolete|| --- Comment #36 from Marcel de Rooy --- Created attachment 162657 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162657=edit 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 -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Marcel de Rooy changed: What|Removed |Added Attachment #162239|0 |1 is obsolete|| --- Comment #35 from Marcel de Rooy --- Created attachment 162656 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162656=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=RestrictionBlockRenewing 6. Change to "block" 7. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab==search=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 301310 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 Signed-off-by: Martin Renvoize Signed-off-by: Marcel de Rooy -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Marcel de Rooy changed: What|Removed |Added Patch complexity|--- |Small patch Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427
--- Comment #34 from Marcel de Rooy ---
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.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427
--- Comment #33 from Marcel de Rooy ---
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?
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427
--- Comment #32 from Marcel de Rooy ---
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.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Marcel de Rooy changed: What|Removed |Added See Also||https://bugs.koha-community ||.org/bugzilla3/show_bug.cgi ||?id=36212 -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #31 from Marcel de Rooy --- 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. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize changed: What|Removed |Added QA Contact|martin.renvoize@ptfs-europe |testo...@bugs.koha-communit |.com|y.org --- Comment #30 from Martin Renvoize --- Rebased.. I still think this is important -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize changed: What|Removed |Added Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize changed: What|Removed |Added Attachment #157894|0 |1 is obsolete|| --- Comment #29 from Martin Renvoize --- Created attachment 162240 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162240=edit 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... -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize changed: What|Removed |Added Attachment #156232|0 |1 is obsolete|| --- Comment #28 from Martin Renvoize --- Created attachment 162239 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162239=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=RestrictionBlockRenewing 6. Change to "block" 7. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab==search=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 301310 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 Signed-off-by: Martin Renvoize -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #27 from Martin Renvoize --- Bug 34924 is now in Koha.. is it time to revisit this -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize 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. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #26 from Martin Renvoize --- 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. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize changed: What|Removed |Added Assignee|koha-b...@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. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 David Cook changed: What|Removed |Added Assignee|dc...@prosentient.com.au|koha-b...@lists.koha-commun ||ity.org --- Comment #25 from David Cook --- (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. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Marcel de Rooy changed: What|Removed |Added Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #24 from Marcel de Rooy --- * 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 .. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #23 from Marcel de Rooy --- 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. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Marcel de Rooy changed: What|Removed |Added CC||m.de.r...@rijksmuseum.nl Assignee|koha-b...@lists.koha-commun |dc...@prosentient.com.au |ity.org | --- Comment #22 from Marcel de Rooy --- No Assignee. Please always use this field, David :) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #21 from Martin Renvoize --- 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. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #20 from Katrin Fischer --- It sounds from the last comment like this needs a little more work? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize changed: What|Removed |Added Assignee|martin.renvoize@ptfs-europe |koha-b...@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. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize changed: What|Removed |Added Assignee|dc...@prosentient.com.au|martin.renvoize@ptfs-europe ||.com -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #19 from Nick Clemens --- (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. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #18 from Martin Renvoize --- 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? -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize changed: What|Removed |Added Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #17 from Martin Renvoize --- Created attachment 157894 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157894=edit 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... -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Georgia Newman changed: What|Removed |Added CC||g.new...@arts.ac.uk -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize changed: What|Removed |Added CC||n...@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #16 from David Cook --- (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. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Nick Clemens changed: What|Removed |Added See Also||https://bugs.koha-community ||.org/bugzilla3/show_bug.cgi ||?id=34924 -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 --- Comment #15 from Martin Renvoize --- 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. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize changed: What|Removed |Added QA Contact|testo...@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize changed: What|Removed |Added Status|Signed Off |Failed QA --- Comment #14 from Martin Renvoize --- 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. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize changed: What|Removed |Added Attachment #153732|0 |1 is obsolete|| --- Comment #13 from Martin Renvoize --- Created attachment 156232 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=156232=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=RestrictionBlockRenewing 6. Change to "block" 7. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab==search=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 301310 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 Signed-off-by: Martin Renvoize -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize changed: What|Removed |Added Severity|enhancement |major -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Martin Renvoize changed: What|Removed |Added CC||martin.renvoize@ptfs-europe ||.com --- Comment #12 from Martin Renvoize --- 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. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Katie Bliss changed: What|Removed |Added CC||kebl...@dmpl.org -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Laura Escamilla changed: What|Removed |Added CC||Laura.escamilla@bywatersolu ||tions.com -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Sam Lau changed: What|Removed |Added Attachment #153696|0 |1 is obsolete|| --- Comment #11 from Sam Lau --- Created attachment 153732 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153732=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=RestrictionBlockRenewing 6. Change to "block" 7. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab==search=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 301310 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 -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 Sam Lau changed: What|Removed |Added Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 31427] Automatic renewal errors should come before many other renewal errors
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31427 David Cook 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. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
