From f1d210019b1f87465a8bad33f57ba6d20bec65f9 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Mon, 16 Jul 2018 19:04:03 -0300
Subject: [PATCH] Bug 20443: Move
 C4::Members::AttributeTypes::GetAttributeTypes to
 Koha::Patron::Attribute::Types

We can then now start to move methods from C4::Members::AttributeTypes
as well.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 C4/Auth_with_ldap.pm                          | 10 +-
 C4/Members/AttributeTypes.pm                  | 44 ---------
 C4/Members/Attributes.pm                      |  7 +-
 Koha/Template/Plugin/Branches.pm              | 23 ++++-
 admin/patron-attr-types.pl                    | 94 ++----------------
 .../en/modules/admin/patron-attr-types.tt     | 96 +++++++------------
 members/memberentry.pl                        |  8 +-
 members/moremember.pl                         |  5 +-
 reports/borrowers_stats.pl                    | 13 ++-
 reports/issues_stats.pl                       |  7 +-
 t/Members_AttributeTypes.t                    |  4 +-
 t/db_dependent/Koha/Patron/Attribute/Types.t  |  2 +
 tools/import_borrowers.pl                     |  7 +-
 tools/modborrowers.pl                         | 20 ++--
 14 files changed, 109 insertions(+), 231 deletions(-)

diff --git a/C4/Auth_with_ldap.pm b/C4/Auth_with_ldap.pm
index cc05550ca3..026178eb3e 100644
--- a/C4/Auth_with_ldap.pm
+++ b/C4/Auth_with_ldap.pm
@@ -233,8 +233,9 @@ sub checkpw_ldap {
         return 0;   # B2, D2
     }
     if (C4::Context->preference('ExtendedPatronAttributes') && $borrowernumber && ($config{update} ||$config{replicate})) {
-        foreach my $attribute_type ( C4::Members::AttributeTypes::GetAttributeTypes() ) {
-            my $code = $attribute_type->{code};
+        my $attribute_types = Koha::Patron::Attribute::Types->filter_by_branch_limitations;
+        while ( my $attribute_type = $attribute_types->next ) {
+            my $code = $attribute_type->code;
             unless (exists($borrower{$code}) && $borrower{$code} !~ m/^\s*$/ ) {
                 next;
             }
@@ -375,8 +376,9 @@ sub update_local {
     # skip extended patron attributes in 'borrowers' attribute update
     my @keys = keys %$borrower;
     if (C4::Context->preference('ExtendedPatronAttributes')) {
-        foreach my $attribute_type ( C4::Members::AttributeTypes::GetAttributeTypes() ) {
-           my $code = $attribute_type->{code};
+        my $attribute_types = Koha::Patron::Attribute::Types->filter_by_branch_limitations;
+        while ( my $attribute_type = $attribute_types->next ) {
+           my $code = $attribute_type->code;
            @keys = grep { $_ ne $code } @keys;
            $debug and printf STDERR "ignoring extended patron attribute '%s' in update_local()\n", $code;
         }
diff --git a/C4/Members/AttributeTypes.pm b/C4/Members/AttributeTypes.pm
index 948acbe3a9..646a97eab3 100644
--- a/C4/Members/AttributeTypes.pm
+++ b/C4/Members/AttributeTypes.pm
@@ -28,8 +28,6 @@ C4::Members::AttributeTypes - mananage extended patron attribute types
 
 =head1 SYNOPSIS
 
-  my @attribute_types = C4::Members::AttributeTypes::GetAttributeTypes();
-
   my $attr_type = C4::Members::AttributeTypes->new($code, $description);
   $attr_type->code($code);
   $attr_type->description($description);
@@ -47,48 +45,6 @@ C4::Members::AttributeTypes - mananage extended patron attribute types
 
 =head1 FUNCTIONS
 
-=head2 GetAttributeTypes
-
-  my @attribute_types = C4::Members::AttributeTypes::GetAttributeTypes($all_fields);
-
-Returns an array of hashrefs of each attribute type defined
-in the database.  The array is sorted by code.  Each hashref contains
-at least the following fields:
-
- - code
- - description
-
-If $all_fields is true, then each hashref also contains the other fields from borrower_attribute_types.
-
-=cut
-
-sub GetAttributeTypes {
-    my $all    = @_   ? shift : 0;
-    my $no_branch_limit = @_ ? shift : 0;
-    my $branch_limit = $no_branch_limit
-        ? 0
-        : C4::Context->userenv ? C4::Context->userenv->{"branch"} : 0;
-    my $select = $all ? '*'   : 'DISTINCT(code), description, class';
-
-    my $dbh = C4::Context->dbh;
-    my $query = "SELECT $select FROM borrower_attribute_types";
-    $query .= qq{
-        LEFT JOIN borrower_attribute_types_branches ON bat_code = code
-        WHERE b_branchcode = ? OR b_branchcode IS NULL
-    } if $branch_limit;
-    $query .= " ORDER BY code";
-    my $sth    = $dbh->prepare($query);
-    $sth->execute( $branch_limit ? $branch_limit : () );
-    my $results = $sth->fetchall_arrayref({});
-    $sth->finish;
-    return @$results;
-}
-
-sub GetAttributeTypes_hashref {
-    my %hash = map {$_->{code} => $_} GetAttributeTypes(@_);
-    return \%hash;
-}
-
 =head1 METHODS 
 
   my $attr_type = C4::Members::AttributeTypes->new($code, $description);
diff --git a/C4/Members/Attributes.pm b/C4/Members/Attributes.pm
index c28d59bac8..17c6286626 100644
--- a/C4/Members/Attributes.pm
+++ b/C4/Members/Attributes.pm
@@ -22,7 +22,8 @@ use warnings;
 
 use Text::CSV;      # Don't be tempted to use Text::CSV::Unicode -- even in binary mode it fails.
 use C4::Context;
-use C4::Members::AttributeTypes;
+
+use Koha::Patron::Attribute::Types;
 
 use vars qw(@ISA @EXPORT_OK @EXPORT %EXPORT_TAGS);
 our ($csv, $AttributeTypes);
@@ -110,7 +111,7 @@ The third option specifies whether repeatable codes are clobbered or collected.
 
 Returns one reference to (merged) array of hashref.
 
-Caches results of C4::Members::AttributeTypes::GetAttributeTypes_hashref(1) for efficiency.
+Caches results of attribute types for efficiency. # FIXME That is very bad and can cause side-effects undef plack
 
 =cut
 
@@ -118,7 +119,7 @@ sub extended_attributes_merge {
     my $old = shift or return;
     my $new = shift or return $old;
     my $keep = @_ ? shift : 0;
-    $AttributeTypes or $AttributeTypes = C4::Members::AttributeTypes::GetAttributeTypes_hashref(1);
+    $AttributeTypes or $AttributeTypes = Koha::Patron::Attribute::Types->search->unblessed;
     my @merged = @$old;
     foreach my $att (@$new) {
         unless ($att->{code}) {
diff --git a/Koha/Template/Plugin/Branches.pm b/Koha/Template/Plugin/Branches.pm
index 08dbcb5c1a..2d246c0a72 100644
--- a/Koha/Template/Plugin/Branches.pm
+++ b/Koha/Template/Plugin/Branches.pm
@@ -58,6 +58,7 @@ sub GetURL {
 sub all {
     my ( $self, $params ) = @_;
     my $selected = $params->{selected};
+    my $selecteds = $params->{selecteds};
     my $unfiltered = $params->{unfiltered} || 0;
     my $search_params = $params->{search_params} || {};
 
@@ -69,11 +70,23 @@ sub all {
       ? Koha::Libraries->search( $search_params, { order_by => ['branchname'] } )->unblessed
       : Koha::Libraries->search_filtered( $search_params, { order_by => ['branchname'] } )->unblessed;
 
-    for my $l ( @$libraries ) {
-        if (       defined $selected and $l->{branchcode} eq $selected
-            or not defined $selected and C4::Context->userenv and $l->{branchcode} eq ( C4::Context->userenv->{branch} // q{} )
-        ) {
-            $l->{selected} = 1;
+    if (defined $selecteds) {
+        # For a select multiple, must be a Koha::Libraries
+        my @selected_branchcodes = $selecteds ? $selecteds->get_column( ['branchcode'] ) : ();
+        $libraries = [ map {
+            my $l = $_;
+            $l->{selected} = 1
+              if grep { $_ eq $l->{branchcode} } @selected_branchcodes;
+            $l;
+        } @$libraries ];
+    }
+    else {
+        for my $l ( @$libraries ) {
+            if (       defined $selected and $l->{branchcode} eq $selected
+                or not defined $selected and C4::Context->userenv and $l->{branchcode} eq ( C4::Context->userenv->{branch} // q{} )
+            ) {
+                $l->{selected} = 1;
+            }
         }
     }
 
diff --git a/admin/patron-attr-types.pl b/admin/patron-attr-types.pl
index 174bac5926..fd87e73c4d 100755
--- a/admin/patron-attr-types.pl
+++ b/admin/patron-attr-types.pl
@@ -29,6 +29,7 @@ use C4::Context;
 use C4::Output;
 use C4::Koha;
 use C4::Members::AttributeTypes;
+use Koha::Patron::Attribute::Types;
 
 use Koha::AuthorisedValues;
 use Koha::Libraries;
@@ -86,23 +87,12 @@ exit 0;
 sub add_attribute_type_form {
     my $template = shift;
 
-    my $branches = Koha::Libraries->search( {}, { order_by => ['branchname'] } )->unblessed;
-    my @branches_loop;
-    foreach my $branch (@$branches) {
-        push @branches_loop, {
-            branchcode => $branch->{branchcode},
-            branchname => $branch->{branchname},
-        };
-    }
-
     my $patron_categories = Koha::Patron::Categories->search_limited({}, {order_by => ['description']});
     $template->param(
         attribute_type_form => 1,
         confirm_op => 'add_attribute_type_confirmed',
         categories => $patron_categories,
-        branches_loop => \@branches_loop,
     );
-    $template->param(classes_val_loop => GetAuthorisedValues( 'PA_CLASS'));
 }
 
 sub error_add_attribute_type_form {
@@ -110,25 +100,6 @@ sub error_add_attribute_type_form {
 
     $template->param(description => scalar $input->param('description'));
 
-    if ($input->param('repeatable')) {
-        $template->param(repeatable_checked => 1);
-    }
-    if ($input->param('unique_id')) {
-        $template->param(unique_id_checked => 1);
-    }
-    if ($input->param('opac_display')) {
-        $template->param(opac_display_checked => 1);
-    }
-    if ($input->param('opac_editable')) {
-        $template->param(opac_editable_checked => 1);
-    }
-    if ($input->param('staff_searchable')) {
-        $template->param(staff_searchable_checked => 1);
-    }
-    if ($input->param('display_checkout')) {
-        $template->param(display_checkout_checked => 'checked="checked"');
-    }
-
     $template->param( category_code => scalar $input->param('category_code') );
     $template->param( class => scalar $input->param('class') );
 
@@ -154,6 +125,8 @@ sub add_update_attribute_type {
         my $existing = C4::Members::AttributeTypes->fetch($code);
         if (defined($existing)) {
             $template->param(duplicate_code_error => $code);
+            # FIXME Regression here
+            # Form will not be refilled with entered values on error
             error_add_attribute_type_form($template);
             return 0;
         }
@@ -231,53 +204,8 @@ sub edit_attribute_type_form {
     my $template = shift;
     my $code = shift;
 
-    my $attr_type = C4::Members::AttributeTypes->fetch($code);
-
-    $template->param(code => $code);
-    $template->param(description => $attr_type->description());
-    $template->param(class => $attr_type->class());
-
-    if ($attr_type->repeatable()) {
-        $template->param(repeatable_checked => 1);
-    }
-    $template->param(repeatable_disabled => 1);
-    if ($attr_type->unique_id()) {
-        $template->param(unique_id_checked => 1);
-    }
-    $template->param(unique_id_disabled => 1);
-    if ($attr_type->opac_display()) {
-        $template->param(opac_display_checked => 1);
-    }
-    if ($attr_type->opac_editable()) {
-        $template->param(opac_editable_checked => 1);
-    }
-    if ($attr_type->staff_searchable()) {
-        $template->param(staff_searchable_checked => 1);
-    }
-    if ($attr_type->display_checkout()) {
-        $template->param(display_checkout_checked => 'checked="checked"');
-    }
-    $template->param( authorised_value_category => $attr_type->authorised_value_category() );
-    $template->param(classes_val_loop => GetAuthorisedValues( 'PA_CLASS' ));
-
-    my $branches = Koha::Libraries->search( {}, { order_by => ['branchname'] } )->unblessed;
-    my @branches_loop;
-    my $selected_branches = $attr_type->branches;
-    foreach my $branch (@$branches) {
-        my $selected = ( grep {$_->{branchcode} eq $branch->{branchcode}} @$selected_branches ) ? 1 : 0;
-        push @branches_loop, {
-            branchcode => $branch->{branchcode},
-            branchname => $branch->{branchname},
-            selected => $selected,
-        };
-    }
-    $template->param( branches_loop => \@branches_loop );
-
-    $template->param(
-        category_code        => $attr_type->category_code,
-        category_class       => $attr_type->class,
-        category_description => $attr_type->category_description,
-    );
+    my $attr_type = Koha::Patron::Attribute::Types->find($code);
+    $template->param(attribute_type => $attr_type);
 
     my $patron_categories = Koha::Patron::Categories->search({}, {order_by => ['description']});
     $template->param(
@@ -292,18 +220,17 @@ sub edit_attribute_type_form {
 sub patron_attribute_type_list {
     my $template = shift;
 
-    my @attr_types = C4::Members::AttributeTypes::GetAttributeTypes( 1, 1 );
+    my @attr_types = Koha::Patron::Attribute::Types->search->as_list;
 
-    my @classes = uniq( map { $_->{class} } @attr_types );
+    my @classes = uniq( map { $_->class } @attr_types );
     @classes = sort @classes;
 
     my @attributes_loop;
+    # FIXME This is not efficient and should be improved
     for my $class (@classes) {
-        my ( @items, $branches );
+        my @items;
         for my $attr (@attr_types) {
-            next if $attr->{class} ne $class;
-            my $attr_type = C4::Members::AttributeTypes->fetch($attr->{code});
-            $attr->{branches} = $attr_type->branches;
+            next if $attr->class ne $class;
             push @items, $attr;
         }
         my $av = Koha::AuthorisedValues->search({ category => 'PA_CLASS', authorised_value => $class });
@@ -312,7 +239,6 @@ sub patron_attribute_type_list {
             class => $class,
             items => \@items,
             lib   => $lib,
-            branches => $branches,
         };
     }
     $template->param(available_attribute_types => \@attributes_loop);
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt
index d4cecaafdf..b207de8c7e 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt
@@ -1,6 +1,8 @@
 [% USE raw %]
 [% USE Asset %]
 [% USE AuthorisedValues %]
+[% USE Branches %]
+[% USE scalar %]
 [% SET footerjs = 1 %]
 [% INCLUDE 'doc-head-open.inc' %]
 <title>Koha &rsaquo; Administration &rsaquo; Patron attribute types
@@ -60,10 +62,10 @@
   <fieldset class="rows">
     <ol>
       <li>
-          [% IF ( edit_attribute_type ) %]
-		  <span class="label">Patron attribute type code: </span>
-            <input type="hidden" name="code" value="[% code | html %]" />
-            [% code | html %]
+          [% IF attribute_type %]
+            <span class="label">Patron attribute type code: </span>
+            <input type="hidden" name="code" value="[% attribute_type.code |html %]" />
+            [% attribute_type.code |html %]
           [% ELSE %]
               <label for="code" class="required">Patron attribute type code: </label>
               <input type="text" id="code" name="code" required="required" class="required" size="10" maxlength="10" />
@@ -71,45 +73,37 @@
           [% END %]
        </li>
        <li><label for="description" class="required">Description: </label>
-           <input type="text" id="description" name="description" required="required" class="required" size="50" maxlength="250" value="[% description | html %]" />
+           <input type="text" id="description" name="description" required="required" class="required" size="50" maxlength="250" value="[% attribute_type.description |html %]" />
            <span class="required">Required</span>
        </li>
        <li><label for="repeatable">Repeatable: </label>
-            [% IF ( repeatable_checked ) %]
-              [% IF ( repeatable_disabled ) %]
-                <input type="checkbox" id="repeatable" name="repeatable" checked="checked" disabled="disabled" />
-              [% ELSE %]
-                <input type="checkbox" id="repeatable" name="repeatable" checked="checked" />
-              [% END %]
+            [% IF attribute_type %]
+                [% IF attribute_type.repeatable %]
+                    <input type="checkbox" id="repeatable" name="repeatable" checked="checked" disabled="disabled" />
+                [% ELSE %]
+                    <input type="checkbox" id="repeatable" name="repeatable" disabled="disabled" />
+                [% END %]
             [% ELSE %]
-              [% IF ( repeatable_disabled ) %]
-                <input type="checkbox" id="repeatable" name="repeatable" disabled="disabled" />
-              [% ELSE %]
                 <input type="checkbox" id="repeatable" name="repeatable" />
-              [% END %]
             [% END %]
-            <span>Check to let a patron record have multiple values of this attribute.  
+            <span>Check to let a patron record have multiple values of this attribute.
                   This setting cannot be changed after an attribute is defined.</span>
        </li>
        <li><label for="unique_id">Unique identifier: </label>
-            [% IF ( unique_id_checked ) %]
-              [% IF ( unique_id_disabled ) %]
-                <input type="checkbox" id="unique_id" name="unique_id" checked="checked" disabled="disabled" />
-              [% ELSE %]
-                <input type="checkbox" id="unique_id" name="unique_id" checked="checked" />
-              [% END %]
+            [% IF attribute_type %]
+                [% IF attribute_type.unique_id %]
+                    <input type="checkbox" id="unique_id" name="unique_id" checked="checked" disabled="disabled" />
+                [% ELSE %]
+                    <input type="checkbox" id="unique_id" name="unique_id" disabled="disabled" />
+                [% END %]
             [% ELSE %]
-              [% IF ( unique_id_disabled ) %]
-                <input type="checkbox" id="unique_id" name="unique_id" disabled="disabled" />
-              [% ELSE %]
                 <input type="checkbox" id="unique_id" name="unique_id" />
-              [% END %]
             [% END %]
             <span>If checked, attribute will be a unique identifier &mdash; if a value is given to a patron record, the same value
                   cannot be given to a different record.  This setting cannot be changed after an attribute is defined.</span>
        </li>
        <li><label for="opac_display">Display in OPAC: </label>
-          [% IF ( opac_display_checked ) %]
+          [% IF attribute_type AND attribute_type.opac_display %]
             <input type="checkbox" id="opac_display" name="opac_display" checked="checked" />
           [% ELSE %]
             <input type="checkbox" id="opac_display" name="opac_display" />
@@ -117,7 +111,7 @@
             <span>Check to display this attribute on a patron's details page in the OPAC.</span>
        </li>
        <li><label for="opac_editable">Editable in OPAC: </label>
-          [% IF ( opac_editable_checked ) %]
+          [% IF attribute_type AND attribute_type.opac_editable %]
             <input type="checkbox" id="opac_editable" name="opac_editable" checked="checked" />
           [% ELSE %]
             <input type="checkbox" id="opac_editable" name="opac_editable" />
@@ -125,7 +119,7 @@
             <span>Check to allow patrons to edit this attribute from their details page in the OPAC. (Requires above, does not work with <a href="/cgi-bin/koha/admin/preferences.pl?op=search&amp;searchfield=PatronSelfRegistrationVerifyByEmail" target="_blank">PatronSelfRegistrationVerifyByEmail</a>.)</span>
        </li>
        <li><label for="staff_searchable">Searchable: </label>
-          [% IF ( staff_searchable_checked ) %]
+          [% IF attribute_type AND attribute_type.staff_searchable %]
             <input type="checkbox" id="staff_searchable" name="staff_searchable" checked="checked" />
           [% ELSE %]
             <input type="checkbox" id="staff_searchable" name="staff_searchable" />
@@ -133,7 +127,7 @@
             <span>Check to make this attribute staff_searchable in the staff patron search.</span>
        </li>
        <li><label for="display_checkout">Display in check-out: </label>
-            [% IF display_checkout_checked %]
+            [% IF attribute_type AND attribute_type.display_checkout %]
                 <input type="checkbox" id="display_checkout" name="display_checkout" checked="checked" />
 	    [% ELSE %]
 	        <input type="checkbox" id="display_checkout" name="display_checkout" />
@@ -144,7 +138,7 @@
         <li><label for="authorised_value_category">Authorized value category: </label>
             <select name="authorised_value_category" id="authorised_value_category">
                 <option value=""></option>
-                [% PROCESS options_for_authorised_value_categories authorised_value_categories => AuthorisedValues.GetCategories( selected => authorised_value_category ) %]
+                [% PROCESS options_for_authorised_value_categories authorised_value_categories => AuthorisedValues.GetCategories( selected => attribute_type.authorised_value_category ) %]
             </select>
             <span>Authorized value category; if one is selected, the patron record input page will only allow values 
                   to be chosen from the authorized value list.  However, an authorized value list is not 
@@ -153,13 +147,7 @@
         <li><label for="branches">Branches limitation: </label>
             <select id="branches" name="branches" multiple size="10">
                 <option value="">All branches</option>
-                [% FOREACH branch IN branches_loop %]
-                  [% IF ( branch.selected ) %]
-                    <option selected="selected" value="[% branch.branchcode | html %]">[% branch.branchname | html %]</option>
-                  [% ELSE %]
-                    <option value="[% branch.branchcode | html %]">[% branch.branchname | html %]</option>
-                  [% END %]
-                [% END %]
+                [% PROCESS options_for_libraries libraries => Branches.all( selecteds => attribute_type.library_limits ) %]
             </select>
             <span>Select All if this attribute type must to be displayed all the time. Otherwise select libraries you want to associate with this value.
             </span>
@@ -169,27 +157,14 @@
             <select name="category_code" id="category">
                 <option value=""></option>
                 [% FOREACH cat IN categories %]
-                    [% IF ( cat.categorycode == category_code ) %]<option value="[% cat.categorycode | html %]" selected="selected">[% cat.description | html %]</option>[% ELSE %]<option value="[% cat.categorycode | html %]">[% cat.description | html %]</option>[% END %]
+                    [% IF ( cat.categorycode == attribute_type.category_code ) %]<option value="[% cat.categorycode | html %]" selected="selected">[% cat.description |html %]</option>[% ELSE %]<option value="[% cat.categorycode | html %]">[% cat.description |html %]</option>[% END %]
                 [% END %]
             </select>
             <span>Choose one to limit this attribute to one patron type. Please leave blank if you want these attributes to be available for all types of patrons.</span>
         </li>
         <li>
             <label for="class">Class: </label>
-            <select name="class" id="class">
-                <option value=""></option>
-                [% FOREACH class IN classes_val_loop %]
-                    [% IF class.authorised_value == category_class %]
-                        <option value="[% class.authorised_value | html %]" selected="selected">
-                            [% class.lib | html %]
-                        </option>
-                    [% ELSE %]
-                        <option value="[% class.authorised_value | html %]" >
-                            [% class.lib | html %]
-                        </option>
-                    [% END %]
-                [% END %]
-            </select>
+            [% PROCESS 'av-build-dropbox.inc' name="class", category="PA_CLASS" default=attribute_type.class %]
             <span>Group attributes types with a block title (based on authorized values category 'PA_CLASS')</span>
         </li>
     </ol>
@@ -259,16 +234,17 @@
             <td>[% item.code | html %]</td>
             <td>[% item.description | html %]</td>
             <td>
-                [% IF ( item.branches && item.branches.size > 0 ) %]
+                [% SET libraries = item.library_limits %]
+                [% IF ( libraries && libraries.count > 0 ) %]
                     [% branches_str = "" %]
-                    [% FOREACH branch IN item.branches %]
+                    [% FOREACH branch IN libraries %]
                         [% branches_str = branches_str _ " " _ branch.branchname _ "(" _ branch.branchcode _ ")" %]
                     [% END %]
                     <span title="[% branches_str | html %]">
-                        [% IF item.branches.size > 1 %]
-                            [% item.branches.size | html %] branches limitations
+                        [% IF libraries.count > 1 %]
+                            [% libraries.count | html %] branches limitations
                         [% ELSE %]
-                            [% item.branches.size | html %] branch limitation
+                            [% libraries.count | html %] branch limitation
                         [% END %]
                     </span>
                 [% ELSE %]
@@ -276,8 +252,8 @@
                 [% END %]
             </td>
             <td class="actions">
-              <a class="btn btn-default btn-xs" href="[% item.script_name | url %]?op=edit_attribute_type&amp;code=[% item.code | uri %]"><i class="fa fa-pencil"></i> Edit</a>
-              <a class="btn btn-default btn-xs" href="[% item.script_name | url %]?op=delete_attribute_type&amp;code=[% item.code | uri %]"><i class="fa fa-trash"></i> Delete</a>
+              <a class="btn btn-default btn-xs" href="[% script_name | url %]?op=edit_attribute_type&amp;code=[% item.code | uri %]"><i class="fa fa-pencil"></i> Edit</a>
+              <a class="btn btn-default btn-xs" href="[% script_name | url %]?op=delete_attribute_type&amp;code=[% item.code | uri %]"><i class="fa fa-trash"></i> Delete</a>
             </td>
           </tr>
         [% END %]
diff --git a/members/memberentry.pl b/members/memberentry.pl
index ff3c1a6ddd..7499382115 100755
--- a/members/memberentry.pl
+++ b/members/memberentry.pl
@@ -43,6 +43,7 @@ use Koha::Cities;
 use Koha::DateUtils;
 use Koha::Libraries;
 use Koha::Patrons;
+use Koha::Patron::Attribute::Types;
 use Koha::Patron::Categories;
 use Koha::Patron::HouseboundRole;
 use Koha::Patron::HouseboundRoles;
@@ -867,8 +868,8 @@ sub patron_attributes_form {
     my $borrowernumber = shift;
     my $op = shift;
 
-    my @types = C4::Members::AttributeTypes::GetAttributeTypes();
-    if (scalar(@types) == 0) {
+    my $attribute_types = Koha::Patron::Attribute::Types->filter_by_branch_limitations;
+    if ( $attribute_types->count == 0 ) {
         $template->param(no_patron_attribute_types => 1);
         return;
     }
@@ -887,8 +888,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(),
diff --git a/members/moremember.pl b/members/moremember.pl
index ea00da74f8..0e3a31b8d6 100755
--- a/members/moremember.pl
+++ b/members/moremember.pl
@@ -35,6 +35,7 @@ use C4::Output;
 use C4::Members::AttributeTypes;
 use C4::Form::MessagingPreferences;
 use List::MoreUtils qw/uniq/;
+use Koha::Patron::Attribute::Types;
 use Koha::Patron::Debarments qw(GetDebarments);
 use Koha::Patron::Messages;
 use Koha::DateUtils;
@@ -154,8 +155,8 @@ if (C4::Context->preference('ExtendedPatronAttributes')) {
         attributes_loop => \@attributes_loop
     );
 
-    my @types = C4::Members::AttributeTypes::GetAttributeTypes();
-    if (scalar(@types) == 0) {
+    my $nb_of_attribute_types = Koha::Patron::Attribute::Types->filter_by_branch_limitations->count;
+    if ( $nb_of_attribute_types == 0 ) {
         $template->param(no_patron_attribute_types => 1);
     }
 }
diff --git a/reports/borrowers_stats.pl b/reports/borrowers_stats.pl
index 27782c5920..34c3353314 100755
--- a/reports/borrowers_stats.pl
+++ b/reports/borrowers_stats.pl
@@ -28,11 +28,11 @@ use C4::Acquisition;
 use C4::Output;
 use C4::Reports;
 use C4::Circulation;
-use C4::Members::AttributeTypes;
 
 use Koha::AuthorisedValues;
 use Koha::DateUtils;
 use Koha::Libraries;
+use Koha::Patron::Attribute::Types;
 use Koha::Patron::Categories;
 
 use Date::Calc qw(
@@ -157,16 +157,15 @@ sub calculate {
 
     # check parameters
     my @valid_names = qw(categorycode zipcode branchcode sex sort1 sort2);
-    my @attribute_types = C4::Members::AttributeTypes::GetAttributeTypes;
     if ($line =~ /^patron_attr\.(.*)/) {
         my $attribute_type = $1;
-        return unless (grep {$attribute_type eq $_->{code}} @attribute_types);
+        return unless Koha::Patron::Attribute::Types->find($attribute_type);
     } else {
         return unless (grep { $_ eq $line } @valid_names);
     }
     if ($column =~ /^patron_attr\.(.*)/) {
         my $attribute_type = $1;
-        return unless (grep {$attribute_type eq $_->{code}} @attribute_types);
+        return unless Koha::Patron::Attribute::Types->find($attribute_type);
     } else {
         return unless (grep { $_ eq $column } @valid_names);
     }
@@ -496,11 +495,11 @@ sub parse_extended_patron_attributes {
 sub patron_attributes_form {
     my $template = shift;
 
-    my @types = C4::Members::AttributeTypes::GetAttributeTypes();
+    my $attribute_types = Koha::Patron::Attribute::Types->filter_by_branch_limitations;
 
     my %items_by_class;
-    foreach my $type (@types) {
-        my $attr_type = C4::Members::AttributeTypes->fetch($type->{code});
+    while ( my $attr_type = $attribute_types->next ) {
+        # TODO The following can be simplified easily
         my $entry = {
             class             => $attr_type->class(),
             code              => $attr_type->code(),
diff --git a/reports/issues_stats.pl b/reports/issues_stats.pl
index 3c1ba1facb..994afc1e2a 100755
--- a/reports/issues_stats.pl
+++ b/reports/issues_stats.pl
@@ -34,7 +34,7 @@ use C4::Members;
 use Koha::AuthorisedValues;
 use Koha::DateUtils;
 use Koha::ItemTypes;
-use C4::Members::AttributeTypes;
+use Koha::Patron::Attribute::Types;
 
 =head1 NAME
 
@@ -166,9 +166,10 @@ foreach (sort {$ccodes->{$a} cmp $ccodes->{$b}} keys %$ccodes) {
 my $CGIextChoice = ( 'CSV' ); # FIXME translation
 my $CGIsepChoice=GetDelimiterChoices;
 
-my @attribute_types = C4::Members::AttributeTypes::GetAttributeTypes(1);
+my $attribute_types = Koha::Patron::Attribute::Types->filter_by_branch_limitations;
 my %attribute_types_by_class;
-foreach my $attribute_type (@attribute_types) {
+while ( my ( $attribute_type ) = $attribute_types->next ) {
+    $attribute_type = $attribute_type->unblessed;
     if ($attribute_type->{authorised_value_category}) {
         my $authorised_values = C4::Koha::GetAuthorisedValues(
             $attribute_type->{authorised_value_category});
diff --git a/t/Members_AttributeTypes.t b/t/Members_AttributeTypes.t
index 42b04d57d4..1c8608db54 100755
--- a/t/Members_AttributeTypes.t
+++ b/t/Members_AttributeTypes.t
@@ -22,6 +22,8 @@ use Test::More;
 
 use Module::Load::Conditional qw/check_install/;
 
+use Koha::Patron::Attribute::Types;
+
 BEGIN {
     if ( check_install( module => 'Test::DBIx::Class' ) ) {
         plan tests => 8;
@@ -57,7 +59,7 @@ fixtures_ok [
 my $db = Test::MockModule->new('Koha::Database');
 $db->mock( _new_schema => sub { return Schema(); } );
 
-my @members_attributetypes = C4::Members::AttributeTypes::GetAttributeTypes(undef, 1);
+my @members_attributetypes = @{Koha::Patron::Attribute::Types->search->unblessed};
 
 is( $members_attributetypes[0]->{'code'}, 'one', 'First code value is one' );
 
diff --git a/t/db_dependent/Koha/Patron/Attribute/Types.t b/t/db_dependent/Koha/Patron/Attribute/Types.t
index ebb31a9512..7f47652536 100644
--- a/t/db_dependent/Koha/Patron/Attribute/Types.t
+++ b/t/db_dependent/Koha/Patron/Attribute/Types.t
@@ -329,6 +329,8 @@ subtest 'search_with_library_limits() tests' => sub {
         4, '4 attribute types are available with no library passed'
     );
 
+    # TODO test if filter_by_branch_limitations restriced on logged-in-user's branch
+
     $schema->storage->txn_rollback;
 };
 
diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl
index 4da2cd0d03..67b6c48042 100755
--- a/tools/import_borrowers.pl
+++ b/tools/import_borrowers.pl
@@ -44,6 +44,7 @@ use Koha::DateUtils;
 use Koha::Token;
 use Koha::Libraries;
 use Koha::Patron::Categories;
+use Koha::Patron::Attribute::Types;
 use Koha::List::Patron;
 
 use Koha::Patrons::Import;
@@ -167,9 +168,9 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) {
 else {
     if ($extended) {
         my @matchpoints = ();
-        my @attr_types = C4::Members::AttributeTypes::GetAttributeTypes( undef, 1 );
-        foreach my $type (@attr_types) {
-            my $attr_type = C4::Members::AttributeTypes->fetch( $type->{code} );
+        my $attribute_types = Koha::Patron::Attribute::Types->search;
+
+        while ( my $attr_type = $attribute_types->next ) {
             if ( $attr_type->unique_id() ) {
                 push @matchpoints,
                   { code => "patron_attribute_" . $attr_type->code(), description => $attr_type->description() };
diff --git a/tools/modborrowers.pl b/tools/modborrowers.pl
index dff86263c2..bc6446c177 100755
--- a/tools/modborrowers.pl
+++ b/tools/modborrowers.pl
@@ -31,7 +31,6 @@ use C4::Auth;
 use C4::Koha;
 use C4::Members;
 use C4::Members::Attributes;
-use C4::Members::AttributeTypes qw/GetAttributeTypes_hashref/;
 use C4::Output;
 use List::MoreUtils qw /any uniq/;
 use Koha::DateUtils qw( dt_from_string );
@@ -115,31 +114,30 @@ if ( $op eq 'show' ) {
     # Construct the patron attributes list
     my @patron_attributes_values;
     my @patron_attributes_codes;
-    my $patron_attribute_types = C4::Members::AttributeTypes::GetAttributeTypes_hashref('all');
+    my $patron_attribute_types = Koha::Patron::Attribute::Types->filter_by_branch_limitations;
     my @patron_categories = Koha::Patron::Categories->search_limited({}, {order_by => ['description']});
-    for ( values %$patron_attribute_types ) {
-        my $attr_type = C4::Members::AttributeTypes->fetch( $_->{code} );
+    while ( my $attr_type = $patron_attribute_types->next ) {
         # TODO Repeatable attributes are not correctly managed and can cause data lost.
         # This should be implemented.
-        next if $attr_type->{repeatable};
-        next if $attr_type->{unique_id}; # Don't display patron attributes that must be unqiue
+        next if $attr_type->repeatable;
+        next if $attr_type->unique_id; # Don't display patron attributes that must be unqiue
         my $options = $attr_type->authorised_value_category
             ? GetAuthorisedValues( $attr_type->authorised_value_category )
             : undef;
         push @patron_attributes_values,
             {
-                attribute_code => $_->{code},
+                attribute_code => $attr_type->code,
                 options        => $options,
             };
 
-        my $category_code = $_->{category_code};
+        my $category_code = $attr_type->category_code;
         my ( $category_lib ) = map {
-            ( defined $category_code and $_->categorycode eq $category_code ) ? $_->description : ()
+            ( defined $category_code and $attr_type->categorycode eq $category_code ) ? $attr_type->description : ()
         } @patron_categories;
         push @patron_attributes_codes,
             {
-                attribute_code => $_->{code},
-                attribute_lib  => $_->{description},
+                attribute_code => $attr_type->code,
+                attribute_lib  => $attr_type->description,
                 category_lib   => $category_lib,
                 type           => $attr_type->authorised_value_category ? 'select' : 'text',
             };
-- 
2.39.5