From 52949f88e0110e52b965bedbdcb885bdb5b00eaf Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 13 Jul 2018 12:10:47 -0300 Subject: [PATCH] Bug 20443: Improve filtering method Koha::Patron::Attributes->search mimicks what is done in Koha::AuthorisedValues->search. But actually it should be more explicit when the caller use it. For instance filter_by_branch_limitation (see discussion on bug 11983). This will be useful for the following patches as we will need a way to replace the $no_branch_limit flag. When the $no_branch_limit flag is called, a simple ->search call should be done. When we want to limit on a specific library we can pass the branchcode in paramter of filter_by_branch_limitation (this is not used yet). If not passed the logged-in user library will be used by default. Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize --- Koha/Patron/Attributes.pm | 67 +++++++++++++++++++++++------------ t/db_dependent/Koha/Patrons.t | 17 ++++++--- 2 files changed, 58 insertions(+), 26 deletions(-) diff --git a/Koha/Patron/Attributes.pm b/Koha/Patron/Attributes.pm index 0cb6875daf..f7289a0be0 100644 --- a/Koha/Patron/Attributes.pm +++ b/Koha/Patron/Attributes.pm @@ -18,6 +18,7 @@ package Koha::Patron::Attributes; use Modern::Perl; use Koha::Patron::Attribute; +use Koha::Patron::Attribute::Types; use base qw(Koha::Objects); @@ -33,35 +34,57 @@ Koha::Patron::Attributes - Koha Patron Attributes Object set class =head3 search -my $attributes-> Koha::Patron::Attributes->search( $params ); +my $attributes = Koha::Patron::Attributes->search( $params ); =cut sub search { my ( $self, $params, $attributes ) = @_; - my $branchcode = $params->{branchcode}; - delete( $params->{branchcode} ); - - my $or = - $branchcode - ? { - '-or' => [ - 'borrower_attribute_types_branches.b_branchcode' => undef, - 'borrower_attribute_types_branches.b_branchcode' => $branchcode, - ] - } : {}; - - my $join = - $branchcode - ? { - join => { - 'code' => 'borrower_attribute_types_branches' - }, - } : {}; - $attributes //= {}; unless ( exists $attributes->{order_by} ) { $attributes->{order_by} = ['code', 'attribute'] } - return $self->SUPER::search( { %$params, %$or }, { %$attributes, %$join } ); + + return $self->SUPER::search( $params, $attributes ); +} + +=head3 filter_by_branch_limitations + +my $attributes = Koha::Patron::Attributes->filter_by_branch_limitations([$branchcode]); + +Search patron attributes filtered by a library + +If $branchcode exists it will be used to filter the result set. + +Otherwise it will be the library of the logged in user. + +=cut + +sub filter_by_branch_limitations { + my ( $self, $branchcode ) = @_; + + # Maybe we should not limit if logged in user is superlibrarian? + my $branch_limit = + $branchcode ? $branchcode + # Do we raise an exception if no userenv defined? + : C4::Context->userenv ? C4::Context->userenv->{"branch"} + : undef; + + my $or = $branch_limit + ? { + '-or' => [ + 'borrower_attribute_types_branches.b_branchcode' => undef, + 'borrower_attribute_types_branches.b_branchcode' => $branch_limit, + ] + } + : {}; + + my $join = $branch_limit + ? { + join => { + code => 'borrower_attribute_types_branches' + }, + } + : {}; + return $self->search( $or, $join ); } =head3 _type diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 146bdbfb95..daee4f9b89 100644 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -2020,7 +2020,7 @@ subtest 'anonymize' => sub { $schema->storage->txn_rollback; subtest 'get_extended_attributes' => sub { - plan tests => 9; + plan tests => 10; my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; @@ -2094,12 +2094,21 @@ subtest 'get_extended_attributes' => sub { # Test branch limitations set_logged_in_user($patron_2); - C4::Members::Attributes::SetBorrowerAttributes($patron_1->borrowernumber, $attributes_for_1); + # Return all $extended_attributes_for_1 = $patron_1->get_extended_attributes; - is( $extended_attributes_for_1->count, 2, 'There should be 2 attributes for patron 1, the limited one should not be returned'); + is( $extended_attributes_for_1->count, 3, 'There should be 2 attributes for patron 1, the limited one should be returned'); + # Return filtered + $extended_attributes_for_1 = $patron_1->get_extended_attributes->filter_by_branch_limitations; + is( $extended_attributes_for_1->count, 2, 'There should be 2 attributes for patron 1, the limited one should be returned'); + + # Not filtered my $limited_value = $patron_1->get_extended_attribute_value( $attribute_type_limited->code ); - is( $limited_value, undef, ); + is( $limited_value, 'my attribute limited', ); + + ## Do we need a filtered? + #$limited_value = $patron_1->get_extended_attribute_value( $attribute_type_limited->code ); + #is( $limited_value, undef, ); $schema->storage->txn_rollback; }; -- 2.39.5