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

[patch] Bug 21249: Code cleaning

Bug-21249-Code-cleaning.patch (text/plain), 4.49 KB, created by Jonathan Druart on 2019-11-06 11:21:27 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2019-11-06 11:21:27 UTC

Size: 4.49 KB

patch

obsolete

>From 783dc82d3b3bc1b983b71f050a8df4061a1f96f4 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 | 28 ++++++++++++++++------------
> 2 files changed, 32 insertions(+), 26 deletions(-)
>
>diff --git a/catalogue/search.pl b/catalogue/search.pl
>index 4a6037de08..f8ab058a6f 100755
>--- a/catalogue/search.pl
>+++ b/catalogue/search.pl
>@@ -412,21 +412,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;
>-     my $multihomebranch = '('.join( " or ", map { 'homebranch:     ' . $_->id } @libraries ) .')';
>-     my $multiholdingbranch = '('.join( " or ", map { 'holdingbranch: ' . $_->id } @libraries ) .')';
>-
>-    if ( $branchfield eq "homebranch") {
>-        push @limits, $multihomebranch if ( $multihomebranch ne '()');
>-    } elsif ( $branchfield eq "holdingbranch") {
>-        push @limits, $multiholdingbranch if ( $multiholdingbranch ne '()');
>-    } else {
>-        my $multihomeandholdingbranch = '('.join( " or ", map { 'homebranch: ' . $_->id, 'holdingbranch: ' . $_->id}  @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 f98cb51ffa..091c042e4d 100755
>--- a/opac/opac-search.pl
>+++ b/opac/opac-search.pl
>@@ -502,19 +502,23 @@ 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;
>-    my $multihomebranch = '('.join( " or ", map { 'homebranch: ' . $_->id } @libraries ) .')';
>-    my $multiholdingbranch = '('.join( " or ", map { 'holdingbranch: ' . $_->id } @libraries ) .')';
>-
>-    if ( $branchfield eq "homebranch" ) {
>-        push @limits, $multihomebranch if ( $multihomebranch ne '()');
>-    } elsif ( $branchfield eq "holdingbranch") {
>-        push @limits, $multiholdingbranch if ( $multiholdingbranch ne '()');
>-    } else {
>-        my $multihomeandholdingbranch = '('.join( " or ", map { 'homebranch: ' . $_->id, 'holdingbranch: ' . $_->id}  @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