Bugzilla – Attachment 31315 Details for
Bug 12788
Refine search displays 4xx fields from authority records
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bub 12788: (regression test) refactor facet extraction code to allow testing
Bub-12788-regression-test-refactor-facet-extractio.patch (text/plain), 8.62 KB, created by
David Cook
on 2014-09-02 03:26:56 UTC
(
hide
)
Description:
Bub 12788: (regression test) refactor facet extraction code to allow testing
Filename:
MIME Type:
Creator:
David Cook
Created:
2014-09-02 03:26:56 UTC
Size:
8.62 KB
patch
obsolete
>From 9b21233f3cc2d569994d4d92478b8dd6523cd863 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@gmail.com> >Date: Wed, 20 Aug 2014 00:33:40 -0300 >Subject: [PATCH] Bub 12788: (regression test) refactor facet extraction code > to allow testing > >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 > >Signed-off-by: Nick Clemens <nick@quecheelibrary.org> >Signed-off-by: David Cook <dcook@prosentient.com.au> >--- > 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.7.7.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 12788
:
31002
|
31003
|
31004
|
31005
|
31026
|
31027
|
31267
|
31268
|
31269
|
31315
|
31316
|
31317
|
31422
|
31423
|
31424