Attachment 103620 Details for Bug 21249 – Bug 21249: Code cleaning

[patch] Bug 21249: Code cleaning

Bug-21249-Code-cleaning.patch (text/plain), 4.57 KB, created by Aleisha Amohia on 2020-04-24 03:06:27 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Aleisha Amohia

Created: 2020-04-24 03:06:27 UTC

Size: 4.57 KB

patch

obsolete

>From 764927894df7e492b0591f600dd52c4cdf54ec34 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Wed, 6 Nov 2019 12:19:11 +0100
>Subject: [PATCH] Bug 21249: Code cleaning
>
>Trying to make the code a bit more readable removing unecessary
>variables and parenthesis.
>
>Code is duplicated but no idea where we could move it to.
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>---
> catalogue/search.pl | 30 ++++++++++++++++--------------
> opac/opac-search.pl | 29 +++++++++++++++++------------
> 2 files changed, 33 insertions(+), 26 deletions(-)
>
>diff --git a/catalogue/search.pl b/catalogue/search.pl
>index 83b73178424..eecfec07128 100755
>--- a/catalogue/search.pl
>+++ b/catalogue/search.pl
>@@ -420,21 +420,23 @@ my @nolimits = map uri_unescape($_), $cgi->multi_param('nolimit');
> my %is_nolimit = map { $_ => 1 } @nolimits;
> @limits = grep { not $is_nolimit{$_} } @limits;
> 
>-my $branchfield = C4::Context->preference('AdvancedSearchBranchFieldToUse');
>-
>-if($params->{'multibranchlimit'}) {
>+if ( $params->{'multibranchlimit'} ) {
>+    my $branchfield  = C4::Context->preference('AdvancedSearchBranchFieldToUse');
>     my $search_group = Koha::Library::Groups->find( $params->{multibranchlimit} );
>-    my @libraries = $search_group->all_libraries;
>-
>-    if ( $branchfield eq "homebranch") {
>-        my $multihomebranch = '('.join( " OR ", map { 'homebranch:     ' . $_->branchcode } @libraries ) .')';
>-        push @limits, $multihomebranch if ( $multihomebranch ne '()');
>-    } elsif ( $branchfield eq "holdingbranch") {
>-        my $multiholdingbranch = '('.join( " OR ", map { 'holdingbranch: ' . $_->branchcode } @libraries ) .')';
>-        push @limits, $multiholdingbranch if ( $multiholdingbranch ne '()');
>-    } else {
>-        my $multihomeandholdingbranch = '('.join( " OR ", map { 'homebranch: ' . $_->branchcode, 'holdingbranch: ' . $_->branchcode}  @libraries ) .')';
>-        push @limits, $multihomeandholdingbranch if ($multihomeandholdingbranch ne  '()');
>+    my @branchcodes  = map { $_->branchcode } $search_group->all_libraries;
>+
>+    if (@branchcodes) {
>+        if ( $branchfield eq "homebranch" ) {
>+            push @limits, sprintf "(%s)", join " or ", map { 'homebranch: ' . $_ } @branchcodes;
>+        }
>+        elsif ( $branchfield eq "holdingbranch" ) {
>+            push @limits, sprintf "(%s)", join " or ", map { 'holdingbranch: ' . $_ } @branchcodes;
>+        }
>+        else {
>+            push @limits, sprintf "(%s or %s)",
>+              join( " or ", map { 'homebranch: ' . $_ } @branchcodes ),
>+              join( " or ", map { 'holdingbranch: ' . $_ } @branchcodes );
>+        }
>     }
> }
> 
>diff --git a/opac/opac-search.pl b/opac/opac-search.pl
>index 99ad3bf05e3..aa8998aefd5 100755
>--- a/opac/opac-search.pl
>+++ b/opac/opac-search.pl
>@@ -497,19 +497,24 @@ if (@searchCategories > 0) {
> 
> my $branchfield = C4::Context->preference('AdvancedSearchBranchFieldToUse');
> 
>-if($params->{'multibranchlimit'}) {
>+if ( $params->{'multibranchlimit'} ) {
>+    my $branchfield  = C4::Context->preference('AdvancedSearchBranchFieldToUse');
>     my $search_group = Koha::Library::Groups->find( $params->{multibranchlimit} );
>-    my @libraries = $search_group->all_libraries;
>-
>-    if ( $branchfield eq "homebranch" ) {
>-        my $multihomebranch = '('.join( " OR ", map { 'homebranch: ' . $_->branchcode } @libraries ) .')';
>-        push @limits, $multihomebranch if ( $multihomebranch ne '()');
>-    } elsif ( $branchfield eq "holdingbranch") {
>-        my $multiholdingbranch = '('.join( " OR ", map { 'holdingbranch: ' . $_->branchcode } @libraries ) .')';
>-        push @limits, $multiholdingbranch if ( $multiholdingbranch ne '()');
>-    } else {
>-        my $multihomeandholdingbranch = '('.join( " OR ", map { 'homebranch: ' . $_->branchcode, 'holdingbranch: ' . $_->branchcode}  @libraries ) .')';
>-        push @limits, $multihomeandholdingbranch if ($multihomeandholdingbranch ne  '()');
>+
>+    my @branchcodes  = map { $_->branchcode } $search_group->all_libraries;
>+
>+    if (@branchcodes) {
>+        if ( $branchfield eq "homebranch" ) {
>+            push @limits, sprintf "(%s)", join " or ", map { 'homebranch: ' . $_ } @branchcodes;
>+        }
>+        elsif ( $branchfield eq "holdingbranch" ) {
>+            push @limits, sprintf "(%s)", join " or ", map { 'holdingbranch: ' . $_ } @branchcodes;
>+        }
>+        else {
>+            push @limits, sprintf "(%s or %s)",
>+              join( " or ", map { 'homebranch: ' . $_ } @branchcodes ),
>+              join( " or ", map { 'holdingbranch: ' . $_ } @branchcodes );
>+        }
>     }
> }
> 
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 21249: 78013 | 78014 | 78921 | 82138 | 83518 | 93345 | 93346 | 95099 | 95100 | 95101 | 103618 | 103619 | 103620 | 103621 | 103622 | 117220 | 117221 | 117222 | 117223 | 117224 | 120481 | 120567 | 120568 | 120569 | 120570 | 120571 | 120572 | 120575 | 120576 | 120577 | 120578 | 120579 | 120580