Bug 28870: Remove traces of Email::Valid
[koha-ffzg.git] / members / memberentry.pl
index 1e8241d..eda147c 100755 (executable)
@@ -23,41 +23,31 @@ use Modern::Perl;
 
 # external modules
 use CGI qw ( -utf8 );
-use List::MoreUtils qw/uniq/;
 
 # internal modules
-use C4::Auth;
+use C4::Auth qw( get_template_and_user haspermission );
 use C4::Context;
-use C4::Output;
-use C4::Members;
-use C4::Members::Attributes;
-use C4::Members::AttributeTypes;
-use C4::Koha;
-use C4::Log;
-use C4::Letters;
+use C4::Output qw( output_and_exit output_and_exit_if_error output_html_with_http_headers );
+use C4::Members qw( checkcardnumber get_cardnumber_length );
+use C4::Koha qw( GetAuthorisedValues );
+use C4::Letters qw( SendAlerts );
 use C4::Form::MessagingPreferences;
 use Koha::AuthUtils;
 use Koha::AuthorisedValues;
-use Koha::Patron::Debarments;
+use Koha::Patron::Debarments qw( AddDebarment DelDebarment GetDebarments );
 use Koha::Cities;
-use Koha::DateUtils;
+use Koha::DateUtils qw( dt_from_string output_pref );
 use Koha::Libraries;
 use Koha::Patrons;
+use Koha::Patron::Attribute::Types;
 use Koha::Patron::Categories;
 use Koha::Patron::HouseboundRole;
 use Koha::Patron::HouseboundRoles;
 use Koha::Token;
-use Email::Valid;
+use Email::Address;
 use Koha::SMS::Providers;
 
-use vars qw($debug);
-
-BEGIN {
-       $debug = $ENV{DEBUG} || 0;
-}
-       
-my $input = new CGI;
-($debug) or $debug = $input->param('debug') || 0;
+my $input = CGI->new;
 my %data;
 
 my $dbh = C4::Context->dbh;
@@ -66,9 +56,7 @@ my ($template, $loggedinuser, $cookie)
     = get_template_and_user({template_name => "members/memberentrygen.tt",
            query => $input,
            type => "intranet",
-           authnotrequired => 0,
            flagsrequired => {borrowers => 'edit_borrowers'},
-           debug => ($debug) ? 1 : 0,
        });
 
 my $borrowernumber = $input->param('borrowernumber');
@@ -102,11 +90,18 @@ my $step          = $input->param('step') || 0;
 my @errors;
 my $borrower_data;
 my $NoUpdateLogin;
+my $NoUpdateEmail;
 my $userenv = C4::Context->userenv;
+my @messages;
 
 ## Deal with guarantor stuff
 $template->param( relationships => scalar $patron->guarantor_relationships ) if $patron;
 
+my @relations = split /\|/, C4::Context->preference('borrowerRelationship'), -1;
+@relations = ('') unless @relations;
+my $empty_relationship_allowed = grep {$_ eq ""} @relations;
+$template->param( empty_relationship_allowed => $empty_relationship_allowed );
+
 my $guarantor_id = $input->param('guarantor_id');
 my $guarantor = undef;
 $guarantor = Koha::Patrons->find( $guarantor_id ) if $guarantor_id;
@@ -166,6 +161,11 @@ if ( $op eq 'modify' or $op eq 'save' or $op eq 'duplicate' ) {
     my $logged_in_user = Koha::Patrons->find( $loggedinuser );
     output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } );
 
+    # check permission to modify email info.
+    if ( $patron->is_superlibrarian && !$logged_in_user->is_superlibrarian ) {
+        $NoUpdateEmail = 1;
+    }
+
     $borrower_data = $patron->unblessed;
     $borrower_data->{category_type} = $patron->category->category_type;
 }
@@ -201,12 +201,12 @@ if ( $op eq 'insert' || $op eq 'modify' || $op eq 'save' || $op eq 'duplicate' )
         if ( $formatteddate ) {
             $newdata{$_} = $formatteddate;
         } else {
-            ($userdate eq '0000-00-00') and warn "Data error: $_ is '0000-00-00'";
             $template->param( "ERROR_$_" => 1 );
             push(@errors,"ERROR_$_");
         }
     }
-  # check permission to modify login info.
+
+    # check permission to modify login info.
     if (ref($borrower_data) && ($borrower_data->{'category_type'} eq 'S') && ! (C4::Auth::haspermission($userenv->{'id'},{'staffaccess'=>1})) )  {
         $NoUpdateLogin = 1;
     }
@@ -233,7 +233,7 @@ if ( $op eq 'insert' || $op eq 'modify' || $op eq 'save' || $op eq 'duplicate' )
         qr/^\d+-DAYS/,
         qr/^patron_attr_/,
         qr/^csrf_token$/,
-        qr/^add_debarment$/, qr/^debarred_expiration$/, qr/^remove_debarment$/, # We already dealt with debarments previously
+        qr/^add_debarment$/, qr/^debarred_comment$/,qr/^debarred_expiration$/, qr/^remove_debarment$/, # We already dealt with debarments previously
         qr/^housebound_chooser$/, qr/^housebound_deliverer$/,
         qr/^select_city$/,
         qr/^new_guarantor_/,
@@ -250,11 +250,10 @@ if ( $op eq 'insert' || $op eq 'modify' || $op eq 'save' || $op eq 'duplicate' )
 
 # Test uniqueness of surname, firstname and dateofbirth
 if ( ( $op eq 'insert' ) and !$nodouble ) {
+    my @dup_fields = split '\|', C4::Context->preference('PatronDuplicateMatchingAddFields');
     my $conditions;
-    $conditions->{surname} = $newdata{surname} if $newdata{surname};
-    if ( $category_type ne 'I' ) {
-        $conditions->{firstname} = $newdata{firstname} if $newdata{firstname};
-        $conditions->{dateofbirth} = $newdata{dateofbirth} if $newdata{dateofbirth};
+    for my $f ( @dup_fields ) {
+        $conditions->{$f} = $newdata{$f} if $newdata{$f};
     }
     $nodouble = 1;
     my $patrons = Koha::Patrons->search($conditions); # FIXME Should be search_limited?
@@ -310,9 +309,8 @@ if ( ( defined $newdata{'userid'} && $newdata{'userid'} eq '' ) || $check_Borrow
         $newdata{'userid'} = $data{'userid'};
     }
 }
-  
-$debug and warn join "\t", map {"$_: $newdata{$_}"} qw(dateofbirth dateenrolled dateexpiry);
-my $extended_patron_attributes = ();
+
+my $extended_patron_attributes;
 if ($op eq 'save' || $op eq 'insert'){
 
     output_and_exit( $input, $cookie, $template,  'wrong_csrf_token' )
@@ -354,7 +352,6 @@ if ($op eq 'save' || $op eq 'insert'){
   
   if (C4::Context->preference("IndependentBranches")) {
     unless ( C4::Context->IsSuperLibrarian() ){
-      $debug and print STDERR "  $newdata{'branchcode'} : ".$userenv->{flags}.":".$userenv->{branch};
       unless (!$newdata{'branchcode'} || $userenv->{branch} eq $newdata{'branchcode'}){
         push @errors, "ERROR_branch";
       }
@@ -364,7 +361,7 @@ if ($op eq 'save' || $op eq 'insert'){
   # 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;
+  my $p = $borrowernumber ? Koha::Patrons->find( $borrowernumber ) : Koha::Patron->new();
   $p->userid( $userid );
   unless ( $p->has_valid_userid ) {
     push @errors, "ERROR_login_exist";
@@ -375,7 +372,7 @@ if ($op eq 'save' || $op eq 'insert'){
   push @errors, "ERROR_password_mismatch" if ( $password ne $password2 );
 
   if ( $password and $password ne '****' ) {
-      my ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $password );
+      my ( $is_valid, $error ) = Koha::AuthUtils::is_password_valid( $password, Koha::Patron::Categories->find($categorycode) );
       unless ( $is_valid ) {
           push @errors, 'ERROR_password_too_short' if $error eq 'too_short';
           push @errors, 'ERROR_password_too_weak' if $error eq 'too_weak';
@@ -389,30 +386,35 @@ if ($op eq 'save' || $op eq 'insert'){
   my $emailalt = $input->param('B_email');
 
   if ($emailprimary) {
-      push (@errors, "ERROR_bad_email") if (!Email::Valid->address($emailprimary));
+      push (@errors, "ERROR_bad_email") if ($emailprimary !~ m/$Email::Address::mailbox/);
   }
   if ($emailsecondary) {
-      push (@errors, "ERROR_bad_email_secondary") if (!Email::Valid->address($emailsecondary));
+      push (@errors, "ERROR_bad_email_secondary") if ($emailsecondary !~ m/$Email::Address::mailbox/);
   }
   if ($emailalt) {
-      push (@errors, "ERROR_bad_email_alternative") if (!Email::Valid->address($emailalt));
+      push (@errors, "ERROR_bad_email_alternative") if ($emailalt !~ m/$Email::Address::mailbox/);
   }
 
-  if (C4::Context->preference('ExtendedPatronAttributes')) {
-    $extended_patron_attributes = parse_extended_patron_attributes($input);
-    foreach my $attr (@$extended_patron_attributes) {
-        unless (C4::Members::Attributes::CheckUniqueness($attr->{code}, $attr->{value}, $borrowernumber)) {
-            my $attr_info = C4::Members::AttributeTypes->fetch($attr->{code});
-            push @errors, "ERROR_extended_unique_id_failed";
-            $template->param(
-                ERROR_extended_unique_id_failed_code => $attr->{code},
-                ERROR_extended_unique_id_failed_value => $attr->{value},
-                ERROR_extended_unique_id_failed_description => $attr_info->description()
-            );
-        }
-    }
+  if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) {
+      $extended_patron_attributes = parse_extended_patron_attributes($input);
+      for my $attr ( @$extended_patron_attributes ) {
+          $attr->{borrowernumber} = $borrowernumber if $borrowernumber;
+          my $attribute = Koha::Patron::Attribute->new($attr);
+          if ( !$attribute->unique_ok ) {
+              push @errors, "ERROR_extended_unique_id_failed";
+              my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
+              $template->param(
+                  ERROR_extended_unique_id_failed_code => $attr->{code},
+                  ERROR_extended_unique_id_failed_value => $attr->{attribute},
+                  ERROR_extended_unique_id_failed_description => $attr_type->description()
+              );
+          }
+      }
   }
 }
+elsif ( $borrowernumber ) {
+    $extended_patron_attributes = Koha::Patrons->find($borrowernumber)->extended_attributes->unblessed;
+}
 
 if ( ($op eq 'modify' || $op eq 'insert' || $op eq 'save'|| $op eq 'duplicate') and ($step == 0 or $step == 3 )){
     unless ($newdata{'dateexpiry'}){
@@ -430,16 +432,19 @@ if ( defined $sms ) {
 ###  Error checks should happen before this line.
 $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};
-        my $patron = eval { Koha::Patron->new(\%newdata)->store };
+        $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'};
+            $op = "add";
         } else {
+            $success = 1;
             add_guarantors( $patron, $input );
             $borrowernumber = $patron->borrowernumber;
             $newdata{'borrowernumber'} = $borrowernumber;
@@ -479,10 +484,7 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){
             }
         }
 
-        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')) {
+        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 +494,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,
@@ -502,72 +504,91 @@ 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'};
         }
 
-        my $patron = Koha::Patrons->find( $borrowernumber );
-        $newdata{debarredcomment} = $newdata{debarred_comment};
-        delete $newdata{debarred_comment};
+        $patron = Koha::Patrons->find( $borrowernumber );
+
+        if ($NoUpdateEmail) {
+            delete $newdata{'email'};
+            delete $newdata{'emailpro'};
+            delete $newdata{'B_email'};
+        }
+
         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 });
+
+        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);
+            }
         }
+    }
 
-        add_guarantors( $patron, $input );
+    if ( $success ) {
         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);
+            $patron->extended_attributes->filter_by_branch_limitations->delete;
+            $patron->extended_attributes($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';
+        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){
@@ -593,14 +614,15 @@ if (C4::Context->preference("IndependentBranches")) {
         }
     }
 }
+
+# Define the fields to be pre-filled in guarantee records
+my $prefillguarantorfields=C4::Context->preference("PrefillGuaranteeField");
+my @prefill_fields=split(/\,/,$prefillguarantorfields);
+
 if ($op eq 'add'){
-    if ( $guarantor_id ) {
-        my $guarantordata = $guarantor->unblessed;
-        foreach (qw(streetnumber address streettype address2
-          zipcode country city state phone phonepro mobile fax email emailpro branchcode
-          B_streetnumber B_streettype B_address B_address2
-          B_city B_state B_zipcode B_country B_email B_phone)) {
-              $newdata{$_} = $guarantordata->{$_};
+    if ($guarantor_id) {
+        foreach (@prefill_fields) {
+            $newdata{$_} = $guarantor->$_;
         }
     }
     $template->param( updtype => 'I', step_1=>1, step_2=>1, step_3=>1, step_4=>1, step_5 => 1, step_6 => 1, step_7 => 1);
@@ -624,6 +646,8 @@ if(!defined($data{'sex'})){
     $template->param( female => 1);
 } elsif ($data{'sex'} eq 'M'){
     $template->param(  male => 1);
+} elsif ($data{'sex'} eq 'O') {
+    $template->param( other => 1);
 } else {
     $template->param(  none => 1);
 }
@@ -634,7 +658,7 @@ my @typeloop;
 my $no_categories = 1;
 my $no_add;
 foreach my $category_type (qw(C A S P I X)) {
-    my $patron_categories = Koha::Patron::Categories->search_limited({ category_type => $category_type }, {order_by => ['categorycode']});
+    my $patron_categories = Koha::Patron::Categories->search_with_library_limits({ category_type => $category_type }, {order_by => ['categorycode']});
     $no_categories = 0 if $patron_categories->count > 0;
 
     my @categoryloop;
@@ -642,6 +666,8 @@ foreach my $category_type (qw(C A S P I X)) {
         push @categoryloop,
           { 'categorycode' => $patron_category->categorycode,
             'categoryname' => $patron_category->description,
+            'effective_min_password_length' => $patron_category->effective_min_password_length,
+            'effective_require_strong_password' => $patron_category->effective_require_strong_password,
             'categorycodeselected' =>
               ( defined($categorycode) && $patron_category->categorycode eq $categorycode ),
           };
@@ -662,9 +688,7 @@ $template->param(
 );
 
 my $cities = Koha::Cities->search( {}, { order_by => 'city_name' } );
-my $roadtypes = C4::Koha::GetAuthorisedValues( 'ROADTYPE' );
 $template->param(
-    roadtypes => $roadtypes,
     cities    => $cities,
 );
 
@@ -734,6 +758,7 @@ $template->param(no_add => $no_add);
 
 $template->param( sort1 => $data{'sort1'});
 $template->param( sort2 => $data{'sort2'});
+$template->param( autorenew => $data{'autorenew'});
 
 if ($nok) {
     foreach my $error (@errors) {
@@ -764,8 +789,8 @@ foreach (qw(dateenrolled dateexpiry dateofbirth)) {
     $template->param( $_ => $data{$_});
 }
 
-if (C4::Context->preference('ExtendedPatronAttributes')) {
-    patron_attributes_form($template, $borrowernumber, $op);
+if ( C4::Context->preference('ExtendedPatronAttributes') ) {
+    patron_attributes_form( $template, $extended_patron_attributes, $op );
 }
 
 if (C4::Context->preference('EnhancedMessagingPreferences')) {
@@ -780,7 +805,6 @@ if (C4::Context->preference('EnhancedMessagingPreferences')) {
 }
 
 $template->param( "show_guarantor" => ( $category_type =~ /A|I|S|X/ ) ? 0 : 1 ); # associate with step to know where you are
-$debug and warn "memberentry step: $step";
 $template->param(%data);
 $template->param( "step_$step"  => 1) if $step;        # associate with step to know where u are
 $template->param(  step  => $step   ) if $step;        # associate with step to know where u are
@@ -804,6 +828,7 @@ $template->param(
   modify          => $modify,
   nok     => $nok,#flag to know if an error
   NoUpdateLogin =>  $NoUpdateLogin,
+  NoUpdateEmail =>  $NoUpdateEmail,
   );
 
 # Generate CSRF token
@@ -837,9 +862,10 @@ 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 {
+sub parse_extended_patron_attributes {
     my ($input) = @_;
     my @patron_attr = grep { /^patron_attr_\d+$/ } $input->multi_param();
 
@@ -851,24 +877,22 @@ sub  parse_extended_patron_attributes {
         my $code     = $input->param("${key}_code");
         next if exists $dups{$code}->{$value};
         $dups{$code}->{$value} = 1;
-        push @attr, { code => $code, value => $value };
+        push @attr, { code => $code, attribute => $value };
     }
     return \@attr;
 }
 
 sub patron_attributes_form {
     my $template = shift;
-    my $borrowernumber = shift;
+    my $attributes = shift;
     my $op = shift;
 
-    my @types = C4::Members::AttributeTypes::GetAttributeTypes();
-    if (scalar(@types) == 0) {
+    my $library_id = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;
+    my $attribute_types = Koha::Patron::Attribute::Types->search_with_library_limits({}, {}, $library_id);
+    if ( $attribute_types->count == 0 ) {
         $template->param(no_patron_attribute_types => 1);
         return;
     }
-    my $attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber);
-    my @classes = uniq( map {$_->{class}} @$attributes );
-    @classes = sort @classes;
 
     # map patron's attributes into a more convenient structure
     my %attr_hash = ();
@@ -879,8 +903,7 @@ sub patron_attributes_form {
     my @attribute_loop = ();
     my $i = 0;
     my %items_by_class;
-    foreach my $type_code (map { $_->{code} } @types) {
-        my $attr_type = C4::Members::AttributeTypes->fetch($type_code);
+    while ( my ( $attr_type ) = $attribute_types->next ) {
         my $entry = {
             class             => $attr_type->class(),
             code              => $attr_type->code(),
@@ -888,15 +911,16 @@ sub patron_attributes_form {
             repeatable        => $attr_type->repeatable(),
             category          => $attr_type->authorised_value_category(),
             category_code     => $attr_type->category_code(),
+            mandatory         => $attr_type->mandatory(),
         };
         if (exists $attr_hash{$attr_type->code()}) {
             foreach my $attr (@{ $attr_hash{$attr_type->code()} }) {
                 my $newentry = { %$entry };
-                $newentry->{value} = $attr->{value};
+                $newentry->{value} = $attr->{attribute};
                 $newentry->{use_dropdown} = 0;
                 if ($attr_type->authorised_value_category()) {
                     $newentry->{use_dropdown} = 1;
-                    $newentry->{auth_val_loop} = GetAuthorisedValues($attr_type->authorised_value_category(), $attr->{value});
+                    $newentry->{auth_val_loop} = GetAuthorisedValues($attr_type->authorised_value_category(), $attr->{attribute});
                 }
                 $i++;
                 undef $newentry->{value} if ($attr_type->unique_id() && $op eq 'duplicate');
@@ -914,12 +938,12 @@ sub patron_attributes_form {
             push @{$items_by_class{$attr_type->class()}}, $newentry;
         }
     }
-    while ( my ($class, @items) = each %items_by_class ) {
+    for my $class ( sort keys %items_by_class ) {
         my $av = Koha::AuthorisedValues->search({ category => 'PA_CLASS', authorised_value => $class });
         my $lib = $av->count ? $av->next->lib : $class;
         push @attribute_loop, {
             class => $class,
-            items => @items,
+            items => $items_by_class{$class},
             lib   => $lib,
         }
     }