Attachment 61224 Details for Bug 18296 – Bug 18295: C4::Items - get_itemnumbers_of

[patch] Bug 18295: C4::Items - get_itemnumbers_of

Bug-18295-C4Items---getitemnumbersof.patch (text/plain), 9.51 KB, created by Jonathan Druart on 2017-03-17 17:25:42 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2017-03-17 17:25:42 UTC

Size: 9.51 KB

patch

obsolete

>From 1f8ef29f305a5610346e752971e7ef984cb299b2 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Wed, 15 Mar 2017 19:02:21 -0300
>Subject: [PATCH] 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.
>
>https://bugs.koha-community.org/show_bug.cgi?id=18296
>---
> 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 fce23ca..4eaa6af 100644
>--- a/C4/Items.pm
>+++ b/C4/Items.pm
>@@ -76,7 +76,6 @@ BEGIN {
> 	GetItemsLocationInfo
> 	GetHostItemsInfo
>         GetItemnumbersForBiblio
>-        get_itemnumbers_of
> 	get_hostitemnumbers_of
>         GetItemnumberFromBarcode
>         GetBarcodeFromItemnumber
>@@ -1352,41 +1351,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);
>@@ -1422,18 +1386,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 07924c8..1e81647 100755
>--- a/labels/label-item-search.pl
>+++ b/labels/label-item-search.pl
>@@ -30,12 +30,13 @@ 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::Koha qw(GetItemTypes);
> use C4::Creators::Lib qw(html_table);
> use C4::Debug;
>-use Koha::DateUtils;
> 
>+use Koha::DateUtils;
>+use Koha::Items;
> use Koha::SearchEngine::Search;
> 
> BEGIN {
>@@ -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 a022c99..d622054 100755
>--- a/reserve/request.pl
>+++ b/reserve/request.pl
>@@ -291,12 +291,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.9.3

Actions: View | Diff | Splinter Review

Attachments on bug 18296: 61224 | 61226 | 63888 | 63944 | 64050 | 64073 | 64106