The routine (after bug 32013) now takes 9 parameters - this is unsustainable as a list
Created attachment 149280 [details] [review] Bug 33444: Update AddRenewal to take a hashref of params
Created attachment 151283 [details] [review] Bug 33444: Update AddRenewal to take a hashref of params Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Feels like an enh ?
Did you see that you solved a bug here (Koha/REST/V1/Checkouts.pm): AddRenewal( - $checkout->borrowernumber, - $checkout->itemnumber, - $checkout->branchcode, - undef, - undef, - $seen + { + borrowernumber => $checkout->borrowernumber, + itemnumber => $checkout->itemnumber, + branch => $checkout->branchcode, + seen => $seen Seen was in the sixth position before. And should have been 7th.
misc/cronjobs/automatic_renewals.pl - my $date_due = AddRenewal( $auto_renew->borrowernumber, $auto_renew->itemnumber, $auto_renew->branchcode, undef, undef, undef, 0, 1 ); + my $date_due = AddRenewal({ + borrowernumber => $auto_renew->borrowernumber, + itemnumber => $auto_renew->itemnumber, + branch => $auto_renew->branchcode, + seen => 0, + automatic => 1, + skip_record_index => 1 This is not the same. skip_record_index was not there before Please clarify
Created attachment 153593 [details] [review] Bug 33444: Update AddRenewal to take a hashref of params Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 153594 [details] [review] Bug 33444: (QA follow-up) Tidy block in automatic_renewals.pl Resolves: The file is less tidy than before (bad/messy lines before: 94, now: 101) Test plan: Run qa tools. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 153595 [details] [review] Bug 33444: (QA follow-up) Add forgotten skipfinecalc in POD No test plan needed. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 153597 [details] [review] Bug 33444: Update AddRenewal to take a hashref of params Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> [EDIT] Removed skip_record_index => 1 from automatic_renewals.pl. See BZ.
Created attachment 153598 [details] [review] Bug 33444: (QA follow-up) Tidy block in automatic_renewals.pl Resolves: The file is less tidy than before (bad/messy lines before: 94, now: 101) Test plan: Run qa tools. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 153599 [details] [review] Bug 33444: (QA follow-up) Add forgotten skipfinecalc in POD No test plan needed. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Marcel de Rooy from comment #5) > misc/cronjobs/automatic_renewals.pl > > - my $date_due = AddRenewal( $auto_renew->borrowernumber, > $auto_renew->itemnumber, $auto_renew->branchcode, undef, undef, undef, 0, 1 > ); > + my $date_due = AddRenewal({ > + borrowernumber => $auto_renew->borrowernumber, > + itemnumber => $auto_renew->itemnumber, > + branch => $auto_renew->branchcode, > + seen => 0, > + automatic => 1, > + skip_record_index => 1 > > This is not the same. skip_record_index was not there before > Please clarify Adjusted in patch 1 in consultation with Nick on IRC.
Pushed to master for 23.11. Nice work everyone, thanks!
Not backporting in 23.05.x Method signature change should have a nice release note