[Koha-patches] [PATCH 1/2] Bub 12788: (regression test) refactor facet extraction code to allow testing
Tomas Cohen Arazi
tomascohen at gmail.com
Wed Aug 20 06:06:41 CEST 2014
This patch refactors the facet extraction loop into a proper function.
The loop is changed so the MARC::Record objects are created only once
instead of the old/current behaviour: once for each defined facet (in
C4::Koha::getFacets).
To test:
- Apply the patch
=> SUCCESS: verify facets functionality remains unchanged.
- Run:
$ prove -v t/db_dependent/Search.t
=> SUCCESS: tests for _get_facets_data_from_record fail, because
100$a is considered for fields with indicator 1=z (field added
by IncludeSeeFromInSearches syspref).
- Sign off :-D
Sponsored-by: Universidad Nacional de Cordoba
---
C4/Search.pm | 104 +++++++++++++++++++++++++++++-------------------
t/db_dependent/Search.t | 30 +++++++++++++-
2 files changed, 91 insertions(+), 43 deletions(-)
diff --git a/C4/Search.pm b/C4/Search.pm
index c704c63..9587f7d 100644
--- a/C4/Search.pm
+++ b/C4/Search.pm
@@ -340,8 +340,8 @@ sub getRecords {
my $results_hashref = ();
# Initialize variables for the faceted results objects
- my $facets_counter = ();
- my $facets_info = ();
+ my $facets_counter = {};
+ my $facets_info = {};
my $facets = getFacets();
my $facets_maxrecs = C4::Context->preference('maxRecordsForFacets')||20;
@@ -499,51 +499,29 @@ sub getRecords {
}
$results_hashref->{ $servers[ $i - 1 ] } = $results_hash;
-# Fill the facets while we're looping, but only for the biblioserver and not for a scan
+ # Fill the facets while we're looping, but only for the
+ # biblioserver and not for a scan
if ( !$scan && $servers[ $i - 1 ] =~ /biblioserver/ ) {
- my $jmax =
- $size > $facets_maxrecs ? $facets_maxrecs : $size;
- for my $facet (@$facets) {
- for ( my $j = 0 ; $j < $jmax ; $j++ ) {
+ my $jmax = $size > $facets_maxrecs
+ ? $facets_maxrecs
+ : $size;
- my $marc_record = new_record_from_zebra (
- 'biblioserver',
- $results[ $i - 1 ]->record($j)->raw()
- );
-
- if ( ! defined $marc_record ) {
- warn "ERROR DECODING RECORD - $@: " .
- $results[ $i - 1 ]->record($j)->raw();
- next;
- }
-
- my @used_datas = ();
+ for ( my $j = 0 ; $j < $jmax ; $j++ ) {
- foreach my $tag ( @{ $facet->{tags} } ) {
+ my $marc_record = new_record_from_zebra (
+ 'biblioserver',
+ $results[ $i - 1 ]->record($j)->raw()
+ );
- # avoid first line
- my $tag_num = substr( $tag, 0, 3 );
- my $subfield_letters = substr( $tag, 3 );
- # Removed when as_string fixed
- my @subfields = $subfield_letters =~ /./sg;
-
- my @fields = $marc_record->field($tag_num);
- foreach my $field (@fields) {
- my $data = $field->as_string( $subfield_letters, $facet->{sep} );
+ if ( ! defined $marc_record ) {
+ warn "ERROR DECODING RECORD - $@: " .
+ $results[ $i - 1 ]->record($j)->raw();
+ next;
+ }
- unless ( grep { /^\Q$data\E$/ } @used_datas ) {
- push @used_datas, $data;
- $facets_counter->{ $facet->{idx} }->{$data}++;
- }
- } # fields
- } # field codes
- } # records
- $facets_info->{ $facet->{idx} }->{label_value} =
- $facet->{label};
- $facets_info->{ $facet->{idx} }->{expanded} =
- $facet->{expanded};
- } # facets
+ _get_facets_data_from_record( $marc_record, $facets, $facets_counter, $facets_info );
+ }
}
# warn "connection ", $i-1, ": $size hits";
@@ -673,6 +651,50 @@ sub getRecords {
return ( undef, $results_hashref, \@facets_loop );
}
+=head2 _get_facets_data_from_record
+
+ C4::Search::_get_facets_data_from_record( $marc_record, $facets, $facets_counter );
+
+Internal function that extracts facets information from a MARC::Record object
+and populates $facets_counter for using in getRecords.
+
+$facets is expected to be filled with C4::Koha::getFacets output (i.e. the configured
+facets for Zebra).
+
+=cut
+
+sub _get_facets_data_from_record {
+
+ my ( $marc_record, $facets, $facets_counter, $facets_info ) = @_;
+
+ for my $facet (@$facets) {
+
+ my @used_datas = ();
+
+ foreach my $tag ( @{ $facet->{ tags } } ) {
+
+ # avoid first line
+ my $tag_num = substr( $tag, 0, 3 );
+ my $subfield_letters = substr( $tag, 3 );
+ # Removed when as_string fixed
+ my @subfields = $subfield_letters =~ /./sg;
+
+ my @fields = $marc_record->field( $tag_num );
+ foreach my $field (@fields) {
+ my $data = $field->as_string( $subfield_letters, $facet->{ sep } );
+
+ unless ( grep { /^\Q$data\E$/ } @used_datas ) {
+ push @used_datas, $data;
+ $facets_counter->{ $facet->{ idx } }->{ $data }++;
+ }
+ }
+ }
+ # update $facets_info so we know what facet categories need to be rendered
+ $facets_info->{ $facet->{ idx } }->{ label_value } = $facet->{ label };
+ $facets_info->{ $facet->{ idx } }->{ expanded } = $facet->{ expanded };
+ }
+}
+
sub pazGetRecords {
my (
$koha_query, $simple_query, $sort_by_ref, $servers_ref,
diff --git a/t/db_dependent/Search.t b/t/db_dependent/Search.t
index cadfedb..11bbba9 100644
--- a/t/db_dependent/Search.t
+++ b/t/db_dependent/Search.t
@@ -857,6 +857,32 @@ if ( $indexing_mode eq 'dom' ) {
getRecords('ccl=( AND )', '', ['title_az'], [ 'biblioserver' ], '20', 0, undef, \%branches, \%itemtypes, 'ccl', undef)
} qr/WARNING: query problem with/, 'got warning instead of crash when attempting to run invalid query (bug 9578)';
+ # Test facet calculation
+ my $facets_counter = {};
+ my $facets_info = {};
+ my $facets = C4::Koha::getFacets();
+ # Create a record with a 100$z field
+ my $marc_record = MARC::Record->new;
+ $marc_record->add_fields(
+ [ '001', '1234' ],
+ [ '100', ' ', ' ', a => 'Cohen Arazi, Tomas' ],
+ [ '100', 'z', ' ', a => 'Tomasito' ],
+ [ '245', ' ', ' ', a => 'First try' ]
+ );
+ C4::Search::_get_facets_data_from_record($marc_record, $facets, $facets_counter,$facets_info);
+ is_deeply( { au => { 'Cohen Arazi, Tomas' => 1 } }, $facets_counter,
+ "_get_facets_data_from_record doesn't count 100\$z (Bug 12788)");
+ $marc_record = MARC::Record->new;
+ $marc_record->add_fields(
+ [ '001', '1234' ],
+ [ '100', ' ', ' ', a => 'Cohen Arazi, Tomas' ],
+ [ '100', 'z', ' ', a => 'Tomasito' ],
+ [ '245', ' ', ' ', a => 'Second try' ]
+ );
+ C4::Search::_get_facets_data_from_record($marc_record, $facets, $facets_counter, $facets_info);
+ is_deeply( { au => { 'Cohen Arazi, Tomas' => 2 } }, $facets_counter,
+ "_get_facets_data_from_record correctly counts author facet twice");
+
cleanup();
}
@@ -932,12 +958,12 @@ sub run_unimarc_search_tests {
}
subtest 'MARC21 + GRS-1' => sub {
- plan tests => 106;
+ plan tests => 108;
run_marc21_search_tests('grs1');
};
subtest 'MARC21 + DOM' => sub {
- plan tests => 106;
+ plan tests => 108;
run_marc21_search_tests('dom');
};
--
1.9.1
More information about the Koha-patches
mailing list