Bug 23883 - sip_cli_emulator.pl - typo in parameter name
Summary: sip_cli_emulator.pl - typo in parameter name
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Marcel de Rooy
URL:
Keywords: release-notes-needed
Depends on: 13159
Blocks:
  Show dependency treegraph
 
Reported: 2019-10-23 10:33 UTC by Andrew Isherwood
Modified: 2025-06-24 21:26 UTC (History)
9 users (show)

See Also:
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:


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
Bug 23883: sip_cli_emulator.pl should display real parameter names (2.28 KB, patch)
2025-06-13 19:55 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 23883: sip_cli_emulator.pl should display real parameter names (2.33 KB, patch)
2025-06-14 05:56 UTC, David Nind
Details | Diff | Splinter Review
Bug 23883: sip_cli_emulator.pl should display real parameter names (2.43 KB, patch)
2025-06-24 14:13 UTC, Marcel de Rooy
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;
             }
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