Bugzilla – Attachment 31424 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]
[PASSED QA] Bug 12788: (followup) minor optimization with proper tests
PASSED-QA-Bug-12788-followup-minor-optimization-wi.patch (text/plain), 7.83 KB, created by
Katrin Fischer
on 2014-09-05 18:34:57 UTC
(
hide
)
Description:
[PASSED QA] Bug 12788: (followup) minor optimization with proper tests
Filename:
MIME Type:
Creator:
Katrin Fischer
Created:
2014-09-05 18:34:57 UTC
Size:
7.83 KB
patch
obsolete
>From 75a01a9bd378c873f7acb7be31d9ee1aaa5332c1 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@gmail.com> >Date: Wed, 20 Aug 2014 11:39:27 -0300 >Subject: [PATCH] [PASSED QA] Bug 12788: (followup) minor optimization with > proper tests > >This patch removes the $facets_info calculation from the _get_facets_data_from_record >sub so it is not done for each record. It introduces a new sub, _get_facets_info >that is called from the getRecords loop, that does the job only once. > >To test: >- Apply on top of the previous patches >- Run > $ prove -v t/db_dependent/Search.t >=> SUCCESS: _get_facets_info gets tested and it passes for both MARC21 and UNIMARC. > Facets rendering should remain unchaged on the UI. >- 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> >Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> >--- > C4/Search.pm | 29 ++++++++++++++++++++---- > t/db_dependent/Search.t | 60 +++++++++++++++++++++++++++++++++++++++++++------ > 2 files changed, 78 insertions(+), 11 deletions(-) > >diff --git a/C4/Search.pm b/C4/Search.pm >index 54ef9ec..932d04b 100644 >--- a/C4/Search.pm >+++ b/C4/Search.pm >@@ -520,7 +520,8 @@ sub getRecords { > next; > } > >- _get_facets_data_from_record( $marc_record, $facets, $facets_counter, $facets_info ); >+ _get_facets_data_from_record( $marc_record, $facets, $facets_counter ); >+ $facets_info = _get_facets_info( $facets ); > } > } > >@@ -656,7 +657,7 @@ sub getRecords { > 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 and $facets_info for using in getRecords. >+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). >@@ -665,7 +666,7 @@ facets for Zebra). > > sub _get_facets_data_from_record { > >- my ( $marc_record, $facets, $facets_counter, $facets_info ) = @_; >+ my ( $marc_record, $facets, $facets_counter ) = @_; > > for my $facet (@$facets) { > >@@ -692,10 +693,30 @@ sub _get_facets_data_from_record { > } > } > } >- # update $facets_info so we know what facet categories need to be rendered >+ } >+} >+ >+=head2 _get_facets_info >+ >+ my $facets_info = C4::Search::_get_facets_info( $facets ) >+ >+Internal function that extracts facets information and properly builds >+the data structure needed to render facet labels. >+ >+=cut >+ >+sub _get_facets_info { >+ >+ my $facets = shift; >+ >+ my $facets_info = {}; >+ >+ for my $facet ( @$facets ) { > $facets_info->{ $facet->{ idx } }->{ label_value } = $facet->{ label }; > $facets_info->{ $facet->{ idx } }->{ expanded } = $facet->{ expanded }; > } >+ >+ return $facets_info; > } > > sub pazGetRecords { >diff --git a/t/db_dependent/Search.t b/t/db_dependent/Search.t >index 11bbba9..a10b71b 100644 >--- a/t/db_dependent/Search.t >+++ b/t/db_dependent/Search.t >@@ -129,6 +129,8 @@ $contextmodule->mock('preference', sub { > return '--'; > } elsif ($pref eq 'DisplayLibraryFacets') { > return 'holding'; >+ } elsif ($pref eq 'UNIMARCAuthorsFacetsSeparator') { >+ return '--'; > } else { > warn "The syspref $pref was requested but I don't know what to say; this indicates that the test requires updating" > unless $pref =~ m/(XSLT|item|branch|holding|image)/i; >@@ -859,7 +861,6 @@ if ( $indexing_mode eq 'dom' ) { > > # 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; >@@ -869,7 +870,7 @@ if ( $indexing_mode eq 'dom' ) { > [ '100', 'z', ' ', a => 'Tomasito' ], > [ '245', ' ', ' ', a => 'First try' ] > ); >- C4::Search::_get_facets_data_from_record($marc_record, $facets, $facets_counter,$facets_info); >+ C4::Search::_get_facets_data_from_record( $marc_record, $facets, $facets_counter ); > 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; >@@ -879,10 +880,33 @@ if ( $indexing_mode eq 'dom' ) { > [ '100', 'z', ' ', a => 'Tomasito' ], > [ '245', ' ', ' ', a => 'Second try' ] > ); >- C4::Search::_get_facets_data_from_record($marc_record, $facets, $facets_counter, $facets_info); >+ C4::Search::_get_facets_data_from_record( $marc_record, $facets, $facets_counter ); > is_deeply( { au => { 'Cohen Arazi, Tomas' => 2 } }, $facets_counter, > "_get_facets_data_from_record correctly counts author facet twice"); > >+ # Test _get_facets_info >+ my $facets_info = C4::Search::_get_facets_info( $facets ); >+ my $expected_facets_info_marc21 = { >+ 'au' => { 'expanded' => undef, >+ 'label_value' => "Authors" }, >+ 'holdingbranch' => { 'expanded' => undef, >+ 'label_value' => "HoldingLibrary" }, >+ 'itype' => { 'expanded' => undef, >+ 'label_value' => "ItemTypes" }, >+ 'location' => { 'expanded' => undef, >+ 'label_value' => "Location" }, >+ 'se' => { 'expanded' => undef, >+ 'label_value' => "Series" }, >+ 'su-geo' => { 'expanded' => undef, >+ 'label_value' => "Places" }, >+ 'su-to' => { 'expanded' => undef, >+ 'label_value' => "Topics" }, >+ 'su-ut' => { 'expanded' => undef, >+ 'label_value' => "Titles" } >+ }; >+ is_deeply( $facets_info, $expected_facets_info_marc21, >+ "_get_facets_info returns the correct data"); >+ > cleanup(); > } > >@@ -954,26 +978,48 @@ sub run_unimarc_search_tests { > ); > is($count, 24, 'UNIMARC authorities: hits on any starts with "jean"'); > >+ # Test _get_facets_info >+ my $facets = C4::Koha::getFacets(); >+ my $facets_info = C4::Search::_get_facets_info( $facets ); >+ my $expected_facets_info_unimarc = { >+ 'au' => { 'expanded' => undef, >+ 'label_value' => "Authors" }, >+ 'holdingbranch' => { 'expanded' => undef, >+ 'label_value' => "HoldingLibrary" }, >+ 'location' => { 'expanded' => undef, >+ 'label_value' => "Location" }, >+ 'se' => { 'expanded' => undef, >+ 'label_value' => "Series" }, >+ 'su-geo' => { 'expanded' => undef, >+ 'label_value' => "Places" }, >+ 'su-to' => { 'expanded' => undef, >+ 'label_value' => "Topics" }, >+ 'su-ut' => { 'expanded' => undef, >+ 'label_value' => "Titles" } >+ }; >+ is_deeply( $facets_info, $expected_facets_info_unimarc, >+ "_get_facets_info returns the correct data"); >+ > cleanup(); > } > > subtest 'MARC21 + GRS-1' => sub { >- plan tests => 108; >+ plan tests => 109; > run_marc21_search_tests('grs1'); > }; > > subtest 'MARC21 + DOM' => sub { >- plan tests => 108; >+ plan tests => 109; > run_marc21_search_tests('dom'); > }; > > subtest 'UNIMARC + GRS-1' => sub { >- plan tests => 13; >+ plan tests => 14; > run_unimarc_search_tests('grs1'); > }; > > subtest 'UNIMARC + DOM' => sub { >- plan tests => 13; >+ plan tests => 14; > run_unimarc_search_tests('dom'); > }; > >-- >1.9.1
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