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

Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/C4/Utils/DataTables/Members.pm b/C4/Utils/DataTables/Members.pm
index a00e116..0005b01 100644 (file)
--- a/C4/Utils/DataTables/Members.pm
+++ b/C4/Utils/DataTables/Members.pm
@@ -23,30 +23,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 (file)
--- 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 782e5ee..24bb8e5 100644 (file)
--- 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;
@@ -735,6 +736,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 54c805d..79d5ab8 100644 (file)
--- 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 );