Attachment 104154 Details for Bug 25311 – Bug 25311: Better error handling when updating a patron

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

Bug-25311-Better-error-handling-when-updating-a-pa.patch (text/plain), 8.04 KB, created by Katrin Fischer on 2020-05-01 22:25:00 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Katrin Fischer

Created: 2020-05-01 22:25:00 UTC

Size: 8.04 KB

patch

obsolete

>From bd45bd4b4e03a813a4d4d956727cb2df321b4c71 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 02f21e8e48..800af2122b 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
>@@ -62,6 +62,8 @@ legend:hover {
>                     [% 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 35eabfbb8a..be5cb22241 100755
>--- a/members/memberentry.pl
>+++ b/members/memberentry.pl
>@@ -437,6 +437,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};
>@@ -448,6 +449,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;
>@@ -507,32 +509,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'};
>@@ -542,24 +518,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')) {
>             $patron->extended_attributes->filter_by_branch_limitations->delete;
>             $patron->extended_attributes($extended_patron_attributes);
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

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