From df1f5da6734fe94b62e4b50fec9e7fadbfce794b Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Wed, 13 Mar 2024 14:56:27 -0300 Subject: [PATCH] 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 Signed-off-by: Kyle M Hall Signed-off-by: Katrin Fischer --- misc/devel/create_superlibrarian.pl | 80 ++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 12 deletions(-) diff --git a/misc/devel/create_superlibrarian.pl b/misc/devel/create_superlibrarian.pl index c0ab6fe6c6..dadf98ea04 100755 --- a/misc/devel/create_superlibrarian.pl +++ b/misc/devel/create_superlibrarian.pl @@ -18,12 +18,19 @@ # along with Koha; if not, see . use Modern::Perl; + use Getopt::Long qw( GetOptions ); -use Pod::Usage qw( pod2usage ); +use Pod::Usage qw( pod2usage ); +use Try::Tiny qw(catch try); -use Koha::Script; +use Koha::Database; +use Koha::Exceptions::Object; +use Koha::Libraries; +use Koha::Patron::Categories; use Koha::Patrons; +use Koha::Script; + my ( $help, $surname, $userid, $password, $branchcode, $categorycode, $cardnumber ); GetOptions( 'help|?' => \$help, @@ -41,16 +48,65 @@ pod2usage("branchcode is mandatory") unless $branchcode; pod2usage("categorycode is mandatory") unless $categorycode; pod2usage("cardnumber is mandatory") unless $cardnumber; -my $patron = Koha::Patron->new({ - surname => $surname, - userid => $userid, - cardnumber => $cardnumber, - branchcode => $branchcode, - categorycode => $categorycode, - flags => 1, -})->store; - -$patron->set_password({ password => $password, skip_validation => 1 }); +try { + Koha::Database->new->schema->txn_do( + sub { + Koha::Exceptions::Object::FKConstraint->throw( + error => 'Broken FK constraint', + broken_fk => 'branchcode' + ) unless Koha::Libraries->find($branchcode); + + Koha::Exceptions::Object::DuplicateID->throw( duplicate_id => 'userid' ) + if Koha::Patrons->find( { userid => $userid } ); + + Koha::Exceptions::Object::DuplicateID->throw( duplicate_id => 'cardnumber' ) + if Koha::Patrons->find( { cardnumber => $cardnumber } ); + + my $patron = Koha::Patron->new( + { + surname => $surname, + userid => $userid, + cardnumber => $cardnumber, + branchcode => $branchcode, + categorycode => $categorycode, + flags => 1, # superlibrarian + } + )->store; + + # password is set on a separate step (store would set the hashed password) + $patron->set_password( { password => $password, skip_validation => 1 } ); + } + ); +} catch { + if ( ref($_) eq 'Koha::Exceptions::Object::FKConstraint' ) { + + my $value = + $_->broken_fk eq 'branchcode' ? $branchcode + : $_->broken_fk eq 'categorycode' ? $categorycode + : 'ERROR'; + + my @valid_values = + $_->broken_fk eq 'branchcode' ? Koha::Libraries->new->get_column('branchcode') + : $_->broken_fk eq 'categorycode' ? Koha::Patron::Categories->new->get_column('categorycode') + : ('UNEXPECTED'); + + printf STDERR "ERROR: '%s' is not valid for the '%s' field\n", $value, $_->broken_fk; + printf STDERR "Possible values are: " . join( ', ', @valid_values ) . "\n"; + + } elsif ( ref($_) eq 'Koha::Exceptions::Object::DuplicateID' ) { + + my $value = + $_->duplicate_id eq 'cardnumber' ? $cardnumber + : $_->duplicate_id eq 'userid' ? $userid + : 'ERROR'; + + printf STDERR "Field '%s' must be unique. Value '%s' is used already.\n", $_->duplicate_id, $value; + } else { + print STDERR "Uncaught exception: $_\n"; + } + + exit 1; +}; =head1 NAME -- 2.39.5