Bug 33444

Summary: AddRenewal should take a hash of parameters
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: Architecture, internals, and plumbingAssignee: Nick Clemens (kidclamp) <nick>
Status: RESOLVED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00
Circulation function:
Bug Depends on: 32013    
Bug Blocks:    
Attachments: Bug 33444: Update AddRenewal to take a hashref of params
Bug 33444: Update AddRenewal to take a hashref of params
Bug 33444: Update AddRenewal to take a hashref of params
Bug 33444: (QA follow-up) Tidy block in automatic_renewals.pl
Bug 33444: (QA follow-up) Add forgotten skipfinecalc in POD
Bug 33444: Update AddRenewal to take a hashref of params
Bug 33444: (QA follow-up) Tidy block in automatic_renewals.pl
Bug 33444: (QA follow-up) Add forgotten skipfinecalc in POD

Description Nick Clemens (kidclamp) 2023-04-07 13:43:24 UTC
The routine (after bug 32013) now takes 9 parameters - this is unsustainable as a list
Comment 1 Nick Clemens (kidclamp) 2023-04-07 15:00:30 UTC
Created attachment 149280 [details] [review]
Bug 33444: Update AddRenewal to take a hashref of params
Comment 2 Jonathan Druart 2023-05-16 14:06:17 UTC
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>
Comment 3 Marcel de Rooy 2023-05-17 13:58:00 UTC
Feels like an enh ?
Comment 4 Marcel de Rooy 2023-07-18 11:23:59 UTC
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.
Comment 5 Marcel de Rooy 2023-07-18 11:39:35 UTC
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
Comment 6 Marcel de Rooy 2023-07-18 12:20:44 UTC
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>
Comment 7 Marcel de Rooy 2023-07-18 12:20:46 UTC
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>
Comment 8 Marcel de Rooy 2023-07-18 12:20:48 UTC
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>
Comment 9 Marcel de Rooy 2023-07-18 12:32:44 UTC
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.
Comment 10 Marcel de Rooy 2023-07-18 12:32:47 UTC
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>
Comment 11 Marcel de Rooy 2023-07-18 12:32:49 UTC
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>
Comment 12 Marcel de Rooy 2023-07-18 12:33:16 UTC
(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.
Comment 13 Tomás Cohen Arazi (tcohen) 2023-07-19 15:08:37 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 14 Fridolin Somers 2023-08-04 01:40:08 UTC
Not backporting in 23.05.x

Method signature change should have a nice release note