Bugzilla – Attachment 78319 Details for
Bug 21205
C4::Acquisition - Remove GetOrderFromItemnumber
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 21205: Replace C4::Items::GetOrderFromItemnumber calls
Bug-21205-Replace-C4ItemsGetOrderFromItemnumber-ca.patch (text/plain), 17.16 KB, created by
Marcel de Rooy
on 2018-08-31 05:46:27 UTC
(
hide
)
Description:
Bug 21205: Replace C4::Items::GetOrderFromItemnumber calls
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2018-08-31 05:46:27 UTC
Size:
17.16 KB
patch
obsolete
>From a96c80959ec5a73d383edc431fa0d15d6f9470ba Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Mon, 6 Aug 2018 18:48:29 -0300 >Subject: [PATCH] Bug 21205: Replace C4::Items::GetOrderFromItemnumber calls >Content-Type: text/plain; charset=utf-8 > >This is done to ease the move of C4::Items (bug 18252) to Koha::Items > > my @itemnumbers = GetItemnumbersFromOrder($order->{ordernumber}); >will become > my @itemnumbers = $order_object->items->get_column('itemnumbers'); > >Test plan: >- Create an order with several items >- Receive some items > >Signed-off-by: Josef Moravec <josef.moravec@gmail.com> > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >--- > C4/Acquisition.pm | 45 ++++++++---------------------- > acqui/basket.pl | 10 ++++--- > acqui/finishreceive.pl | 28 +++++++++++-------- > acqui/orderreceive.pl | 7 +++-- > acqui/parcel.pl | 13 +++++---- > opac/opac-detail.pl | 7 ++--- > t/db_dependent/Acquisition/CancelReceipt.t | 18 ++++++------ > t/db_dependent/Acquisition/TransferOrder.t | 12 ++++---- > 8 files changed, 64 insertions(+), 76 deletions(-) > >diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm >index 016d33e..df7aafb 100644 >--- a/C4/Acquisition.pm >+++ b/C4/Acquisition.pm >@@ -83,8 +83,6 @@ BEGIN { > &DelInvoice > &MergeInvoices > >- &GetItemnumbersFromOrder >- > &AddClaim > &GetBiblioCountByBasketno > >@@ -120,28 +118,6 @@ sub GetOrderFromItemnumber { > > } > >-# Returns the itemnumber(s) associated with the ordernumber given in parameter >-sub GetItemnumbersFromOrder { >- my ($ordernumber) = @_; >- my $dbh = C4::Context->dbh; >- my $query = "SELECT itemnumber FROM aqorders_items WHERE ordernumber=?"; >- my $sth = $dbh->prepare($query); >- $sth->execute($ordernumber); >- my @tab; >- >- while (my $order = $sth->fetchrow_hashref) { >- push @tab, $order->{'itemnumber'}; >- } >- >- return @tab; >- >-} >- >- >- >- >- >- > =head1 NAME > > C4::Acquisition - Koha functions for dealing with orders and acquisitions >@@ -1591,8 +1567,8 @@ sub CancelReceipt { > > my $parent_ordernumber = $order->{'parent_ordernumber'}; > >- my @itemnumbers = GetItemnumbersFromOrder( $ordernumber ); > my $order_obj = Koha::Acquisition::Orders->find( $ordernumber ); # FIXME rewrite all this subroutine using this object >+ my @itemnumbers = $order_obj->items->get_column('itemnumber'); > > if($parent_ordernumber == $ordernumber || not $parent_ordernumber) { > # The order line has no parent, just mark it as not received >@@ -1668,7 +1644,7 @@ sub CancelReceipt { > my @affects = split q{\|}, C4::Context->preference("AcqItemSetSubfieldsWhenReceiptIsCancelled"); > if ( @affects ) { > for my $in ( @itemnumbers ) { >- my $item = Koha::Items->find( $in ); >+ my $item = Koha::Items->find( $in ); # FIXME We do not need that, we already have Koha::Items from $order_obj->items > my $biblio = $item->biblio; > my ( $itemfield ) = GetMarcFromKohaField( 'items.itemnumber', $biblio->frameworkcode ); > my $item_marc = C4::Items::GetMarcItem( $biblio->biblionumber, $in ); >@@ -1690,7 +1666,7 @@ sub _cancel_items_receipt { > my ( $order, $parent_ordernumber ) = @_; > $parent_ordernumber ||= $order->ordernumber; > >- my @itemnumbers = GetItemnumbersFromOrder($order->ordernumber); # FIXME Must be $order->items >+ my $items = $order->items; > if ( $order->basket->effective_create_items eq 'receiving' ) { > # Remove items that were created at receipt > my $query = qq{ >@@ -1700,13 +1676,13 @@ sub _cancel_items_receipt { > }; > my $dbh = C4::Context->dbh; > my $sth = $dbh->prepare($query); >- foreach my $itemnumber (@itemnumbers) { >- $sth->execute($itemnumber, $itemnumber); >+ while ( my $item = $items->next ) { >+ $sth->execute($item->itemnumber, $item->itemnumber); > } > } else { > # Update items >- foreach my $itemnumber (@itemnumbers) { >- ModItemOrder($itemnumber, $parent_ordernumber); >+ while ( my $item = $items->next ) { >+ ModItemOrder($item->itemnumber, $parent_ordernumber); > } > } > } >@@ -1910,9 +1886,10 @@ sub DelOrder { > } > $sth->finish; > >- my @itemnumbers = GetItemnumbersFromOrder( $ordernumber ); >- foreach my $itemnumber (@itemnumbers){ >- my $delcheck = C4::Items::DelItemCheck( $bibnum, $itemnumber ); >+ my $order = Koha::Acquisition::Orders->find($ordernumber); >+ my $items = $order->items; >+ while ( my $item = $items->next ) { # Should be moved to Koha::Acquisition::Order->delete >+ my $delcheck = C4::Items::DelItemCheck( $bibnum, $item->itemnumber ); > > if($delcheck != 1) { > $error->{'delitem'} = 1; >diff --git a/acqui/basket.pl b/acqui/basket.pl >index 9f2ffdf..1b55e74 100755 >--- a/acqui/basket.pl >+++ b/acqui/basket.pl >@@ -34,6 +34,7 @@ use C4::Items; > use C4::Suggestions; > use Koha::Biblios; > use Koha::Acquisition::Booksellers; >+use Koha::Acquisition::Orders; > use Koha::Libraries; > use C4::Letters qw/SendAlerts/; > use Date::Calc qw/Add_Delta_Days/; >@@ -474,12 +475,13 @@ sub get_order_infos { > my $cnt_subscriptions = $biblio->subscriptions->count; > my $itemcount = $biblio->items->count; > my $holds_count = $biblio->holds->count; >- my @items = GetItemnumbersFromOrder( $ordernumber ); >- my $itemholds = $biblio->holds->search({ itemnumber => { -in => \@items } })->count; >+ my $order = Koha::Acquisition::Orders->find($ordernumber); # FIXME We should certainly do that at the beginning of this sub >+ my $items = $order->items; >+ my $itemholds = $biblio->holds->search({ itemnumber => { -in => [ $items->get_column('itemnumber') ] } })->count; > > # if the biblio is not in other orders and if there is no items elsewhere and no subscriptions and no holds we can then show the link "Delete order and Biblio" see bug 5680 >- $line{can_del_bib} = 1 if $countbiblio <= 1 && $itemcount == scalar @items && !($cnt_subscriptions) && !($holds_count); >- $line{items} = ($itemcount) - (scalar @items); >+ $line{can_del_bib} = 1 if $countbiblio <= 1 && $itemcount == $items->count && !($cnt_subscriptions) && !($holds_count); >+ $line{items} = $itemcount - $items->count; > $line{left_item} = 1 if $line{items} >= 1; > $line{left_biblio} = 1 if $countbiblio > 1; > $line{biblios} = $countbiblio - 1; >diff --git a/acqui/finishreceive.pl b/acqui/finishreceive.pl >index 8c1439c..3608dce 100755 >--- a/acqui/finishreceive.pl >+++ b/acqui/finishreceive.pl >@@ -150,17 +150,21 @@ if ($quantityrec > $origquantityrec ) { > } > } > >-ModItem( >- { >- booksellerid => $booksellerid, >- dateaccessioned => $datereceived, >- datelastseen => $datereceived, >- price => $unitprice, >- replacementprice => $order->{rrp}, >- replacementpricedate => $datereceived, >- }, >- $biblionumber, >- $_ >-) foreach GetItemnumbersFromOrder($new_ordernumber); >+my $new_order_object = Koha::Acquisition::Orders->find( $new_ordernumber ); # FIXME we should not need to refetch it >+my $items = $new_order_object->items; >+while ( my $item = $items->next ) { >+ ModItem( >+ { >+ booksellerid => $booksellerid, >+ dateaccessioned => $datereceived, >+ datelastseen => $datereceived, >+ price => $unitprice, >+ replacementprice => $order->{rrp}, >+ replacementpricedate => $datereceived, >+ }, >+ $biblionumber, >+ $item->itemnumber, >+ ); >+} > > print $input->redirect("/cgi-bin/koha/acqui/parcel.pl?invoiceid=$invoiceid&sticky_filters=1"); >diff --git a/acqui/orderreceive.pl b/acqui/orderreceive.pl >index 4dbb5f0..6f04257 100755 >--- a/acqui/orderreceive.pl >+++ b/acqui/orderreceive.pl >@@ -111,7 +111,8 @@ unless ( $results and @$results) { > > # prepare the form for receiving > my $order = $results->[0]; >-my $basket = Koha::Acquisition::Orders->find( $ordernumber )->basket; >+my $order_object = Koha::Acquisition::Orders->find( $ordernumber ); >+my $basket = $order_object->basket; > > # Check if ACQ framework exists > my $acq_fw = GetMarcStructure( 1, 'ACQ', { unsafe => 1 } ); >@@ -127,10 +128,10 @@ if ($AcqCreateItem eq 'receiving') { > ); > } elsif ($AcqCreateItem eq 'ordering') { > my $fw = ($acq_fw) ? 'ACQ' : ''; >- my @itemnumbers = GetItemnumbersFromOrder($order->{ordernumber}); >+ my @itemnumbers = $order_object->items->get_column('itemnumbers'); > my @items; > foreach (@itemnumbers) { >- my $item = GetItem($_); >+ my $item = GetItem($_); # FIXME We do not need this call, we already have the Koha::Items > my $descriptions; > $descriptions = Koha::AuthorisedValues->get_description_by_koha_field({frameworkcode => $fw, kohafield => 'items.notforloan', authorised_value => $item->{notforloan} }); > $item->{notforloan} = $descriptions->{lib} // ''; >diff --git a/acqui/parcel.pl b/acqui/parcel.pl >index ec7332d..da0b4cf 100755 >--- a/acqui/parcel.pl >+++ b/acqui/parcel.pl >@@ -67,6 +67,7 @@ use C4::Suggestions; > > use Koha::Acquisition::Baskets; > use Koha::Acquisition::Bookseller; >+use Koha::Acquisition::Orders; > use Koha::Biblios; > use Koha::DateUtils; > use Koha::Biblios; >@@ -125,6 +126,7 @@ my $subtotal_for_funds; > for my $order ( @orders ) { > $order->{'unitprice'} += 0; > >+ my $order_object = Koha::Acquisition::Orders->find($order->{ordernumber}); > if ( $bookseller->invoiceincgst ) { > $order->{ecost} = $order->{ecost_tax_included}; > $order->{unitprice} = $order->{unitprice_tax_included}; >@@ -138,7 +140,7 @@ for my $order ( @orders ) { > > my %line = %{ $order }; > $line{invoice} = $invoice->{invoicenumber}; >- my @itemnumbers = GetItemnumbersFromOrder( $order->{ordernumber} ); >+ my @itemnumbers = $order_object->items->get_column('itemnumbers'); > my $biblio = Koha::Biblios->find( $line{biblionumber} ); > $line{total_holds} = $biblio ? $biblio->holds->count : 0; > $line{item_holds} = $biblio ? $biblio->current_holds->search( >@@ -241,11 +243,12 @@ unless( defined $invoice->{closedate} ) { > my $biblio = Koha::Biblios->find( $biblionumber ); > my $countbiblio = CountBiblioInOrders($biblionumber); > my $ordernumber = $line{'ordernumber'}; >+ my $order_object = Koha::Acquisition::Orders->find($ordernumber); > my $cnt_subscriptions = $biblio ? $biblio->subscriptions->count: 0; > my $itemcount = $biblio ? $biblio->items->count : 0; > my $holds_count = $biblio ? $biblio->holds->count : 0; >- my @items = GetItemnumbersFromOrder( $ordernumber ); >- my $itemholds = $biblio ? $biblio->holds->search({ itemnumber => { -in => \@items } })->count : 0; >+ my @itemnumbers = $order_object->items->get_column('itemnumbers'); >+ my $itemholds = $biblio ? $biblio->holds->search({ itemnumber => { -in => \@itemnumbers } })->count : 0; > > my $suggestion = GetSuggestionInfoFromBiblionumber($line{biblionumber}); > $line{suggestionid} = $suggestion->{suggestionid}; >@@ -253,8 +256,8 @@ unless( defined $invoice->{closedate} ) { > $line{firstnamesuggestedby} = $suggestion->{firstnamesuggestedby}; > > # if the biblio is not in other orders and if there is no items elsewhere and no subscriptions and no holds we can then show the link "Delete order and Biblio" see bug 5680 >- $line{can_del_bib} = 1 if $countbiblio <= 1 && $itemcount == scalar @items && !($cnt_subscriptions) && !($holds_count); >- $line{items} = ($itemcount) - (scalar @items); >+ $line{can_del_bib} = 1 if $countbiblio <= 1 && $itemcount == scalar @itemnumbers && !($cnt_subscriptions) && !($holds_count); >+ $line{items} = ($itemcount) - (scalar @itemnumbers); > $line{left_item} = 1 if $line{items} >= 1; > $line{left_biblio} = 1 if $countbiblio > 1; > $line{biblios} = $countbiblio - 1; >diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl >index 2c61b36..868a8c5 100755 >--- a/opac/opac-detail.pl >+++ b/opac/opac-detail.pl >@@ -641,11 +641,10 @@ if ( C4::Context->preference('OPACAcquisitionDetails' ) ) { > }); > my $total_quantity = 0; > for my $order ( @$orders ) { >- my $basket = Koha::Acquisition::Orders->find( $order->{ordernumber} )->basket; >+ my $order = Koha::Acquisition::Orders->find( $order->{ordernumber} ); >+ my $basket = $order->basket; > if ( $basket->effective_create_items eq 'ordering' ) { >- for my $itemnumber ( C4::Acquisition::GetItemnumbersFromOrder( $order->{ordernumber} ) ) { >- push @itemnumbers_on_order, $itemnumber; >- } >+ @itemnumbers_on_order = $order->items->get_column('itemnumber'); > } > $total_quantity += $order->{quantity}; > } >diff --git a/t/db_dependent/Acquisition/CancelReceipt.t b/t/db_dependent/Acquisition/CancelReceipt.t >index 3b650b0..a900885 100644 >--- a/t/db_dependent/Acquisition/CancelReceipt.t >+++ b/t/db_dependent/Acquisition/CancelReceipt.t >@@ -89,7 +89,7 @@ $order->add_item( $itemnumber ); > > CancelReceipt($ordernumber); > >-is(scalar GetItemnumbersFromOrder($ordernumber), 0, "Create items on receiving: 0 item exist after cancelling a receipt"); >+is($order->items->count, 0, "Create items on receiving: 0 item exist after cancelling a receipt"); > > my $itemnumber1 = AddItem( { itype => $itemtype }, $biblionumber ); > my $itemnumber2 = AddItem( { itype => $itemtype }, $biblionumber ); >@@ -114,7 +114,7 @@ $order->add_item( $itemnumber1 ); > $order->add_item( $itemnumber2 ); > > is( >- scalar( GetItemnumbersFromOrder( $order->ordernumber ) ), >+ $order->items->count, > 2, > "Create items on ordering: 2 items should be linked to the order before receiving" > ); >@@ -128,23 +128,23 @@ my ( undef, $new_ordernumber ) = ModReceiveOrder( > } > ); > >-my $new_order = GetOrder( $new_ordernumber ); >+my $new_order = Koha::Acquisition::Orders->find( $new_ordernumber ); > >-is( $new_order->{ordernumber}, $new_ordernumber, >+is( $new_order->ordernumber, $new_ordernumber, > "ModReceiveOrder should return a correct ordernumber" ); > isnt( $new_ordernumber, $ordernumber, > "ModReceiveOrder should return a different ordernumber" ); >-is( $new_order->{parent_ordernumber}, $ordernumber, >+is( $new_order->parent_ordernumber, $ordernumber, > "The new order created by ModReceiveOrder should be linked to the parent order" > ); > > is( >- scalar( GetItemnumbersFromOrder( $order->ordernumber ) ), >+ $order->items->count, > 1, > "Create items on ordering: 1 item should still be linked to the original order after receiving" > ); > is( >- scalar( GetItemnumbersFromOrder($new_ordernumber) ), >+ $new_order->items->count, > 1, > "Create items on ordering: 1 item should be linked to new order after receiving" > ); >@@ -152,12 +152,12 @@ is( > CancelReceipt($new_ordernumber); > > is( >- scalar( GetItemnumbersFromOrder($new_ordernumber) ), >+ $new_order->items->count, > 0, > "Create items on ordering: no item should be linked to the cancelled order" > ); > is( >- scalar( GetItemnumbersFromOrder( $order->ordernumber ) ), >+ $order->items->count, > 2, > "Create items on ordering: items are not deleted after cancelling a receipt" > ); >diff --git a/t/db_dependent/Acquisition/TransferOrder.t b/t/db_dependent/Acquisition/TransferOrder.t >index c0d001b..7a5dcf7 100644 >--- a/t/db_dependent/Acquisition/TransferOrder.t >+++ b/t/db_dependent/Acquisition/TransferOrder.t >@@ -72,7 +72,8 @@ $order->add_item( $itemnumber ); > # Begin tests > is(scalar GetOrders($basketno1), 1, "1 order in basket1"); > ($order) = GetOrders($basketno1); >-is(scalar GetItemnumbersFromOrder($order->{ordernumber}), 1, "1 item in basket1's order"); >+$order = Koha::Acquisition::Orders->find($order->{ordernumber}); >+is($order->items->count, 1, "1 item in basket1's order"); > is(scalar GetOrders($basketno2), 0, "0 order in basket2"); > > # Transfering order to basket2 >@@ -81,12 +82,13 @@ is(scalar GetOrders($basketno1), 0, "0 order in basket1"); > is(scalar GetOrders($basketno2), 1, "1 order in basket2"); > > # Determine if the transfer marked things cancelled properly. >-is($order->{orderstatus},'new','Before the transfer, the order status should be new'); >-($order) = GetOrders($basketno1, { 'cancelled' => 1 }); >-is($order->{orderstatus},'cancelled','After the transfer, the order status should be set to cancelled'); >+is($order->orderstatus,'new','Before the transfer, the order status should be new'); >+$order = Koha::Acquisition::Orders->find($order->ordernumber); >+is($order->orderstatus,'cancelled','After the transfer, the order status should be set to cancelled'); > > ($order) = GetOrders($basketno2); >-is(scalar GetItemnumbersFromOrder($order->{ordernumber}), 1, "1 item in basket2's order"); >+$order = Koha::Acquisition::Orders->find($order->{ordernumber}); >+is($order->items->count, 1, "1 item in basket2's order"); > > # Bug 11552 > my $orders = SearchOrders({ ordernumber => $newordernumber }); >-- >2.1.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 21205
:
77698
|
78042
|
78043
| 78319 |
78320