From ef9d9a006a59c2a44cc579309904f3019f2e004a Mon Sep 17 00:00:00 2001 From: Kyle M Hall 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 Signed-off-by: Tomas Cohen Arazi --- 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 5dc5c3ad46..4c593e4eb6 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 62772d5d54..ab895b2575 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 f5a9d9fcef..99a5f0dfa3 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 89914b3f3e..eef01d42cd 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.39.5