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

[patch] Bug 21249: Code cleaning

Bug-21249-Code-cleaning.patch (text/plain), 4.68 KB, created by David Cook on 2021-05-06 03:45:49 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Cook

Created: 2021-05-06 03:45:49 UTC

Size: 4.68 KB

patch

obsolete

>From 9e92e64acbb07c99f61589e2a6aceea15211d5d6 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>
>Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
>
>Signed-off-by: David Cook <dcook@prosentient.com.au>
>---
> 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 7c1265e220..2da010e001 100755
>--- a/catalogue/search.pl
>+++ b/catalogue/search.pl
>@@ -379,21 +379,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 d4694e105d..410f3f37c5 100755
>--- a/opac/opac-search.pl
>+++ b/opac/opac-search.pl
>@@ -486,19 +486,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