From 9f16ab9e758aeb1a9e64bde03c6c8dcfb7f6e094 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Wed, 16 Oct 2019 16:43:46 +0200
Subject: [PATCH] Bug 18743: Fix suggestion listing when organized by library
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

There are some weird behaviors happening when using the "Organize by:
library" dropdown along with the library filter (in the "Acquisition
information" box).

I am suggesting the following test plan:
0. Create several suggestion from different libraries

A. You are superlibrarian and IndependentBranches is not set (=No)
1. Hit /suggestion/suggestion.pl
=> Default view shows the suggestions from your library
2. Filter by another library
=> You see the suggestions from this library
3. Filter by "Any" libraries
=> You see all the suggestions
4. "Organize by library"
=> You see all the suggestions, organized by library
5. Filter by a specific library
=> You see the suggestion from your library, all in one tab

B. You are not superlibrarian and IndependentBranches is not set (=No)
Same as A.

C. You are superlibrarian and IndependentBranches is set
Same as A.

D. You are not superlibrarian and IndependentBranches is set
You will never see suggestions coming from outside your library

QA: To be clear: the whole script needs a rewrite, but here we are just
trying to fix weird behaviors.

Sponsored-by: BULAC - http://www.bulac.fr/
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 C4/Suggestions.pm        | 24 +++++++++++++++---------
 suggestion/suggestion.pl | 14 ++++++++++----
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/C4/Suggestions.pm b/C4/Suggestions.pm
index 2cc06c99e0..15b02c51fa 100644
--- a/C4/Suggestions.pm
+++ b/C4/Suggestions.pm
@@ -135,10 +135,13 @@ sub SearchSuggestion {
     }
 
     # filter on user branch
-    if ( C4::Context->preference('IndependentBranches') ) {
+    if (   C4::Context->preference('IndependentBranches')
+        && !C4::Context->IsSuperLibrarian() )
+    {
+        # If IndependentBranches is set and the logged in user is not superlibrarian
+        # Then we want to filter by the user's library (i.e. cannot see suggestions from other libraries)
         my $userenv = C4::Context->userenv;
         if ($userenv) {
-            if ( !C4::Context->IsSuperLibrarian() && !$suggestion->{branchcode} )
             {
                 push @sql_params, $$userenv{branch};
                 push @query,      q{
@@ -146,13 +149,16 @@ sub SearchSuggestion {
                 };
             }
         }
-    } else {
-        if ( defined $suggestion->{branchcode} && $suggestion->{branchcode} ) {
-            unless ( $suggestion->{branchcode} eq '__ANY__' ) {
-                push @sql_params, $suggestion->{branchcode};
-                push @query,      qq{ AND suggestions.branchcode=? };
-            }
-        }
+    }
+    elsif (defined $suggestion->{branchcode}
+        && $suggestion->{branchcode}
+        && $suggestion->{branchcode} ne '__ANY__' )
+    {
+        # If IndependentBranches is not set OR the logged in user is not superlibrarian
+        # AND the branchcode filter is passed and not '__ANY__'
+        # Then we want to filter using this parameter
+        push @sql_params, $suggestion->{branchcode};
+        push @query,      qq{ AND suggestions.branchcode=? };
     }
 
     # filter on nillable fields
diff --git a/suggestion/suggestion.pl b/suggestion/suggestion.pl
index e04e2ef629..e1fca2611e 100755
--- a/suggestion/suggestion.pl
+++ b/suggestion/suggestion.pl
@@ -117,6 +117,7 @@ my ( $template, $borrowernumber, $cookie, $userflags ) = get_template_and_user(
 
 $borrowernumber = $input->param('borrowernumber') if ( $input->param('borrowernumber') );
 $template->param('borrowernumber' => $borrowernumber);
+my $branchfilter = $input->param('branchcode') || C4::Context->userenv->{'branch'};
 
 #########################################
 ##  Operations
@@ -253,7 +254,6 @@ if ($op=~/else/) {
     $op='else';
     
     $displayby||="STATUS";
-    delete $$suggestion_ref{'branchcode'} if($displayby eq "branchcode");
     # distinct values of display by
     my $criteria_list=GetDistinctValues("suggestions.".$displayby);
     my (@criteria_dv, $criteria_has_empty);
@@ -267,6 +267,14 @@ if ($op=~/else/) {
     # aggregate null and empty values under empty value
     push @criteria_dv, '' if $criteria_has_empty;
 
+    # Hack to not modify GetDistinctValues for this specific case
+    if (   $displayby eq 'branchcode'
+        && C4::Context->preference('IndependentBranches')
+        && not C4::Context->IsSuperLibrarian )
+    {
+        @criteria_dv = ( C4::Context->userenv->{'branch'} );
+    }
+
     my @allsuggestions;
     foreach my $criteriumvalue ( @criteria_dv ) {
         # By default, display suggestions from current working branch
@@ -275,7 +283,7 @@ if ($op=~/else/) {
         }
         my $definedvalue = defined $$suggestion_ref{$displayby} && $$suggestion_ref{$displayby} ne "";
 
-        next if ( $definedvalue && $$suggestion_ref{$displayby} ne $criteriumvalue );
+        next if ( $definedvalue && $$suggestion_ref{$displayby} ne $criteriumvalue ) and ($displayby ne 'branchcode' or $branchfilter ne '__ANY__' );
         $$suggestion_ref{$displayby} = $criteriumvalue;
 
         my $suggestions = &SearchSuggestion($suggestion_ref);
@@ -330,8 +338,6 @@ if(defined($returnsuggested) and $returnsuggested ne "noone")
     print $input->redirect("/cgi-bin/koha/members/moremember.pl?borrowernumber=".$returnsuggested."#suggestions");
 }
 
-my $branchfilter = ($displayby ne "branchcode") ? $input->param('branchcode') : C4::Context->userenv->{'branch'};
-
 $template->param(
     branchfilter => $branchfilter,
 );
-- 
2.11.4.GIT