From b9f2de194fb4b6e4c223fc0a6d0c2bfd41c219f9 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 29 Apr 2020 14:38:41 +0200 Subject: [PATCH] Bug 25311: Better error handling when creating a patron This is still not ideal but brings a bit of enhancement. One possible problem is that the patron creation will fail if the streetnumber field is too long (borrowers.streetnumber is varchar(10). Test plan: 0. Don't apply this patch 1. Create a new patron with a streetnumber longer than 10 characters 2. Save => The patron has not been created and the app explodes The error is about extended_attributes and not meaningful Can't call method "extended_attributes" on an undefined value at /kohadevbox/koha/members/memberentry.pl line 560 3. Apply the patch 4. Repeat 1. and 2 => You get a warning on the interface and you still see the creation form 5. Check the logs => The error is meaningful "Data too long for column 'streetnumber'" Signed-off-by: Martin Renvoize Signed-off-by: Katrin Fischer --- .../prog/en/modules/members/memberentrygen.tt | 11 ++++++ members/memberentry.pl | 39 +++++++++++-------- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt index 1be9e1f95e..65775445e4 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt @@ -34,6 +34,17 @@
+ + [% IF messages %] + [% FOR message IN messages %] + [% SWITCH message.error %] + [% CASE 'error_on_insert_patron' %] +
Something went wrong when creating the patron. Check the logs.
+ [% CASE %]Unhandled error: [% message.error %] + [% END %] + [% END %] + [% END %] + [% IF ( opadd ) %]
[% ELSE %] diff --git a/members/memberentry.pl b/members/memberentry.pl index d5b0f08d55..6bcc6d44ff 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -103,6 +103,7 @@ my @errors; my $borrower_data; my $NoUpdateLogin; my $userenv = C4::Context->userenv; +my @messages; ## Deal with guarantor stuff $template->param( relationships => scalar $patron->guarantor_relationships ) if $patron; @@ -439,6 +440,8 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ # FIXME Urgent error handling here, we cannot fail without relevant feedback # Lot of code will need to be removed from this script to handle exceptions raised by Koha::Patron->store warn "Patron creation failed! - $@"; # Maybe we must die instead of just warn + push @messages, {error => 'error_on_insert_patron'}; + $op = "add"; } else { add_guarantors( $patron, $input ); $borrowernumber = $patron->borrowernumber; @@ -479,10 +482,10 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ } } - if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) { + if ($patron && C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) { C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $extended_patron_attributes); } - if (C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) { + if ($patron && C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) { C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template, 1, $newdata{'categorycode'}); } @@ -492,7 +495,7 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ $hsbnd_chooser = 1 if $input->param('housebound_chooser'); $hsbnd_deliverer = 1 if $input->param('housebound_deliverer'); # Only create a HouseboundRole if patron has a role. - if ( $hsbnd_chooser || $hsbnd_deliverer ) { + if ( $patron && ( $hsbnd_chooser || $hsbnd_deliverer ) ) { Koha::Patron::HouseboundRole->new({ borrowernumber_id => $borrowernumber, housebound_chooser => $hsbnd_chooser, @@ -549,25 +552,28 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ } add_guarantors( $patron, $input ); - if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) { - C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $extended_patron_attributes); - } if (C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) { C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template); } } - if ( $destination eq 'circ' and not C4::Auth::haspermission( C4::Context->userenv->{id}, { circulate => 'circulate_remaining_permissions' } ) ) { - # If we want to redirect to circulation.pl and need to check if the logged in user has the necessary permission - $destination = 'not_circ'; + if ( $patron ) { + if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) { + C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $extended_patron_attributes); + } + + if ( $destination eq 'circ' and not C4::Auth::haspermission( C4::Context->userenv->{id}, { circulate => 'circulate_remaining_permissions' } ) ) { + # If we want to redirect to circulation.pl and need to check if the logged in user has the necessary permission + $destination = 'not_circ'; + } + print scalar( $destination eq "circ" ) + ? $input->redirect( + "/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber") + : $input->redirect( + "/cgi-bin/koha/members/moremember.pl?borrowernumber=$borrowernumber" + ); + exit; # You can only send 1 redirect! After that, content or other headers don't matter. } - print scalar( $destination eq "circ" ) - ? $input->redirect( - "/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrowernumber") - : $input->redirect( - "/cgi-bin/koha/members/moremember.pl?borrowernumber=$borrowernumber" - ); - exit; # You can only send 1 redirect! After that, content or other headers don't matter. } if ($delete){ @@ -828,6 +834,7 @@ if ( C4::Context->preference('TranslateNotices') ) { $template->param( languages => $translated_languages ); } +$template->param( messages => \@messages ); output_html_with_http_headers $input, $cookie, $template->output; sub parse_extended_patron_attributes { -- 2.20.1