Attachment 62025 Details for Bug 18403 – Bug 18403: Refactor and add Koha::Patron->libraries_where_can_see_patrons

[patch] Bug 18403: Refactor and add Koha::Patron->libraries_where_can_see_patrons

Bug-18403-Refactor-and-add-KohaPatron-librarieswhe.patch (text/plain), 7.00 KB, created by Jonathan Druart on 2017-04-10 21:33:31 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2017-04-10 21:33:31 UTC

Size: 7.00 KB

patch

obsolete

>From a85ed655070350156a32fad5e5fa6014886e2f51 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>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

Actions: View | Diff | Splinter Review

Attachments on bug 18403: 62010 | 62011 | 62012 | 62013 | 62014 | 62015 | 62016 | 62017 | 62018 | 62019 | 62020 | 62021 | 62022 | 62023 | 62024 | 62025 | 62026 | 62027 | 62028 | 62029 | 62030 | 62031 | 62032 | 62033 | 62302 | 62303 | 62830 | 62831 | 62832 | 62833 | 62834 | 62835 | 62836 | 62837 | 62838 | 62839 | 62840 | 62841 | 62842 | 62843 | 62844 | 62845 | 62846 | 62847 | 62848 | 62849 | 62850 | 62851 | 62852 | 62853 | 62854 | 62855 | 71392 | 71394 | 71500 | 71501 | 71502 | 71503 | 71504 | 71505 | 71506 | 71507 | 71508 | 71509 | 71510 | 71511 | 71512 | 71513 | 71514 | 71515 | 71516 | 71517 | 71518 | 71519 | 71520 | 71521 | 71522 | 71523 | 71524 | 71525 | 71526 | 71527 | 71528 | 71529 | 71530 | 71767