Attachment 131129 Details for Bug 29771 – Bug 29771: Remove trivial cases

[patch] Bug 29771: Remove trivial cases

Bug-29771-Remove-trivial-cases.patch (text/plain), 4.70 KB, created by Nick Clemens (kidclamp) on 2022-02-25 19:20:24 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2022-02-25 19:20:24 UTC

Size: 4.70 KB

patch

obsolete

>From 072179fd4d49c9943305b67c999060cf1cb7e96a Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Thu, 23 Dec 2021 16:16:58 -0300
>Subject: [PATCH] Bug 29771: Remove trivial cases
>
>I squashed the patches because they are too trivial to have a test plan.
>Or it is too much work to write the test plan for such trivial cases. I
>leave the original commit messages just in case.
>
>Generally, this are all cases in which CGI::param is being called in a
>trivially identifiable _list context_. i.e. they are assigned to a
>@variable.
>
>I left one case out on purpose: admin/auth_subfields_structure.pl
>
>Paul introduced this:
>
>my @kohafield = ''.$input->param('kohafield');
>and then:
>my $kohafield = $kohafield[$i];
>
>My intuition says it is forcing scalar context on the first assignment
>so the list contains a single element and then inside the loop some
>$kohafield assignments should lead to undef, and even warnings. I leave
>it for a separate patch because it is not that easy testable and is a
>sensible area.
>
>Bug 29771: Remove warning from  acqui/finishreceive.pl
>
>This patch removes a warning that shows when receiving.
>
>To test:
>1. Do the acq workflow up to the receive step.
>2. Once you choose the items and click on Finish
>=> FAIL: There's a warning in the logs
>3. Revert receipt
>4. Apply this patch
>5. Receive
>=> SUCCESS: No more warnings
>6. Sign off :-D
>
>Bug 29771: Remove warning from svc/members/add_to_list
>
>To test:
>1. Run:
>   $ tail -f /var/log/koha/kohadev/*-error.log
>2. Generate a patron list
>3. Perform a patron search that gives you a few
>4. Select some, and choose to add them to the list
>=> FAIL: The logs show the infamous warn:
>CGI::param called in list context from /kohadevbox/koha/svc/members/add_to_list
>5. Apply this patch
>6. Restart plack and repeat 4
>=> SUCCESS: No warn!
>7. Sign off :-D
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>
>Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
>---
> acqui/finishreceive.pl    | 2 +-
> svc/cover_images          | 2 +-
> svc/members/add_to_list   | 6 +++---
> tools/import_borrowers.pl | 2 +-
> tools/stockrotation.pl    | 2 +-
> 5 files changed, 7 insertions(+), 7 deletions(-)
>
>diff --git a/acqui/finishreceive.pl b/acqui/finishreceive.pl
>index 94b73d53a4..c2d51686eb 100755
>--- a/acqui/finishreceive.pl
>+++ b/acqui/finishreceive.pl
>@@ -80,7 +80,7 @@ my $basket = $order_obj->basket;
> if ($quantityrec > $origquantityrec ) {
>     my @received_items = ();
>     if ($basket->effective_create_items eq 'ordering') {
>-        @received_items = $input->param('items_to_receive');
>+        @received_items = $input->multi_param('items_to_receive');
>         my @affects = split q{\|}, C4::Context->preference("AcqItemSetSubfieldsWhenReceived");
>         if ( @affects ) {
>             my $frameworkcode = GetFrameworkCode($biblionumber);
>diff --git a/svc/cover_images b/svc/cover_images
>index 7531e8c2b4..a1d487a664 100755
>--- a/svc/cover_images
>+++ b/svc/cover_images
>@@ -37,7 +37,7 @@ if ( $auth_status ne "ok" ) {
> }
> 
> my $action       = $input->param('action');
>-my @imagenumbers = $input->param('imagenumber');
>+my @imagenumbers = $input->multi_param('imagenumber');
> 
> # Array to store the reponse JSON
> my $response = [];
>diff --git a/svc/members/add_to_list b/svc/members/add_to_list
>index fd23e9c842..2219717003 100755
>--- a/svc/members/add_to_list
>+++ b/svc/members/add_to_list
>@@ -32,9 +32,9 @@ my ( $auth_status ) = check_cookie_auth(
> );
> 
> exit 0 if $auth_status ne "ok";
>-my $add_to_patron_list       = $input->param('add_to_patron_list');
>-my $new_patron_list          = $input->param('new_patron_list');
>-my @borrowernumbers          = $input->param('borrowernumbers[]');
>+my $add_to_patron_list = $input->param('add_to_patron_list');
>+my $new_patron_list    = $input->param('new_patron_list');
>+my @borrowernumbers    = $input->multi_param('borrowernumbers[]');
> 
> my $response;
> if ($add_to_patron_list) {
>diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl
>index d02d671d41..b635b46e0a 100755
>--- a/tools/import_borrowers.pl
>+++ b/tools/import_borrowers.pl
>@@ -93,7 +93,7 @@ if ( $input->param('sample') ) {
>     exit 0;
> }
> 
>-my @preserve_fields = $input->param('preserve_existing');
>+my @preserve_fields = $input->multi_param('preserve_existing');
> 
> my $uploadborrowers = $input->param('uploadborrowers');
> my $matchpoint      = $input->param('matchpoint');
>diff --git a/tools/stockrotation.pl b/tools/stockrotation.pl
>index 04ff184da5..deaad70dcd 100755
>--- a/tools/stockrotation.pl
>+++ b/tools/stockrotation.pl
>@@ -376,7 +376,7 @@ if (!defined $op) {
> } elsif ($op eq 'move_items_to_rota') {
> 
>     # The barcodes of the items we're moving
>-    my @move = $input->param('move_item');
>+    my @move = $input->multi_param('move_item');
> 
>     foreach my $item(@move) {
> 
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 29771: 128908 | 128909 | 128910 | 131129 | 131130 | 131187 | 131188