[Koha-patches] [PATCH] Bug5063: C4::Bookseller Changes
Colin Campbell
colin.campbell at ptfs-europe.com
Mon Jul 26 17:56:04 CEST 2010
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
---
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/lib/KohaTest.pm | 2 +-
16 files changed, 84 insertions(+), 121 deletions(-)
diff --git a/C4/Bookseller.pm b/C4/Bookseller.pm
index 8d60938..8b5c3d0 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
- at results = &GetBookSeller($searchstring);
+ at 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, discount,
notes
)
- VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
- ";
+ VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?) |
+ ;
my $sth = $dbh->prepare($query);
$sth->execute(
$data->{'name'}, $data->{'address1'},
@@ -195,21 +166,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
@@ -225,17 +189,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'},
@@ -248,27 +210,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 005fcb9..28ae83d 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 7641e68..0918f5d 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 e948d2f..810f665 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 2333f89..9e63492 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 a580ae7..f64cffe 100755
--- a/acqui/neworderempty.pl
+++ b/acqui/neworderempty.pl
@@ -76,7 +76,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 156dab8..b65e4ea 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 a1e6fce..5e365f3 100755
--- a/acqui/orderreceive.pl
+++ b/acqui/orderreceive.pl
@@ -65,7 +65,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 675b07d..7b9782a 100755
--- a/acqui/parcel.pl
+++ b/acqui/parcel.pl
@@ -57,7 +57,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 33792b4..1e8d44e 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 6c49a90..5505c52 100755
--- a/acqui/supplier.pl
+++ b/acqui/supplier.pl
@@ -47,7 +47,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 e324a29..3f20796 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 e671e96..5839cd4 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 b723136..888df33 100755
--- a/serials/claims.pl
+++ b/serials/claims.pl
@@ -22,7 +22,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;
my $input = CGI->new;
diff --git a/t/db_dependent/Acquisition.t b/t/db_dependent/Acquisition.t
index 241f442..1d4f36a 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/lib/KohaTest.pm b/t/lib/KohaTest.pm
index 323b55a..1282e38 100644
--- a/t/lib/KohaTest.pm
+++ b/t/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;
--
1.7.1.1
More information about the Koha-patches
mailing list