Bugzilla – Attachment 147708 Details for
Bug 32426
Make userid generation pluggable
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 32426: Changes for members/memberentry.pl
Bug-32426-Changes-for-membersmemberentrypl.patch (text/plain), 7.53 KB, created by
Kyle M Hall
on 2023-03-03 16:08:48 UTC
(
hide
)
Description:
Bug 32426: Changes for members/memberentry.pl
Filename:
MIME Type:
Creator:
Kyle M Hall
Created:
2023-03-03 16:08:48 UTC
Size:
7.53 KB
patch
obsolete
>From 74ae4812ea6f9d3f01cb10f311315c79ebaddb4b Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Tue, 24 Jan 2023 15:34:51 +0100 >Subject: [PATCH] Bug 32426: Changes for members/memberentry.pl > >Test plan: >Note: We will address this again when installing a plugin, but >first we test with the legacy userid code. > >Add a new user with members/memberentry in staff. >Edit this user, change userid in staff. Try full form and partial >one. >If you remove userid or replace by a space (when mandatory), Koha >should regenerate a legacy userid. > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > members/memberentry.pl | 93 ++++++++++++++++++------------------------ > 1 file changed, 39 insertions(+), 54 deletions(-) > >diff --git a/members/memberentry.pl b/members/memberentry.pl >index 0193cc3777..7d58011f62 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -20,6 +20,7 @@ > > # pragma > use Modern::Perl; >+use Try::Tiny; > > # external modules > use CGI qw ( -utf8 ); >@@ -273,31 +274,7 @@ $newdata{'country'} = $input->param('country') if defined($input->param('country > > $newdata{'lang'} = $input->param('lang') if defined($input->param('lang')); > >-# builds default userid >-# userid input text may be empty or missing because of syspref BorrowerUnwantedField >-if ( ( defined $newdata{'userid'} && $newdata{'userid'} eq '' ) || $check_BorrowerUnwantedField =~ /userid/ && !defined $data{'userid'} ) { >- my $fake_patron = Koha::Patron->new; >- $fake_patron->userid($patron->userid) if $patron; # editing >- if ( ( defined $newdata{'firstname'} || $category->category_type eq 'I' ) && ( defined $newdata{'surname'} ) ) { >- # Full page edit, firstname and surname input zones are present >- $fake_patron->firstname($newdata{firstname}); >- $fake_patron->surname($newdata{surname}); >- $fake_patron->generate_userid; >- $newdata{'userid'} = $fake_patron->userid; >- } >- elsif ( ( defined $data{'firstname'} || $category->category_type eq 'I' ) && ( defined $data{'surname'} ) ) { >- # Partial page edit (access through "Details"/"Library details" tab), firstname and surname input zones are not used >- # Still, if the userid field is erased, we can create a new userid with available firstname and surname >- # FIXME clean thiscode newdata vs data is very confusing >- $fake_patron->firstname($data{firstname}); >- $fake_patron->surname($data{surname}); >- $fake_patron->generate_userid; >- $newdata{'userid'} = $fake_patron->userid; >- } >- else { >- $newdata{'userid'} = $data{'userid'}; >- } >-} >+# Bug 32426 removed the fake_patron code here that filled $newdata{userid}. We should leave it now to patron->store. > > my $extended_patron_attributes; > if ($op eq 'save' || $op eq 'insert'){ >@@ -350,15 +327,8 @@ if ($op eq 'save' || $op eq 'insert'){ > } > } > } >- # Check if the 'userid' is unique. 'userid' might not always be present in >- # the edited values list when editing certain sub-forms. Get it straight >- # from the DB if absent. >- my $userid = $newdata{ userid } // $borrower_data->{ userid }; >- my $p = $borrowernumber ? Koha::Patrons->find( $borrowernumber ) : Koha::Patron->new(); >- $p->userid( $userid ); >- unless ( $p->has_valid_userid ) { >- push @errors, "ERROR_login_exist"; >- } >+ >+ # Bug 32426 removed the userid unique-check here. Postpone it to patron->store. > > my $password = $input->param('password'); > my $password2 = $input->param('password2'); >@@ -428,15 +398,25 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ > if ($op eq 'insert'){ > # we know it's not a duplicate borrowernumber or there would already be an error > delete $newdata{password2}; >- $patron = eval { Koha::Patron->new(\%newdata)->store }; >- if ( $@ ) { >- # 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'}; >+ $success = 1; >+ $patron = try { >+ Koha::Patron->new(\%newdata)->store; >+ } catch { >+ $success = 0; >+ $nok = 1; >+ if( ref($_) eq 'Koha::Exceptions::Patron::InvalidUserid' ) { >+ push @errors, "ERROR_login_exist"; >+ } else { >+ # 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 { >- $success = 1; >+ return; >+ }; >+ >+ if ( $success ) { > add_guarantors( $patron, $input ); > $borrowernumber = $patron->borrowernumber; > $newdata{'borrowernumber'} = $borrowernumber; >@@ -444,7 +424,7 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ > } > > # If 'AutoEmailNewUser' syspref is on, email user their account details from the 'notice' that matches the user's branchcode. >- if ( C4::Context->preference("AutoEmailNewUser") ) { >+ if ( $patron && C4::Context->preference("AutoEmailNewUser") ) { > #look for defined primary email address, if blank - attempt to use borr.email and borr.emailpro instead > my $emailaddr = $patron->notice_email_address; > # if we manage to find a valid email address, send notice >@@ -513,19 +493,24 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ > > delete $newdata{password2}; > >- 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'}; >+ try { >+ $patron->set(\%newdata)->store if scalar(keys %newdata) > 1; >+ # bug 4508 - avoid crash if we're not updating any columns in the borrowers table (editing patron attrs or msg prefs) >+ $success = 1; >+ } catch { >+ $success = 0; >+ $nok = 1; >+ if( ref($_) eq 'Koha::Exceptions::Patron::InvalidUserid' ) { >+ push @errors, "ERROR_login_exist"; >+ } else { >+ warn "Patron modification failed! - $@"; # Maybe we must die instead of just warn >+ push @messages, {error => 'error_on_update_patron'}; >+ } > $op = "modify"; >- } else { >+ }; >+ >+ if ( $success ) { > >- $success = 1; > # Update or create our HouseboundRole if necessary. > my $housebound_role = Koha::Patron::HouseboundRoles->find($borrowernumber); > my ( $hsbnd_chooser, $hsbnd_deliverer ) = ( 0, 0 ); >-- >2.30.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 32426
:
146947
|
146948
|
146949
|
146950
|
146951
|
146952
|
146953
|
146954
|
147252
|
147253
|
147254
|
147255
|
147704
|
147705
|
147706
|
147707
|
147708
|
147709
|
147710
|
147711
|
147712
|
148203
|
148204
|
148205
|
148206
|
148207
|
148208
|
148209
|
148210
|
148211
|
148782
|
148793