Bugzilla – Attachment 3264 Details for
Bug 5063
Improve C4::Booksellers interface
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Proposed patch
0001-Bug5063-C4-Bookseller-Changes.patch (text/plain), 16.06 KB, created by
Colin Campbell
on 2011-03-08 13:59:22 UTC
(
hide
)
Description:
Proposed patch
Filename:
MIME Type:
Creator:
Colin Campbell
Created:
2011-03-08 13:59:22 UTC
Size:
16.06 KB
patch
obsolete
>From bc892be163c6ec5c0fd5e73a91cd54479c7ac445 Mon Sep 17 00:00:00 2001 >From: Colin Campbell <colin.campbell@ptfs-europe.com> >Date: Mon, 26 Jul 2010 16:30:49 +0100 >Subject: [PATCH] Bug5063: C4::Bookseller Changes >Content-Type: text/plain; charset="utf-8" > >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/db_dependent/lib/KohaTest.pm | 2 +- > 16 files changed, 84 insertions(+), 121 deletions(-) > >diff --git a/C4/Bookseller.pm b/C4/Bookseller.pm >index 21ad348..4ad42cb 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 221a26b..8ea2929 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 6af057d..1cba13e 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 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 96e4556..98ba544 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 6c8f5f0..ad95944 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 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 80660d9..52d940c 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 1d487cb..7c7aa2c 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 735d945..e51148c 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 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/db_dependent/lib/KohaTest.pm b/t/db_dependent/lib/KohaTest.pm >index 323b55a..1282e38 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; >-- >1.7.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 5063
: 3264