Attachment 62055 Details for Bug 9988 – Bug 9988: [QA Follow-up] Satisfy QA issues

[patch] Bug 9988: [QA Follow-up] Satisfy QA issues

Bug-9988-QA-Follow-up-Satisfy-QA-issues.patch (text/plain), 6.44 KB, created by Marcel de Rooy on 2017-04-11 14:27:11 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2017-04-11 14:27:11 UTC

Size: 6.44 KB

patch

obsolete

>From 1f6e0dbffaec50eda406afd064b4741eee8b1795 Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Tue, 11 Apr 2017 14:58:41 +0200
>Subject: [PATCH] Bug 9988: [QA Follow-up] Satisfy QA issues
>Content-Type: text/plain; charset=utf-8
>
>[1] See comment102. Moved sub reporting_tag_xml to MergeRequest.pm.
>    Adjusted t/db_dependent/Koha/Authorities.t accordingly.
>    This resolves the C3 inconsistent hierarchy errors.
>[2] Removed empty POD section Instance Methods from MergeRequests.
>    This resolves the POD error in comment102 point 2.
>[3] Include a tag 100 for UNIMARC in reporting_tag_xml to resolve an error
>    on encoding in MARC::File::XML. Subtest for oldmarc and subtest for
>    reporting_tag_xml adjusted accordingly.
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>---
> Koha/Authority/MergeRequest.pm    | 36 ++++++++++++++++++++++++++++++++++--
> Koha/Authority/MergeRequests.pm   | 27 ---------------------------
> t/db_dependent/Koha/Authorities.t | 27 +++++++++++++++------------
> 3 files changed, 49 insertions(+), 41 deletions(-)
>
>diff --git a/Koha/Authority/MergeRequest.pm b/Koha/Authority/MergeRequest.pm
>index 20c138c..4c67910 100644
>--- a/Koha/Authority/MergeRequest.pm
>+++ b/Koha/Authority/MergeRequest.pm
>@@ -22,7 +22,6 @@ use Modern::Perl;
> use parent qw(Koha::Object);
> 
> use Koha::Authorities;
>-use Koha::Authority::MergeRequests;
> use Koha::Authority::Types;
> 
> =head1 NAME
>@@ -67,7 +66,7 @@ sub new {
>     if( $self->authid && $oldrecord ) {
>         my $auth = Koha::Authorities->find( $self->authid );
>         my $type = $auth ? Koha::Authority::Types->find($auth->authtypecode) : undef;
>-        $self->reportxml( Koha::Authority::MergeRequests->reporting_tag_xml({ record => $oldrecord, tag => $type->auth_tag_to_report })) if $type;
>+        $self->reportxml( $self->reporting_tag_xml({ record => $oldrecord, tag => $type->auth_tag_to_report })) if $type;
>     }
>     return $self;
> }
>@@ -88,6 +87,39 @@ sub oldmarc {
> 
> =head2 CLASS METHODS
> 
>+=head3 reporting_tag_xml
>+
>+    my $xml = Koha::Authority::MergeRequest->reporting_tag_xml({
>+        record => $record, tag => $tag,
>+    });
>+
>+=cut
>+
>+sub reporting_tag_xml {
>+    my ( $class, $params ) = @_;
>+    return if !$params->{record} || !$params->{tag};
>+
>+    my $newrecord = MARC::Record->new;
>+    $newrecord->encoding( 'UTF-8' );
>+    my $reportfield = $params->{record}->field( $params->{tag} );
>+    return if !$reportfield;
>+
>+    # For UNIMARC we need a field 100 that includes the encoding
>+    # at position 13 and 14
>+    if( C4::Context->preference('marcflavour') eq 'UNIMARC' ) {
>+        $newrecord->append_fields(
>+            MARC::Field->new( '100', '', '', a => ' 'x13 . '50' ),
>+        );
>+    }
>+
>+    $newrecord->append_fields( $reportfield );
>+    return $newrecord->as_xml(
>+        C4::Context->preference('marcflavour') eq 'UNIMARC' ?
>+        'UNIMARCAUTH' :
>+        'MARC21'
>+    );
>+}
>+
> =head3 _type
> 
> Returns name of corresponding DBIC resultset
>diff --git a/Koha/Authority/MergeRequests.pm b/Koha/Authority/MergeRequests.pm
>index dd274cf..c3f5ca7 100644
>--- a/Koha/Authority/MergeRequests.pm
>+++ b/Koha/Authority/MergeRequests.pm
>@@ -42,35 +42,8 @@ Description
> 
> =head1 METHODS
> 
>-=head2 INSTANCE METHODS
>-
> =head2 CLASS METHODS
> 
>-=head3 reporting_tag_xml
>-
>-    my $xml = Koha::Authority::MergeRequests->reporting_tag_xml({
>-        record => $record, tag => $tag,
>-    });
>-
>-=cut
>-
>-sub reporting_tag_xml {
>-    my ( $class, $params ) = @_;
>-    return if !$params->{record} || !$params->{tag};
>-
>-    my $newrecord = MARC::Record->new;
>-    $newrecord->encoding( 'UTF-8' );
>-    my $reportfield = $params->{record}->field( $params->{tag} );
>-    return if !$reportfield;
>-
>-    $newrecord->append_fields( $reportfield );
>-    return $newrecord->as_xml(
>-        C4::Context->preference('marcflavour') eq 'UNIMARC' ?
>-        'UNIMARCAUTH' :
>-        'MARC21'
>-    );
>-}
>-
> =head3 cron_cleanup
> 
>     Koha::Authority::MergeRequests->cron_cleanup({
>diff --git a/t/db_dependent/Koha/Authorities.t b/t/db_dependent/Koha/Authorities.t
>index a690663..501e62b 100644
>--- a/t/db_dependent/Koha/Authorities.t
>+++ b/t/db_dependent/Koha/Authorities.t
>@@ -32,7 +32,6 @@ use C4::Context;
> use Koha::Authority;
> use Koha::Authorities;
> use Koha::Authority::MergeRequest;
>-use Koha::Authority::MergeRequests;
> use Koha::Authority::Type;
> use Koha::Authority::Types;
> use Koha::Database;
>@@ -91,35 +90,39 @@ subtest 'New merge request, method oldmarc' => sub {
>     });
>     like( $req->reportxml, qr/b_findme/, 'Reportxml initialized' );
> 
>-    # Check if oldmarc is a MARC::Record and has one field
>+    # Check if oldmarc is a MARC::Record and has one or two fields
>     is( ref( $req->oldmarc ), 'MARC::Record', 'Check oldmarc method' );
>-    is( scalar $req->oldmarc->fields, 1, 'Contains one field' );
>+    if( C4::Context->preference('marcflavour') eq 'UNIMARC' ) {
>+        is( scalar $req->oldmarc->fields, 2, 'UNIMARC contains two fields' );
>+    } else {
>+        is( scalar $req->oldmarc->fields, 1, 'MARC21 contains one field' );
>+    }
> };
> 
>-subtest 'Testing reporting_tag_xml in MergeRequests' => sub {
>+subtest 'Testing reporting_tag_xml in MergeRequest' => sub {
>     plan tests => 2;
> 
>     my $record = MARC::Record->new;
>     $record->append_fields(
>         MARC::Field->new( '024', '', '', a => 'aaa' ),
>-        MARC::Field->new( '100', '', '', a => 'Best author' ),
>+        MARC::Field->new( '110', '', '', a => 'Best author' ),
>         MARC::Field->new( '234', '', '', a => 'Just a field' ),
>     );
>-    my $xml = Koha::Authority::MergeRequests->reporting_tag_xml({
>-        record => $record, tag => '110',
>+    my $xml = Koha::Authority::MergeRequest->reporting_tag_xml({
>+        record => $record, tag => '100',
>     });
>     is( $xml, undef, 'Expected no result for wrong tag' );
>-    $xml = Koha::Authority::MergeRequests->reporting_tag_xml({
>-        record => $record, tag => '100',
>+    $xml = Koha::Authority::MergeRequest->reporting_tag_xml({
>+        record => $record, tag => '110',
>     });
>     my $newrecord = MARC::Record->new_from_xml(
>         $xml, 'UTF-8',
>         C4::Context->preference('marcflavour') eq 'UNIMARC' ?
>         'UNIMARCAUTH' :
>         'MARC21',
>-    ); # MARC format does not actually matter here
>-    cmp_deeply( $record->field('100')->subfields,
>-        $newrecord->field('100')->subfields,
>+    );
>+    cmp_deeply( $record->field('110')->subfields,
>+        $newrecord->field('110')->subfields,
>         'Compare reporting tag in both records',
>     );
> };
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 9988: 59158 | 59159 | 60256 | 60257 | 60258 | 60259 | 60594 | 60595 | 60596 | 60597 | 60598 | 60599 | 60600 | 60601 | 60602 | 60603 | 60604 | 60605 | 60606 | 60607 | 60668 | 60669 | 60670 | 60671 | 60672 | 60673 | 60674 | 60675 | 61306 | 61307 | 61308 | 61309 | 61779 | 61780 | 61781 | 61782 | 61783 | 61784 | 61785 | 61786 | 61787 | 61788 | 61789 | 61790 | 61791 | 61792 | 61890 | 61891 | 61892 | 61893 | 61894 | 61895 | 61897 | 61898 | 61899 | 61900 | 61901 | 61902 | 61903 | 61904 | 61912 | 61913 | 61914 | 61915 | 61916 | 61917 | 61918 | 61919 | 61920 | 61921 | 61922 | 61923 | 61924 | 61925 | 62055 | 62123 | 62124 | 62125 | 62126 | 62127 | 62128 | 62129 | 62130 | 62131 | 62132 | 62133 | 62134 | 62135 | 62136 | 62137