Bugzilla – Attachment 18512 Details for
Bug 9755
Record merge code needs to be refactored
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 9755 QA follow-up: move MARC-specific functionality to utility class
Bug-9755-QA-follow-up-move-MARC-specific-functiona.patch (text/plain), 12.62 KB, created by
Jared Camins-Esakov
on 2013-05-30 10:27:57 UTC
(
hide
)
Description:
Bug 9755 QA follow-up: move MARC-specific functionality to utility class
Filename:
MIME Type:
Creator:
Jared Camins-Esakov
Created:
2013-05-30 10:27:57 UTC
Size:
12.62 KB
patch
obsolete
>From dabf4cd2b9a35f1122c7815680c4e530f5bb942d Mon Sep 17 00:00:00 2001 >From: Jared Camins-Esakov <jcamins@cpbibliography.com> >Date: Thu, 30 May 2013 06:23:43 -0400 >Subject: [PATCH] Bug 9755 QA follow-up: move MARC-specific functionality to > utility class > >This follow-up moves all the MARC-specific functionality of Koha::Record >to a Koha::Util::MARC utility class. > >To test, run relevant unit tests: >> prove t/Koha_Record.t t/Koha_Util_MARC.t t/db_dependent/Authority.t >and optionally try to merge a record. >--- > Koha/Authority.pm | 9 +++-- > Koha/Record.pm | 75 +++++--------------------------------- > Koha/Util/MARC.pm | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++ > cataloguing/merge.pl | 8 ++--- > t/Koha_Record.t | 5 ++- > t/Koha_Util_MARC.t | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 6 files changed, 217 insertions(+), 76 deletions(-) > create mode 100644 Koha/Util/MARC.pm > create mode 100755 t/Koha_Util_MARC.t > >diff --git a/Koha/Authority.pm b/Koha/Authority.pm >index 855ce1f..21f78cd 100644 >--- a/Koha/Authority.pm >+++ b/Koha/Authority.pm >@@ -40,7 +40,7 @@ use C4::Charset; > > use base qw(Koha::Record); > >-__PACKAGE__->mk_accessors(qw( authid authtype marcflavour )); >+__PACKAGE__->mk_accessors(qw( authid authtype )); > > =head2 new > >@@ -65,12 +65,15 @@ sub new { > my $auth = Koha::Authority->get_from_authid($authid); > > Create the Koha::Authority object associated with the provided authid. >+Note that this routine currently retrieves a MARC record because >+authorities in Koha are MARC records by definition. This is an >+unfortunate but unavoidable fact. > > =cut > sub get_from_authid { > my $class = shift; > my $authid = shift; >- my $marcflavour = C4::Context->preference("marcflavour"); >+ my $marcflavour = lc C4::Context->preference("marcflavour"); > > my $dbh=C4::Context->dbh; > my $sth=$dbh->prepare("select authtypecode, marcxml from auth_header where authid=?"); >@@ -82,8 +85,8 @@ sub get_from_authid { > $record->encoding('UTF-8'); > > my $self = $class->SUPER::new( { authid => $authid, >- marcflavour => $marcflavour, > authtype => $authtypecode, >+ schema => $marcflavour, > record => $record }); > > bless $self, $class; >diff --git a/Koha/Record.pm b/Koha/Record.pm >index 1e3c7a3..373d5f0 100644 >--- a/Koha/Record.pm >+++ b/Koha/Record.pm >@@ -19,7 +19,7 @@ package Koha::Record; > > =head1 NAME > >-Koha::Record - base class for MARC records >+Koha::Record - base class for records > > =head1 SYNOPSIS > >@@ -34,82 +34,25 @@ Object-oriented class that encapsulates all records in Koha. > use strict; > use warnings; > use C4::Context; >-use MARC::Record; >+use Koha::Util::MARC; > > use base qw(Class::Accessor); > >-__PACKAGE__->mk_accessors(qw( record marcflavour )); >+__PACKAGE__->mk_accessors(qw( record schema )); > > >-=head2 createMarcHash >+=head2 createMergeHash > >-Create a MARC hash for use when merging records. >+Create a hash for use when merging records. At the moment the only >+metadata schema supported is MARC. > > =cut > >-sub createMarcHash { >+sub createMergeHash { > my ($self, $tagslib) = @_; >- my $record = $self->record; >- my @array; >- my @fields = $record->fields(); >- >- >- foreach my $field (@fields) { >- my $fieldtag = $field->tag(); >- if ($fieldtag < 10) { >- if (!defined($tagslib) || $tagslib->{$fieldtag}->{'@'}->{'tab'} >= 0) { >- push @array, { >- field => [ >- { >- tag => $fieldtag, >- key => _createKey(), >- value => $field->data(), >- } >- ] >- }; >- } >- } else { >- my @subfields = $field->subfields(); >- my @subfield_array; >- foreach my $subfield (@subfields) { >- if (!defined($tagslib) || $tagslib->{$fieldtag}->{@$subfield[0]}->{'tab'} >= 0) { >- push @subfield_array, { >- subtag => @$subfield[0], >- subkey => _createKey(), >- value => @$subfield[1], >- }; >- } >- >- } >- >- if ((!defined($tagslib) || $tagslib->{$fieldtag}->{'tab'} >= 0) && $fieldtag ne '995' && $fieldtag ne '999') { >- push @array, { >- field => [ >- { >- tag => $fieldtag, >- key => _createKey(), >- indicator1 => $field->indicator(1), >- indicator2 => $field->indicator(2), >- subfield => [@subfield_array], >- } >- ] >- }; >- } >- >- } >+ if ($self->schema =~ m/marc/) { >+ return Koha::Util::MARC::createMergeHash($self->record, $tagslib); > } >- return [@array]; >- >-} >- >-=head2 _createKey >- >-Create a random value to set it into the input name >- >-=cut >- >-sub _createKey { >- return int(rand(1000000)); > } > > 1; >diff --git a/Koha/Util/MARC.pm b/Koha/Util/MARC.pm >new file mode 100644 >index 0000000..251d565 >--- /dev/null >+++ b/Koha/Util/MARC.pm >@@ -0,0 +1,98 @@ >+package Koha::Util::MARC; >+ >+# Copyright 2013 C & P Bibliography Services >+# >+# 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, write to the Free Software Foundation, Inc., >+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. >+ >+use Modern::Perl; >+use MARC::Record; >+ >+=head1 NAME >+ >+Koha::Util::MARC - utility class with routines for working with MARC records >+ >+=head1 METHODS >+ >+=head2 createMergeHash >+ >+Create a hash to use when merging MARC records >+ >+=cut >+ >+sub createMergeHash { >+ my ($record, $tagslib) = @_; >+ my @array; >+ my @fields = $record->fields(); >+ >+ >+ foreach my $field (@fields) { >+ my $fieldtag = $field->tag(); >+ if ($fieldtag < 10) { >+ if (!defined($tagslib) || $tagslib->{$fieldtag}->{'@'}->{'tab'} >= 0) { >+ push @array, { >+ field => [ >+ { >+ tag => $fieldtag, >+ key => _createKey(), >+ value => $field->data(), >+ } >+ ] >+ }; >+ } >+ } else { >+ my @subfields = $field->subfields(); >+ my @subfield_array; >+ foreach my $subfield (@subfields) { >+ if (!defined($tagslib) || $tagslib->{$fieldtag}->{@$subfield[0]}->{'tab'} >= 0) { >+ push @subfield_array, { >+ subtag => @$subfield[0], >+ subkey => _createKey(), >+ value => @$subfield[1], >+ }; >+ } >+ >+ } >+ >+ if ((!defined($tagslib) || $tagslib->{$fieldtag}->{'tab'} >= 0) && @subfield_array) { >+ push @array, { >+ field => [ >+ { >+ tag => $fieldtag, >+ key => _createKey(), >+ indicator1 => $field->indicator(1), >+ indicator2 => $field->indicator(2), >+ subfield => [@subfield_array], >+ } >+ ] >+ }; >+ } >+ >+ } >+ } >+ return [@array]; >+} >+ >+=head2 _createKey >+ >+Create a random value to set it into the input name >+ >+=cut >+ >+sub _createKey { >+ return int(rand(1000000)); >+} >+ >+1; >diff --git a/cataloguing/merge.pl b/cataloguing/merge.pl >index 0fc4f52..14dd56c 100755 >--- a/cataloguing/merge.pl >+++ b/cataloguing/merge.pl >@@ -170,11 +170,11 @@ if ($merge) { > > # Creating a loop for display > >- my $recordObj1 = new Koha::Record({ 'record' => GetMarcBiblio($mergereference) }); >- my $recordObj2 = new Koha::Record({ 'record' => GetMarcBiblio($notreference) }); >+ my $recordObj1 = new Koha::Record({ 'record' => GetMarcBiblio($mergereference), 'schema' => lc C4::Context->preference('marcflavour') }); >+ my $recordObj2 = new Koha::Record({ 'record' => GetMarcBiblio($notreference), 'schema' => lc C4::Context->preference('marcflavour') }); > >- my @record1 = $recordObj1->createMarcHash($tagslib); >- my @record2 = $recordObj2->createMarcHash($tagslib); >+ my @record1 = $recordObj1->createMergeHash($tagslib); >+ my @record2 = $recordObj2->createMergeHash($tagslib); > > # Parameters > $template->param( >diff --git a/t/Koha_Record.t b/t/Koha_Record.t >index ca9a246..b45fb50 100755 >--- a/t/Koha_Record.t >+++ b/t/Koha_Record.t >@@ -33,7 +33,7 @@ $marcrecord->add_fields( > [ '150', ' ', ' ', a => 'Cooking' ], > [ '450', ' ', ' ', a => 'Cookery', z => 'Instructional manuals' ], > ); >-my $record = Koha::Record->new({ 'record' => $marcrecord }); >+my $record = Koha::Record->new({ 'record' => $marcrecord, 'schema' => 'marc21' }); > > is(ref($record), 'Koha::Record', 'Created valid Koha::Record object'); > >@@ -82,9 +82,8 @@ my $samplehash = [ > } > ]; > >-my $hash = $record->createMarcHash(); >+my $hash = $record->createMergeHash(); > my %fieldkeys; >-require Data::Dumper; > foreach my $field (@$hash) { > $fieldkeys{delete $field->{'field'}->[0]->{'key'}}++; > if (defined $field->{'field'}->[0]->{'subfield'}) { >diff --git a/t/Koha_Util_MARC.t b/t/Koha_Util_MARC.t >new file mode 100755 >index 0000000..0c7f3ee >--- /dev/null >+++ b/t/Koha_Util_MARC.t >@@ -0,0 +1,98 @@ >+#!/usr/bin/perl >+ >+# Copyright 2013 C & P Bibliography Services >+# >+# 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 2 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, write to the Free Software Foundation, Inc., >+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. >+ >+# Note that at present this test is almost identical to the one testing >+# the encapsulating method in Koha::Record. >+ >+use strict; >+use warnings; >+ >+use Test::More tests => 3; >+ >+BEGIN { >+ use_ok('Koha::Util::MARC'); >+} >+ >+my $marcrecord = MARC::Record->new; >+ >+$marcrecord->add_fields( >+ [ '001', '1234' ], >+ [ '150', ' ', ' ', a => 'Cooking' ], >+ [ '450', ' ', ' ', a => 'Cookery', z => 'Instructional manuals' ], >+ ); >+my $samplehash = [ >+ { >+ 'field' => [ >+ { >+ 'value' => '1234', >+ 'tag' => '001', >+ } >+ ] >+ }, >+ { >+ 'field' => [ >+ { >+ 'subfield' => [ >+ { >+ 'value' => 'Cooking', >+ 'subtag' => 'a' >+ } >+ ], >+ 'indicator2' => ' ', >+ 'tag' => 150, >+ 'indicator1' => ' ', >+ } >+ ] >+ }, >+ { >+ 'field' => [ >+ { >+ 'subfield' => [ >+ { >+ 'value' => 'Cookery', >+ 'subtag' => 'a' >+ }, >+ { >+ 'value' => 'Instructional manuals', >+ 'subtag' => 'z' >+ } >+ ], >+ 'indicator2' => ' ', >+ 'tag' => 450, >+ 'indicator1' => ' ', >+ } >+ ] >+ } >+]; >+ >+my $hash = Koha::Util::MARC::createMergeHash($marcrecord); >+my %fieldkeys; >+foreach my $field (@$hash) { >+ $fieldkeys{delete $field->{'field'}->[0]->{'key'}}++; >+ if (defined $field->{'field'}->[0]->{'subfield'}) { >+ foreach my $subfield (@{$field->{'field'}->[0]->{'subfield'}}) { >+ $fieldkeys{delete $subfield->{'subkey'}}++; >+ } >+ } >+} >+ >+is_deeply($hash, $samplehash, 'Generated hash correctly'); >+my $dupkeys = grep { $_ > 1 } values %fieldkeys; >+is($dupkeys, 0, 'No duplicate keys'); >+ >-- >1.7.9.5
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 9755
:
15913
|
15922
|
18446
|
18512
|
18513
|
18531
|
18909
|
18910
|
19038
|
19464
|
19465
|
19466