From 34ec88eb0a9316f38e3c65be0a79f72a58471ca5 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Wed, 19 Jul 2017 16:54:40 +0200 Subject: [PATCH] Bug 10306: Add tests for module changes Content-Type: text/plain; charset=utf-8 GetMarcSubfieldStructure: In Biblio.t we are adding a subtest that checks the structure returned by this routine. Is it a hashref pointing to arrayrefs of hashrefs? ;) In Search.t this routine was mocked. The change in the returned structure is now applied to this mock too (moving the marc tag hashes into arrayrefs). GetMarcFromKohaField: In Biblio.t we add a subtest for it. We are checking if it returns multiple mappings (per kohafield) and calling it in scalar context too. GetMarcSubfieldStructureFromKohaField: The existing subtest in Biblio.t is expanded to also test the call in list context. TransformKohaToMarc: This sub has its own test script. We are adding a subtest in TransformKohaToMarc.t for a test with multiple mappings, and for mapping to a control field in another framework. This also tests the additional framework parameter. Additionally, we add a test for the new no_split option used for items. TransformMarcToKoha: This implicitly tests its helper sub _get_inverted. This patch adds a new test script for this routine. TransformMarcToKohaOneField: A few tests are added to the previous new test script. Test plan: [1] Run t/db_dependent/Biblio.t [2] Run t/db_dependent/Biblio/TransformKohaToMarc.t [3] Run t/db_dependent/Biblio/TransformMarcToKoha.t [4] Run t/db_dependent/Search.t --- t/db_dependent/Biblio.t | 73 ++++++++++++++++++++--- t/db_dependent/Biblio/TransformKohaToMarc.t | 76 +++++++++++++++++++++++- t/db_dependent/Biblio/TransformMarcToKoha.t | 89 +++++++++++++++++++++++++++++ t/db_dependent/Search.t | 76 ++++++++++++------------ 4 files changed, 266 insertions(+), 48 deletions(-) create mode 100644 t/db_dependent/Biblio/TransformMarcToKoha.t diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t index 08e09a1..b57216c 100755 --- a/t/db_dependent/Biblio.t +++ b/t/db_dependent/Biblio.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 7; +use Test::More tests => 9; use Test::MockModule; use List::MoreUtils qw( uniq ); @@ -25,6 +25,8 @@ use MARC::Record; use t::lib::Mocks qw( mock_preference ); use Koha::Database; +use Koha::Caches; +use Koha::MarcSubfieldStructures; BEGIN { use_ok('C4::Biblio'); @@ -33,9 +35,10 @@ BEGIN { my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; my $dbh = C4::Context->dbh; +Koha::Caches->get_instance->clear_from_cache( "MarcSubfieldStructure-" ); subtest 'GetMarcSubfieldStructureFromKohaField' => sub { - plan tests => 23; + plan tests => 25; my @columns = qw( tagfield tagsubfield liblibrarian libopac repeatable mandatory kohafield tab @@ -54,11 +57,61 @@ subtest 'GetMarcSubfieldStructureFromKohaField' => sub { is($marc_subfield_structure->{kohafield}, 'biblio.biblionumber', "Result is the good result"); like($marc_subfield_structure->{tagfield}, qr/^\d{3}$/, "tagfield is a valid tagfield"); + # Add a test for list context (BZ 10306) + my @results = GetMarcSubfieldStructureFromKohaField('biblio.biblionumber', ''); + is( @results, 1, 'We expect only one mapping' ); + is_deeply( $results[0], $marc_subfield_structure, + 'The first entry should be the same hashref as we had before' ); + # foo.bar does not exist so this should return undef $marc_subfield_structure = GetMarcSubfieldStructureFromKohaField('foo.bar', ''); is($marc_subfield_structure, undef, "invalid kohafield returns undef"); + }; +subtest "GetMarcSubfieldStructure" => sub { + plan tests => 5; + + # Add multiple Koha to Marc mappings + my $mapping1 = Koha::MarcSubfieldStructures->find('','399','a') // Koha::MarcSubfieldStructure->new({ frameworkcode => '', tagfield => '399', tagsubfield => 'a' }); + $mapping1->kohafield( "mytable.nicepages" ); + $mapping1->store; + my $mapping2 = Koha::MarcSubfieldStructures->find('','399','b') // Koha::MarcSubfieldStructure->new({ frameworkcode => '', tagfield => '399', tagsubfield => 'b' }); + $mapping2->kohafield( "mytable.nicepages" ); + $mapping2->store; + Koha::Caches->get_instance->clear_from_cache( "MarcSubfieldStructure-" ); + my $structure = C4::Biblio::GetMarcSubfieldStructure(''); + + is( @{ $structure->{"mytable.nicepages"} }, 2, + 'GetMarcSubfieldStructure should return two entries for nicepages' ); + is( $structure->{"mytable.nicepages"}->[0]->{tagfield}, '399', + 'Check tagfield for first entry' ); + is( $structure->{"mytable.nicepages"}->[0]->{tagsubfield}, 'a', + 'Check tagsubfield for first entry' ); + is( $structure->{"mytable.nicepages"}->[1]->{tagfield}, '399', + 'Check tagfield for second entry' ); + is( $structure->{"mytable.nicepages"}->[1]->{tagsubfield}, 'b', + 'Check tagsubfield for second entry' ); +}; + +subtest "GetMarcFromKohaField" => sub { + plan tests => 6; + + #NOTE: We are building on data from the previous subtest + # With: field 399 / mytable.nicepages + + # Check call in list context for multiple mappings + my @retval = C4::Biblio::GetMarcFromKohaField('mytable.nicepages', ''); + is( @retval, 4, 'Should return two tags and subfields' ); + is( $retval[0], '399', 'Check first tag' ); + is( $retval[1], 'a', 'Check first subfield' ); + is( $retval[2], '399', 'Check second tag' ); + is( $retval[3], 'b', 'Check second subfield' ); + + # Check same call in scalar context + is( C4::Biblio::GetMarcFromKohaField('mytable.nicepages', ''), '399', + 'GetMarcFromKohaField returns first tag in scalar context' ); +}; # Mocking variables my $biblio_module = new Test::MockModule('C4::Biblio'); @@ -75,12 +128,12 @@ $biblio_module->mock( my ( $itemnumber_field, $itemnumber_subfield ) = get_itemnumber_field(); return { - 'biblio.title' => { tagfield => $title_field, tagsubfield => $title_subfield }, - 'biblio.biblionumber' => { tagfield => $biblionumber_field, tagsubfield => $biblionumber_subfield }, - 'biblioitems.isbn' => { tagfield => $isbn_field, tagsubfield => $isbn_subfield }, - 'biblioitems.issn' => { tagfield => $issn_field, tagsubfield => $issn_subfield }, - 'biblioitems.biblioitemnumber' => { tagfield => $biblioitemnumber_field, tagsubfield => $biblioitemnumber_subfield }, - 'items.itemnumber' => { tagfield => $itemnumber_subfield, tagsubfield => $itemnumber_subfield }, + 'biblio.title' => [ { tagfield => $title_field, tagsubfield => $title_subfield } ], + 'biblio.biblionumber' => [ { tagfield => $biblionumber_field, tagsubfield => $biblionumber_subfield } ], + 'biblioitems.isbn' => [ { tagfield => $isbn_field, tagsubfield => $isbn_subfield } ], + 'biblioitems.issn' => [ { tagfield => $issn_field, tagsubfield => $issn_subfield } ], + 'biblioitems.biblioitemnumber' => [ { tagfield => $biblioitemnumber_field, tagsubfield => $biblioitemnumber_subfield } ], + 'items.itemnumber' => [ { tagfield => $itemnumber_subfield, tagsubfield => $itemnumber_subfield } ], }; } ); @@ -389,4 +442,6 @@ subtest 'deletedbiblio_metadata' => sub { is( $moved, $biblionumber, 'Found in deletedbiblio_metadata' ); }; -1; +# Cleanup +Koha::Caches->get_instance->clear_from_cache( "MarcSubfieldStructure-" ); +$schema->storage->txn_rollback; diff --git a/t/db_dependent/Biblio/TransformKohaToMarc.t b/t/db_dependent/Biblio/TransformKohaToMarc.t index 34a2a5e..978861f 100644 --- a/t/db_dependent/Biblio/TransformKohaToMarc.t +++ b/t/db_dependent/Biblio/TransformKohaToMarc.t @@ -1,8 +1,10 @@ use Modern::Perl; -use Test::More tests => 1; +use Test::More tests => 4; use MARC::Record; use t::lib::Mocks; +use t::lib::TestBuilder; + use Koha::Database; use Koha::Caches; use Koha::MarcSubfieldStructures; @@ -37,6 +39,78 @@ is_deeply( \@subfields, [ ], 'TransformKohaToMarc should return sorted subfields (regression test for bug 12343)' ); + +# Now test multiple mappings per kohafield too +subtest "Multiple Koha to MARC mappings (BZ 10306)" => sub { + plan tests => 4; + + # 300a and 260d mapped to mytable.nicepages + my $mapping3 = Koha::MarcSubfieldStructures->find('','260','d') // Koha::MarcSubfieldStructure->new({ frameworkcode => '', tagfield => '260', tagsubfield => 'd' }); + $mapping3->kohafield( "mytable.nicepages" ); + $mapping3->store; + Koha::Caches->get_instance->clear_from_cache( "MarcSubfieldStructure-" ); + + # Include two values in goodillustrations too: should result in two + # subfields. + my $record = C4::Biblio::TransformKohaToMarc({ + "mytable2.goodillustrations" => "good | better", + "mytable.nicepages" => "nice", + }); + is( $record->subfield('260','d'), "nice", "Check 260d" ); + is( $record->subfield('300','a'), "nice", "Check 300a" ); + is( $record->subfield('300','b'), "good", "Check first 300b" ); + is( ($record->field('300')->subfield('b'))[1], "better", + "Check second 300b" ); +}; + +subtest "Working with control fields in another framework" => sub { + plan tests => 2; + + # Add a new framework + my $fw = t::lib::TestBuilder->new->build_object({ + class => 'Koha::BiblioFrameworks' + }); + + # Map a controlfield to 'fullcontrol' + my $mapping = Koha::MarcSubfieldStructure->new({ frameworkcode => $fw->frameworkcode, tagfield => '001', tagsubfield => '@', kohafield => 'fullcontrol' }); + $mapping->store; + + # First test in the wrong framework + my @cols = ( notexist => 'i am not here', fullcontrol => 'all' ); + my $record = C4::Biblio::TransformKohaToMarc( { @cols } ); + is( $record->field('001'), undef, + 'With default framework we should not find a 001 controlfield' ); + # Now include the framework parameter and test 001 + $record = C4::Biblio::TransformKohaToMarc( { @cols }, $fw->frameworkcode ); + is( $record->field('001')->data, "all", "Check controlfield 001 with right frameworkcode" ); + + # Remove from cache + Koha::Caches->get_instance->clear_from_cache( "MarcSubfieldStructure-".$fw->frameworkcode ); +}; + +subtest "Add test for no_split option" => sub { + plan tests => 4; + + my $fwc = t::lib::TestBuilder->new->build({ source => 'MarcSubfieldStructure' })->{frameworkcode}; + Koha::MarcSubfieldStructure->new({ frameworkcode => $fwc, tagfield => '952', tagsubfield => 'a', kohafield => 'items.fld1' })->store; + Koha::MarcSubfieldStructure->new({ frameworkcode => $fwc, tagfield => '952', tagsubfield => 'b', kohafield => 'items.fld1' })->store; + Koha::Caches->get_instance->clear_from_cache( "MarcSubfieldStructure-$fwc" ); + + # Test single value in fld1 + my @cols = ( 'items.fld1' => '01' ); + my $record = C4::Biblio::TransformKohaToMarc( { @cols }, $fwc, { no_split => 1 } ); + is( $record->subfield( '952', 'a' ), '01', 'Check single in 952a' ); + is( $record->subfield( '952', 'b' ), '01', 'Check single in 952b' ); + + # Test glued (composite) value in fld1 + @cols = ( 'items.fld1' => '01 | 02' ); + $record = C4::Biblio::TransformKohaToMarc( { @cols }, $fwc, { no_split => 1 } ); + is( $record->subfield( '952', 'a' ), '01 | 02', 'Check composite in 952a' ); + is( $record->subfield( '952', 'b' ), '01 | 02', 'Check composite in 952b' ); + + Koha::Caches->get_instance->clear_from_cache( "MarcSubfieldStructure-$fwc" ); +}; + # Cleanup Koha::Caches->get_instance->clear_from_cache( "MarcSubfieldStructure-" ); $schema->storage->txn_rollback; diff --git a/t/db_dependent/Biblio/TransformMarcToKoha.t b/t/db_dependent/Biblio/TransformMarcToKoha.t new file mode 100644 index 0000000..806b759 --- /dev/null +++ b/t/db_dependent/Biblio/TransformMarcToKoha.t @@ -0,0 +1,89 @@ +#!/usr/bin/perl + +# Tests for C4::Biblio::TransformMarcToKoha, TransformMarcToKohaOneField + +# Copyright 2017 Rijksmuseum +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it under the +# terms of the GNU General Public License as published by the Free Software +# Foundation; either version 3 of the License, or (at your option) any later +# version. +# +# Koha is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with Koha; if not, see . + +use Modern::Perl; +use Test::More tests => 2; +use MARC::Record; + +use t::lib::Mocks; +use t::lib::TestBuilder; + +use Koha::Database; +use Koha::Caches; +use Koha::MarcSubfieldStructures; +use C4::Biblio; + +my $schema = Koha::Database->new->schema; +$schema->storage->txn_begin; + +# Create a new framework with a few mappings +# Note: TransformMarcToKoha wants a table name (biblio, biblioitems or items) +our $fwc = t::lib::TestBuilder->new->build_object({ class => 'Koha::BiblioFrameworks' })->frameworkcode; +Koha::MarcSubfieldStructure->new({ frameworkcode => $fwc, tagfield => '300', tagsubfield => 'a', kohafield => 'biblio.field1' })->store; +Koha::MarcSubfieldStructure->new({ frameworkcode => $fwc, tagfield => '300', tagsubfield => 'b', kohafield => 'biblio.field2' })->store; +Koha::MarcSubfieldStructure->new({ frameworkcode => $fwc, tagfield => '500', tagsubfield => 'a', kohafield => 'biblio.field3' })->store; + +subtest 'Test a few mappings' => sub { + plan tests => 6; + + my $marc = MARC::Record->new; + $marc->append_fields( + MARC::Field->new( '300', '', '', a => 'a1', b => 'b1' ), + MARC::Field->new( '300', '', '', a => 'a2', b => 'b2' ), + MARC::Field->new( '500', '', '', a => 'note1', a => 'note2' ), + ); + my $result = C4::Biblio::TransformMarcToKoha( $marc, $fwc ); + # Note: TransformMarcToKoha stripped the table prefix biblio. + is( keys %{$result}, 3, 'Found all three mappings' ); + is( $result->{field1}, 'a1 | a2', 'Check field1 results' ); + is( $result->{field2}, 'b1 | b2', 'Check field2 results' ); + is( $result->{field3}, 'note1 | note2', 'Check field3 results' ); + + is( C4::Biblio::TransformMarcToKohaOneField( 'biblio.field1', $marc, $fwc ), + $result->{field1}, 'TransformMarcToKohaOneField returns biblio.field1'); + is( C4::Biblio::TransformMarcToKohaOneField( 'field4', $marc, $fwc ), + undef, 'TransformMarcToKohaOneField returns undef' ); +}; + +subtest 'Multiple mappings for one kohafield' => sub { + plan tests => 4; + + # Add another mapping to field1 + Koha::MarcSubfieldStructure->new({ frameworkcode => $fwc, tagfield => '510', tagsubfield => 'a', kohafield => 'biblio.field1' })->store; + Koha::Caches->get_instance->clear_from_cache( "MarcSubfieldStructure-$fwc" ); + + my $marc = MARC::Record->new; + $marc->append_fields( MARC::Field->new( '300', '', '', a => '3a' ) ); + my $result = C4::Biblio::TransformMarcToKoha( $marc, $fwc ); + is_deeply( $result, { field1 => '3a' }, 'Simple start' ); + $marc->append_fields( MARC::Field->new( '510', '', '', a => '' ) ); + $result = C4::Biblio::TransformMarcToKoha( $marc, $fwc ); + is_deeply( $result, { field1 => '3a' }, 'An empty 510a makes no difference' ); + $marc->append_fields( MARC::Field->new( '510', '', '', a => '51' ) ); + $result = C4::Biblio::TransformMarcToKoha( $marc, $fwc ); + is_deeply( $result, { field1 => '3a | 51' }, 'Got 300a and 510a' ); + + is( C4::Biblio::TransformMarcToKohaOneField( 'biblio.field1', $marc, $fwc ), + '3a | 51', 'TransformMarcToKohaOneField returns biblio.field1' ); +}; + +# Cleanup +Koha::Caches->get_instance->clear_from_cache( "MarcSubfieldStructure-$fwc" ); +$schema->storage->txn_rollback; diff --git a/t/db_dependent/Search.t b/t/db_dependent/Search.t index c8cbc48..7687e8f 100644 --- a/t/db_dependent/Search.t +++ b/t/db_dependent/Search.t @@ -159,44 +159,44 @@ sub mock_GetMarcSubfieldStructure { if ($marc_type eq 'marc21') { $bibliomodule->mock('GetMarcSubfieldStructure', sub { return { - 'biblio.biblionumber' => { tagfield => '999', tagsubfield => 'c' }, - 'biblio.isbn' => { tagfield => '020', tagsubfield => 'a' }, - 'biblio.title' => { tagfield => '245', tagsubfield => 'a' }, - 'biblio.notes' => { tagfield => '500', tagsubfield => 'a' }, - 'items.barcode' => { tagfield => '952', tagsubfield => 'p' }, - 'items.booksellerid' => { tagfield => '952', tagsubfield => 'e' }, - 'items.ccode' => { tagfield => '952', tagsubfield => '8' }, - 'items.cn_sort' => { tagfield => '952', tagsubfield => '6' }, - 'items.cn_source' => { tagfield => '952', tagsubfield => '2' }, - 'items.coded_location_qualifier' => { tagfield => '952', tagsubfield => 'f' }, - 'items.copynumber' => { tagfield => '952', tagsubfield => 't' }, - 'items.damaged' => { tagfield => '952', tagsubfield => '4' }, - 'items.dateaccessioned' => { tagfield => '952', tagsubfield => 'd' }, - 'items.datelastborrowed' => { tagfield => '952', tagsubfield => 's' }, - 'items.datelastseen' => { tagfield => '952', tagsubfield => 'r' }, - 'items.enumchron' => { tagfield => '952', tagsubfield => 'h' }, - 'items.holdingbranch' => { tagfield => '952', tagsubfield => 'b' }, - 'items.homebranch' => { tagfield => '952', tagsubfield => 'a' }, - 'items.issues' => { tagfield => '952', tagsubfield => 'l' }, - 'items.itemcallnumber' => { tagfield => '952', tagsubfield => 'o' }, - 'items.itemlost' => { tagfield => '952', tagsubfield => '1' }, - 'items.itemnotes' => { tagfield => '952', tagsubfield => 'z' }, - 'items.itemnumber' => { tagfield => '952', tagsubfield => '9' }, - 'items.itype' => { tagfield => '952', tagsubfield => 'y' }, - 'items.location' => { tagfield => '952', tagsubfield => 'c' }, - 'items.materials' => { tagfield => '952', tagsubfield => '3' }, - 'items.nonpublicnote' => { tagfield => '952', tagsubfield => 'x' }, - 'items.notforloan' => { tagfield => '952', tagsubfield => '7' }, - 'items.onloan' => { tagfield => '952', tagsubfield => 'q' }, - 'items.price' => { tagfield => '952', tagsubfield => 'g' }, - 'items.renewals' => { tagfield => '952', tagsubfield => 'm' }, - 'items.replacementprice' => { tagfield => '952', tagsubfield => 'v' }, - 'items.replacementpricedate' => { tagfield => '952', tagsubfield => 'w' }, - 'items.reserves' => { tagfield => '952', tagsubfield => 'n' }, - 'items.restricted' => { tagfield => '952', tagsubfield => '5' }, - 'items.stack' => { tagfield => '952', tagsubfield => 'j' }, - 'items.uri' => { tagfield => '952', tagsubfield => 'u' }, - 'items.withdrawn' => { tagfield => '952', tagsubfield => '0' }, + 'biblio.biblionumber' => [{ tagfield => '999', tagsubfield => 'c' }], + 'biblio.isbn' => [{ tagfield => '020', tagsubfield => 'a' }], + 'biblio.title' => [{ tagfield => '245', tagsubfield => 'a' }], + 'biblio.notes' => [{ tagfield => '500', tagsubfield => 'a' }], + 'items.barcode' => [{ tagfield => '952', tagsubfield => 'p' }], + 'items.booksellerid' => [{ tagfield => '952', tagsubfield => 'e' }], + 'items.ccode' => [{ tagfield => '952', tagsubfield => '8' }], + 'items.cn_sort' => [{ tagfield => '952', tagsubfield => '6' }], + 'items.cn_source' => [{ tagfield => '952', tagsubfield => '2' }], + 'items.coded_location_qualifier' => [{ tagfield => '952', tagsubfield => 'f' }], + 'items.copynumber' => [{ tagfield => '952', tagsubfield => 't' }], + 'items.damaged' => [{ tagfield => '952', tagsubfield => '4' }], + 'items.dateaccessioned' => [{ tagfield => '952', tagsubfield => 'd' }], + 'items.datelastborrowed' => [{ tagfield => '952', tagsubfield => 's' }], + 'items.datelastseen' => [{ tagfield => '952', tagsubfield => 'r' }], + 'items.enumchron' => [{ tagfield => '952', tagsubfield => 'h' }], + 'items.holdingbranch' => [{ tagfield => '952', tagsubfield => 'b' }], + 'items.homebranch' => [{ tagfield => '952', tagsubfield => 'a' }], + 'items.issues' => [{ tagfield => '952', tagsubfield => 'l' }], + 'items.itemcallnumber' => [{ tagfield => '952', tagsubfield => 'o' }], + 'items.itemlost' => [{ tagfield => '952', tagsubfield => '1' }], + 'items.itemnotes' => [{ tagfield => '952', tagsubfield => 'z' }], + 'items.itemnumber' => [{ tagfield => '952', tagsubfield => '9' }], + 'items.itype' => [{ tagfield => '952', tagsubfield => 'y' }], + 'items.location' => [{ tagfield => '952', tagsubfield => 'c' }], + 'items.materials' => [{ tagfield => '952', tagsubfield => '3' }], + 'items.nonpublicnote' => [{ tagfield => '952', tagsubfield => 'x' }], + 'items.notforloan' => [{ tagfield => '952', tagsubfield => '7' }], + 'items.onloan' => [{ tagfield => '952', tagsubfield => 'q' }], + 'items.price' => [{ tagfield => '952', tagsubfield => 'g' }], + 'items.renewals' => [{ tagfield => '952', tagsubfield => 'm' }], + 'items.replacementprice' => [{ tagfield => '952', tagsubfield => 'v' }], + 'items.replacementpricedate' => [{ tagfield => '952', tagsubfield => 'w' }], + 'items.reserves' => [{ tagfield => '952', tagsubfield => 'n' }], + 'items.restricted' => [{ tagfield => '952', tagsubfield => '5' }], + 'items.stack' => [{ tagfield => '952', tagsubfield => 'j' }], + 'items.uri' => [{ tagfield => '952', tagsubfield => 'u' }], + 'items.withdrawn' => [{ tagfield => '952', tagsubfield => '0' }], }; }); } -- 2.1.4