From 22d4c83e48f941e4953a38d14ab1202b71669b56 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 15 Mar 2017 19:02:21 -0300 Subject: [PATCH] [SIGNED-OFF] Bug 18295: C4::Items - get_itemnumbers_of The code from scripts and subroutines using this subroutine was not very elegant. Most of the time the code was unnecessarily complex. This patch removes this subroutine and adapt the code to use Koha::Items instead. 1. C4::Items::get_hostitemnumbers_of I did not understand why the code was so complicated, it seems that we only want to know if a given item has a given biblionumber 2. cataloguing/merge.pl We want to retrieve the itemnumber for a given biblio. We could also have done that with: Koha::Biblios->find( $biblionumber )->items; 3. labels/label-item-search.pl We want to loop over the items for a given biblio, no need to use get_itemnumbers_of and GetItemInfosOf. We just need to use: Koha::Items->search({ biblionumber => $biblionumber }) 4. reserve/request.pl We want to retrieve the itemnumbers of the biblio's items We could also have done that with: Koha::Biblios->find( $biblionumber )->items->get_column('itemnumber'); Test plan: 1.You need to create analytical record relationships ( EasyAnalyticalRecords needs to be set). Link an item to a biblio using the 'Edit > Link to host item' menu from the biblio detail page. From the staff interface place a hold on the biblio. You should see the items from the biblio and the one you just linked 2. Merge two bibliographic records (with items), the resulting record should contain items from both original records 3. Create a new label batch, edit it. Add items to this batch ('Add items' button). Fill the input with a barcode. You should see all the items of a biblio. Signed-off-by: Josef Moravec https://bugs.koha-community.org/show_bug.cgi?id=18285 --- C4/Items.pm | 51 +++------------------------------------------ cataloguing/merge.pl | 13 ++++++------ labels/label-item-search.pl | 34 +++++++++++++----------------- reserve/request.pl | 6 ++---- 4 files changed, 26 insertions(+), 78 deletions(-) diff --git a/C4/Items.pm b/C4/Items.pm index 6378249..4cf3424 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -77,7 +77,6 @@ BEGIN { GetItemsLocationInfo GetHostItemsInfo GetItemnumbersForBiblio - get_itemnumbers_of get_hostitemnumbers_of GetItemnumberFromBarcode GetBarcodeFromItemnumber @@ -1478,41 +1477,6 @@ sub GetItemnumbersForBiblio { return \@items; } -=head2 get_itemnumbers_of - - my @itemnumbers_of = get_itemnumbers_of(@biblionumbers); - -Given a list of biblionumbers, return the list of corresponding itemnumbers -for each biblionumber. - -Return a reference on a hash where keys are biblionumbers and values are -references on array of itemnumbers. - -=cut - -sub get_itemnumbers_of { - my @biblionumbers = @_; - - my $dbh = C4::Context->dbh; - - my $query = ' - SELECT itemnumber, - biblionumber - FROM items - WHERE biblionumber IN (?' . ( ',?' x scalar @biblionumbers - 1 ) . ') - '; - my $sth = $dbh->prepare($query); - $sth->execute(@biblionumbers); - - my %itemnumbers_of; - - while ( my ( $itemnumber, $biblionumber ) = $sth->fetchrow_array ) { - push @{ $itemnumbers_of{$biblionumber} }, $itemnumber; - } - - return \%itemnumbers_of; -} - =head2 get_hostitemnumbers_of my @itemnumbers_of = get_hostitemnumbers_of($biblionumber); @@ -1548,18 +1512,9 @@ sub get_hostitemnumbers_of { foreach my $hostfield ( $marcrecord->field($tag) ) { my $hostbiblionumber = $hostfield->subfield($biblio_s); my $linkeditemnumber = $hostfield->subfield($item_s); - my @itemnumbers; - if ( my $itemnumbers = - get_itemnumbers_of($hostbiblionumber)->{$hostbiblionumber} ) - { - @itemnumbers = @$itemnumbers; - } - foreach my $itemnumber (@itemnumbers) { - if ( $itemnumber eq $linkeditemnumber ) { - push( @returnhostitemnumbers, $itemnumber ); - last; - } - } + my $is_from_biblio = Koha::Items->search({ itemnumber => $linkeditemnumber, biblionumber => $hostbiblionumber }); + push @returnhostitemnumbers, $linkeditemnumber + if $is_from_biblio; } return @returnhostitemnumbers; diff --git a/cataloguing/merge.pl b/cataloguing/merge.pl index ee9f1fa..11bb4e2 100755 --- a/cataloguing/merge.pl +++ b/cataloguing/merge.pl @@ -31,6 +31,7 @@ use C4::Reserves qw/MergeHolds/; use C4::Acquisition qw/ModOrder GetOrdersByBiblionumber/; use Koha::BiblioFrameworks; +use Koha::Items; use Koha::MetadataRecord; my $input = new CGI; @@ -87,14 +88,14 @@ if ($merge) { # Moving items from the other record to the reference record foreach my $biblionumber (@biblionumbers) { - my $itemnumbers = get_itemnumbers_of($biblionumber); - foreach my $itemnumber (@{ $itemnumbers->{$biblionumber} }) { - my $res = MoveItemFromBiblio($itemnumber, $biblionumber, $ref_biblionumber); - if (not defined $res) { - push @notmoveditems, $itemnumber; + my $items = Koha::Items->search({ biblionumber => $biblionumber }); + while ( my $item = $items->next) { + my $res = MoveItemFromBiblio( $item->itemnumber, $biblionumber, $ref_biblionumber ); + if ( not defined $res ) { + push @notmoveditems, $item->itemnumber; + } } } - } # If some items could not be moved : if (scalar(@notmoveditems) > 0) { my $itemlist = join(' ',@notmoveditems); diff --git a/labels/label-item-search.pl b/labels/label-item-search.pl index 3ef8dbf..d318987 100755 --- a/labels/label-item-search.pl +++ b/labels/label-item-search.pl @@ -30,11 +30,12 @@ use C4::Output qw(output_html_with_http_headers); use C4::Context; use C4::Search qw(SimpleSearch); use C4::Biblio qw(TransformMarcToKoha); -use C4::Items qw(GetItemInfosOf get_itemnumbers_of); +use C4::Items qw(GetItemInfosOf); use C4::Creators::Lib qw(html_table); use C4::Debug; -use Koha::DateUtils; +use Koha::DateUtils; +use Koha::Items; use Koha::ItemTypes; use Koha::SearchEngine::Search; @@ -139,28 +140,21 @@ if ($show_results) { push (@results_set, $biblio); my $biblionumber = $biblio->{'biblionumber'}; #DEBUG Notes: Grab the item numbers associated with this MARC record... - my $itemnums = get_itemnumbers_of($biblionumber); + my $items = Koha::Items->search({ biblionumber => $biblionumber }, { order_by => { -desc => 'itemnumber' }}); #DEBUG Notes: Retrieve the item data for each number... - if (my $iii = $itemnums->{$biblionumber}) { - my $item_results = GetItemInfosOf(@$iii); - foreach my $item ( keys %$item_results ) { - #DEBUG Notes: Build an array element 'item' of the correct bib (results) hash which contains item-specific data... - if ($item_results->{$item}->{'biblionumber'} eq $results_set[$i]->{'biblionumber'}) { - my $item_data; - $item_data->{'_item_number'} = $item_results->{$item}->{'itemnumber'}; - $item_data->{'_item_call_number'} = ($item_results->{$item}->{'itemcallnumber'} ? $item_results->{$item}->{'itemcallnumber'} : 'NA'); - $item_data->{'_date_accessioned'} = $item_results->{$item}->{'dateaccessioned'}; - $item_data->{'_barcode'} = ( $item_results->{$item}->{'barcode'} ? $item_results->{$item}->{'barcode'} : 'NA'); - $item_data->{'_add'} = $item_results->{$item}->{'itemnumber'}; - unshift (@row_data, $item_data); # item numbers are given to us in descending order by get_itemnumbers_of()... - } + while ( my $item = $items->next ) { + #DEBUG Notes: Build an array element 'item' of the correct bib (results) hash which contains item-specific data... + if ( $item->biblionumber eq $results_set[$i]->{'biblionumber'} ) { + my $item_data; + $item_data->{'_item_number'} = $item->itemnumber; + $item_data->{'_item_call_number'} = ( $item->itemcallnumber || 'NA' ); + $item_data->{'_date_accessioned'} = $item->dateaccessioned; + $item_data->{'_barcode'} = ( $item->barcode || 'NA' ); + $item_data->{'_add'} = $item->itemnumber; + push @row_data, $item_data; } $results_set[$i]->{'item_table'} = html_table($display_columns, \@row_data); } - else { - # FIXME: Some error trapping code needed - warn sprintf('No item numbers retrieved for biblio number: %s', $biblionumber); - } } ( $template, $loggedinuser, $cookie ) = get_template_and_user( diff --git a/reserve/request.pl b/reserve/request.pl index ba4eb0a..b33ae70 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -293,12 +293,10 @@ foreach my $biblionumber (@biblionumbers) { my $fixedRank = $count+1; my %itemnumbers_of_biblioitem; - my @itemnumbers; ## $items is array of 'item' table numbers - if (my $items = get_itemnumbers_of($biblionumber)->{$biblionumber}){ - @itemnumbers = @$items; - } + my $items = Koha::Items->search({ biblionumber => $biblionumber }); + my @itemnumbers = $items->get_column('itemnumber'); my @hostitems = get_hostitemnumbers_of($biblionumber); if (@hostitems){ $template->param('hostitemsflag' => 1); -- 2.1.4