From d600f22c691b686a330ee18dd8a58668507757be Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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=UTF-8 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 set (=Yes) Same as A. C. You are superlibrarian and IndependentBranches is set Same as A. D. You are not superlibrarian and IndependentBranches is not 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 Signed-off-by: Nick Clemens --- 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.0