Attachment 138122 Details for Bug 19693 – Bug 19693: Add test sub to Merge.t

[patch] Bug 19693: Add test sub to Merge.t

0001-Bug-19693-Add-test-sub-to-Merge.t.patch (text/plain), 7.05 KB, created by Janusz Kaczmarek on 2022-07-26 09:32:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Janusz Kaczmarek

Created: 2022-07-26 09:32:30 UTC

Size: 7.05 KB

patch

obsolete

>From 99cef606b57a1f9fc9fe5480cf446047722e9a23 Mon Sep 17 00:00:00 2001
>From: Janusz Kaczmarek <januszop@gmail.com>
>Date: Mon, 25 Jul 2022 22:27:32 +0200
>Subject: [PATCH 1/2] Bug 19693: Add test sub to Merge.t
>
>In those rare cases when authorities are updated by an external agency (or
>even internally, by reviewing and correcting an exported authority file)
>when the heading tag will be changed (seems odd but happens:
>
>111 Congress ==> 110 Corporate body.Congress ;
>100 Person ==> 110 Corporate body (a company named with person's name ;
>151 City--object ==> 150 Object (city) etc.)
>
>and then the authority record in Koha database will be updated with
>bulkmarcimport or by calling directly ModAuthority from a custom script,
>the merge function "doesn't know" that the change to the authority type
>has been made and, consequently, doesn't adequately change the tag in
>related fields in biblio records (as it would if two different records
>with different authtypecode were merged with Koha interface).
>
>This is because at the moment when merge function is being called
>by ModAuthority:
>Koha::Authority::Types->find($autfrom->authtypecode)
>Koha::Authority::Types->find($authto->authtypecode)
>both have the same value (because $mergefrom == $mergeto).
>
>Therefore in case when $mergefrom == $mergeto and the heading tag changes,
>$authtypefrom and $authfrom calculated in merge in the ordinar way are
>misleading and should not be taken unto consideration.
>
>Test plan:
>==========
>
>1. run t/db_dependent/Authority/Merge.t
>2. you should see problems in "Test update A with modified heading tag
>   (changing authtype)"
>3. apply the patch
>4. run t/db_dependent/Authority/Merge.t again
>5. the test should pass
>
>Alternatively:
>
>1. have an authority record used in biblio;
>   export it to file;
>   change 1XX heading tag to a different (but reasonable) value
>   and possibly change also the content of the heading
>   (one can delete also 942 but it doesn't matter);
>   make bulkmarcimport.pl -a -update -file <modified_auth_file> and
>   see that the tag in biblio record has not been changed (whereas
>   the type of authority record did change);
>
>2. make orders in database (so that the authority type and the tag of
>   the field in biblio record correspond); apply the patch;
>
>3. repeat the test from 1.
>---
> t/db_dependent/Authority/Merge.t | 84 +++++++++++++++++++++++++++++++-
> 1 file changed, 82 insertions(+), 2 deletions(-)
>
>diff --git a/t/db_dependent/Authority/Merge.t b/t/db_dependent/Authority/Merge.t
>index 3041662f50..0720ab329b 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 => 10;
>+use Test::More tests => 11;
> 
> use Getopt::Long;
> use MARC::Record;
>@@ -21,7 +21,7 @@ use Koha::Biblios;
> use Koha::Database;
> 
> BEGIN {
>-        use_ok('C4::AuthoritiesMarc', qw( merge AddAuthority compare_fields DelAuthority ));
>+        use_ok('C4::AuthoritiesMarc', qw( merge AddAuthority compare_fields DelAuthority ModAuthority ));
> }
> 
> # Optionally change marc flavour
>@@ -245,6 +245,86 @@ subtest 'Test merge A1 to B1 (changing authtype)' => sub {
>         'Check 612x' );
> };
> 
>+subtest 'Test update A with modified heading tag (changing authtype)' => sub {
>+# Bug 19693
>+# This would happen rarely when updating authorities from authority file
>+# when the tag of a heading field is being changed (and thus also
>+# the authtype)
>+    plan tests => 13;
>+
>+    # Get back to loose mode now
>+    t::lib::Mocks::mock_preference('AuthorityMergeMode', 'loose');
>+
>+    # create an auth rec
>+    my $auth1 = MARC::Record->new;
>+    $auth1->append_fields( MARC::Field->new( '109', '0', '0', 'a' => 'George Orwell', b => 'bb' ));
>+    my $authid1 = AddAuthority( $auth1, undef, $authtype1 );
>+
>+    # create a biblio with one 109 and two 609s to be touched
>+    # seems exceptional see bug 13760 comment10
>+    my $marc = MARC::Record->new;
>+    $marc->append_fields(
>+        MARC::Field->new( '003', 'some_003' ),
>+        MARC::Field->new( '109', '', '', a => 'G. Orwell', b => 'bb', d => 'd', 9 => $authid1 ),
>+        MARC::Field->new( '245', '', '', a => 'My title' ),
>+        MARC::Field->new( '609', '', '', a => 'Orwell', 9 => "$authid1" ),
>+        MARC::Field->new( '609', '', '', a => 'Orwell', x => 'xx', 9 => "$authid1" ),
>+        MARC::Field->new( '611', '', '', a => 'Added for testing order' ),
>+        MARC::Field->new( '612', '', '', a => 'unrelated', 9 => 'other' ),
>+    );
>+    my ( $biblionumber ) = C4::Biblio::AddBiblio( $marc, '' );
>+    my $oldbiblio = Koha::Biblios->find($biblionumber)->metadata->record;
>+
>+    # Time to merge
>+    @linkedrecords = ( $biblionumber );
>+    my $auth2 = MARC::Record->new;
>+    $auth2->append_fields( MARC::Field->new( '112', '0', '0', 'a' => 'Batman', c => 'cc' ));
>+    my $authid2 = ModAuthority($authid1, $auth2, $authtype2 );
>+    # inside ModAuthority the following is executed:
>+    # merge({ mergefrom => $authid1, MARCfrom => $auth1, mergeto => $authid1, MARCto => $auth2 });
>+    is( $authid2, $authid1, 'authid after ModAuthority OK' );
>+
>+    # Get new marc record for compares
>+    my $newbiblio = Koha::Biblios->find($biblionumber)->metadata->record;
>+    compare_fields( $oldbiblio, $newbiblio, {}, 'count' );
>+    # Exclude 109/609 and 112/612 in comparing order
>+    my $excl = { '109' => 1, '112' => 1, '609' => 1, '612' => 1 };
>+    compare_fields( $oldbiblio, $newbiblio, $excl, 'order' );
>+    # Check position of 612s in the new record
>+    my $full_order = join q/,/, map { $_->tag } $newbiblio->fields;
>+    is( $full_order =~ /611(,612){3}/, 1, 'Check position of all 612s' );
>+
>+    # Check some fields
>+    is( $newbiblio->field('003')->data,
>+        $oldbiblio->field('003')->data,
>+        'Check contents of a control field not expected to be touched' );
>+    is( $newbiblio->subfield( '245', 'a' ),
>+        $oldbiblio->subfield( '245', 'a' ),
>+        'Check contents of a data field not expected to be touched' );
>+    is( $newbiblio->subfield( '112', 'a' ),
>+        $auth2->subfield( '112', 'a' ), 'Check modified 112a' );
>+    is( $newbiblio->subfield( '112', 'c' ),
>+        $auth2->subfield( '112', 'c' ), 'Check new 112c' );
>+
>+    # Check 112b; this subfield was cleared when moving from 109 to 112
>+    # Note that this fix only applies to the current loose mode only
>+    is( $newbiblio->subfield( '112', 'b' ), undef,
>+        'Merge respects a cleared subfield in loose mode' );
>+
>+    # Check the original 612
>+    is( ( $newbiblio->field('612') )[0]->subfield( 'a' ),
>+        $oldbiblio->subfield( '612', 'a' ), 'Check untouched 612a' );
>+    # Check second 612
>+    is( ( $newbiblio->field('612') )[1]->subfield( 'a' ),
>+        $auth2->subfield( '112', 'a' ), 'Check second touched 612a' );
>+    # Check second new 612ax (in LOOSE mode)
>+    is( ( $newbiblio->field('612') )[2]->subfield( 'a' ),
>+        $auth2->subfield( '112', 'a' ), 'Check touched 612a' );
>+    is( ( $newbiblio->field('612') )[2]->subfield( 'x' ),
>+        ( $oldbiblio->field('609') )[1]->subfield('x'),
>+        'Check 612x' );
>+};
>+
> subtest 'Merging authorities should handle deletes (BZ 18070)' => sub {
>     plan tests => 2;
> 
>-- 
>2.30.2
>

Actions: View | Diff | Splinter Review

Attachments on bug 19693: 69344 | 69349 | 69350 | 138121 | 138122 | 140915 | 140916 | 141480 | 141481 | 141482