There are several places where we fetch the soonest renewal date when CanBookBeRenewed returns too soon We have already calculated this date and should pass it back
Created attachment 131046 [details] [review] Bug 30167: Return soonest renewal date when renewal is to soon This patch adds an 'info' return param to CanBookBeRenewed and passes the soonest renewal date when returning too_soon errors To test: 1 - prove -v t/db_dependent/Circulation.t Fix whitespace
Created attachment 131047 [details] [review] Bug 30167: Use returned renewal date rather than fetching This patch updates the three scripts that fetched the soonest renewal date to use the return from CanBookBeRenewed To test: 1 - Set a circulation rule with a 'no renewal before' set to 3, loan length set to 5 2 - Check out an item to a patron that uses this rule 3 - Verify the checkouts for the patron show the correct 'No renewal before' date 4 - Sign in to the patron's opac account 5 - Verify the item shows it cannot be renewed, and shows the correct date 6 - Go to Circulation->Renew 7 - Attempt to renew using barcode 8 - Confirm error shows the soonest renewal date
Patch no longer applies 8-(.. git bz apply 30167 Bug 30167 - Return soonest renewal date when CanBookBeRenewed returns %too_soon 131046 - Bug 30167: Return soonest renewal date when renewal is to soon 131047 - Bug 30167: Use returned renewal date rather than fetching Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 30167: Return soonest renewal date when renewal is to soon Using index info to reconstruct a base tree... M C4/Circulation.pm M t/db_dependent/Circulation.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Circulation.t CONFLICT (content): Merge conflict in t/db_dependent/Circulation.t Auto-merging C4/Circulation.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 30167: Return soonest renewal date when renewal is to soon
Created attachment 134082 [details] [review] Bug 30167: Return soonest renewal date when renewal is to soon This patch adds an 'info' return param to CanBookBeRenewed and passes the soonest renewal date when returning too_soon errors To test: 1 - prove -v t/db_dependent/Circulation.t Fix whitespace
Created attachment 134083 [details] [review] Bug 30167: Use returned renewal date rather than fetching This patch updates the three scripts that fetched the soonest renewal date to use the return from CanBookBeRenewed To test: 1 - Set a circulation rule with a 'no renewal before' set to 3, loan length set to 5 2 - Check out an item to a patron that uses this rule 3 - Verify the checkouts for the patron show the correct 'No renewal before' date 4 - Sign in to the patron's opac account 5 - Verify the item shows it cannot be renewed, and shows the correct date 6 - Go to Circulation->Renew 7 - Attempt to renew using barcode 8 - Confirm error shows the soonest renewal date
Created attachment 134197 [details] [review] Bug 30167: Return soonest renewal date when renewal is to soon This patch adds an 'info' return param to CanBookBeRenewed and passes the soonest renewal date when returning too_soon errors To test: 1 - prove -v t/db_dependent/Circulation.t Fix whitespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 134198 [details] [review] Bug 30167: Use returned renewal date rather than fetching This patch updates the three scripts that fetched the soonest renewal date to use the return from CanBookBeRenewed To test: 1 - Set a circulation rule with a 'no renewal before' set to 3, loan length set to 5 2 - Check out an item to a patron that uses this rule 3 - Verify the checkouts for the patron show the correct 'No renewal before' date 4 - Sign in to the patron's opac account 5 - Verify the item shows it cannot be renewed, and shows the correct date 6 - Go to Circulation->Renew 7 - Attempt to renew using barcode 8 - Confirm error shows the soonest renewal date Signed-off-by: David Nind <david@davidnind.com>
Testing notes (koha-testing-docker): - Tested on 04/28/2022 (American date format) with . 'no renewal before' set to 3 . 'loan period' set to 5 - Checked out on date is 04/28/2022 00:09 - For the patron from the checkouts: No renewal before 04/30/2022 - When trying to renew, the message is: "Cannot renew: Programming Perl / ( 39999000011418 ) cannot be renewed before 04/30/2022. " - Date is calculated correctly including the current day (3 days - 28th, 29th + 30th)
+ ($ok,$error,$info) = &CanBookBeRenewed($borrowernumber, $itemnumber[, $override_limit]); Find out whether a borrowed item may be renewed. + $info will contain the soonest renewal date if the error is 'too soon' I would suggest to let $info either be a hash containing a key for soonest renewal date (among others, room for extension) in order to honor its name or rename it to soonestrenewaldate. This is somewhat obsure.
Changing status to get some feedback on Marcel's last comment.
Created attachment 134380 [details] [review] Bug 30167: (follow-up) Return a hash with soonest_renew_date
FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt FAIL valid_template Attempt to reload Koha/Template/Plugin/Branches.pm aborted. Compilation failed in require at /usr/lib/x86_64-linux-gnu/perl5/5.28/Template/Plugins.pm line 206. I am going to ignore this one. It pops up now and then on some reports. And several people have not succeeded in finding the exact cause. See also https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27944#c46
Created attachment 134454 [details] [review] Bug 30167: Return soonest renewal date when renewal is to soon This patch adds an 'info' return param to CanBookBeRenewed and passes the soonest renewal date when returning too_soon errors To test: 1 - prove -v t/db_dependent/Circulation.t Fix whitespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 134455 [details] [review] Bug 30167: Use returned renewal date rather than fetching This patch updates the three scripts that fetched the soonest renewal date to use the return from CanBookBeRenewed To test: 1 - Set a circulation rule with a 'no renewal before' set to 3, loan length set to 5 2 - Check out an item to a patron that uses this rule 3 - Verify the checkouts for the patron show the correct 'No renewal before' date 4 - Sign in to the patron's opac account 5 - Verify the item shows it cannot be renewed, and shows the correct date 6 - Go to Circulation->Renew 7 - Attempt to renew using barcode 8 - Confirm error shows the soonest renewal date Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 134456 [details] [review] Bug 30167: (follow-up) Return a hash with soonest_renew_date Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Not sure about 'major' btw. More like an enhancement.
(In reply to Marcel de Rooy from comment #16) > Not sure about 'major' btw. More like an enhancement. could be less than major - but can affect performance, so not an enh, should be eligible for backport
Sorry, does not apply any anymore since Bug 29537 was pushed
Created attachment 134576 [details] [review] Bug 30167: Return soonest renewal date when renewal is to soon This patch adds an 'info' return param to CanBookBeRenewed and passes the soonest renewal date when returning too_soon errors To test: 1 - prove -v t/db_dependent/Circulation.t Fix whitespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 134577 [details] [review] Bug 30167: Use returned renewal date rather than fetching This patch updates the three scripts that fetched the soonest renewal date to use the return from CanBookBeRenewed To test: 1 - Set a circulation rule with a 'no renewal before' set to 3, loan length set to 5 2 - Check out an item to a patron that uses this rule 3 - Verify the checkouts for the patron show the correct 'No renewal before' date 4 - Sign in to the patron's opac account 5 - Verify the item shows it cannot be renewed, and shows the correct date 6 - Go to Circulation->Renew 7 - Attempt to renew using barcode 8 - Confirm error shows the soonest renewal date Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 134578 [details] [review] Bug 30167: (follow-up) Return a hash with soonest_renew_date Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 134579 [details] [review] Bug 30167: (follow-up) Compare dt objects
Pushed to master for 22.05, thanks to everybody involved [U+1F984]
Created attachment 135016 [details] [review] Bug 30167: [21.11.x] Return soonest renewal date when renewal is to soon This patch adds an 'info' return param to CanBookBeRenewed and passes the soonest renewal date when returning too_soon errors To test: 1 - prove -v t/db_dependent/Circulation.t Fix whitespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 30167: Use returned renewal date rather than fetching This patch updates the three scripts that fetched the soonest renewal date to use the return from CanBookBeRenewed To test: 1 - Set a circulation rule with a 'no renewal before' set to 3, loan length set to 5 2 - Check out an item to a patron that uses this rule 3 - Verify the checkouts for the patron show the correct 'No renewal before' date 4 - Sign in to the patron's opac account 5 - Verify the item shows it cannot be renewed, and shows the correct date 6 - Go to Circulation->Renew 7 - Attempt to renew using barcode 8 - Confirm error shows the soonest renewal date Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 30167: (follow-up) Return a hash with soonest_renew_date Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Bug 30167: (follow-up) Compare dt objects