Bugzilla – Attachment 154967 Details for
Bug 19316
Items should have sort1 and sort2 fields
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 19316: (follow-up) Tidy Items.pm
Bug-19316-follow-up-Tidy-Itemspm.patch (text/plain), 17.84 KB, created by
Matt Blenkinsop
on 2023-08-30 14:09:54 UTC
(
hide
)
Description:
Bug 19316: (follow-up) Tidy Items.pm
Filename:
MIME Type:
Creator:
Matt Blenkinsop
Created:
2023-08-30 14:09:54 UTC
Size:
17.84 KB
patch
obsolete
>From b0e49eea32d68532da0ea767e585f0e68bcb5a33 Mon Sep 17 00:00:00 2001 >From: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> >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 <http://www.gnu.org/licenses>. > > 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)
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=154967">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 19316
:
154963
|
154964
|
154965
|
154966
|
154967
|
166298
|
166299
|
166300
|
166301
|
166578
|
166593
|
169466
|
169467
|
169468
|
169469
|
169470
|
169503
|
169504
|
169505
|
169506
|
169507