From 0d082204ea3c7d3d51a9bdc6eccc436910cdc455 Mon Sep 17 00:00:00 2001 From: Colin Campbell Date: Mon, 7 Mar 2011 21:30:25 -0500 Subject: [PATCH] Bug5063: C4::Bookseller Changes Merge unfao changes to C4::Bookseller Enable warnings in Bookseller.pm Some cleanups in Bookseller code Do not export everything by default Display vendors more rationally Was displaying by id make it name as the searchstring is for all embedded substrings Have removed "if mysql" logic as we want to deal with this by abstracting the DB interaction and it makes cleaner code until then Sponsered by UN FAO, Rome Signed-off-by: Nicole C. Engard Signed-off-by: Chris Cormack --- C4/Bookseller.pm | 172 +++++++++++++-------------------- acqui/basket.pl | 2 +- acqui/booksellers.pl | 2 +- acqui/lateorders.pl | 4 +- acqui/neworderbiblio.pl | 2 +- acqui/neworderempty.pl | 2 +- acqui/newordersuggestion.pl | 2 +- acqui/orderreceive.pl | 2 +- acqui/parcel.pl | 2 +- acqui/parcels.pl | 2 +- acqui/supplier.pl | 2 +- acqui/updatesupplier.pl | 3 +- serials/acqui-search-result.pl | 2 +- serials/claims.pl | 2 +- t/db_dependent/Acquisition.t | 2 +- t/db_dependent/lib/KohaTest.pm | 2 +- 16 files changed, 84 insertions(+), 121 deletions(-) diff --git a/C4/Bookseller.pm b/C4/Bookseller.pm index 21ad34848d..4ad42cb6f2 100644 --- a/C4/Bookseller.pm +++ b/C4/Bookseller.pm @@ -1,6 +1,7 @@ package C4::Bookseller; # Copyright 2000-2002 Katipo Communications +# Copyright 2010 PTFS Europe # # This file is part of Koha. # @@ -18,23 +19,18 @@ package C4::Bookseller; # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. use strict; -#use warnings; FIXME - Bug 2505 - -use vars qw($VERSION @ISA @EXPORT); - -BEGIN { - # set the version for version checking - $VERSION = 3.01; - require Exporter; - @ISA = qw(Exporter); - @EXPORT = qw( - &GetBookSeller &GetBooksellersWithLateOrders &GetBookSellerFromId - &ModBookseller - &DelBookseller - &AddBookseller - ); -} +use warnings; + +use base qw( Exporter ); +# set the version for version checking +our $VERSION = 4.01; +our @EXPORT_OK = qw( + GetBookSeller GetBooksellersWithLateOrders GetBookSellerFromId + ModBookseller + DelBookseller + AddBookseller +); =head1 NAME @@ -54,92 +50,67 @@ a bookseller. =head2 GetBookSeller -@results = &GetBookSeller($searchstring); +@results = GetBookSeller($searchstring); Looks up a book seller. C<$searchstring> may be either a book seller ID, or a string to look for in the book seller's name. -C<@results> is an array of references-to-hash, whose keys are the fields of of the +C<@results> is an array of hash_refs whose keys are the fields of of the aqbooksellers table in the Koha database. =cut -# FIXME: This function is badly named. It should be something like -# SearchBookSellersByName. It is NOT a singular return value. +sub GetBookSeller { + my $searchstring = shift; + $searchstring = q{%} . $searchstring . q{%}; + my $query = +'select aqbooksellers.*, count(*) as basketcount from aqbooksellers left join aqbasket ' + . 'on aqbasket.booksellerid = aqbooksellers.id where name lIke ? group by aqbooksellers.id order by name'; + + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare($query); + $sth->execute($searchstring); + my $resultset_ref = $sth->fetchall_arrayref( {} ); + return @{$resultset_ref}; +} -sub GetBookSeller($) { - my ($searchstring) = @_; +sub GetBookSellerFromId { + my $id = shift or return; my $dbh = C4::Context->dbh; - my $query = "SELECT * FROM aqbooksellers WHERE name LIKE ?"; - my $sth =$dbh->prepare($query); - $sth->execute( "%$searchstring%" ); - my @results; - # count how many baskets this bookseller has. - # if it has none, the bookseller can be deleted - my $sth2 = $dbh->prepare("SELECT count(*) FROM aqbasket WHERE booksellerid=?"); - while ( my $data = $sth->fetchrow_hashref ) { - $sth2->execute($data->{id}); - $data->{basketcount} = $sth2->fetchrow(); - push( @results, $data ); + my $vendor = + $dbh->selectrow_hashref( 'SELECT * FROM aqbooksellers WHERE id = ?', + {}, $id ); + if ($vendor) { + ( $vendor->{basketcount} ) = $dbh->selectrow_array( + 'SELECT count(*) FROM aqbasket where booksellerid = ?', + {}, $id ); } - $sth->finish; - return @results ; + return $vendor; } - -sub GetBookSellerFromId($) { - my $id = shift or return; - my $dbh = C4::Context->dbh(); - my $query = "SELECT * FROM aqbooksellers WHERE id = ?"; - my $sth =$dbh->prepare($query); - $sth->execute( $id ); - if (my $data = $sth->fetchrow_hashref()){ - my $sth2 = $dbh->prepare("SELECT count(*) FROM aqbasket WHERE booksellerid=?"); - $sth2->execute($id); - $data->{basketcount}=$sth2->fetchrow(); - return $data; - } - return; -} #-----------------------------------------------------------------# =head2 GetBooksellersWithLateOrders -%results = &GetBooksellersWithLateOrders; +%results = GetBooksellersWithLateOrders($delay); Searches for suppliers with late orders. =cut sub GetBooksellersWithLateOrders { - my ($delay,$branch) = @_; # FIXME: Branch argument unused. + my $delay = shift; my $dbh = C4::Context->dbh; -# FIXME NOT quite sure that this operation is valid for DBMs different from Mysql, HOPING so -# should be tested with other DBMs - - my $strsth; - my $dbdriver = C4::Context->config("db_scheme") || "mysql"; - if ( $dbdriver eq "mysql" ) { - $strsth = " - SELECT DISTINCT aqbasket.booksellerid, aqbooksellers.name - FROM aqorders LEFT JOIN aqbasket ON aqorders.basketno=aqbasket.basketno - LEFT JOIN aqbooksellers ON aqbasket.booksellerid = aqbooksellers.id - WHERE (closedate < DATE_SUB(CURDATE( ),INTERVAL $delay DAY) - AND (datereceived = '' OR datereceived IS NULL)) - "; - } - else { - $strsth = " - SELECT DISTINCT aqbasket.booksellerid, aqbooksellers.name - FROM aqorders LEFT JOIN aqbasket ON aqorders.basketno=aqbasket.basketno - LEFT JOIN aqbooksellers ON aqbasket.aqbooksellerid = aqbooksellers.id - WHERE (closedate < (CURDATE( )-(INTERVAL $delay DAY))) - AND (datereceived = '' OR datereceived IS NULL)) - "; - } + # TODO delay should be verified + my $query_string = + "SELECT DISTINCT aqbasket.booksellerid, aqbooksellers.name + FROM aqorders LEFT JOIN aqbasket ON aqorders.basketno=aqbasket.basketno + LEFT JOIN aqbooksellers ON aqbasket.booksellerid = aqbooksellers.id + WHERE (closedate < DATE_SUB(CURDATE( ),INTERVAL $delay DAY) + AND (datereceived = '' OR datereceived IS NULL))"; - my $sth = $dbh->prepare($strsth); + my $sth = $dbh->prepare($query_string); $sth->execute; my %supplierlist; while ( my ( $id, $name ) = $sth->fetchrow ) { @@ -165,8 +136,8 @@ Returns the ID of the newly-created bookseller. sub AddBookseller { my ($data) = @_; - my $dbh = C4::Context->dbh; - my $query = " + my $dbh = C4::Context->dbh; + my $query = q| INSERT INTO aqbooksellers ( name, address1, address2, address3, address4, @@ -176,8 +147,8 @@ sub AddBookseller { listincgst,invoiceincgst, gstrate, discount, notes ) - VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?) - "; + VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?) | + ; my $sth = $dbh->prepare($query); $sth->execute( $data->{'name'}, $data->{'address1'}, @@ -196,21 +167,14 @@ sub AddBookseller { ); # return the id of this new supplier - # FIXME: no protection against simultaneous addition: max(id) might be wrong! - $query = " - SELECT max(id) - FROM aqbooksellers - "; - $sth = $dbh->prepare($query); - $sth->execute; - return scalar($sth->fetchrow); + return $dbh->{'mysql_insertid'}; } #-----------------------------------------------------------------# =head2 ModBookseller -&ModBookseller($bookseller); +ModBookseller($bookseller); Updates the information for a given bookseller. C<$bookseller> is a reference-to-hash whose keys are the fields of the aqbooksellers table @@ -226,17 +190,15 @@ C<&ModBookseller> with the result. sub ModBookseller { my ($data) = @_; my $dbh = C4::Context->dbh; - my $query = " - UPDATE aqbooksellers + my $query = 'UPDATE aqbooksellers SET name=?,address1=?,address2=?,address3=?,address4=?, postal=?,phone=?,fax=?,url=?,contact=?,contpos=?, contphone=?,contfax=?,contaltphone=?,contemail=?, contnotes=?,active=?,listprice=?, invoiceprice=?, gstreg=?,listincgst=?,invoiceincgst=?, - discount=?, notes=?, gstrate=? - WHERE id=? - "; - my $sth = $dbh->prepare($query); + discount=?,notes=?,gstrate=? + WHERE id=?'; + my $sth = $dbh->prepare($query); $sth->execute( $data->{'name'}, $data->{'address1'}, $data->{'address2'}, $data->{'address3'}, @@ -249,27 +211,27 @@ sub ModBookseller { $data->{'active'}, $data->{'listprice'}, $data->{'invoiceprice'}, $data->{'gstreg'}, $data->{'listincgst'}, $data->{'invoiceincgst'}, - $data->{'discount'}, - $data->{'notes'}, $data->{'gstrate'}, - $data->{'id'} + $data->{'discount'}, $data->{'notes'}, + $data->{'gstrate'}, $data->{'id'} ); - $sth->finish; + return; } =head2 DelBookseller -&DelBookseller($booksellerid); +DelBookseller($booksellerid); -delete the supplier identified by $booksellerid -This sub can be called only if the supplier has no order. +delete the supplier record identified by $booksellerid +This sub assumes it is called only if the supplier has no order. =cut sub DelBookseller { - my ($id) = @_; - my $dbh=C4::Context->dbh; - my $sth=$dbh->prepare("DELETE FROM aqbooksellers WHERE id=?"); + my $id = shift; + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare('DELETE FROM aqbooksellers WHERE id=?'); $sth->execute($id); + return; } 1; diff --git a/acqui/basket.pl b/acqui/basket.pl index 221a26b25b..8ea292940a 100755 --- a/acqui/basket.pl +++ b/acqui/basket.pl @@ -29,7 +29,7 @@ use CGI; use C4::Acquisition; use C4::Budgets; -use C4::Bookseller; +use C4::Bookseller qw( GetBookSellerFromId); use C4::Dates qw/format_date/; use C4::Debug; diff --git a/acqui/booksellers.pl b/acqui/booksellers.pl index 7641e689e6..0918f5d2c4 100755 --- a/acqui/booksellers.pl +++ b/acqui/booksellers.pl @@ -62,7 +62,7 @@ use CGI; use C4::Acquisition; use C4::Dates qw/format_date/; -use C4::Bookseller; +use C4::Bookseller qw/ GetBookSellerFromId GetBookSeller /; use C4::Members qw/GetMember/; my $query = new CGI; diff --git a/acqui/lateorders.pl b/acqui/lateorders.pl index e948d2fafa..810f665b87 100755 --- a/acqui/lateorders.pl +++ b/acqui/lateorders.pl @@ -45,7 +45,7 @@ To know on which branch this script have to display late order. use strict; use warnings; use CGI; -use C4::Bookseller; +use C4::Bookseller qw( GetBooksellersWithLateOrders ); use C4::Auth; use C4::Koha; use C4::Output; @@ -76,7 +76,7 @@ unless ($delay =~ /^\d{1,3}$/) { $delay = 30; #default value for delay } -my %supplierlist = GetBooksellersWithLateOrders($delay,$branch); +my %supplierlist = GetBooksellersWithLateOrders($delay); my (@sloopy); # supplier loop foreach (keys %supplierlist){ push @sloopy, (($supplierid and $supplierid eq $_ ) ? diff --git a/acqui/neworderbiblio.pl b/acqui/neworderbiblio.pl index 2333f89d07..9e6349282a 100755 --- a/acqui/neworderbiblio.pl +++ b/acqui/neworderbiblio.pl @@ -60,7 +60,7 @@ use strict; use C4::Search; use CGI; -use C4::Bookseller; +use C4::Bookseller qw/ GetBookSellerFromId /; use C4::Biblio; use C4::Auth; use C4::Output; diff --git a/acqui/neworderempty.pl b/acqui/neworderempty.pl index 6af057d29a..1cba13ecac 100755 --- a/acqui/neworderempty.pl +++ b/acqui/neworderempty.pl @@ -77,7 +77,7 @@ use C4::Budgets; use C4::Input; use C4::Dates; -use C4::Bookseller; # GetBookSellerFromId +use C4::Bookseller qw/ GetBookSellerFromId /; use C4::Acquisition; use C4::Suggestions; # GetSuggestion use C4::Biblio; # GetBiblioData diff --git a/acqui/newordersuggestion.pl b/acqui/newordersuggestion.pl index 156dab8f0b..b65e4ea624 100755 --- a/acqui/newordersuggestion.pl +++ b/acqui/newordersuggestion.pl @@ -93,7 +93,7 @@ use CGI; use C4::Auth; # get_template_and_user use C4::Output; use C4::Suggestions; -use C4::Bookseller; +use C4::Bookseller qw/ GetBookSellerFromId /; use C4::Biblio; my $input = new CGI; diff --git a/acqui/orderreceive.pl b/acqui/orderreceive.pl index 96e4556ece..98ba544246 100755 --- a/acqui/orderreceive.pl +++ b/acqui/orderreceive.pl @@ -69,7 +69,7 @@ use C4::Acquisition; use C4::Auth; use C4::Output; use C4::Dates qw/format_date/; -use C4::Bookseller; +use C4::Bookseller qw/ GetBookSellerFromId /; use C4::Members; use C4::Branch; # GetBranches use C4::Items; diff --git a/acqui/parcel.pl b/acqui/parcel.pl index 6c8f5f019e..ad95944111 100755 --- a/acqui/parcel.pl +++ b/acqui/parcel.pl @@ -61,7 +61,7 @@ use strict; use C4::Auth; use C4::Acquisition; use C4::Budgets; -use C4::Bookseller; +use C4::Bookseller qw/ GetBookSellerFromId /; use C4::Biblio; use C4::Items; use CGI; diff --git a/acqui/parcels.pl b/acqui/parcels.pl index 33792b47bc..1e8d44e3d7 100755 --- a/acqui/parcels.pl +++ b/acqui/parcels.pl @@ -74,7 +74,7 @@ use C4::Output; use C4::Dates qw/format_date/; use C4::Acquisition; -use C4::Bookseller; +use C4::Bookseller qw/ GetBookSellerFromId /; my $input = CGI->new; my $supplierid = $input->param('supplierid'); diff --git a/acqui/supplier.pl b/acqui/supplier.pl index 80660d9f40..52d940c616 100755 --- a/acqui/supplier.pl +++ b/acqui/supplier.pl @@ -49,7 +49,7 @@ use C4::Output; use C4::Dates qw/format_date /; use CGI; -use C4::Bookseller; +use C4::Bookseller qw( GetBookSellerFromId DelBookseller ); use C4::Budgets; my $query = CGI->new; diff --git a/acqui/updatesupplier.pl b/acqui/updatesupplier.pl index 1d487cb7ad..7c7aa2c87f 100755 --- a/acqui/updatesupplier.pl +++ b/acqui/updatesupplier.pl @@ -48,7 +48,8 @@ use strict; #use warnings; FIXME - Bug 2505 use C4::Context; use C4::Auth; -use C4::Bookseller; + +use C4::Bookseller qw( ModBookseller AddBookseller ); use C4::Biblio; use C4::Output; use CGI; diff --git a/serials/acqui-search-result.pl b/serials/acqui-search-result.pl index e671e96bd8..5839cd498e 100755 --- a/serials/acqui-search-result.pl +++ b/serials/acqui-search-result.pl @@ -47,7 +47,7 @@ use C4::Output; use CGI; use C4::Acquisition; use C4::Dates qw/format_date/; -use C4::Bookseller; +use C4::Bookseller qw( GetBookSeller ); my $query=new CGI; my ($template, $loggedinuser, $cookie) diff --git a/serials/claims.pl b/serials/claims.pl index 735d945bf6..e51148c744 100755 --- a/serials/claims.pl +++ b/serials/claims.pl @@ -24,7 +24,7 @@ use C4::Auth; use C4::Serials; use C4::Acquisition; use C4::Output; -use C4::Bookseller; +use C4::Bookseller qw( GetBookSeller ); use C4::Context; use C4::Letters; use C4::Branch; # GetBranches GetBranchesLoop diff --git a/t/db_dependent/Acquisition.t b/t/db_dependent/Acquisition.t index 241f4429e2..1d4f36a4a8 100755 --- a/t/db_dependent/Acquisition.t +++ b/t/db_dependent/Acquisition.t @@ -7,7 +7,7 @@ use strict; use warnings; use Data::Dumper; -use C4::Bookseller; +use C4::Bookseller qw( GetBookSellerFromId ); use Test::More tests => 37; diff --git a/t/db_dependent/lib/KohaTest.pm b/t/db_dependent/lib/KohaTest.pm index 323b55a67b..1282e3886c 100644 --- a/t/db_dependent/lib/KohaTest.pm +++ b/t/db_dependent/lib/KohaTest.pm @@ -10,7 +10,7 @@ plan skip_all => "Test::Class required for performing database tests" if $@; use C4::Auth; use C4::Biblio; -use C4::Bookseller; +use C4::Bookseller qw( AddBookseller ); use C4::Context; use C4::Items; use C4::Members; -- 2.20.1