From 71f6e56824b7d621af70f838266fe488d4990d25 Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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: Signed-off-by: Nick Clemens --- 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' %] Koha › Administration › 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 — 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&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&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&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&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&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 88f21e1903..77c8b88c9b 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; @@ -854,8 +855,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; } @@ -874,8 +875,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.20.1