Attachment 104744 Details for Bug 25311 – [19.11.x] Bug 25311: Better error handling when updating a patron

[patch] [19.11.x] Bug 25311: Better error handling when updating a patron

Bug-25311-Better-error-handling-when-updating-a-pa.patch (text/plain), 7.95 KB, created by Jonathan Druart on 2020-05-12 08:48:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2020-05-12 08:48:30 UTC

Size: 7.95 KB

patch

obsolete

>From c80cd1ba537f5963b86faee1c1e034793641239e Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Wed, 29 Apr 2020 14:48:38 +0200
>Subject: [PATCH] Bug 25311: Better error handling when updating a patron
>
>Same as the precedent patch for patron's modification
>
>Test plan is identical but with an existing patron
>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>
>Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
>---
> .../prog/en/modules/members/memberentrygen.tt |  2 +
> members/memberentry.pl                        | 94 +++++++++++--------
> 2 files changed, 55 insertions(+), 41 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 65775445e4..2f6851b326 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
>@@ -40,6 +40,8 @@
>                 [% SWITCH message.error %]
>                     [% CASE 'error_on_insert_patron' %]
>                         <div class="dialog alert">Something went wrong when creating the patron. Check the logs.</div>
>+                    [% CASE 'error_on_update_patron' %]
>+                        <div class="dialog alert">Something went wrong when updating the patron. Check the logs.</div>
>                     [% CASE %]Unhandled error: [% message.error %]
>                 [% END %]
>             [% END %]
>diff --git a/members/memberentry.pl b/members/memberentry.pl
>index 6bcc6d44ff..2fbf25fd82 100755
>--- a/members/memberentry.pl
>+++ b/members/memberentry.pl
>@@ -432,6 +432,7 @@ if ( defined $sms ) {
> $nok = $nok || scalar(@errors);
> if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){
> 	$debug and warn "$op dates: " . join "\t", map {"$_: $newdata{$_}"} qw(dateofbirth dateenrolled dateexpiry);
>+    my $success;
> 	if ($op eq 'insert'){
> 		# we know it's not a duplicate borrowernumber or there would already be an error
>         delete $newdata{password2};
>@@ -443,6 +444,7 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){
>             push @messages, {error => 'error_on_insert_patron'};
>             $op = "add";
>         } else {
>+            $success = 1;
>             add_guarantors( $patron, $input );
>             $borrowernumber = $patron->borrowernumber;
>             $newdata{'borrowernumber'} = $borrowernumber;
>@@ -505,32 +507,6 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){
> 
>     } elsif ($op eq 'save') {
> 
>-        # Update or create our HouseboundRole if necessary.
>-        my $housebound_role = Koha::Patron::HouseboundRoles->find($borrowernumber);
>-        my ( $hsbnd_chooser, $hsbnd_deliverer ) = ( 0, 0 );
>-        $hsbnd_chooser = 1 if $input->param('housebound_chooser');
>-        $hsbnd_deliverer = 1 if $input->param('housebound_deliverer');
>-        if ( $housebound_role ) {
>-            if ( $hsbnd_chooser || $hsbnd_deliverer ) {
>-                # Update our HouseboundRole.
>-                $housebound_role
>-                    ->housebound_chooser($hsbnd_chooser)
>-                    ->housebound_deliverer($hsbnd_deliverer)
>-                    ->store;
>-            } else {
>-                $housebound_role->delete; # No longer needed.
>-            }
>-        } else {
>-            # Only create a HouseboundRole if patron has a role.
>-            if ( $hsbnd_chooser || $hsbnd_deliverer ) {
>-                $housebound_role = Koha::Patron::HouseboundRole->new({
>-                    borrowernumber_id    => $borrowernumber,
>-                    housebound_chooser   => $hsbnd_chooser,
>-                    housebound_deliverer => $hsbnd_deliverer,
>-                })->store;
>-            }
>-        }
>-
>         if ($NoUpdateLogin) {
>             delete $newdata{'password'};
>             delete $newdata{'userid'};
>@@ -540,24 +516,60 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){
>         $newdata{debarredcomment} = $newdata{debarred_comment};
>         delete $newdata{debarred_comment};
>         delete $newdata{password2};
>-        $patron->set(\%newdata)->store if scalar(keys %newdata) > 1; # bug 4508 - avoid crash if we're not
>-                                                                # updating any columns in the borrowers table,
>-                                                                # which can happen if we're only editing the
>-                                                                # patron attributes or messaging preferences sections
>-
>-        # should never raise an exception as password validity is checked above
>-        my $password = $newdata{password};
>-        if ( $password and $password ne '****' ) {
>-            $patron->set_password({ password => $password });
>-        }
> 
>-        add_guarantors( $patron, $input );
>-        if (C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) {
>-            C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template);
>+        eval {
>+            $patron->set(\%newdata)->store if scalar(keys %newdata) > 1; # bug 4508 - avoid crash if we're not
>+                                                                    # updating any columns in the borrowers table,
>+                                                                    # which can happen if we're only editing the
>+                                                                    # patron attributes or messaging preferences sections
>+        };
>+        if ( $@ ) {
>+            warn "Patron modification failed! - $@"; # Maybe we must die instead of just warn
>+            push @messages, {error => 'error_on_update_patron'};
>+            $op = "modify";
>+        } else {
>+
>+            $success = 1;
>+            # Update or create our HouseboundRole if necessary.
>+            my $housebound_role = Koha::Patron::HouseboundRoles->find($borrowernumber);
>+            my ( $hsbnd_chooser, $hsbnd_deliverer ) = ( 0, 0 );
>+            $hsbnd_chooser = 1 if $input->param('housebound_chooser');
>+            $hsbnd_deliverer = 1 if $input->param('housebound_deliverer');
>+            if ( $housebound_role ) {
>+                if ( $hsbnd_chooser || $hsbnd_deliverer ) {
>+                    # Update our HouseboundRole.
>+                    $housebound_role
>+                        ->housebound_chooser($hsbnd_chooser)
>+                        ->housebound_deliverer($hsbnd_deliverer)
>+                        ->store;
>+                } else {
>+                    $housebound_role->delete; # No longer needed.
>+                }
>+            } else {
>+                # Only create a HouseboundRole if patron has a role.
>+                if ( $hsbnd_chooser || $hsbnd_deliverer ) {
>+                    $housebound_role = Koha::Patron::HouseboundRole->new({
>+                        borrowernumber_id    => $borrowernumber,
>+                        housebound_chooser   => $hsbnd_chooser,
>+                        housebound_deliverer => $hsbnd_deliverer,
>+                    })->store;
>+                }
>+            }
>+
>+            # should never raise an exception as password validity is checked above
>+            my $password = $newdata{password};
>+            if ( $password and $password ne '****' ) {
>+                $patron->set_password({ password => $password });
>+            }
>+
>+            add_guarantors( $patron, $input );
>+            if (C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) {
>+                C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template);
>+            }
>         }
>-	}
>+    }
> 
>-    if ( $patron ) {
>+    if ( $success ) {
>         if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) {
>             C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $extended_patron_attributes);
>         }
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 25311: 103930 | 103931 | 103939 | 103940 | 104154 | 104155 | 104156 | 104157 | 104158 | 104743 | 104744 | 104745