From 48818bdfeaa9f3f8d46942ad7623632c6d70d410 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 14 Aug 2013 10:43:07 +0200 Subject: [PATCH] Bug 10723: Merge GetPendingOrders and SearchOrders routines 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, acqui/parcel.pl scripts. Note: on acqui/parcel the basket filter is a search on basket name (was on basket id, which was not relevant). Signed-off-by: Pierre Angot Signed-off-by: Kyle M Hall Passes koha-qa.pm, no adverse bahaviors noted. All sub calls updated. --- 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 | 140 ------------ .../lib/KohaTest/Acquisition/GetPendingOrders.pm | 82 ------- 8 files changed, 132 insertions(+), 377 deletions(-) delete mode 100644 t/db_dependent/lib/KohaTest/Acquisition.pm delete mode 100644 t/db_dependent/lib/KohaTest/Acquisition/GetPendingOrders.pm diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index 1a210b5..ec72a9a 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 &GetOrdersByBiblionumber + &NewOrder &DelOrder &ModOrder &GetOrder &GetOrders &GetOrdersByBiblionumber &GetLateOrders &GetOrderFromItemnumber - &SearchOrder &GetHistory &GetRecentAcqui + &SearchOrders &GetHistory &GetRecentAcqui &ModReceiveOrder &CancelReceipt &GetCancelledOrders &GetLastOrderNotReceivedFromSubscriptionid &GetLastOrderReceivedFromSubscriptionid @@ -827,96 +827,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("IndependentBranches") ) { - 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); @@ -1489,80 +1399,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("IndependentBranches") ) { + 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({}); } #------------------------------------------------------------# @@ -2365,8 +2315,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 8fc760b..ab7b341 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 ee96e3f..27be3c8 100755 --- a/acqui/parcel.pl +++ b/acqui/parcel.pl @@ -162,8 +162,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}; @@ -207,13 +206,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 c7b3c4a..90d5a88 100755 --- a/t/db_dependent/Acquisition.t +++ b/t/db_dependent/Acquisition.t @@ -80,8 +80,10 @@ my ($biblionumber2, $biblioitemnumber2) = AddBiblio(MARC::Record->new, ''); } ); -my $grouped = 0; -my $orders = GetPendingOrders( $booksellerid, $grouped ); +my $orders = SearchOrders({ + booksellerid => $booksellerid, + pending => 1 +}); isa_ok( $orders, 'ARRAY' ); C4::Acquisition::CloseBasket( $basketno ); diff --git a/t/db_dependent/lib/KohaTest/Acquisition.pm b/t/db_dependent/lib/KohaTest/Acquisition.pm deleted file mode 100644 index 3f0b789..0000000 --- a/t/db_dependent/lib/KohaTest/Acquisition.pm +++ /dev/null @@ -1,140 +0,0 @@ -package KohaTest::Acquisition; -use base qw( KohaTest ); - -use strict; -use warnings; - -use Test::More; - -use C4::Acquisition; -use C4::Budgets; -use C4::Context; -use C4::Members; -use Time::localtime; - -sub testing_class { 'C4::Acquisition' }; - - -sub methods : Test( 1 ) { - my $self = shift; - my @methods = qw( GetBasket - NewBasket - CloseBasket - GetPendingOrders - GetOrders - GetOrder - NewOrder - ModOrder - ModReceiveOrder - SearchOrder - DelOrder - GetParcel - GetParcels - GetLateOrders - GetHistory - GetRecentAcqui - ); - - can_ok( $self->testing_class, @methods ); -} - -=head3 create_new_basket - - creates a baseket by creating an order with no baseket number. - - named parameters: - authorizedby - invoice - date - - returns: baseket number, order number - - runs 4 tests - -=cut - -sub create_new_basket { - my $self = shift; - my %param = @_; - $param{'authorizedby'} = $self->{'memberid'} unless exists $param{'authorizedby'}; - $param{'invoice'} = 123 unless exists $param{'invoice'}; - - my $today = sprintf( '%04d-%02d-%02d', - localtime->year() + 1900, - localtime->mon() + 1, - localtime->mday() ); - - # I actually think that this parameter is unused. - $param{'date'} = $today unless exists $param{'date'}; - - $self->add_biblios( add_items => 1 ); - ok( scalar @{$self->{'biblios'}} > 0, 'we have added at least one biblio' ); - - my $rand = int(rand(10000)); - my $basketno = NewBasket( $self->{'booksellerid'}, $param{'authorizedby'}, "Basket $rand"); -# $basketnote, $basketbooksellernote, $basketcontractnumber ); -# The following keys are used: "biblionumber", "title", "basketno", "quantity", "notes", "biblioitemnumber", "rrp", "ecost", "gst", "unitprice", "subscription", "sort1", "sort2", "booksellerinvoicenumber", "listprice", "budgetdate", "purchaseordernumber", "branchcode", "booksellerinvoicenumber", "bookfundid". - my $budget_id = AddBudget( { budget_name => "Budget $rand" } ); - my ( undef, $ordernumber ) = NewOrder( { - basketno => $basketno, - budget_id => $budget_id, - biblionumber => $self->{'biblios'}[0], - quantity => 1, - bookfundid => $self->{'bookfundid'}, - rrp => 1, - ecost => 1, - booksellerinvoicenumber => $param{'invoice'}, - } ); - ok( $basketno, "my basket number is $basketno" ); - ok( $ordernumber, "my order number is $ordernumber" ); - - my $order = GetOrder( $ordernumber ); - is( $order->{'ordernumber'}, $ordernumber, 'got the right order' ) - or diag( Data::Dumper->Dump( [ $order ], [ 'order' ] ) ); - - is( $order->{'budgetdate'}, $today, "the budget date is $today" ); - - # XXX should I stuff these in $self? - return ( $basketno, $ordernumber ); - -} - - -sub enable_independant_branches { - my $self = shift; - - my $member = GetMember( 'borrowernumber' =>$self->{'memberid'} ); - - C4::Context::set_userenv( 0, # usernum - $self->{'memberid'}, # userid - undef, # usercnum - undef, # userfirstname - undef, # usersurname - $member->{'branchcode'}, # userbranch - undef, # branchname - 0, # userflags - undef, # emailaddress - undef, # branchprinter - ); - - # set a preference. There's surely a method for this, but I can't find it. - my $retval = C4::Context->dbh->do( q(update systempreferences set value = '1' where variable = 'IndependentBranches') ); - ok( $retval, 'set the preference' ); - - ok( C4::Context->userenv, 'usernev' ); - isnt( C4::Context->userenv->{flags}, 1, 'flag != 1' ) - or diag( Data::Dumper->Dump( [ C4::Context->userenv ], [ 'userenv' ] ) ); - - is( C4::Context->userenv->{branch}, $member->{'branchcode'}, 'we have set the right branch in C4::Context: ' . $member->{'branchcode'} ); - -} - -sub disable_independant_branches { - my $self = shift; - - my $retval = C4::Context->dbh->do( q(update systempreferences set value = '0' where variable = 'IndependentBranches') ); - ok( $retval, 'set the preference back' ); - - -} -1; diff --git a/t/db_dependent/lib/KohaTest/Acquisition/GetPendingOrders.pm b/t/db_dependent/lib/KohaTest/Acquisition/GetPendingOrders.pm deleted file mode 100644 index cf4bb15..0000000 --- a/t/db_dependent/lib/KohaTest/Acquisition/GetPendingOrders.pm +++ /dev/null @@ -1,82 +0,0 @@ -package KohaTest::Acquisition::GetPendingOrders; -use base qw( KohaTest::Acquisition ); - -use strict; -use warnings; - -use Test::More; - -use C4::Acquisition; - -=head3 no_orders - -at first, there should be no orders for our bookseller. - -=cut - -sub no_orders : Test( 1 ) { - my $self = shift; - - my $orders = GetPendingOrders( $self->{'booksellerid'} ); - is( scalar @$orders, 0, 'our new bookseller has no pending orders' ) - or diag( Data::Dumper->Dump( [ $orders ], [ 'orders' ] ) ); -} - -=head3 new_order - -we make an order, then see if it shows up in the pending orders - -=cut - -sub one_new_order : Test( 49 ) { - my $self = shift; - - my ( $basketno, $ordernumber ) = $self->create_new_basket(); - - ok( $basketno, "basketno is $basketno" ); - ok( $ordernumber, "ordernumber is $ordernumber" ); - - my $orders = GetPendingOrders( $self->{'booksellerid'} ); - is( scalar @$orders, 1, 'we successfully entered one order.' ); - - my @expectedfields = qw( basketno - biblioitemnumber - biblionumber - booksellerinvoicenumber - budgetdate - cancelledby - closedate - creationdate - currency - datecancellationprinted - datereceived - ecost - entrydate - firstname - freight - gst - listprice - notes - ordernumber - purchaseordernumber - quantity - quantityreceived - rrp - serialid - sort1 - sort2 - subscription - supplierreference - surname - timestamp - title - totalamount - unitprice ); - my $firstorder = $orders->[0]; - for my $field ( @expectedfields ) { - ok( exists( $firstorder->{ $field } ), "This order has a $field field" ); - } - -} - -1; -- 1.7.2.5