gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
This commit is contained in:
Tomas Cohen Arazi 2014-08-20 11:39:27 -03:00
parent eefef42abd
commit ead0d88c74
2 changed files with 78 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
C4/Search.pm
View File

@ -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 {

60
t/db_dependent/Search.t
View File

@ -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');
};