Bugzilla – Attachment 18493 Details for
Bug 10124
Pending orders: ergonomic improvement
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 10124: Merge GetPendingOrders and SearchOrders routines
Bug-10124-Merge-GetPendingOrders-and-SearchOrders-.patch (text/plain), 17.38 KB, created by
Jonathan Druart
on 2013-05-30 07:39:11 UTC
(
hide
)
Description:
Bug 10124: Merge GetPendingOrders and SearchOrders routines
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2013-05-30 07:39:11 UTC
Size:
17.38 KB
patch
obsolete
>From 60ad226a8e46b40f6e441e93f03c991a784f4fa0 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >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 | 235 ++++++++------------ > 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, 144 insertions(+), 160 deletions(-) > >diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm >index c766753..1be4724 100644 >--- a/C4/Acquisition.pm >+++ b/C4/Acquisition.pm >@@ -53,9 +53,10 @@ BEGIN { > &ModBasketgroup &NewBasketgroup &DelBasketgroup &GetBasketgroup &CloseBasketgroup > &GetBasketgroups &ReOpenBasketgroup > >- &NewOrder &DelOrder &ModOrder &GetPendingOrders &GetOrder &GetOrders &GetOrdersByBiblionumber >+ &NewOrder &DelOrder &ModOrder &GetOrder &GetOrders &GetOrdersByBiblionumber >+ &NewOrder &DelOrder &ModOrder &GetOrder &GetOrders > &GetOrderNumber &GetLateOrders &GetOrderFromItemnumber >- &SearchOrder &GetHistory &GetRecentAcqui >+ &SearchOrders &GetHistory &GetRecentAcqui > &ModReceiveOrder &CancelReceipt &ModOrderBiblioitemNumber > &GetCancelledOrders > &GetLastOrderNotReceivedFromSubscriptionid &GetLastOrderReceivedFromSubscriptionid >@@ -822,96 +823,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); >@@ -1572,80 +1483,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<yes>, 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<author> >+C<$owner> Finds order for the logged in user. >+C<$pending> Finds pending orders. Ignores completed and cancelled orders. > >-=item C<seriestitle> > >-=item C<branchcode> >- >-=item C<budget_id> >- >-=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({}); > } > > #------------------------------------------------------------# >@@ -2448,8 +2399,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 8e82283..8fba052 100755 >--- a/acqui/parcel.pl >+++ b/acqui/parcel.pl >@@ -169,8 +169,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}; >@@ -214,13 +213,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 c859038..e93877d 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
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 10124
:
17691
|
17692
|
17693
|
17701
|
18493
|
18494
|
18495
|
18501
|
18502
|
18503
|
19309
|
19636
|
19637
|
19638
|
19639
|
19640