From 99c271f398c914b1fbbab720828c56ec28a78ae8 Mon Sep 17 00:00:00 2001 From: Olli-Antti Kivilahti Date: Tue, 22 Jul 2014 13:01:54 +0300 Subject: [PATCH] Bug 11677 - Limit to Only items currently available for loan or reference not working! This row: $availability_limit .= "( ( allrecords,AlwaysMatches='' not onloan,AlwaysMatches='') and (lost,st-numeric=0) )"; Causes all records in Zebra to be selected and removes records with even one item onloan (even if items would be available)! This is obviously wrong, but because Zebra (and not many other text indexers) cannot deal with (If undef)-search terms, a fix might be to set 952$q (onloan) datetime to 0000-00-00 instead of NULL in the DB. Then we could find Records with items where onloan is 0000-00-00. Comments out that Zebra availability search and deals with availability on Koha-side. Gets the branch or holdingbranch limit from opac-search.pl or search.pl and uses that to skip Zebra results which are onloan or notforloan. Updates the result set size when results are removed. Fixed a bug where pagination broke, due to the $results_hash->{'RECORDS'} -array getting populated always starting from 0, where it should have started from the given $offset. --- C4/Search.pm | 73 +++++++++++++++++++++++++++++++++++++++++++++++---- catalogue/search.pl | 11 ++++++-- opac/opac-search.pl | 11 ++++++-- 3 files changed, 86 insertions(+), 9 deletions(-) diff --git a/C4/Search.pm b/C4/Search.pm index 9b8bd4e..c939a63 100644 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -251,6 +251,7 @@ sub SimpleSearch { $zoom_queries[$i] = new ZOOM::Query::PQF( $query, $zconns[$i]); } else { $query =~ s/:/=/g; + $query =~ s/\?//g; #Remove ? because this error is thrown :> CCL parsing error (10014) Right truncation not supported ZOOM for query: title=MISSÄ ON JATKOT? at /C4/Search.pm line 276. $zoom_queries[$i] = new ZOOM::Query::CCL2RPN( $query, $zconns[$i]); } $tmpresults[$i] = $zconns[$i]->search( $zoom_queries[$i] ); @@ -327,7 +328,7 @@ sub getRecords { my ( $koha_query, $simple_query, $sort_by_ref, $servers_ref, $results_per_page, $offset, $expanded_facet, $branches, - $itemtypes, $query_type, $scan, $opac + $itemtypes, $query_type, $scan, $opac, $availableLimit ) = @_; my @servers = @$servers_ref; @@ -347,6 +348,15 @@ sub getRecords { my @facets_loop; # stores the ref to array of hashes for template facets loop + #Get the required marc tags for determining branch-based availability. + my ( $holdingbranchField, $holdingbranchSubfield, $onloanField, $onloanSubfield, $notforloanField, $notforloanSubfield ); + if ($availableLimit) { + ( $holdingbranchField, $holdingbranchSubfield ) = GetMarcFromKohaField( "items.holdingbranch" ); + ( $onloanField, $onloanSubfield ) = GetMarcFromKohaField( "items.onloan" ); + ( $notforloanField, $notforloanSubfield ) = GetMarcFromKohaField( "items.notforloan" ); + } + + ### LOOP THROUGH THE SERVERS for ( my $i = 0 ; $i < @servers ; $i++ ) { $zconns[$i] = C4::Context->Zconn( $servers[$i], 1 ); @@ -450,6 +460,13 @@ sub getRecords { $times = $size; } + #If we are using pagination to display other resultset pages. + #The results_hash needs to contain Records starting from the given + #offset. + if ($offset > 0) { + $results_hash->{'RECORDS'}->[$offset-1] = undef; + } + for ( my $j = $offset ; $j < $times ; $j++ ) { my $records_hash; my $record; @@ -491,9 +508,19 @@ sub getRecords { # not an index scan else { - $record = $results[ $i - 1 ]->record($j)->raw(); + $record = $results[ $i - 1 ]->record($j); + last() unless $record; + $record = $record->raw(); # warn "RECORD $j:".$record; - $results_hash->{'RECORDS'}[$j] = $record; + + if ( $availableLimit && #Availablelimit is 1 if there is no limit=branch:FTL given + not(isAvailableFromMARCXML( $record, $availableLimit, $holdingbranchField, $holdingbranchSubfield, $onloanField, $onloanSubfield, $notforloanField, $notforloanSubfield )) ) { + $times++; #Skip this record, but make sure we still get the $results_per_page results to display. + $results_hash->{'hits'}--; #This wasn't a hit after all :( + next(); + } + + push @{$results_hash->{'RECORDS'}}, $record; } } @@ -1581,8 +1608,8 @@ sub buildQuery { ## 'available' is defined as (items.onloan is NULL) and (items.itemlost = 0) ## In English: ## all records not indexed in the onloan register (zebra) and all records with a value of lost equal to 0 - $availability_limit .= -"( ( allrecords,AlwaysMatches='' not onloan,AlwaysMatches='') and (lost,st-numeric=0) )"; #or ( allrecords,AlwaysMatches='' not lost,AlwaysMatches='')) )"; +# $availability_limit .= +#"( ( allrecords,AlwaysMatches='' not onloan,AlwaysMatches='') and (lost,st-numeric=0) )"; #or ( allrecords,AlwaysMatches='' not lost,AlwaysMatches='')) )"; $limit_cgi .= "&limit=available"; $limit_desc .= ""; } @@ -2461,6 +2488,42 @@ sub new_record_from_zebra { } +#Check each item in the MARCXML-String for onloan or notforloan +#Return 1 when the first available item is found. +#PARAM $limitHoldingbranch, counterintuitively is 1, if no holdingbranch based limiting is desired, +# and the holdingbranch code if we want to limit by branch. + +sub isAvailableFromMARCXML { + my ( $recordXML, $limitHoldingbranch, $holdingbranchField, $holdingbranchSubfield, $onloanField, $onloanSubfield, $notforloanField, $notforloanSubfield ) = @_; + + my @itemsFields = $recordXML =~ /(.*?)<\/datafield>/sg; + + foreach my $fieldStr (@itemsFields) { + my ($itemHoldingbranch, $itemOnloan, $itemNotforloan); + + if ($fieldStr =~ /(.*?)<\/subfield>/s) { + $itemOnloan = $1; + next() if $itemOnloan; + } + if ($fieldStr =~ /(.*?)<\/subfield>/s) { + $itemNotforloan = $1; + next() if $itemNotforloan; + } + if ($limitHoldingbranch != 1) { #Check for holdingbranch-based availability. + if ($fieldStr =~ /(.*?)<\/subfield>/s) { + $itemHoldingbranch = $1; + } + if ($itemHoldingbranch && $itemHoldingbranch eq $limitHoldingbranch) { + return 1; #Available ! wooee! + } + } + else { + return 1; #We are not checking for holdingbranch-based availability, so any availability is fine! + } + } + return 0; +} + END { } # module clean-up code here (global destructor) 1; diff --git a/catalogue/search.pl b/catalogue/search.pl index 629a317..711a61d 100755 --- a/catalogue/search.pl +++ b/catalogue/search.pl @@ -425,12 +425,19 @@ if($params->{'multibranchlimit'}) { push @limits, $multibranch if ($multibranch ne '()'); } -my $available; +my ($available, $availableBranch); foreach my $limit(@limits) { if ($limit =~/available/) { $available = 1; } + elsif ($limit =~ /branch:(.*?)$/) { #Catch holdingbranch from facets and branch from advanced search + $availableBranch = $1; + } +} +if ($availableBranch && $available) { + $available = $availableBranch; } +undef $availableBranch; #Don't pollute namespace! $template->param(available => $available); # append year limits if they exist @@ -528,7 +535,7 @@ my @results_array; my $results_hashref; eval { - ($error, $results_hashref, $facets) = getRecords($query,$simple_query,\@sort_by,\@servers,$results_per_page,$offset,$expanded_facet,$branches,$itemtypes,$query_type,$scan); + ($error, $results_hashref, $facets) = getRecords($query,$simple_query,\@sort_by,\@servers,$results_per_page,$offset,$expanded_facet,$branches,$itemtypes,$query_type,$scan,undef,$available); }; # This sorts the facets into alphabetical order diff --git a/opac/opac-search.pl b/opac/opac-search.pl index cc6beb7..28e54f5 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -410,12 +410,19 @@ if($params->{'multibranchlimit'}) { push @limits, $multibranch if ($multibranch ne '()'); } -my $available; +my ($available, $availableBranch); foreach my $limit(@limits) { if ($limit =~/available/) { $available = 1; } + elsif ($limit =~ /branch:(.*?)$/) { #Catch holdingbranch from facets and branch from advanced search + $availableBranch = $1; + } +} +if ($availableBranch && $available) { + $available = $availableBranch; } +undef $availableBranch; #Don't pollute namespace! $template->param(available => $available); # append year limits if they exist @@ -519,7 +526,7 @@ if ($tag) { $pasarParams .= '&simple_query=' . $simple_query; $pasarParams .= '&query_type=' . $query_type if ($query_type); eval { - ($error, $results_hashref, $facets) = getRecords($query,$simple_query,\@sort_by,\@servers,$results_per_page,$offset,$expanded_facet,$branches,$itemtypes,$query_type,$scan,1); + ($error, $results_hashref, $facets) = getRecords($query,$simple_query,\@sort_by,\@servers,$results_per_page,$offset,$expanded_facet,$branches,$itemtypes,$query_type,$scan,1,$available); }; } # This sorts the facets into alphabetical order -- 1.7.9.5