From 067ff5201e49cd8659f5bc9a6fe8fc74eb75083d Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 26 Apr 2013 09:27:11 +0200 Subject: [PATCH] Bug 10124: Merge GetPendingOrders and SearchOrders routines Preliminary work: In the C4::Acquisition module, 2 routines do the same work. This patch merges these 2 routines. Test plan: test the acqui/orderreceive.pl, acqui/uncertainprice.pl and serials/acqui-search-result.pl scripts. acqui/parcel.pl will be tested with the others patches. --- C4/Acquisition.pm | 234 ++++++++------------ acqui/orderreceive.pl | 4 +- acqui/parcel.pl | 27 ++- acqui/uncertainprice.pl | 9 +- serials/acqui-search-result.pl | 7 +- t/db_dependent/Acquisition.t | 6 +- t/db_dependent/lib/KohaTest/Acquisition.pm | 2 +- .../lib/KohaTest/Acquisition/GetPendingOrders.pm | 14 +- 8 files changed, 143 insertions(+), 160 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index b78a51e..4a37a68 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -53,9 +53,9 @@ BEGIN { &ModBasketgroup &NewBasketgroup &DelBasketgroup &GetBasketgroup &CloseBasketgroup &GetBasketgroups &ReOpenBasketgroup - &NewOrder &DelOrder &ModOrder &GetPendingOrders &GetOrder &GetOrders + &NewOrder &DelOrder &ModOrder &GetOrder &GetOrders &GetOrderNumber &GetLateOrders &GetOrderFromItemnumber - &SearchOrder &GetHistory &GetRecentAcqui + &SearchOrders &GetHistory &GetRecentAcqui &ModReceiveOrder &CancelReceipt &ModOrderBiblioitemNumber &GetCancelledOrders &GetLastOrderNotReceivedFromSubscriptionid &GetLastOrderReceivedFromSubscriptionid @@ -819,96 +819,6 @@ sub GetBasketgroups { =head2 FUNCTIONS ABOUT ORDERS -=cut - -#------------------------------------------------------------# - -=head3 GetPendingOrders - -$orders = &GetPendingOrders($supplierid,$grouped,$owner,$basketno,$ordernumber,$search,$ean); - -Finds pending orders from the bookseller with the given ID. Ignores -completed and cancelled orders. - -C<$booksellerid> contains the bookseller identifier -C<$owner> contains 0 or 1. 0 means any owner. 1 means only the list of orders entered by the user itself. -C<$grouped> is a boolean that, if set to 1 will group all order lines of the same basket -in a single result line -C<$orders> is a reference-to-array; each element is a reference-to-hash. - -Used also by the filter in parcel.pl -I have added: - -C<$ordernumber> -C<$search> -C<$ean> - -These give the value of the corresponding field in the aqorders table -of the Koha database. - -Results are ordered from most to least recent. - -=cut - -sub GetPendingOrders { - my ($supplierid,$grouped,$owner,$basketno,$ordernumber,$search,$ean) = @_; - my $dbh = C4::Context->dbh; - my $strsth = " - SELECT ".($grouped?"count(*),":"")."aqbasket.basketno, - surname,firstname,biblio.*,biblioitems.isbn, - aqbasket.closedate, aqbasket.creationdate, aqbasket.basketname, - aqorders.* - FROM aqorders - LEFT JOIN aqbasket ON aqbasket.basketno=aqorders.basketno - LEFT JOIN borrowers ON aqbasket.authorisedby=borrowers.borrowernumber - LEFT JOIN biblio ON biblio.biblionumber=aqorders.biblionumber - LEFT JOIN biblioitems ON biblioitems.biblionumber=biblio.biblionumber - WHERE (quantity > quantityreceived OR quantityreceived is NULL) - AND datecancellationprinted IS NULL"; - my @query_params; - my $userenv = C4::Context->userenv; - if ( C4::Context->preference("IndependantBranches") ) { - if ( ($userenv) && ( $userenv->{flags} != 1 ) ) { - $strsth .= " AND (borrowers.branchcode = ? - or borrowers.branchcode = '')"; - push @query_params, $userenv->{branch}; - } - } - if ($supplierid) { - $strsth .= " AND aqbasket.booksellerid = ?"; - push @query_params, $supplierid; - } - if($ordernumber){ - $strsth .= " AND (aqorders.ordernumber=?)"; - push @query_params, $ordernumber; - } - if($search){ - $strsth .= " AND (biblio.title like ? OR biblio.author LIKE ? OR biblioitems.isbn like ?)"; - push @query_params, ("%$search%","%$search%","%$search%"); - } - if ($ean) { - $strsth .= " AND biblioitems.ean = ?"; - push @query_params, $ean; - } - if ($basketno) { - $strsth .= " AND aqbasket.basketno=? "; - push @query_params, $basketno; - } - if ($owner) { - $strsth .= " AND aqbasket.authorisedby=? "; - push @query_params, $userenv->{'number'}; - } - $strsth .= " group by aqbasket.basketno" if $grouped; - $strsth .= " order by aqbasket.basketno"; - my $sth = $dbh->prepare($strsth); - $sth->execute( @query_params ); - my $results = $sth->fetchall_arrayref({}); - $sth->finish; - return $results; -} - -#------------------------------------------------------------# - =head3 GetOrders @orders = &GetOrders($basketnumber, $orderby); @@ -1531,80 +1441,120 @@ sub CancelReceipt { #------------------------------------------------------------# -=head3 SearchOrder +=head3 SearchOrders -@results = &SearchOrder($search, $biblionumber, $complete); +@results = &SearchOrders({ + ordernumber => $ordernumber, + search => $search, + biblionumber => $biblionumber, + ean => $ean, + booksellerid => $booksellerid, + basketno => $basketno, + owner => $owner, + pending => $pending +}); Searches for orders. -C<$search> may take one of several forms: if it is an ISBN, -C<&ordersearch> returns orders with that ISBN. If C<$search> is an -order number, C<&ordersearch> returns orders with that order number -and biblionumber C<$biblionumber>. Otherwise, C<$search> is considered -to be a space-separated list of search terms; in this case, all of the -terms must appear in the title (matching the beginning of title -words). - -If C<$complete> is C, the results will include only completed -orders. In any case, C<&ordersearch> ignores cancelled orders. - -C<&ordersearch> returns an array. -C<@results> is an array of references-to-hash with the following keys: - -=over 4 - -=item C +C<$owner> Finds order for the logged in user. +C<$pending> Finds pending orders. Ignores completed and cancelled orders. -=item C -=item C - -=item C - -=back +C<@results> is an array of references-to-hash with the keys are fields +from aqorders, biblio, biblioitems and aqbasket tables. =cut -sub SearchOrder { -#### -------- SearchOrder------------------------------- - my ( $ordernumber, $search, $ean, $supplierid, $basket ) = @_; +sub SearchOrders { + my ( $params ) = @_; + my $ordernumber = $params->{ordernumber}; + my $search = $params->{search}; + my $ean = $params->{ean}; + my $booksellerid = $params->{booksellerid}; + my $basketno = $params->{basketno}; + my $basketname = $params->{basketname}; + my $basketgroupname = $params->{basketgroupname}; + my $owner = $params->{owner}; + my $pending = $params->{pending}; my $dbh = C4::Context->dbh; my @args = (); - my $query = - "SELECT * - FROM aqorders + my $query = q{ + SELECT aqbasket.basketno, + borrowers.surname, + borrowers.firstname, + biblio.*, + biblioitems.isbn, + biblioitems.biblioitemnumber, + aqbasket.closedate, + aqbasket.creationdate, + aqbasket.basketname, + aqorders.* + FROM aqorders + LEFT JOIN aqbasket ON aqorders.basketno = aqbasket.basketno + LEFT JOIN aqbasketgroups ON aqbasket.basketgroupid = aqbasketgroups.id + LEFT JOIN borrowers ON aqbasket.authorisedby=borrowers.borrowernumber LEFT JOIN biblio ON aqorders.biblionumber=biblio.biblionumber LEFT JOIN biblioitems ON biblioitems.biblionumber=biblio.biblionumber - LEFT JOIN aqbasket ON aqorders.basketno = aqbasket.basketno - WHERE (datecancellationprinted is NULL)"; + WHERE (datecancellationprinted is NULL) + }; + + $query .= q{ + AND (quantity > quantityreceived OR quantityreceived is NULL) + }; + + my $userenv = C4::Context->userenv; + if ( C4::Context->preference("IndependantBranches") ) { + if ( ( $userenv ) and ( $userenv->{flags} != 1 ) ) { + $query .= q{ + AND ( + borrowers.branchcode = ? + OR borrowers.branchcode = '' + ) + }; + push @args, $userenv->{branch}; + } + } - if($ordernumber){ - $query .= " AND (aqorders.ordernumber=?)"; + if ( $ordernumber ) { + $query .= ' AND (aqorders.ordernumber=?)'; push @args, $ordernumber; } - if($search){ - $query .= " AND (biblio.title like ? OR biblio.author LIKE ? OR biblioitems.isbn like ?)"; + if( $search ) { + $query .= ' AND (biblio.title LIKE ? OR biblio.author LIKE ? OR biblioitems.isbn LIKE ?)'; push @args, ("%$search%","%$search%","%$search%"); } - if ($ean) { - $query .= " AND biblioitems.ean = ?"; + if ( $ean ) { + $query .= ' AND biblioitems.ean = ?'; push @args, $ean; } - if ($supplierid) { - $query .= "AND aqbasket.booksellerid = ?"; - push @args, $supplierid; + if ( $booksellerid ) { + $query .= 'AND aqbasket.booksellerid = ?'; + push @args, $booksellerid; } - if($basket){ - $query .= "AND aqorders.basketno = ?"; - push @args, $basket; + if( $basketno ) { + $query .= 'AND aqbasket.basketno = ?'; + push @args, $basketno; } + if( $basketname ) { + $query .= 'AND aqbasket.basketname LIKE ?'; + push @args, "%$basketname%"; + } + if( $basketgroupname ) { + $query .= ' AND aqbasketgroups.name LIKE ?'; + push @args, "%$basketgroupname%"; + } + + if ( $owner ) { + $query .= ' AND aqbasket.authorisedby=? '; + push @args, $userenv->{'number'}; + } + + $query .= ' ORDER BY aqbasket.basketno'; my $sth = $dbh->prepare($query); $sth->execute(@args); - my $results = $sth->fetchall_arrayref({}); - $sth->finish; - return $results; + return $sth->fetchall_arrayref({}); } #------------------------------------------------------------# @@ -2406,8 +2356,10 @@ sub GetInvoiceDetails { my $invoice = $sth->fetchrow_hashref; $query = qq{ - SELECT aqorders.*, biblio.* + SELECT aqorders.*, biblio.*, + aqbasket.basketname FROM aqorders + LEFT JOIN aqbasket ON aqorders.basketno = aqbasket.basketno LEFT JOIN biblio ON aqorders.biblionumber = biblio.biblionumber WHERE invoiceid = ? }; diff --git a/acqui/orderreceive.pl b/acqui/orderreceive.pl index fc79ccb..0f438fe 100755 --- a/acqui/orderreceive.pl +++ b/acqui/orderreceive.pl @@ -91,7 +91,9 @@ $datereceived = $datereceived ? C4::Dates->new($datereceived, 'iso') : C4::Dates my $bookseller = GetBookSellerFromId($booksellerid); my $results; -$results = SearchOrder($ordernumber) if $ordernumber; +$results = SearchOrders({ + ordernumber => $ordernumber +}) if $ordernumber; my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { diff --git a/acqui/parcel.pl b/acqui/parcel.pl index 15578a9..ad0648d 100755 --- a/acqui/parcel.pl +++ b/acqui/parcel.pl @@ -173,8 +173,7 @@ for my $item ( @parcelitems ) { $item->{unitprice} = get_value_with_gst_params( $item->{unitprice}, $item->{gstrate}, $bookseller ); $total = ( $item->{'unitprice'} ) * $item->{'quantityreceived'}; $item->{'unitprice'} += 0; - my %line; - %line = %{ $item }; + my %line = %{ $item }; my $ecost = get_value_with_gst_params( $line{ecost}, $line{gstrate}, $bookseller ); $line{ecost} = sprintf( "%.2f", $ecost ); $line{invoice} = $invoice->{invoicenumber}; @@ -218,13 +217,27 @@ if(!defined $invoice->{closedate}) { if($input->param('op') eq "search"){ my $search = $input->param('summaryfilter') || ''; my $ean = $input->param('eanfilter') || ''; - my $basketno = $input->param('basketfilter') || ''; + my $basketname = $input->param('basketfilter') || ''; my $orderno = $input->param('orderfilter') || ''; - my $grouped; - my $owner; - $pendingorders = GetPendingOrders($booksellerid,$grouped,$owner,$basketno,$orderno,$search,$ean); + $pendingorders = SearchOrders({ + booksellerid => $booksellerid, + basketname => $basketname, + ordernumber => $orderno, + search => $search, + ean => $ean, + pending => 1, + }); + $template->param( + summaryfilter => $search, + eanfilter => $ean, + basketfilter => $basketname, + orderfilter => $orderno, + ); }else{ - $pendingorders = GetPendingOrders($booksellerid); + $pendingorders = SearchOrders({ + booksellerid => $booksellerid, + pending => 1 + }); } my $countpendings = scalar @$pendingorders; diff --git a/acqui/uncertainprice.pl b/acqui/uncertainprice.pl index 924f9bd..fd76e47 100755 --- a/acqui/uncertainprice.pl +++ b/acqui/uncertainprice.pl @@ -52,7 +52,7 @@ use C4::Output; use CGI; use C4::Bookseller qw/GetBookSellerFromId/; -use C4::Acquisition qw/GetPendingOrders GetOrder ModOrder/; +use C4::Acquisition qw/SearchOrders GetOrder ModOrder/; use C4::Biblio qw/GetBiblioData/; my $input=new CGI; @@ -73,7 +73,12 @@ my $owner = $input->param('owner') || 0 ; # flag to see only "my" orders, or eve my $bookseller = &GetBookSellerFromId($booksellerid); #show all orders that have uncertain price for the bookseller -my $pendingorders = &GetPendingOrders($booksellerid,0,$owner,$basketno); +my $pendingorders = SearchOrders({ + booksellerid => $booksellerid, + owner => $owner, + basketno => $basketno, + pending => 1, +}); my @orders; foreach my $order (@{$pendingorders}) { diff --git a/serials/acqui-search-result.pl b/serials/acqui-search-result.pl index 5839cd4..e7d1e61 100755 --- a/serials/acqui-search-result.pl +++ b/serials/acqui-search-result.pl @@ -45,7 +45,7 @@ use C4::Auth; use C4::Biblio; use C4::Output; use CGI; -use C4::Acquisition; +use C4::Acquisition qw( SearchOrders ); use C4::Dates qw/format_date/; use C4::Bookseller qw( GetBookSeller ); @@ -66,7 +66,10 @@ my @suppliers = GetBookSeller($supplier); #build result page my $loop_suppliers = []; for my $s (@suppliers) { - my $orders = GetPendingOrders($s->{'id'}); + my $orders = SearchOrders({ + booksellerid => $s->{'id'}, + pending => 1 + }); my $loop_basket = []; for my $ord ( @{$orders} ) { diff --git a/t/db_dependent/Acquisition.t b/t/db_dependent/Acquisition.t index c015425..c5593a8 100755 --- a/t/db_dependent/Acquisition.t +++ b/t/db_dependent/Acquisition.t @@ -28,8 +28,10 @@ SKIP: { my $supplierid = 1; -my $grouped = 0; -my $orders = GetPendingOrders( $supplierid, $grouped ); +my $orders = SearchOrders({ + booksellerid => $supplierid, + pending => 1 +}); isa_ok( $orders, 'ARRAY' ); my @lateorders = GetLateOrders(0); diff --git a/t/db_dependent/lib/KohaTest/Acquisition.pm b/t/db_dependent/lib/KohaTest/Acquisition.pm index e7618c9..39c2f43 100644 --- a/t/db_dependent/lib/KohaTest/Acquisition.pm +++ b/t/db_dependent/lib/KohaTest/Acquisition.pm @@ -20,7 +20,7 @@ sub methods : Test( 1 ) { my @methods = qw( GetBasket NewBasket CloseBasket - GetPendingOrders + SearchOrders GetOrders GetOrderNumber GetOrder diff --git a/t/db_dependent/lib/KohaTest/Acquisition/GetPendingOrders.pm b/t/db_dependent/lib/KohaTest/Acquisition/GetPendingOrders.pm index cf4bb15..3f58154 100644 --- a/t/db_dependent/lib/KohaTest/Acquisition/GetPendingOrders.pm +++ b/t/db_dependent/lib/KohaTest/Acquisition/GetPendingOrders.pm @@ -1,4 +1,4 @@ -package KohaTest::Acquisition::GetPendingOrders; +package KohaTest::Acquisition::SearchOrders; use base qw( KohaTest::Acquisition ); use strict; @@ -17,7 +17,10 @@ at first, there should be no orders for our bookseller. sub no_orders : Test( 1 ) { my $self = shift; - my $orders = GetPendingOrders( $self->{'booksellerid'} ); + my $orders = SearchOrders({ + booksellerid => $self->{booksellerid}, + pending => 1 + }); is( scalar @$orders, 0, 'our new bookseller has no pending orders' ) or diag( Data::Dumper->Dump( [ $orders ], [ 'orders' ] ) ); } @@ -35,8 +38,11 @@ sub one_new_order : Test( 49 ) { ok( $basketno, "basketno is $basketno" ); ok( $ordernumber, "ordernumber is $ordernumber" ); - - my $orders = GetPendingOrders( $self->{'booksellerid'} ); + + my $orders = Searchorders({ + booksellerid => $self->{booksellerid}, + pending => 1 + }); is( scalar @$orders, 1, 'we successfully entered one order.' ); my @expectedfields = qw( basketno -- 1.7.10.4