Bug 18743: Fix suggestion listing when organized by library

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>

diff --git a/C4/Suggestions.pm b/C4/Suggestions.pm
index 2cc06c9..15b02c5 100644 (file)
--- 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 e04e2ef..e1fca26 100755 (executable)
--- 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,
 );