From 1a5416ba9ebef98bdc9dcbbaeb4739b5aa061541 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 28 Nov 2013 12:34:41 +0100 Subject: [PATCH] Bug 11319: POC Marc modification templates improvements This patch is a POC. Please give me your feedback on the implementation of the fields/subfields split. This patch only implements the delete and the read routines. See new UT at the end of t/SimpleMARC.t --- C4/MarcModificationTemplates.pm | 4 +- Koha/SimpleMARC.pm | 102 +++++++++++----- installer/data/mysql/updatedatabase.pl | 12 ++ .../modules/tools/marc_modification_templates.tt | 8 +- t/SimpleMARC.t | 123 +++++++++++++------- tools/marc_modification_templates.pl | 2 +- 6 files changed, 174 insertions(+), 77 deletions(-) diff --git a/C4/MarcModificationTemplates.pm b/C4/MarcModificationTemplates.pm index 50b77df..55f8e0f 100644 --- a/C4/MarcModificationTemplates.pm +++ b/C4/MarcModificationTemplates.pm @@ -587,8 +587,8 @@ sub ModifyRecordWithTemplate { when ( /^move_field$/ ) { move_field( @params ); } - when ( /^delete_field$/ ) { - delete_field( @params ); + when ( /^delete$/ ) { + Koha::SimpleMarc::delete( @params ); } } } diff --git a/Koha/SimpleMARC.pm b/Koha/SimpleMARC.pm index 2195143..ad42ddf 100644 --- a/Koha/SimpleMARC.pm +++ b/Koha/SimpleMARC.pm @@ -16,11 +16,11 @@ our %EXPORT_TAGS = ( 'all' => [ qw( our @EXPORT_OK = ( @{ $EXPORT_TAGS{'all'} } ); our @EXPORT = qw( - read_field + read update_field copy_field move_field - delete_field + delete field_exists field_equals ); @@ -77,7 +77,7 @@ sub copy_field { if ( ! ( $record && $fromFieldName && $toFieldName ) ) { return; } - my @values = read_field( $record, $fromFieldName, $fromSubfieldName ); + my @values = Koha::SimpleMARC::read( $record, $fromFieldName, $fromSubfieldName ); @values = ( $values[$n-1] ) if ( $n ); if ( $regex and $regex->{search} ) { @@ -180,24 +180,55 @@ sub update_field { =cut -sub read_field { - my ( $record, $fieldName, $subfieldName, $n ) = @_; +sub read { + my ( $record, $fieldName, $subfieldName, $n ) = @_; - my @fields = $record->field( $fieldName ); + if ( not $subfieldName or $subfieldName eq '' ) { + _read_field( $record, $fieldName, $n ); + } else { + _read_subfield( $record, $fieldName, $subfieldName, $n ); + } - return map { $_->data() } @fields unless $subfieldName; +} - my @subfields; - foreach my $field ( @fields ) { - my @sf = $field->subfield( $subfieldName ); - push( @subfields, @sf ); - } +sub _read_field { + my ( $record, $fieldName, $n ) = @_; + + my @fields = $record->field( $fieldName ); + + return unless @fields; + + return map { $_->data() } @fields + if $fieldName < 10; + + my @values; + foreach my $field ( @fields ) { + my @sf = $field->subfields; + push( @values, @sf ); + } + + return $n + ? $values[$n-1] # I don't think it is what we want here + : @values; - if ( $n ) { - return $subfields[$n-1]; - } else { - return @subfields; - } +} + +sub _read_subfield { + my ( $record, $fieldName, $subfieldName, $n ) = @_; + + my @fields = $record->field( $fieldName ); + + return unless @fields; + + my @values; + foreach my $field ( @fields ) { + my @sf = $field->subfield( $subfieldName ); + push( @values, @sf ); + } + + return $n + ? $values[$n-1] + : @values; } =head2 field_exists @@ -243,7 +274,7 @@ sub field_equals { if ( ! $record ) { return; } - my @field_values = read_field( $record, $fieldName, $subfieldName, $n ); + my @field_values = Koha::SimpleMARC::read( $record, $fieldName, $subfieldName, $n ); my $field_value = $field_values[$n-1]; if ( $regex ) { @@ -269,9 +300,17 @@ sub field_equals { sub move_field { my ( $record, $fromFieldName, $fromSubfieldName, $toFieldName, $toSubfieldName, $regex, $n ) = @_; copy_field( $record, $fromFieldName, $fromSubfieldName, $toFieldName, $toSubfieldName, $regex, $n , 'dont_erase' ); - delete_field( $record, $fromFieldName, $fromSubfieldName, $n ); + Koha::SimpleMARC::delete( $record, $fromFieldName, $fromSubfieldName, $n ); } +sub delete { + my ( $record, $fieldName, $subfieldName, $n ) = @_; + if ( not $subfieldName or $subfieldName eq '' ) { + _delete_field( $record, $fieldName, $n ); + } else { + _delete_subfield( $record, $fieldName, $subfieldName, $n ); + } +} =head2 delete_field delete_field( $record, $fieldName[, $subfieldName [, $n ] ] ); @@ -283,22 +322,27 @@ sub move_field { =cut -sub delete_field { - my ( $record, $fieldName, $subfieldName, $n ) = @_; +sub _delete_field { + my ( $record, $fieldName, $n ) = @_; - my @fields = $record->field( $fieldName ); + my @fields = $record->field( $fieldName ); - @fields = ( $fields[$n-1] ) if ( $n ); - - if ( @fields && !$subfieldName ) { + @fields = ( $fields[$n-1] ) if ( $n ); foreach my $field ( @fields ) { - $record->delete_field( $field ); + $record->delete_field( $field ); } - } elsif ( @fields && $subfieldName ) { +} + +sub _delete_subfield { + my ( $record, $fieldName, $subfieldName, $n ) = @_; + + my @fields = $record->field( $fieldName ); + + @fields = ( $fields[$n-1] ) if ( $n ); + foreach my $field ( @fields ) { - $field->delete_subfield( code => $subfieldName ); + $field->delete_subfield( code => $subfieldName ); } - } } 1; diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index a9bfbbf..d01150c 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -7778,6 +7778,18 @@ if(CheckVersion($DBversion)) { SetVersion($DBversion); } + + +$DBversion = "3.15.00.XXX"; +if(CheckVersion($DBversion)) { + $dbh->do(q| + ALTER TABLE marc_modification_template_actions + CHANGE action action VARCHAR(16) NOT NULL; + |); + print "Upgrade to $DBversion done (Bug 11275: alter deleteditems.materials from varchar(10) to text)\n"; + SetVersion($DBversion); +} + =head1 FUNCTIONS =head2 TableExists($table) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/marc_modification_templates.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/marc_modification_templates.tt index 569b7f8..85a9e1c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/marc_modification_templates.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/marc_modification_templates.tt @@ -24,7 +24,7 @@ function onActionChange(selectObj) { var action = selectObj.options[idx].value; switch( action ) { - case 'delete_field': + case 'delete': show('field_number_block'); hide('with_value_block'); hide('to_field_block'); @@ -198,7 +198,7 @@ function editAction( mmta_id, ordering, action, field_number, from_field, from_s function cancelEditAction() { document.getElementById('mmta_id').value = ''; - setSelectByValue( 'action', 'delete_field' ); + setSelectByValue( 'action', 'delete' ); document.getElementById('action').onchange(); document.getElementById('from_field').value = ''; @@ -323,7 +323,7 @@ function setSelectByValue( selectId, value ) { [% ActionsLoo.ordering %] - [% IF ( ActionsLoo.action_delete_field ) %] Delete [% END %] + [% IF ( ActionsLoo.action_delete ) %] Delete [% END %] [% IF ( ActionsLoo.action_update_field ) %] Update [% END %] [% IF ( ActionsLoo.action_move_field ) %] Move [% END %] [% IF ( ActionsLoo.action_copy_field ) %] Copy [% END %] @@ -402,7 +402,7 @@ function setSelectByValue( selectId, value ) { Add a new action