From b0e49eea32d68532da0ea767e585f0e68bcb5a33 Mon Sep 17 00:00:00 2001 From: Matt Blenkinsop Date: Wed, 30 Aug 2023 14:03:14 +0000 Subject: [PATCH] Bug 19316: (follow-up) Tidy Items.pm Items.pm kept failing the QA script despite all new lines having been tidied with perltidy. There must be a larger tidying issue in the file that is affecting all the new lines so I've tidied the whole file and added a separate commit. --- Koha/Items.pm | 286 +++++++++++++++++++++++++------------------------- 1 file changed, 144 insertions(+), 142 deletions(-) diff --git a/Koha/Items.pm b/Koha/Items.pm index f058ba03ad..56887fa1ab 100644 --- a/Koha/Items.pm +++ b/Koha/Items.pm @@ -18,7 +18,7 @@ package Koha::Items; # along with Koha; if not, see . use Modern::Perl; -use Array::Utils qw( array_minus ); +use Array::Utils qw( array_minus ); use List::MoreUtils qw( uniq ); use Try::Tiny; @@ -69,21 +69,21 @@ sub filter_by_for_hold { rule_value => 'not_allowed', } )->get_column('itemtype'); - push @hold_not_allowed_itypes, Koha::ItemTypes->search({ notforloan => 1 })->get_column('itemtype'); + push @hold_not_allowed_itypes, Koha::ItemTypes->search( { notforloan => 1 } )->get_column('itemtype'); my $params = { itemlost => 0, withdrawn => 0, notforloan => { '<=' => 0 }, # items with negative or zero notforloan value are holdable - ( C4::Context->preference('AllowHoldsOnDamagedItems')? (): ( damaged => 0 ) ), - ( C4::Context->only_my_library() ? ( homebranch => C4::Context::mybranch() ) : () ), + ( C4::Context->preference('AllowHoldsOnDamagedItems') ? () : ( damaged => 0 ) ), + ( C4::Context->only_my_library() ? ( homebranch => C4::Context::mybranch() ) : () ), }; if ( C4::Context->preference("item-level_itypes") ) { return $self->search( { %$params, - itype => { -not_in => \@hold_not_allowed_itypes }, + itype => { -not_in => \@hold_not_allowed_itypes }, } ); } else { @@ -116,7 +116,7 @@ are honoured. =cut sub filter_by_visible_in_opac { - my ($self, $params) = @_; + my ( $self, $params ) = @_; my $patron = $params->{patron}; @@ -128,14 +128,14 @@ sub filter_by_visible_in_opac { my $rules_params; foreach my $field ( keys %$rules ) { - $rules_params->{'me.'.$field} = - [ { '-not_in' => $rules->{$field} }, undef ]; + $rules_params->{ 'me.' . $field } = + [ { '-not_in' => $rules->{$field} }, undef ]; } - $result = $result->search( $rules_params ); + $result = $result->search($rules_params); } - if (C4::Context->preference('hidelostitems')) { + if ( C4::Context->preference('hidelostitems') ) { $result = $result->filter_out_lost; } @@ -155,10 +155,9 @@ sub filter_out_lost { my $params = { itemlost => 0 }; - return $self->search( $params ); + return $self->search($params); } - =head3 move_to_biblio $items->move_to_biblio($to_biblio); @@ -175,7 +174,7 @@ sub move_to_biblio { $biblionumbers->{ $item->biblionumber } = 1; $item->move_to_biblio( $to_biblio, { skip_record_index => 1 } ); } - my $indexer = Koha::SearchEngine::Indexer->new({ index => $Koha::SearchEngine::BIBLIOS_INDEX }); + my $indexer = Koha::SearchEngine::Indexer->new( { index => $Koha::SearchEngine::BIBLIOS_INDEX } ); for my $biblionumber ( keys %{$biblionumbers} ) { $indexer->index_records( $biblionumber, "specialUpdate", "biblioserver" ); } @@ -251,148 +250,153 @@ sub batch_update { my $sort2 = $params->{sort2}; my $callback = $params->{callback}; - my (@modified_itemnumbers, $modified_fields); + my ( @modified_itemnumbers, $modified_fields ); my $i; my $schema = Koha::Database->new->schema; while ( my $item = $self->next ) { - try {$schema->txn_do(sub { - my $modified_holds_priority = 0; - my $item_returned = 0; - my $sort_fields_modified = 0; - if ( defined $exclude_from_local_holds_priority ) { - if(!defined $item->exclude_from_local_holds_priority || $item->exclude_from_local_holds_priority != $exclude_from_local_holds_priority) { - $item->exclude_from_local_holds_priority($exclude_from_local_holds_priority)->store; - $modified_holds_priority = 1; - } - } - if ( defined $sort1 || defined $sort2 ) { - $item->sort1($sort1)->store if $sort1 ne ""; - $sort_fields_modified++ if $sort1 ne ""; - $item->sort2($sort2)->store if $sort2 ne ""; - $sort_fields_modified++ if $sort2 ne ""; - } + try { + $schema->txn_do( + sub { + my $modified_holds_priority = 0; + my $item_returned = 0; + my $sort_fields_modified = 0; + if ( defined $exclude_from_local_holds_priority ) { + if ( !defined $item->exclude_from_local_holds_priority + || $item->exclude_from_local_holds_priority != $exclude_from_local_holds_priority ) + { + $item->exclude_from_local_holds_priority($exclude_from_local_holds_priority)->store; + $modified_holds_priority = 1; + } + } + if ( defined $sort1 || defined $sort2 ) { + $item->sort1($sort1)->store if $sort1 ne ""; + $sort_fields_modified++ if $sort1 ne ""; + $item->sort2($sort2)->store if $sort2 ne ""; + $sort_fields_modified++ if $sort2 ne ""; + } - my $modified = 0; - my $new_values = {%$new_values}; # Don't modify the original + my $modified = 0; + my $new_values = {%$new_values}; # Don't modify the original - my $old_values = $item->unblessed; - if ( $item->more_subfields_xml ) { - $old_values = { - %$old_values, - %{$item->additional_attributes->to_hashref}, - }; - } + my $old_values = $item->unblessed; + if ( $item->more_subfields_xml ) { + $old_values = { + %$old_values, + %{ $item->additional_attributes->to_hashref }, + }; + } + + for my $attr ( keys %$regex_mod ) { + my $old_value = $old_values->{$attr}; - for my $attr ( keys %$regex_mod ) { - my $old_value = $old_values->{$attr}; + next unless $old_value; - next unless $old_value; + my $value = apply_regex( + { + %{ $regex_mod->{$attr} }, + value => $old_value, + } + ); - my $value = apply_regex( - { - %{ $regex_mod->{$attr} }, - value => $old_value, + $new_values->{$attr} = $value; } - ); - $new_values->{$attr} = $value; - } + for my $attribute ( keys %$new_values ) { + next if $attribute eq 'more_subfields_xml'; # Already counted before - for my $attribute ( keys %$new_values ) { - next if $attribute eq 'more_subfields_xml'; # Already counted before + my $old = $old_values->{$attribute}; + my $new = $new_values->{$attribute}; + $modified++ + if ( defined $old xor defined $new ) + || ( defined $old && defined $new && $new ne $old ); + } - my $old = $old_values->{$attribute}; - my $new = $new_values->{$attribute}; - $modified++ - if ( defined $old xor defined $new ) - || ( defined $old && defined $new && $new ne $old ); - } + { # Dealing with more_subfields_xml - { # Dealing with more_subfields_xml - - my $frameworkcode = $item->biblio->frameworkcode; - my $tagslib = C4::Biblio::GetMarcStructure( 1, $frameworkcode, { unsafe => 1 }); - my ( $itemtag, $itemsubfield ) = C4::Biblio::GetMarcFromKohaField( "items.itemnumber" ); - - my @more_subfield_tags = map { - ( - ref($_) - && %$_ - && !$_->{kohafield} # Get subfields that are not mapped - ) - ? $_->{tagsubfield} - : () - } values %{ $tagslib->{$itemtag} }; - - my $more_subfields_xml = Koha::Item::Attributes->new( - { - map { - exists $new_values->{$_} ? ( $_ => $new_values->{$_} ) - : exists $old_values->{$_} - ? ( $_ => $old_values->{$_} ) - : () - } @more_subfield_tags - } - )->to_marcxml($frameworkcode); + my $frameworkcode = $item->biblio->frameworkcode; + my $tagslib = C4::Biblio::GetMarcStructure( 1, $frameworkcode, { unsafe => 1 } ); + my ( $itemtag, $itemsubfield ) = C4::Biblio::GetMarcFromKohaField("items.itemnumber"); - $new_values->{more_subfields_xml} = $more_subfields_xml; + my @more_subfield_tags = map { + ( + ref($_) + && %$_ + && !$_->{kohafield} # Get subfields that are not mapped + ) + ? $_->{tagsubfield} + : () + } values %{ $tagslib->{$itemtag} }; - delete $new_values->{$_} for @more_subfield_tags; # Clean the hash + my $more_subfields_xml = Koha::Item::Attributes->new( + { + map { + exists $new_values->{$_} ? ( $_ => $new_values->{$_} ) + : exists $old_values->{$_} ? ( $_ => $old_values->{$_} ) + : () + } @more_subfield_tags + } + )->to_marcxml($frameworkcode); - } + $new_values->{more_subfields_xml} = $more_subfields_xml; - if ( $modified ) { - my $itemlost_pre = $item->itemlost; - $item->set($new_values)->store({skip_record_index => 1}); + delete $new_values->{$_} for @more_subfield_tags; # Clean the hash - C4::Circulation::LostItem( - $item->itemnumber, 'batchmod', undef, - { skip_record_index => 1 } - ) if $item->itemlost - and not $itemlost_pre; - } - if ( $mark_items_returned ){ - my $issue = $item->checkout; - if( $issue ){ - $item_returned = 1; - C4::Circulation::MarkIssueReturned( - $issue->borrowernumber, - $item->itemnumber, - undef, - $issue->patron->privacy, - { - skip_record_index => 1, - skip_holds_queue => 1, + } + + if ($modified) { + my $itemlost_pre = $item->itemlost; + $item->set($new_values)->store( { skip_record_index => 1 } ); + + C4::Circulation::LostItem( + $item->itemnumber, 'batchmod', undef, + { skip_record_index => 1 } + ) + if $item->itemlost + and not $itemlost_pre; + } + if ($mark_items_returned) { + my $issue = $item->checkout; + if ($issue) { + $item_returned = 1; + C4::Circulation::MarkIssueReturned( + $issue->borrowernumber, + $item->itemnumber, + undef, + $issue->patron->privacy, + { + skip_record_index => 1, + skip_holds_queue => 1, + } + ); } - ); - } - } + } - push @modified_itemnumbers, $item->itemnumber - if $modified || $modified_holds_priority || $item_returned || $sort_fields_modified; - $modified_fields += $modified + $modified_holds_priority + $item_returned + $sort_fields_modified; - })} - catch { + push @modified_itemnumbers, $item->itemnumber + if $modified || $modified_holds_priority || $item_returned || $sort_fields_modified; + $modified_fields += $modified + $modified_holds_priority + $item_returned + $sort_fields_modified; + } + ) + } catch { warn $_ }; - if ( $callback ) { - $callback->(++$i); + if ($callback) { + $callback->( ++$i ); } } if (@modified_itemnumbers) { - my @biblionumbers = uniq( - Koha::Items->search( { itemnumber => \@modified_itemnumbers } ) - ->get_column('biblionumber')); + my @biblionumbers = + uniq( Koha::Items->search( { itemnumber => \@modified_itemnumbers } )->get_column('biblionumber') ); - if ( @biblionumbers ) { - my $indexer = Koha::SearchEngine::Indexer->new( - { index => $Koha::SearchEngine::BIBLIOS_INDEX } ); + if (@biblionumbers) { + my $indexer = Koha::SearchEngine::Indexer->new( { index => $Koha::SearchEngine::BIBLIOS_INDEX } ); - $indexer->index_records( \@biblionumbers, 'specialUpdate', - "biblioserver", undef ); + $indexer->index_records( + \@biblionumbers, 'specialUpdate', + "biblioserver", undef + ); } } @@ -400,32 +404,30 @@ sub batch_update { } sub apply_regex { + # FIXME Should be moved outside of Koha::Items # FIXME This is nearly identical to Koha::SimpleMARC::_modify_values - my ($params) = @_; - my $search = $params->{search}; - my $replace = $params->{replace}; + my ($params) = @_; + my $search = $params->{search}; + my $replace = $params->{replace}; my $modifiers = $params->{modifiers} || q{}; - my $value = $params->{value}; + my $value = $params->{value}; - $replace =~ s/"/\\"/g; # Protection from embedded code - $replace = '"' . $replace . '"'; # Put in a string for /ee + $replace =~ s/"/\\"/g; # Protection from embedded code + $replace = '"' . $replace . '"'; # Put in a string for /ee my @available_modifiers = qw( i g ); my $retained_modifiers = q||; for my $modifier ( split //, $modifiers ) { $retained_modifiers .= $modifier - if grep { /$modifier/ } @available_modifiers; + if grep { /$modifier/ } @available_modifiers; } if ( $retained_modifiers =~ m/^(ig|gi)$/ ) { $value =~ s/$search/$replace/igee; - } - elsif ( $retained_modifiers eq 'i' ) { + } elsif ( $retained_modifiers eq 'i' ) { $value =~ s/$search/$replace/iee; - } - elsif ( $retained_modifiers eq 'g' ) { + } elsif ( $retained_modifiers eq 'g' ) { $value =~ s/$search/$replace/gee; - } - else { + } else { $value =~ s/$search/$replace/ee; } @@ -441,13 +443,13 @@ Search and sort items in a specific order, depending if serials are present or n =cut sub search_ordered { - my ($self, $params, $attributes) = @_; + my ( $self, $params, $attributes ) = @_; - $self = $self->search($params, $attributes); + $self = $self->search( $params, $attributes ); my @biblionumbers = uniq $self->get_column('biblionumber'); - if ( scalar ( @biblionumbers ) == 1 + if ( scalar(@biblionumbers) == 1 && Koha::Biblios->find( $biblionumbers[0] )->serial ) { return $self->search( @@ -465,7 +467,7 @@ sub search_ordered { 'homebranch.branchname', 'me.enumchron', \"LPAD( me.copynumber, 8, '0' )", - {-desc => 'me.dateaccessioned'} + { -desc => 'me.dateaccessioned' } ], join => ['homebranch'] } -- 2.37.1 (Apple Git-137.1)