Bug 23883

Summary: sip_cli_emulator.pl - typo in parameter name
Product: Koha Reporter: Andrew Isherwood <bugzilla>
Component: Command-line UtilitiesAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: Pushed to main --- QA Contact: Marcel de Rooy <m.de.rooy>
Severity: trivial    
Priority: P5 - low CC: colin.campbell, david, jonathan.druart, joonas.kylmala, kyle, m.de.rooy, martin.renvoize, robin, tomascohen
Version: MainKeywords: release-notes-needed
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Trivial patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
25.11.00
Circulation function:
Bug Depends on: 13159    
Bug Blocks:    
Attachments: Bug 23883: SIP - Fix typo in parameter name
Bug 23883: SIP - Fix typo in parameter name
Bug 23883: sip_cli_emulator.pl should display real parameter names
Bug 23883: sip_cli_emulator.pl should display real parameter names
Bug 23883: sip_cli_emulator.pl should display real parameter names

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;
             }
Comment 5 Tomás Cohen Arazi (tcohen) 2025-06-13 19:55:53 UTC
Created attachment 183252 [details] [review]
Bug 23883: sip_cli_emulator.pl should display real parameter names

For creating this patch I looked at all parameters defined in the
handler mapping, removed duplicated ones and tracked down which ones
really were parameters in `GetOptions`. This included tracking down
variables that were later assigned other values and things like that.

The mapping is used to display the right parameter name if missing a
mandatory one.
Comment 6 David Nind 2025-06-14 05:56:16 UTC
Created attachment 183255 [details] [review]
Bug 23883: sip_cli_emulator.pl should display real parameter names

For creating this patch I looked at all parameters defined in the
handler mapping, removed duplicated ones and tracked down which ones
really were parameters in `GetOptions`. This included tracking down
variables that were later assigned other values and things like that.

The mapping is used to display the right parameter name if missing a
mandatory one.

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2025-06-14 05:57:24 UTC
Not sure that I tested properly, the test plan in the original works though.
Comment 8 Marcel de Rooy 2025-06-24 14:13:56 UTC
Created attachment 183460 [details] [review]
Bug 23883: sip_cli_emulator.pl should display real parameter names

For creating this patch I looked at all parameters defined in the
handler mapping, removed duplicated ones and tracked down which ones
really were parameters in `GetOptions`. This included tracking down
variables that were later assigned other values and things like that.

The mapping is used to display the right parameter name if missing a
mandatory one.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Lucas Gass (lukeg) 2025-06-24 21:26:15 UTC
Nice work everyone!

Pushed to main for 25.11