Bugzilla – Attachment 66798 Details for
Bug 17380
Resolve several problems related to Default authority framework
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 17380: Graceful resolution of missing reporting tag in merge
Bug-17380-Graceful-resolution-of-missing-reporting.patch (text/plain), 6.09 KB, created by
Marcel de Rooy
on 2017-09-04 08:29:58 UTC
(
hide
)
Description:
Bug 17380: Graceful resolution of missing reporting tag in merge
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2017-09-04 08:29:58 UTC
Size:
6.09 KB
patch
obsolete
>From 8e5681599891dd4c5fc31ad61fa6192fb2b6b175 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Tue, 4 Jul 2017 14:31:21 +0200 >Subject: [PATCH] Bug 17380: Graceful resolution of missing reporting tag in > merge >Content-Type: text/plain; charset=utf-8 > >Altough this patch deals with a mostly hypothetical case and this report >makes it practically impossible anymore to merge with records in the >Default framework (having no reporting tag), we can make the code of >sub merge still a bit more robust here. > >If you would merge biblio records from one authtype to another and the new >framework would not have a reporting tag, before this patch the result would >be data loss. Merge would handle this request as a delete. This patch makes >merge handle it differently: instead of clearing the biblio records, it >keeps $a and $9 in order to make a future corrective merge possible. > >Note: The additional condition on line 1468 for $tags_using_authtype >makes sure that we do not select all fields when the authtype should >unexpectedly be empty string (Default). This prevents crashing on >a "Control fields do not have subfields" error. > >Test plan: >[1] Run t/db_dependent/Authorities/Merge.t > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> > >Signed-off-by: Josef Moravec <josef.moravec@gmail.com> >--- > C4/AuthoritiesMarc.pm | 14 +++++++++++--- > t/db_dependent/Authority/Merge.t | 33 ++++++++++++++++++++++++++++++++- > 2 files changed, 43 insertions(+), 4 deletions(-) > >diff --git a/C4/AuthoritiesMarc.pm b/C4/AuthoritiesMarc.pm >index 8b9c6c1..c198912 100644 >--- a/C4/AuthoritiesMarc.pm >+++ b/C4/AuthoritiesMarc.pm >@@ -1443,6 +1443,14 @@ sub merge { > > my @record_to; > @record_to = $MARCto->field($auth_tag_to_report_to)->subfields() if $auth_tag_to_report_to && $MARCto && $MARCto->field($auth_tag_to_report_to); >+ # Exceptional: If MARCto and authtypeto exist but $auth_tag_to_report_to >+ # is empty, make sure that $9 and $a remain (instead of clearing the >+ # reference) in order to allow for data recovery. >+ # Note: We need $a too, since a single $9 does not pass ModBiblio. >+ if( $MARCto && $authtypeto && !@record_to ) { >+ push @record_to, [ 'a', ' ' ]; # do not remove the space >+ } >+ > my @record_from; > if( !$authfrom && $MARCfrom && $MARCfrom->field('1..','2..') ) { > # postponed merge, authfrom was deleted and MARCfrom only contains the old reporting tag (and possibly a 100 for UNIMARC) >@@ -1457,7 +1465,7 @@ sub merge { > # For a deleted authority record, we scan all auth controlled fields > my $dbh = C4::Context->dbh; > my $sql = "SELECT DISTINCT tagfield FROM marc_subfield_structure WHERE authtypecode=?"; >- my $tags_using_authtype = $authtypefrom ? $dbh->selectcol_arrayref( $sql, undef, ( $authtypefrom->authtypecode )) : $dbh->selectcol_arrayref( "SELECT DISTINCT tagfield FROM marc_subfield_structure WHERE authtypecode IS NOT NULL AND authtypecode<>''" ); >+ my $tags_using_authtype = $authtypefrom && $authtypefrom->authtypecode ? $dbh->selectcol_arrayref( $sql, undef, ( $authtypefrom->authtypecode )) : $dbh->selectcol_arrayref( "SELECT DISTINCT tagfield FROM marc_subfield_structure WHERE authtypecode IS NOT NULL AND authtypecode<>''" ); > my $tags_new; > if( $authtypeto && ( !$authtypefrom || $authtypeto->authtypecode ne $authtypefrom->authtypecode )) { > $tags_new = $dbh->selectcol_arrayref( $sql, undef, ( $authtypeto->authtypecode )); >@@ -1493,7 +1501,7 @@ sub merge { > $update = 1; > next; > } >- my $newtag = $tags_new >+ my $newtag = $tags_new && @$tags_new > ? _merge_newtag( $tag, $tags_new ) > : $tag; > my $field_to = MARC::Field->new( >@@ -1512,7 +1520,7 @@ sub merge { > $field_to->add_subfields( $subfield->[0], $subfield->[1] ); > } > } >- if ($tags_new) { >+ if ($tags_new && @$tags_new) { > $marcrecord->delete_field($field); > append_fields_ordered( $marcrecord, $field_to ); > } else { >diff --git a/t/db_dependent/Authority/Merge.t b/t/db_dependent/Authority/Merge.t >index 29f6a80..08a582c 100755 >--- a/t/db_dependent/Authority/Merge.t >+++ b/t/db_dependent/Authority/Merge.t >@@ -4,7 +4,7 @@ > > use Modern::Perl; > >-use Test::More tests => 7; >+use Test::More tests => 8; > > use Getopt::Long; > use MARC::Record; >@@ -329,6 +329,37 @@ subtest "Test some specific postponed merge issues" => sub { > is( $biblio->subfield('109', '9'), $id, 'If the 109 is no longer present, another modify merge would not bring it back' ); > }; > >+subtest "Graceful resolution of missing reporting tag" => sub { >+ plan tests => 2; >+ >+ # Simulate merge with authority in Default fw without reporting tag >+ # We expect data loss in biblio, but we keep $a and the reference in $9 >+ # in order to allow a future merge to restore data. >+ >+ # Accomplish the above by clearing reporting tag in $authtype2 >+ my $fw2 = Koha::Authority::Types->find( $authtype2 ); >+ $fw2->auth_tag_to_report('')->store; >+ >+ my $authmarc = MARC::Record->new; >+ $authmarc->append_fields( MARC::Field->new( '109', '', '', 'a' => 'aa', b => 'bb' )); >+ my $id1 = AddAuthority( $authmarc, undef, $authtype1 ); >+ my $id2 = AddAuthority( $authmarc, undef, $authtype2 ); >+ >+ my $biblio = MARC::Record->new; >+ $biblio->append_fields( >+ MARC::Field->new( '609', '', '', a => 'aa', 9 => $id1 ), >+ ); >+ my ( $biblionumber ) = C4::Biblio::AddBiblio( $biblio, '' ); >+ >+ # Merge >+ merge({ mergefrom => $id1, MARCfrom => $authmarc, mergeto => $id2, MARCto => $authmarc, biblionumbers => [ $biblionumber ] }); >+ $biblio = C4::Biblio::GetMarcBiblio( $biblionumber ); >+ is( $biblio->subfield('612', '9'), $id2, 'id2 saved in $9' ); >+ is( $biblio->subfield('612', 'a'), ' ', 'Kept an empty $a too' ); >+ >+ $fw2->auth_tag_to_report('112')->store; >+}; >+ > sub set_mocks { > # After we removed the Zebra code from merge, we only need to mock > # get_usage_count and linked_biblionumbers here. >-- >2.1.4
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 17380
:
55947
|
60592
|
64952
|
64953
|
64954
|
64955
|
66277
|
66278
|
66279
|
66280
|
66313
|
66314
|
66315
|
66316
|
66750
|
66751
|
66796
|
66797
|
66798
|
66799
|
66800
|
66801
|
66912
|
66913
|
66914
|
66915
|
66916
|
66917