Attachment 158341 Details for Bug 33749 – Bug 33749: (QA follow-up) Tidy code for qa script

[patch] Bug 33749: (QA follow-up) Tidy code for qa script

Bug-33749-QA-follow-up-Tidy-code-for-qa-script.patch (text/plain), 11.69 KB, created by Kyle M Hall (khall) on 2023-11-03 14:54:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2023-11-03 14:54:52 UTC

Size: 11.69 KB

patch

obsolete

>From e5669323bbe83b12fabf0befdde6c2fc3c87e17e Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Fri, 3 Nov 2023 14:51:35 +0000
>Subject: [PATCH] Bug 33749: (QA follow-up) Tidy code for qa script
>
>Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
>---
> C4/AuthoritiesMarc.pm          | 163 +++++++++++++++++----------------
> C4/Biblio.pm                   |   4 +-
> Koha/Filter/MARC/TrimFields.pm |   6 +-
> t/RecordProcessor.t            |   8 +-
> 4 files changed, 93 insertions(+), 88 deletions(-)
>
>diff --git a/C4/AuthoritiesMarc.pm b/C4/AuthoritiesMarc.pm
>index 5dc5c3ad461..4c593e4eb6e 100644
>--- a/C4/AuthoritiesMarc.pm
>+++ b/C4/AuthoritiesMarc.pm
>@@ -562,136 +562,141 @@ returns authid of the newly created authority
> =cut
> 
> sub AddAuthority {
>-# pass the MARC::Record to this function, and it will create the records in the authority table
>+
>+    # pass the MARC::Record to this function, and it will create the records in the authority table
>     my ( $record, $authid, $authtypecode, $params ) = @_;
> 
>     my $skip_record_index = $params->{skip_record_index} || 0;
> 
>-  my $dbh=C4::Context->dbh;
>-	my $leader='     nz  a22     o  4500';#Leader for incomplete MARC21 record
>+    my $dbh    = C4::Context->dbh;
>+    my $leader = '     nz  a22     o  4500';    #Leader for incomplete MARC21 record
> 
>-# if authid empty => true add, find a new authid number
>+    # if authid empty => true add, find a new authid number
>     my $format;
>-    if (uc(C4::Context->preference('marcflavour')) eq 'UNIMARC') {
>-        $format= 'UNIMARCAUTH';
>-    }
>-    else {
>-        $format= 'MARC21';
>+    if ( uc( C4::Context->preference('marcflavour') ) eq 'UNIMARC' ) {
>+        $format = 'UNIMARCAUTH';
>+    } else {
>+        $format = 'MARC21';
>     }
> 
>     #update date/time to 005 for marc and unimarc
>-    my $time=POSIX::strftime("%Y%m%d%H%M%S",localtime);
>-    my $f5=$record->field('005');
>-    if (!$f5) {
>-      $record->insert_fields_ordered( MARC::Field->new('005',$time.".0") );
>-    }
>-    else {
>-      $f5->update($time.".0");
>+    my $time = POSIX::strftime( "%Y%m%d%H%M%S", localtime );
>+    my $f5   = $record->field('005');
>+    if ( !$f5 ) {
>+        $record->insert_fields_ordered( MARC::Field->new( '005', $time . ".0" ) );
>+    } else {
>+        $f5->update( $time . ".0" );
>     }
> 
>     SetUTF8Flag($record);
>-	if ($format eq "MARC21") {
>+    if ( $format eq "MARC21" ) {
>         my $userenv = C4::Context->userenv;
>         my $library;
>         my $marcorgcode = C4::Context->preference('MARCOrgCode');
>         if ( $userenv && $userenv->{'branch'} ) {
>             $library = Koha::Libraries->find( $userenv->{'branch'} );
>+
>             # userenv's library could not exist because of a trick in misc/commit_file.pl (see FIXME and set_userenv statement)
>             $marcorgcode = $library ? $library->get_effective_marcorgcode : $marcorgcode;
>         }
>-		if (!$record->leader) {
>-			$record->leader($leader);
>-		}
>-		if (!$record->field('003')) {
>-			$record->insert_fields_ordered(
>-                MARC::Field->new('003', $marcorgcode),
>-			);
>-		}
>-		my $date=POSIX::strftime("%y%m%d",localtime);
>-		if (!$record->field('008')) {
>+        if ( !$record->leader ) {
>+            $record->leader($leader);
>+        }
>+        if ( !$record->field('003') ) {
>+            $record->insert_fields_ordered(
>+                MARC::Field->new( '003', $marcorgcode ),
>+            );
>+        }
>+        my $date = POSIX::strftime( "%y%m%d", localtime );
>+        if ( !$record->field('008') ) {
>+
>             # Get a valid default value for field 008
>             my $default_008 = C4::Context->preference('MARCAuthorityControlField008');
>-            if(!$default_008 or length($default_008)<34) {
>+            if ( !$default_008 or length($default_008) < 34 ) {
>                 $default_008 = '|| aca||aabn           | a|a     d';
>+            } else {
>+                $default_008 = substr( $default_008, 0, 34 );
>             }
>-            else {
>-                $default_008 = substr($default_008,0,34);
>-            }
> 
>-            $record->insert_fields_ordered( MARC::Field->new('008',$date.$default_008) );
>-		}
>-		if (!$record->field('040')) {
>-		 $record->insert_fields_ordered(
>-        MARC::Field->new('040','','',
>-            'a' => $marcorgcode,
>-            'c' => $marcorgcode,
>-				) 
>-			);
>+            $record->insert_fields_ordered( MARC::Field->new( '008', $date . $default_008 ) );
>+        }
>+        if ( !$record->field('040') ) {
>+            $record->insert_fields_ordered(
>+                MARC::Field->new(
>+                    '040', '', '',
>+                    'a' => $marcorgcode,
>+                    'c' => $marcorgcode,
>+                )
>+            );
>+        }
>+    }
>+
>+    if ( $format eq "UNIMARCAUTH" ) {
>+        $record->leader("     nx  j22             ") unless ( $record->leader() );
>+        my $date            = POSIX::strftime( "%Y%m%d", localtime );
>+        my $defaultfield100 = C4::Context->preference('UNIMARCAuthorityField100');
>+        if ( my $string = $record->subfield( '100', "a" ) ) {
>+            $string =~ s/fre50/frey50/;
>+            $record->field('100')->update( 'a' => $string );
>+        } elsif ( $record->field('100') ) {
>+            $record->field('100')->update( 'a' => $date . $defaultfield100 );
>+        } else {
>+            $record->append_fields(
>+                MARC::Field->new(
>+                    '100', ' ', ' '
>+                    , 'a' => $date . $defaultfield100
>+                )
>+            );
>+        }
>     }
>-	}
>-
>-  if ($format eq "UNIMARCAUTH") {
>-        $record->leader("     nx  j22             ") unless ($record->leader());
>-        my $date=POSIX::strftime("%Y%m%d",localtime);
>-	my $defaultfield100 = C4::Context->preference('UNIMARCAuthorityField100');
>-    if (my $string=$record->subfield('100',"a")){
>-      	$string=~s/fre50/frey50/;
>-      	$record->field('100')->update('a'=>$string);
>+    my ( $auth_type_tag, $auth_type_subfield ) = get_auth_type_location($authtypecode);
>+    if ( !$authid and $format eq "MARC21" ) {
>+
>+        # only need to do this fix when modifying an existing authority
>+        C4::AuthoritiesMarc::MARC21::fix_marc21_auth_type_location( $record, $auth_type_tag, $auth_type_subfield );
>+    }
>+    if ( my $field = $record->field($auth_type_tag) ) {
>+        $field->update( $auth_type_subfield => $authtypecode );
>+    } else {
>+        $record->add_fields( $auth_type_tag, '', '', $auth_type_subfield => $authtypecode );
>     }
>-    elsif ($record->field('100')){
>-          $record->field('100')->update('a'=>$date.$defaultfield100);
>-    } else {      
>-        $record->append_fields(
>-        MARC::Field->new('100',' ',' '
>-            ,'a'=>$date.$defaultfield100)
>-        );
>-    }      
>-  }
>-  my ($auth_type_tag, $auth_type_subfield) = get_auth_type_location($authtypecode);
>-  if (!$authid and $format eq "MARC21") {
>-    # only need to do this fix when modifying an existing authority
>-    C4::AuthoritiesMarc::MARC21::fix_marc21_auth_type_location($record, $auth_type_tag, $auth_type_subfield);
>-  } 
>-  if (my $field=$record->field($auth_type_tag)){
>-    $field->update($auth_type_subfield=>$authtypecode);
>-  }
>-  else {
>-    $record->add_fields($auth_type_tag,'','', $auth_type_subfield=>$authtypecode);
>-  }
> 
>     if ( C4::Context->preference('StripWhitespaceChars') ) {
>-        my $p = Koha::RecordProcessor->new({ filters => qw(TrimFields) });
>-        $p->process( $record );
>+        my $p = Koha::RecordProcessor->new( { filters => qw(TrimFields) } );
>+        $p->process($record);
>     }
> 
>     # Save record into auth_header, update 001
>     my $action;
>     my $authority;
>-    if (!$authid ) {
>+    if ( !$authid ) {
>         $action = 'create';
>+
>         # Save a blank record, get authid
>-        $authority = Koha::Authority->new({ datecreated => \'NOW()', marcxml => '' })->store();
>+        $authority = Koha::Authority->new( { datecreated => \'NOW()', marcxml => '' } )->store();
>         $authority->discard_changes();
>         $authid = $authority->authid;
>         logaction( "AUTHORITIES", "ADD", $authid, "authority" ) if C4::Context->preference("AuthoritiesLog");
>     } else {
>-        $action = 'modify';
>+        $action    = 'modify';
>         $authority = Koha::Authorities->find($authid);
>     }
> 
>     # Insert/update the recordID in MARC record
>     $record->delete_field( $record->field('001') );
>     $record->insert_fields_ordered( MARC::Field->new( '001', $authid ) );
>+
>     # Update
>-    $authority->update({ authtypecode => $authtypecode, marc => $record->as_usmarc, marcxml => $record->as_xml_record($format) });
>+    $authority->update(
>+        { authtypecode => $authtypecode, marc => $record->as_usmarc, marcxml => $record->as_xml_record($format) } );
> 
>-    unless ( $skip_record_index ) {
>-        my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::AUTHORITIES_INDEX });
>+    unless ($skip_record_index) {
>+        my $indexer = Koha::SearchEngine::Indexer->new( { index => $Koha::SearchEngine::AUTHORITIES_INDEX } );
>         $indexer->index_records( $authid, "specialUpdate", "authorityserver", $record );
>     }
> 
>-    _after_authority_action_hooks({ action => $action, authority_id => $authid });
>-    return ( $authid );
>+    _after_authority_action_hooks( { action => $action, authority_id => $authid } );
>+    return ($authid);
> }
> 
> =head2 DelAuthority
>diff --git a/C4/Biblio.pm b/C4/Biblio.pm
>index 62772d5d545..ab895b25753 100644
>--- a/C4/Biblio.pm
>+++ b/C4/Biblio.pm
>@@ -2841,8 +2841,8 @@ sub ModBiblioMarc {
>     }
> 
>     if ( C4::Context->preference('StripWhitespaceChars') ) {
>-        my $p = Koha::RecordProcessor->new({ filters => qw(TrimFields) });
>-        $p->process( $record );
>+        my $p = Koha::RecordProcessor->new( { filters => qw(TrimFields) } );
>+        $p->process($record);
>     }
> 
>     my $metadata = {
>diff --git a/Koha/Filter/MARC/TrimFields.pm b/Koha/Filter/MARC/TrimFields.pm
>index f5a9d9fcef8..99a5f0dfa39 100644
>--- a/Koha/Filter/MARC/TrimFields.pm
>+++ b/Koha/Filter/MARC/TrimFields.pm
>@@ -51,7 +51,7 @@ sub filter {
>     my ( $self, $record ) = @_;
> 
>     return
>-      unless $record and ref($record) eq 'MARC::Record';
>+        unless $record and ref($record) eq 'MARC::Record';
> 
>     foreach my $field ( $record->fields ) {
>         unless ( $field->is_control_field ) {
>@@ -60,8 +60,8 @@ sub filter {
>                 my $value = $subfield->[1];
>                 $value =~ s/[\n\r]+/ /g;
>                 $value =~ s/^\s+|\s+$//g;
>-                $field->add_subfields( $key => $value ); # add subfield to the end of the subfield list
>-                $field->delete_subfield( pos => 0 ); # delete the subfield at the top of the subfield list
>+                $field->add_subfields( $key => $value );    # add subfield to the end of the subfield list
>+                $field->delete_subfield( pos => 0 );        # delete the subfield at the top of the subfield list
>             }
>         }
>     }
>diff --git a/t/RecordProcessor.t b/t/RecordProcessor.t
>index 89914b3f3e5..eef01d42cdd 100755
>--- a/t/RecordProcessor.t
>+++ b/t/RecordProcessor.t
>@@ -217,13 +217,13 @@ subtest "'TrimFields' filter tests" => sub {
>         [ '521', ' ', ' ', a => "This is a\t test!\t" ],
>     );
> 
>-    my $p = Koha::RecordProcessor->new({ filters => ['TrimFields'] });
>-    $p->process( $record );
>+    my $p = Koha::RecordProcessor->new( { filters => ['TrimFields'] } );
>+    $p->process($record);
> 
>-    my $get520a = $record->subfield('520','a');
>+    my $get520a = $record->subfield( '520', 'a' );
>     is( $get520a, "This is a test!", "Whitespace characters are appropriately stripped or replaced with spaces" );
> 
>-    my $get521a = $record->subfield('521','a');
>+    my $get521a = $record->subfield( '521', 'a' );
>     is( $get521a, "This is a\t test!", "Trailing tabs are stripped while inner tabs are kept" );
> };
> 
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 33749: 152135 | 152136 | 156442 | 156443 | 158338 | 158339 | 158340 | 158341