Bug 36309 - create_superlibrarian.pl output could be more helpful
Summary: create_superlibrarian.pl output could be more helpful
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-03-13 17:54 UTC by Tomás Cohen Arazi
Modified: 2024-03-18 17:40 UTC (History)
4 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 36309: Make create_superlibrarian.pl output more useful (5.02 KB, patch)
2024-03-13 18:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 36309: Make create_superlibrarian.pl output more useful (5.06 KB, patch)
2024-03-13 22:03 UTC, David Nind
Details | Diff | Splinter Review
Bug 36309: Make create_superlibrarian.pl output more useful (5.12 KB, patch)
2024-03-18 17:39 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2024-03-13 17:54:23 UTC
This helpful script could provide better output instead of just exploding. For instance, it could display the valid values for branchcode and categorycode in the event of a broken FK while creating the patron.
Comment 1 Tomás Cohen Arazi 2024-03-13 18:28:56 UTC
Created attachment 163118 [details] [review]
Bug 36309: Make create_superlibrarian.pl output more useful

In situations in which you are not familiar with all the Koha settings,
and table structure, the fact this script just fails telling there's a
broken FK is just not practical.

We should capture those exceptions and display a useful message instead.

This script does that. It adds some validations and some exception
handling too. It prints a nice message about the bad value the user
passed, and the valid values too!

To test:
1. Run this on a fresh KTD:
   $ ktd --shell
  k$ perl  misc/devel/create_superlibrarian.pl \
      --userid tcohen \
      --password tomasito \
      --cardnumber 123456789 \
      --categorycode POT \
      --branchcode ATO
=> FAIL: It explodes with a MySQL exception message!
2. Apply this patch
3. Repeat 1
=> SUCCESS: It tells you which value is wrong and what values you can
pick to make the command work
4. Pick a valid value, and repeat
=> SUCCESS: Now the other value is wrong, a nice message is displayed!
5. Fix with a valid value and repeat
=> SUCCESS: Patron created!
6. Sign off :-D
Comment 2 Tomás Cohen Arazi 2024-03-13 18:34:22 UTC
Comment for QA:

I'm not 100% happy with our exception system as there's no consistent way to implement this without several manual checks and disambiguations... and disabling STDERR warns at DBIx::Class level to avoid noise for the end user.

That said, it is a discussion to have on another report. And the extra checks inside a transaction I implemented here cause no harm either. I think the benefits are worth the (tiny) extra overhead.
Comment 3 David Nind 2024-03-13 22:03:06 UTC
Created attachment 163124 [details] [review]
Bug 36309: Make create_superlibrarian.pl output more useful

In situations in which you are not familiar with all the Koha settings,
and table structure, the fact this script just fails telling there's a
broken FK is just not practical.

We should capture those exceptions and display a useful message instead.

This script does that. It adds some validations and some exception
handling too. It prints a nice message about the bad value the user
passed, and the valid values too!

To test:
1. Run this on a fresh KTD:
   $ ktd --shell
  k$ perl  misc/devel/create_superlibrarian.pl \
      --userid tcohen \
      --password tomasito \
      --cardnumber 123456789 \
      --categorycode POT \
      --branchcode ATO
=> FAIL: It explodes with a MySQL exception message!
2. Apply this patch
3. Repeat 1
=> SUCCESS: It tells you which value is wrong and what values you can
pick to make the command work
4. Pick a valid value, and repeat
=> SUCCESS: Now the other value is wrong, a nice message is displayed!
5. Fix with a valid value and repeat
=> SUCCESS: Patron created!
6. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2024-03-13 22:10:09 UTC
Testing notes (using KTD):

1. Before the patch, the message was:
   Invalid parameter passed, categorycode=POT does not exist

2. After patch - no changes to parameters:
   ERROR: 'ATO' is not valid for the 'branchcode' field
   Possible values are: CPL, FFL, FPL, FRL, IPT, LPL, MPL, PVL, RPL, SPL, TPL, UPL

3. After patch - change branchcode to valid value:
   ERROR: 'ATO' is not valid for the 'branchcode' field
   Possible values are: CPL, FFL, FPL, FRL, IPT, LPL, MPL, PVL, RPL, SPL, TPL, UPL

4. After path - change categorycode to valid value: no output, successfully adds user
Comment 5 Kyle M Hall 2024-03-18 17:39:37 UTC
Created attachment 163363 [details] [review]
Bug 36309: Make create_superlibrarian.pl output more useful

In situations in which you are not familiar with all the Koha settings,
and table structure, the fact this script just fails telling there's a
broken FK is just not practical.

We should capture those exceptions and display a useful message instead.

This script does that. It adds some validations and some exception
handling too. It prints a nice message about the bad value the user
passed, and the valid values too!

To test:
1. Run this on a fresh KTD:
   $ ktd --shell
  k$ perl  misc/devel/create_superlibrarian.pl \
      --userid tcohen \
      --password tomasito \
      --cardnumber 123456789 \
      --categorycode POT \
      --branchcode ATO
=> FAIL: It explodes with a MySQL exception message!
2. Apply this patch
3. Repeat 1
=> SUCCESS: It tells you which value is wrong and what values you can
pick to make the command work
4. Pick a valid value, and repeat
=> SUCCESS: Now the other value is wrong, a nice message is displayed!
5. Fix with a valid value and repeat
=> SUCCESS: Patron created!
6. Sign off :-D

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>