From 50fb4612ed573dd180c5e98e7d6e83c4265817f0 Mon Sep 17 00:00:00 2001 From: Jared Camins-Esakov Date: Wed, 6 Mar 2013 11:30:40 -0500 Subject: [PATCH] Bug 9755: Refactor record merge functionality This patch refactors the merge record interface and code a little bit in preparation for making it possible to merge authority records. To test: 1) Apply patch. 2) Try merging two records: a) Create a list. b) Add two records you would like to (or be willing to) merge to said list. c) View said list. d) Check the checkboxes next to the two records you added. e) Click "Merge selected records." f) Choose a merge reference. g) Choose fields from each record that you want to keep. h) Click "Merge." 3) Confirm that your merged record has the fields and subfields you wanted. 4) Run the unit tests for the two files that were changed: prove t/Koha_Record.t t/db_dependent/Koha_Authority.t 5) Sign off. Signed-off-by: Mathieu Saby Signed-off-by: Chris Cormack Signed-off-by: Katrin Fischer Signed-off-by: Galen Charlton --- Koha/Authority.pm | 5 +- Koha/Record.pm | 115 ++++++++ cataloguing/merge.pl | 74 +---- .../prog/en/includes/merge-record.inc | 219 +++++++++++++++ .../prog/en/modules/cataloguing/merge.tt | 264 +----------------- t/Koha_Record.t | 99 +++++++ 6 files changed, 449 insertions(+), 327 deletions(-) create mode 100644 Koha/Record.pm create mode 100644 koha-tmpl/intranet-tmpl/prog/en/includes/merge-record.inc create mode 100755 t/Koha_Record.t diff --git a/Koha/Authority.pm b/Koha/Authority.pm index b07db92895..855ce1f5c6 100644 --- a/Koha/Authority.pm +++ b/Koha/Authority.pm @@ -38,9 +38,9 @@ use MARC::Record; use MARC::File::XML; use C4::Charset; -use base qw(Class::Accessor); +use base qw(Koha::Record); -__PACKAGE__->mk_accessors(qw( authid authtype record marcflavour )); +__PACKAGE__->mk_accessors(qw( authid authtype marcflavour )); =head2 new @@ -59,6 +59,7 @@ sub new { return $self; } + =head2 get_from_authid my $auth = Koha::Authority->get_from_authid($authid); diff --git a/Koha/Record.pm b/Koha/Record.pm new file mode 100644 index 0000000000..1e3c7a3a8e --- /dev/null +++ b/Koha/Record.pm @@ -0,0 +1,115 @@ +package Koha::Record; + +# 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. + +=head1 NAME + +Koha::Record - base class for MARC records + +=head1 SYNOPSIS + + my $record = new Koha::Record({ 'record' => $marcrecord }); + +=head1 DESCRIPTION + +Object-oriented class that encapsulates all records in Koha. + +=cut + +use strict; +use warnings; +use C4::Context; +use MARC::Record; + +use base qw(Class::Accessor); + +__PACKAGE__->mk_accessors(qw( record marcflavour )); + + +=head2 createMarcHash + +Create a MARC hash for use when merging records. + +=cut + +sub createMarcHash { + 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], + } + ] + }; + } + + } + } + 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 019b468836..0fc4f52c2b 100755 --- a/cataloguing/merge.pl +++ b/cataloguing/merge.pl @@ -30,6 +30,7 @@ use C4::Serials; use C4::Koha; use C4::Reserves qw/MergeHolds/; use C4::Acquisition qw/ModOrder GetOrdersByBiblionumber/; +use Koha::Record; my $input = new CGI; my @biblionumber = $input->param('biblionumber'); @@ -168,8 +169,12 @@ if ($merge) { my $notreference = ($biblionumber[0] == $mergereference) ? $biblionumber[1] : $biblionumber[0]; # Creating a loop for display - my @record1 = _createMarcHash(GetMarcBiblio($mergereference), $tagslib); - my @record2 = _createMarcHash(GetMarcBiblio($notreference), $tagslib); + + my $recordObj1 = new Koha::Record({ 'record' => GetMarcBiblio($mergereference) }); + my $recordObj2 = new Koha::Record({ 'record' => GetMarcBiblio($notreference) }); + + my @record1 = $recordObj1->createMarcHash($tagslib); + my @record2 = $recordObj2->createMarcHash($tagslib); # Parameters $template->param( @@ -231,68 +236,3 @@ exit; # ------------------------ # Functions # ------------------------ -sub _createMarcHash { - my $record = shift; - my $tagslib = shift; - my @array; - my @fields = $record->fields(); - - - foreach my $field (@fields) { - my $fieldtag = $field->tag(); - if ($fieldtag < 10) { - if ($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 ($tagslib->{$fieldtag}->{@$subfield[0]}->{'tab'} >= 0) { - push @subfield_array, { - subtag => @$subfield[0], - subkey => createKey(), - value => @$subfield[1], - }; - } - - } - - if ($tagslib->{$fieldtag}->{'tab'} >= 0 && $fieldtag ne '995') { - 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)); -} - diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/merge-record.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/merge-record.inc new file mode 100644 index 0000000000..5b8524aedf --- /dev/null +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/merge-record.inc @@ -0,0 +1,219 @@ +[% BLOCK sourcetab %] +
+ [% IF ( records ) %] + +
+
    + [% FOREACH record IN records %] + [% FOREACH fiel IN record.field %] +
  • + + [% fiel.tag %] + + + + [% IF ( fiel.value ) %] / [% fiel.value %] + + + [% END %] + + [% IF ( fiel.subfield ) %] +
      + [% FOREACH subfiel IN fiel.subfield %] +
    • + + [% subfiel.subtag %] / [% subfiel.value %] + + +
      • + [% END %] +
    + [% END %] + [% END %] +
    • + [% END %] +
+
+ [% END %] +
+[% END %] +[% BLOCK mergejs %] + // Creating tabs + $("#tabs").tabs(); + + // Toggle a field / subfield + function toggleField(pField) { + + // Getting the key of the clicked checkbox + var ckid = $(pField).attr("id"); + var tab = ckid.split('_'); + var source = tab[1]; // From which record the click came from + var key = tab[2]; + var type = $(pField).attr("class"); + + // Getting field/subfield + var field; + var subfield; + if (type == "subfieldpick") { + + field = $(pField).parent().parent().parent().find("span.field").text(); + subfield = $(pField).parent().find("span.subfield").text(); + } else { + + field = $(pField).parent().find("span.field").text(); + } + + // If the field has just been checked + if (pField.checked) { + + // We check for repeatability + var canbeadded = true; + if (type == "subfieldpick") { + var repeatable = 1; + var alreadyexists = 0; + if (tagslib[field] && tagslib[field][subfield]) { + repeatable = tagslib[field][subfield].repeatable; // Note : we can't use the dot notation here (tagslib.021) because the key is a number + // TODO : Checking for subfields + } + } else { + if (tagslib[field]) { + repeatable = tagslib[field].repeatable; + alreadyexists = $("#resultul span.field:contains(" + field + ")"); + if (repeatable == 0 && alreadyexists.length != 0) { + canbeadded = false; + } + } + } + // If the field is not repeatable, we check if it already exists in the result table + if (canbeadded == false) { + alert(_("The field is non-repeatable and already exists in the destination record. Therefore, you cannot add it.")); + pField.checked = 0; + } else { + + // Cloning the field or subfield we picked + var clone = $(pField).parent().clone(); + + // Removing the checkboxes from it + $(clone).find("input.subfieldpick, input.fieldpick").each(function() { + $(this).remove(); + }); + + + // If we are a subfield + if (type == "subfieldpick") { + // then we need to find who is our parent field... + fieldkey = $(pField).parent().parent().parent().attr("id"); + + // Find where to add the subfield + + // First, check if the field is not already in the destination record + if ($("#resultul li#" + fieldkey).length > 0) { + // If so, we add our field to it + $("#resultul li#" + fieldkey + " ul").append(clone); + } else { + // If not, we add the subfield to the first matching field + var where = 0; + $("#resultul li span.field").each(function() { + if (where == 0 && $(this).text() == field) { + where = this; + } + }); + + // If there is no matching field in the destination record + if (where == 0) { + + // TODO: + // We select the whole field and removing non-selected subfields, instead of... + + // Alerting the user + alert(_("This subfield cannot be added: there is no") + " " + field + " " + _("field in the destination record.")); + pField.checked = false; + + } else { + $(where).nextAll("ul").append(clone); + } + + } + + + + } else { + // If we are a field + var where = 0; + // Find where to add the field + $("#resultul li span.field").each(function() { + if (where == 0 && $(this).text() > field) { + where = this; + } + }); + + $(where).parent().before(clone); + } + } + } else { + + // Else, we remove it from the results tab + $("ul#resultul li#k" + key).remove(); + } +} + + + // When a field is checked / unchecked + $('input.fieldpick').click(function() { + toggleField(this); + // (un)check all subfields + var ischecked = this.checked; + $(this).parent().find("input.subfieldpick").each(function() { + this.checked = ischecked; + }); + }); + + // When a field or subfield is checked / unchecked + $("input.subfieldpick").click(function() { + toggleField(this); + }); +[% END %] +[% BLOCK mergesource %] +
+

Source records

+ + [% PROCESS sourcetab records=record1 recordnumber=1 defaultrecord=1 %] + [% PROCESS sourcetab records=record2 recordnumber=2 defaultrecord=0 %] +
+[% END %] +[% BLOCK mergetarget %] +
+

Destination record

+
+
    + [% FOREACH record IN record1 %] + [% FOREACH fiel IN record.field %]
  • + [% fiel.tag %] + + + [% IF ( fiel.value ) %] / + [% fiel.value %] + + + [% END %] + + [% IF ( fiel.subfield ) %] +
      + [% FOREACH subfiel IN fiel.subfield %] +
    • + [% subfiel.subtag %] / [% subfiel.value %] + + +
      • + [% END %] +
    + [% END %] +
    • [% END %] + [% END %] +
+
+
+[% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/merge.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/merge.tt index 3ccd77f679..4453b98ee3 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/merge.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/merge.tt @@ -1,3 +1,4 @@ +[% PROCESS 'merge-record.inc' %] [% INCLUDE 'doc-head-open.inc' %] Koha › Cataloging › Merging records [% INCLUDE 'greybox.inc' %] @@ -16,152 +17,16 @@ div#result { margin-top: 1em; } $("ul#ulrecord2").remove(); } - $(document).ready(function(){ - // Creating tabs - $("#tabs").tabs(); - // Getting marc structure via ajax tagslib = []; $.getJSON("/cgi-bin/koha/cataloguing/merge_ajax.pl", {frameworkcode : "[% framework %]" }, function(json) { - tagslib = json; + tagslib = json; }); - - - // Toggle a field / subfield - function toggleField(pField) { - - // Getting the key of the clicked checkbox - var ckid = $(pField).attr("id"); - var tab = ckid.split('_'); - var source = tab[1]; // From which record the click came from - var key = tab[2]; - var type = $(pField).attr("class"); - - // Getting field/subfield - var field; - var subfield; - if (type == "subfieldpick") { - - field = $(pField).parent().parent().parent().find("span.field").text(); - subfield = $(pField).parent().find("span.subfield").text(); - } else { - - field = $(pField).parent().find("span.field").text(); - } - - // If the field has just been checked - if (pField.checked) { - - // We check for repeatability - var canbeadded = true; - if (type == "subfieldpick") { - var repeatable = 1; - var alreadyexists = 0; - if (tagslib[field] && tagslib[field][subfield]) { - repeatable = tagslib[field][subfield].repeatable; // Note : we can't use the dot notation here (tagslib.021) because the key is a number - // TODO : Checking for subfields - } - } else { - if (tagslib[field]) { - repeatable = tagslib[field].repeatable; - alreadyexists = $("#resultul span.field:contains(" + field + ")"); - if (repeatable == 0 && alreadyexists.length != 0) { - canbeadded = false; - } - } - } - // If the field is not repeatable, we check if it already exists in the result table - if (canbeadded == false) { - alert(_("The field is non-repeatable and already exists in the destination record. Therefore, you cannot add it.")); - pField.checked = 0; - } else { - - // Cloning the field or subfield we picked - var clone = $(pField).parent().clone(); - - // Removing the checkboxes from it - $(clone).find("input.subfieldpick, input.fieldpick").each(function() { - $(this).remove(); - }); - - - // If we are a subfield - if (type == "subfieldpick") { - // then we need to find who is our parent field... - fieldkey = $(pField).parent().parent().parent().attr("id"); - - // Find where to add the subfield - - // First, check if the field is not already in the destination record - if ($("#resultul li#" + fieldkey).length > 0) { - // If so, we add our field to it - $("#resultul li#" + fieldkey + " ul").append(clone); - } else { - // If not, we add the subfield to the first matching field - var where = 0; - $("#resultul li span.field").each(function() { - if (where == 0 && $(this).text() == field) { - where = this; - } - }); - - // If there is no matching field in the destination record - if (where == 0) { - - // TODO: - // We select the whole field and removing non-selected subfields, instead of... - - // Alerting the user - alert(_("This subfield cannot be added: there is no") + " " + field + " " + _("field in the destination record.")); - pField.checked = false; - - } else { - $(where).nextAll("ul").append(clone); - } - - } - - - - } else { - // If we are a field - var where = 0; - // Find where to add the field - $("#resultul li span.field").each(function() { - if (where == 0 && $(this).text() > field) { - where = this; - } - }); - - $(where).parent().before(clone); - } - } - } else { - - // Else, we remove it from the results tab - $("ul#resultul li#k" + key).remove(); - } -} - - - // When a field is checked / unchecked - $('input.fieldpick').click(function() { - toggleField(this); - // (un)check all subfields - var ischecked = this.checked; - $(this).parent().find("input.subfieldpick").each(function() { - this.checked = ischecked; - }); - }); - - // When a field or subfield is checked / unchecked - $("input.subfieldpick").click(function() { - toggleField(this); - }); - + [% PROCESS mergejs %] }); + function changeFramework(fw) { $("#Frameworks").val(fw); } @@ -253,127 +118,10 @@ function changeFramework(fw) {
-
-

Source records

- -
- [% IF ( record1 ) %] - -
-
    - [% FOREACH record IN record1 %] - [% FOREACH fiel IN record.field %] -
  • - - [% fiel.tag %] - - - - [% IF ( fiel.value ) %] / [% fiel.value %] - - - [% END %] - - [% IF ( fiel.subfield ) %] -
      - [% FOREACH subfiel IN fiel.subfield %] -
    • - - [% subfiel.subtag %] / [% subfiel.value %] - - -
      • - [% END %] -
    - [% END %] - [% END %] -
    • - [% END %] -
-
-[% END %] -
-
- [% IF ( record2 ) %] - -
-
    - [% FOREACH record IN record2 %] - [% FOREACH fiel IN record.field %] -
  • - - [% fiel.tag %] - - - - [% IF ( fiel.value ) %] / [% fiel.value %] - - - [% END %] - - [% IF ( fiel.subfield ) %] -
      - [% FOREACH subfiel IN fiel.subfield %] -
    • - - [% subfiel.subtag %] / [% subfiel.value %] - - -
      • - [% END %] -
    - [% END %] - [% END %] -
    • - [% END %] -
-
- - - - - - [% END %] -
-
+[% PROCESS mergesource recordid1=biblio1 recordid2=biblio2 %]
-
-

Destination record

-
-
    - [% FOREACH record IN record1 %] - [% FOREACH fiel IN record.field %]
  • [% fiel.tag %] - - - [% IF ( fiel.value ) %] / - [% fiel.value %] - - - [% END %] - - [% IF ( fiel.subfield ) %] -
      - [% FOREACH subfiel IN fiel.subfield %] -
    • - [% subfiel.subtag %] / [% subfiel.value %] - - -
      • - [% END %] -
    - [% END %] - - [% END %] -
    • - [% END %] - -
-
-
+[% PROCESS mergetarget %]
diff --git a/t/Koha_Record.t b/t/Koha_Record.t new file mode 100755 index 0000000000..ca9a246e8b --- /dev/null +++ b/t/Koha_Record.t @@ -0,0 +1,99 @@ +#!/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. + +use strict; +use warnings; + +use Test::More tests => 4; + +BEGIN { + use_ok('Koha::Record'); +} + +my $marcrecord = MARC::Record->new; + +$marcrecord->add_fields( + [ '001', '1234' ], + [ '150', ' ', ' ', a => 'Cooking' ], + [ '450', ' ', ' ', a => 'Cookery', z => 'Instructional manuals' ], + ); +my $record = Koha::Record->new({ 'record' => $marcrecord }); + +is(ref($record), 'Koha::Record', 'Created valid Koha::Record object'); + +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 = $record->createMarcHash(); +my %fieldkeys; +require Data::Dumper; +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'); -- 2.39.5