From 94a30097e18772b02cade8c9f74ad32f778feb35 Mon Sep 17 00:00:00 2001 From: David Gustafsson Date: Wed, 17 Apr 2019 17:34:53 +0200 Subject: [PATCH] Bug 14957: Add tests and pod for new options param to ModBiblio https://bugs.koha-community.org/show_bug.cgi?id=14956 --- C4/Biblio.pm | 8 +++- Koha/MarcMergeRule.pm | 12 ++++++ admin/marc-merge-rules.pl | 4 +- t/db_dependent/Biblio/MarcMergeRules.t | 73 ++++++++++++++++++++++++++++++++-- 4 files changed, 91 insertions(+), 6 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index b837ff718b..7d0f988697 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -257,6 +257,12 @@ in the C and C tables, as well as which fields are used to store embedded item, biblioitem, and biblionumber data for indexing. +The C<$options> argument is a hashref with additional parameters +that currently has one available option: C. C +is forwared to L where it is used for selecting the +current rule set if Marc Merge Rules is enabled. +See L for more details. + Returns 1 on success 0 on failure =cut @@ -3279,7 +3285,7 @@ sub _koha_delete_biblio_metadata { =head2 ModBiblioMarc - &ModBiblioMarc($newrec, $biblionumber, $frameworkcode, $options); + ModBiblioMarc($newrec, $biblionumber, $frameworkcode); Add MARC XML data for a biblio to koha diff --git a/Koha/MarcMergeRule.pm b/Koha/MarcMergeRule.pm index 989827f66a..d1bdbbac18 100644 --- a/Koha/MarcMergeRule.pm +++ b/Koha/MarcMergeRule.pm @@ -39,6 +39,18 @@ sub store { $self->SUPER::store(@_); } +=head2 delete + +Override C to clear marc merge rules cache. + +=cut + +sub delete { + my $self = shift @_; + $cache->clear_from_cache('marc_merge_rules'); + $self->SUPER::delete(@_); +} + sub _type { return 'MarcMergeRule'; } diff --git a/admin/marc-merge-rules.pl b/admin/marc-merge-rules.pl index f9cc61b3e5..123dd5f86c 100755 --- a/admin/marc-merge-rules.pl +++ b/admin/marc-merge-rules.pl @@ -15,8 +15,7 @@ # You should have received a copy of the GNU General Public License # along with Koha; if not, see . -use strict; -use warnings; +use Modern::Perl; # standard or CPAN modules used use CGI qw ( -utf8 ); @@ -144,6 +143,7 @@ elsif ($op eq 'doedit' || $op eq 'add') { }; if (!@{$errors}) { my $rule = Koha::MarcMergeRules->find_or_create($rule_data); + # Need to call set and store here in case we have an update $rule->set($rule_data); $rule->store(); } diff --git a/t/db_dependent/Biblio/MarcMergeRules.t b/t/db_dependent/Biblio/MarcMergeRules.t index c05e92d74c..106b9c1be5 100755 --- a/t/db_dependent/Biblio/MarcMergeRules.t +++ b/t/db_dependent/Biblio/MarcMergeRules.t @@ -24,7 +24,7 @@ use C4::Context; use C4::Biblio; use Koha::Database; #?? -use Test::More tests => 22; +use Test::More tests => 23; use Test::MockModule; use Koha::MarcMergeRules; @@ -86,7 +86,7 @@ subtest 'Record fields has been overwritten when no merge rules are defined' => my $rule = Koha::MarcMergeRules->find_or_create({ tag => '*', - module => $modules[0]->id, + module => $modules[0]->name, filter => '*', add => 0, append => 0, @@ -585,7 +585,7 @@ subtest 'Record fields has been overwritten when add = 1, append = 1, remove = 1 # Protect field 500 with more specific tag value my $skip_all_rule = Koha::MarcMergeRules->find_or_create({ tag => '500', - module => $modules[0]->id, + module => $modules[0]->name, filter => '*', add => 0, append => 0, @@ -712,6 +712,73 @@ subtest 'An exception is thrown when rule tag is set to invalid regexp' => sub { ok($exception->isa('Koha::Exceptions::MarcMergeRule::InvalidTagRegExp'), "Exception is of correct class"); }; +$skip_all_rule->delete(); + +subtest 'context option in ModBiblio is handled correctly' => sub { + plan tests => 6; + $rule->set( + { + tag => '250', + module => $modules[0]->name, + filter => '*', + 'add' => 0, + 'append' => 0, + 'remove' => 0, + 'delete' => 0, + } + ); + $rule->store(); + my ($biblionumber) = AddBiblio($orig_record, ''); + + # Since marc merc rules are not run on save, only update + # saved record should be identical to orig_record + my $saved_record = GetMarcBiblio({ biblionumber => $biblionumber }); + + my @all_fields = $saved_record->fields(); + # Koha also adds 999c field, therefore 4 not 3 + cmp_ok(scalar @all_fields, '==', 4, 'Saved record has the expected number of fields'); + is_deeply( + [map { $_->subfield('a') } $saved_record->field('250') ], + ['250 bottles of beer on the wall', '256 bottles of beer on the wall'], + 'All "250" fields of saved record are identical to original record passed to AddBiblio' + ); + + is_deeply( + [map { $_->subfield('a') } $saved_record->field('500') ], + ['One bottle of beer in the fridge'], + 'All "500" fields of saved record are identical to original record passed to AddBiblio' + ); + + $saved_record->append_fields( + MARC::Field->new('250', '', '', 'a' => '251 bottles of beer on the wall'), # Appended + MARC::Field->new('500', '', '', 'a' => 'One cold bottle of beer in the fridge'), # Appended + ); + + ModBiblio($saved_record, $biblionumber, '', { context => { $modules[0]->name => 'test' } }); + + my $updated_record = GetMarcBiblio({ biblionumber => $biblionumber }); + + @all_fields = $updated_record->fields(); + cmp_ok(scalar @all_fields, '==', 5, 'Updated record has the expected number of fields'); + is_deeply( + [map { $_->subfield('a') } $updated_record->field('250') ], + ['250 bottles of beer on the wall', '256 bottles of beer on the wall'], + '"250" fields have retained their original values' + ); + + is_deeply( + [map { $_->subfield('a') } $updated_record->field('500') ], + ['One bottle of beer in the fridge', 'One cold bottle of beer in the fridge'], + '"500" field has been appended' + ); + + # To trigger removal from search index etc + DelBiblio($biblionumber); +}; + +# Explicityly delete rule to trigger clearing of cache +$rule->delete(); + $schema->storage->txn_rollback; 1; -- 2.11.0