Bug 23883 - sip_cli_emulator.pl - typo in parameter name
Summary: sip_cli_emulator.pl - typo in parameter name
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 13159
Blocks:
  Show dependency treegraph
 
Reported: 2019-10-23 10:33 UTC by Andrew Isherwood
Modified: 2023-09-12 14:43 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 23883: SIP - Fix typo in parameter name (1.21 KB, patch)
2019-12-17 13:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23883: SIP - Fix typo in parameter name (1.28 KB, patch)
2019-12-17 14:55 UTC, Andrew Isherwood
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Isherwood 2019-10-23 10:33:33 UTC
When trying to make a fee payment via sip_cli_emulator.pl, if a payment amount is not passed, the error reports "fee_amount is required for fee_paid", the parameter is actually "fee-amount", not "fee_amount"
Comment 1 Jonathan Druart 2019-12-17 13:38:17 UTC
Created attachment 96374 [details] [review]
Bug 23883: SIP - Fix typo in parameter name

The options of the script have hypens whereas the hash keys have
underscore.
In my understanding the problem only occurs on display purpose.

Test plan:
Try a sip command like
   % perl misc/sip_cli_emulator.pl -a localhost -su term1 -sp term1 -l CPL -m fee_paid

Without this patch you will see:
  fee_amount is required for fee_paid
With this patch the correct option is listed:
  fee-amount is required for fee_paid
Comment 2 Andrew Isherwood 2019-12-17 14:55:51 UTC
Created attachment 96382 [details] [review]
Bug 23883: SIP - Fix typo in parameter name

The options of the script have hypens whereas the hash keys have
underscore.
In my understanding the problem only occurs on display purpose.

Test plan:
Try a sip command like
   % perl misc/sip_cli_emulator.pl -a localhost -su term1 -sp term1 -l CPL -m fee_paid

Without this patch you will see:
  fee_amount is required for fee_paid
With this patch the correct option is listed:
  fee-amount is required for fee_paid

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Comment 3 Joonas Kylmälä 2019-12-20 14:02:35 UTC
Well, this same issue happens if you don't specify the --patron parameter:

> patron_identifier is required for fee_paid

Here if instead of --patron you try --patron-identifier (patron_identifier with hyphen instead) it doesn't work. So the proposed fix doesn't really solve this problem but only the specific case of --fee-amount parameter.
Comment 4 Jonathan Druart 2019-12-23 13:38:33 UTC
Maybe that should be moved one line above, but I am not sure how to test that correctly actually.

diff --git a/misc/sip_cli_emulator.pl b/misc/sip_cli_emulator.pl
index 1b987279b5..f28c067674 100755
--- a/misc/sip_cli_emulator.pl
+++ b/misc/sip_cli_emulator.pl
@@ -319,8 +319,8 @@ sub build_command_message {

     foreach my $key ( keys %$parameters ) {
         unless ( $parameters->{$key} ) {
+            $key =~ s|_|-|g; # options of this script have hyphens
             unless ( $optional{$key} ) {
-                $key =~ s|_|-|g; # options of this script have hyphens
                 say "$key is required for $message";
                 return;
             }