Bugzilla – Attachment 101132 Details for
Bug 20443
Move C4::Members::Attributes to Koha namespace
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20443: Move C4::Members::AttributeTypes::GetAttributeTypes to Koha::Patron::Attribute::Types
Bug-20443-Move-C4MembersAttributeTypesGetAttribute.patch (text/plain), 35.91 KB, created by
Jonathan Druart
on 2020-03-20 14:47:34 UTC
(
hide
)
Description:
Bug 20443: Move C4::Members::AttributeTypes::GetAttributeTypes to Koha::Patron::Attribute::Types
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2020-03-20 14:47:34 UTC
Size:
35.91 KB
patch
obsolete
>From 71f6e56824b7d621af70f838266fe488d4990d25 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: Signed-off-by: Nick Clemens <nick@bywatersolutions.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 › 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20443
:
101122
|
101123
|
101124
|
101125
|
101126
|
101127
|
101128
|
101129
|
101130
|
101131
|
101132
|
101133
|
101134
|
101135
|
101136
|
101137
|
101138
|
101139
|
101140
|
101141
|
101142
|
101143
|
101144
|
101145
|
101146
|
101147
|
101148
|
101149
|
101150
|
101209
|
101210
|
101211
|
101212
|
101213
|
101214
|
101215
|
101216
|
101217
|
101218
|
101219
|
101220
|
101221
|
101222
|
101223
|
101224
|
101225
|
101226
|
101227
|
101228
|
101229
|
101230
|
101231
|
101232
|
101233
|
101234
|
101235
|
101236
|
101237
|
101472
|
104519