From a85ed655070350156a32fad5e5fa6014886e2f51 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 6 Apr 2017 12:59:46 -0300 Subject: [PATCH] Bug 18403: Refactor and add Koha::Patron->libraries_where_can_see_patrons Technical note: Here we are just refactoring a code that have been copied into 3 different places. libraries_where_can_see_patrons is a terrible method's name, feel free to suggest something better. The method return a list of branchcodes to be more efficient, instead of Koha::Libraries --- C4/Utils/DataTables/Members.pm | 26 ++---------------------- Koha/Libraries.pm | 29 +++++++-------------------- Koha/Patron.pm | 45 ++++++++++++++++++++++++++++++++++++++++++ Koha/Patrons.pm | 20 +++---------------- 4 files changed, 57 insertions(+), 63 deletions(-) diff --git a/C4/Utils/DataTables/Members.pm b/C4/Utils/DataTables/Members.pm index ce90f9f..ddc2b8a 100644 --- a/C4/Utils/DataTables/Members.pm +++ b/C4/Utils/DataTables/Members.pm @@ -22,30 +22,8 @@ sub search { # If branches are independent and user is not superlibrarian # The search has to be only on the user branch my $userenv = C4::Context->userenv; - my @restricted_branchcodes; - if (C4::Context::only_my_library) { - push @restricted_branchcodes, $userenv->{branch}; - } - else { - my $logged_in_user = Koha::Patrons->find( $userenv->{number} ); - unless ( - $logged_in_user->can( - { borrowers => 'view_borrower_infos_from_any_libraries' } - ) - ) - { - if ( my $library_groups = $logged_in_user->library->library_groups ) - { - while ( my $library_group = $library_groups->next ) { - push @restricted_branchcodes, - $library_group->parent->children->get_column('branchcode'); - } - } - else { - push @restricted_branchcodes, $userenv->{branch}; - } - } - } + my $logged_in_user = Koha::Patrons->find( $userenv->{number} ); + my @restricted_branchcodes = $logged_in_user->libraries_where_can_see_patrons; my ($sth, $query, $iTotalQuery, $iTotalRecords, $iTotalDisplayRecords); my $dbh = C4::Context->dbh; diff --git a/Koha/Libraries.pm b/Koha/Libraries.pm index 927368e..5d88c6b 100644 --- a/Koha/Libraries.pm +++ b/Koha/Libraries.pm @@ -47,32 +47,17 @@ sub search_filtered { my @branchcodes; if ( my $userenv = C4::Context->userenv ) { - if ( C4::Context::only_my_library ) { - push @branchcodes, $userenv->{branch}; - } - else { + my $only_from_group = $params->{only_from_group}; + if ( $only_from_group ) { my $logged_in_user = Koha::Patrons->find( $userenv->{number} ); - unless ( - $logged_in_user->can( - { borrowers => 'view_borrower_infos_from_any_libraries' } - ) - ) - { - if ( my $library_groups = $logged_in_user->library->library_groups ) - { - while ( my $library_group = $library_groups->next ) { - push @branchcodes, - $library_group->parent->children->get_column('branchcode'); - } - } - else { - push @branchcodes, $userenv->{branch}; - } + my @branchcodes = $logged_in_user->libraries_where_can_see_patrons; + $params->{branchcode} = { -in => \@branchcodes } if @branchcodes; + } else { + if ( C4::Context::only_my_library ) { + $params->{branchcode} = C4::Context->userenv->{branch}; } } } - - $params->{branchcode} = { -in => \@branchcodes } if @branchcodes; delete $params->{only_from_group}; return $self->SUPER::search( $params, $attributes ); } diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 751dc77..af62ecb 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -21,6 +21,7 @@ package Koha::Patron; use Modern::Perl; use Carp; +use List::MoreUtils qw( uniq ); use C4::Context; use C4::Log; @@ -610,6 +611,50 @@ sub can_see_patron_infos { return $can; } +=head3 libraries_where_can_see_patrons + +my $libraries = $patron-libraries_where_can_see_patrons; + +Return the list of branchcodes(!) of libraries the patron is allowed to see other patron's infos. +The branchcodes are arbitrarily returned sorted. +We are supposing here that the object is related to the logged in patron (use of C4::Context::only_my_library) + +An empty array means no restriction, the patron can see patron's infos from any libraries. + +=cut + +sub libraries_where_can_see_patrons { + my ( $self ) = @_; + my $userenv = C4::Context->userenv; + + return () unless $userenv; # For tests, but userenv should be defined in tests... + + my @restricted_branchcodes; + if (C4::Context::only_my_library) { + push @restricted_branchcodes, $self->branchcode; + } + else { + unless ( + $self->can( + { borrowers => 'view_borrower_infos_from_any_libraries' } + ) + ) + { + my $library_groups = $self->library->library_groups; + if ( $library_groups->count ) + { + while ( my $library_group = $library_groups->next ) { + push @restricted_branchcodes, $library_group->parent->children->get_column('branchcode'); + } + } + else { + push @restricted_branchcodes, $self->branchcode; + } + } + } + return sort(uniq(@restricted_branchcodes)); +} + sub can { my ( $self, $flagsrequired ) = @_; return unless $self->userid; diff --git a/Koha/Patrons.pm b/Koha/Patrons.pm index 4749b12..9ae5efd 100644 --- a/Koha/Patrons.pm +++ b/Koha/Patrons.pm @@ -54,23 +54,9 @@ sub search_limited { my $userenv = C4::Context->userenv; my @restricted_branchcodes; - my $logged_in_user = Koha::Patrons->find( $userenv->{number} ); - if ( $logged_in_user and not - $logged_in_user->can( - { borrowers => 'view_borrower_infos_from_any_libraries' } - ) - ) - { - if ( my $library_groups = $logged_in_user->library->library_groups ) - { - while ( my $library_group = $library_groups->next ) { - push @restricted_branchcodes, - $library_group->parent->children->get_column('branchcode'); - } - } - else { - push @restricted_branchcodes, $userenv->{branch}; - } + if ( $userenv ) { + my $logged_in_user = Koha::Patrons->find( $userenv->{number} ); + @restricted_branchcodes = $logged_in_user->libraries_where_can_see_patrons; } $params->{'me.branchcode'} = { -in => \@restricted_branchcodes } if @restricted_branchcodes; return $self->search( $params, $attributes ); -- 2.9.3