Attachment 29494 Details for Bug 12493 – [SIGNED-OFF] Bug 12493: moving the subroutines GetContract and GetContracts from C4::Acquisition.pm to C4::Contract.pm

[patch] [SIGNED-OFF] Bug 12493: moving the subroutines GetContract and GetContracts from C4::Acquisition.pm to C4::Contract.pm

SIGNED-OFF-Bug-12493-moving-the-subroutines-GetCon.patch (text/plain), 14.18 KB, created by Bernardo Gonzalez Kriegel on 2014-07-03 15:55:55 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Bernardo Gonzalez Kriegel

Created: 2014-07-03 15:55:55 UTC

Size: 14.18 KB

patch

obsolete

>From 2dc3d098c14cd52213c7e38f5be5ada0ebbf567b Mon Sep 17 00:00:00 2001
>From: Yohann Dufour <dufour.yohann@gmail.com>
>Date: Fri, 27 Jun 2014 16:00:50 +0200
>Subject: [PATCH] [SIGNED-OFF] Bug 12493: moving the subroutines GetContract
> and GetContracts from C4::Acquisition.pm to C4::Contract.pm
>
>This patch includes:
>- the subroutines GetContract and GetContracts has been moved from C4::Acquisition.pm to C4::Contract.pm and adapted for a general use
>- adaptation of acqui/basket.pl, acqui/basketheader.pl, acqui/neworderempty.pl, acqui/supplier.pl and admin/aqcontract.pl
>- the unit tests for the module C4::Contract.pm
>
>Test plan:
>1) Apply the patch
>2) Execute the unit tests by launching:
>prove t/db_dependent/Contract.t t/Acquisition/ t/db_dependent/Acquisition/ t/db_dependent/Acquisition.t
>3) The command has to be a success :
>t/db_dependent/Contract.t ................................. ok
>t/Acquisition/CanUserManageBasket.t ....................... ok
>t/Acquisition/Invoice.t ................................... ok
>t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t .. ok
>t/db_dependent/Acquisition/GetOrdersByBiblionumber.t ...... ok
>t/db_dependent/Acquisition/Invoices.t ..................... ok
>t/db_dependent/Acquisition/OrderFromSubscription.t ........ ok
>t/db_dependent/Acquisition/TransferOrder.t ................ 1/11 # Transfering order to basket2
>t/db_dependent/Acquisition/TransferOrder.t ................ ok
>t/db_dependent/Acquisition/close_reopen_basket.t .......... ok
>t/db_dependent/Acquisition.t .............................. ok
>All tests successful.
>Files=10, Tests=284, 15 wallclock secs ( 0.11 usr  0.02 sys + 12.88 cusr  0.77 csys = 13.78 CPU)
>Result: PASS
>
>4) Log on with a superlibrarian permission
>5) Go on the page acqui/supplier.pl (Acquisitions > Button "New vendor")
>6) Record a vendor with a nonzero "name"
>7) Go on the page admin/aqcontract.pl (click on the "Contracts" item in the menu)
>8) Click on the button "New" > "Contract" and record a new one
>9) Verify the displayed data are correct about the contract
>10) "Edit" the contract with different values and verify the data are updated
>11) Click on "Delete" in order to delete the contract, verify the displayed data are correct but cancel the operation
>12) Click on "New" > "Basket" and verify there is the created contract in field "Contract", then record a basket by selectioning the created contract
>13) Verify the contract name displayed is correct
>14) Record an active budget and a fund linked to this budget
>15) Go on the new basket (Home > Acquisitions > Search the created vendor)
>16) Click on "Add to basket" then "From a new (empty) record" and verify the displayed contract name is correct
>
>Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
>Tested with both patches applied.
>Works as described following test plan, all points (I did 14 first)
>All test pass
>No koha-qa errors
>---
> C4/Acquisition.pm      |   74 ++++--------------------------------------
> C4/Contract.pm         |   84 ++++++++++++++++++++++++++++++++++++++++++++----
> acqui/basket.pl        |    9 ++++--
> acqui/basketheader.pl  |   16 +++++++--
> acqui/neworderempty.pl |    5 ++-
> acqui/supplier.pl      |    5 ++-
> admin/aqcontract.pl    |    9 +++---
> 7 files changed, 115 insertions(+), 87 deletions(-)
>
>diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm
>index 2e013ac..b7f834e 100644
>--- a/C4/Acquisition.pm
>+++ b/C4/Acquisition.pm
>@@ -27,6 +27,7 @@ use C4::Dates qw(format_date format_date_in_iso);
> use MARC::Record;
> use C4::Suggestions;
> use C4::Biblio;
>+use C4::Contract;
> use C4::Debug;
> use C4::SQLHelper qw(InsertInTable UpdateInTable);
> use C4::Bookseller qw(GetBookSellerFromId);
>@@ -65,7 +66,6 @@ BEGIN {
>         &NewOrderItem &ModItemOrder
> 
>         &GetParcels &GetParcel
>-        &GetContracts &GetContract
> 
>         &GetInvoices
>         &GetInvoice
>@@ -293,7 +293,9 @@ sub GetBasketAsCSV {
>     my ($basketno, $cgi) = @_;
>     my $basket = GetBasket($basketno);
>     my @orders = GetOrders($basketno);
>-    my $contract = GetContract($basket->{'contractnumber'});
>+    my $contract = GetContract({
>+        contractnumber => $basket->{'contractnumber'}
>+    });
> 
>     my $template = C4::Templates::gettemplate("acqui/csv/basket.tmpl", "intranet", $cgi);
> 
>@@ -361,7 +363,9 @@ sub GetBasketGroupAsCSV {
>     my @rows;
>     for my $basket (@$baskets) {
>         my @orders     = GetOrders( $$basket{basketno} );
>-        my $contract   = GetContract( $$basket{contractnumber} );
>+        my $contract   = GetContract({
>+            contractnumber => $$basket{contractnumber}
>+        });
>         my $bookseller = GetBookSellerFromId( $$basket{booksellerid} );
>         my $basketgroup = GetBasketgroup( $$basket{basketgroupid} );
> 
>@@ -2453,72 +2457,8 @@ sub GetRecentAcqui {
>     return $results;
> }
> 
>-=head3 GetContracts
>-
>-  $contractlist = &GetContracts($booksellerid, $activeonly);
>-
>-Looks up the contracts that belong to a bookseller
>-
>-Returns a list of contracts
>-
>-=over
>-
>-=item C<$booksellerid> is the "id" field in the "aqbooksellers" table.
>-
>-=item C<$activeonly> if exists get only contracts that are still active.
>-
>-=back
>-
>-=cut
>-
>-sub GetContracts {
>-    my ( $booksellerid, $activeonly ) = @_;
>-    my $dbh = C4::Context->dbh;
>-    my $query;
>-    if (! $activeonly) {
>-        $query = "
>-            SELECT *
>-            FROM   aqcontract
>-            WHERE  booksellerid=?
>-        ";
>-    } else {
>-        $query = "SELECT *
>-            FROM aqcontract
>-            WHERE booksellerid=?
>-                AND contractenddate >= CURDATE( )";
>-    }
>-    my $result_set =
>-      $dbh->selectall_arrayref( $query, { Slice => {} }, $booksellerid );
>-    return @{$result_set};
>-}
>-
> #------------------------------------------------------------#
> 
>-=head3 GetContract
>-
>-  $contract = &GetContract($contractID);
>-
>-Looks up the contract that has PRIMKEY (contractnumber) value $contractID
>-
>-Returns a contract
>-
>-=cut
>-
>-sub GetContract {
>-    my ( $contractno ) = @_;
>-    my $dbh = C4::Context->dbh;
>-    my $query = "
>-        SELECT *
>-        FROM   aqcontract
>-        WHERE  contractnumber=?
>-        ";
>-
>-    my $sth = $dbh->prepare($query);
>-    $sth->execute( $contractno );
>-    my $result = $sth->fetchrow_hashref;
>-    return $result;
>-}
>-
> =head3 AddClaim
> 
> =over
>diff --git a/C4/Contract.pm b/C4/Contract.pm
>index 28e4e75..77fefaa 100644
>--- a/C4/Contract.pm
>+++ b/C4/Contract.pm
>@@ -17,8 +17,10 @@ package C4::Contract;
> # with Koha; if not, write to the Free Software Foundation, Inc.,
> # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> 
>+use Modern::Perl;
> use strict;
> #use warnings; FIXME - Bug 2505
>+use C4::Context;
> use C4::SQLHelper qw(:all);
> 
> use vars qw($VERSION @ISA @EXPORT);
>@@ -29,10 +31,11 @@ BEGIN {
>     require Exporter;
> 	@ISA    = qw(Exporter);
> 	@EXPORT = qw(
>-		&GetContract
>-		&AddContract
>-		&ModContract
>-		&DelContract
>+        &GetContracts
>+        &GetContract
>+        &AddContract
>+        &ModContract
>+        &DelContract
> 	);
> }
> 
>@@ -50,12 +53,79 @@ The functions in this module deal with contracts. They allow to
> add a new contract, to modify it or to get some informations around
> a contract.
> 
>-This module is just a wrapper for C4::SQLHelper functions, so take a look at
>-SQLHelper centralised documentation to know how to use the following subs.
>+=cut
>+
>+
>+=head2 GetContracts
>+
>+$contractlist = GetContracts({
>+    booksellerid => $booksellerid,
>+    activeonly => $activeonly
>+});
>+
>+Looks up the contracts that belong to a bookseller
>+
>+Returns a list of contracts
>+
>+=over
>+
>+=item C<$booksellerid> is the "id" field in the "aqbooksellers" table.
>+
>+=item C<$activeonly> if exists get only contracts that are still active.
>+
>+=back
> 
> =cut
> 
>-sub GetContract { SearchInTable("aqcontract", shift); }
>+sub GetContracts {
>+    my ($params) = @_;
>+    my $booksellerid = $params->{booksellerid};
>+    my $activeonly = $params->{activeonly};
>+
>+    my $dbh = C4::Context->dbh;
>+    my $query = "SELECT * FROM aqcontract";
>+    my $result_set;
>+    if($booksellerid) {
>+        $query .= " WHERE booksellerid=?";
>+
>+        if($activeonly) {
>+            $query .= " AND contractenddate >= CURDATE( )";
>+        }
>+
>+        $result_set = $dbh->selectall_arrayref( $query, { Slice => {} }, $booksellerid );
>+    }
>+    else {
>+        $result_set = $dbh->selectall_arrayref( $query, { Slice => {} } );
>+    }
>+
>+    return $result_set;
>+}
>+
>+=head2 GetContract
>+
>+$contract = GetContract( { contractnumber => $contractnumber }Â );
>+
>+Looks up the contract that has PRIMKEY (contractnumber) value $contractID
>+
>+Returns a contract
>+
>+=cut
>+
>+sub GetContract {
>+    my ($params) = @_;
>+    my $contractno = $params->{contractnumber};
>+
>+    my $dbh = C4::Context->dbh;
>+    my $query = "SELECT * FROM aqcontract WHERE contractnumber=?";
>+
>+    my $sth = $dbh->prepare($query);
>+    $sth->execute($contractno);
>+    my $result = $sth->fetchrow_hashref;
>+    return $result;
>+}
>+
>+
>+#sub GetContract { SearchInTable("aqcontract", shift); }
> 
> sub AddContract { InsertInTable("aqcontract", shift); }
> 
>diff --git a/acqui/basket.pl b/acqui/basket.pl
>index 4f2a37e..d951ef8 100755
>--- a/acqui/basket.pl
>+++ b/acqui/basket.pl
>@@ -30,6 +30,7 @@ use C4::Acquisition;
> use C4::Budgets;
> use C4::Branch;
> use C4::Bookseller qw( GetBookSellerFromId);
>+use C4::Contract;
> use C4::Debug;
> use C4::Biblio;
> use C4::Members qw/GetMember/;  #needed for permissions checking for changing basketgroup of a basket
>@@ -167,7 +168,9 @@ if ( $op eq 'delete_confirm' ) {
>     }
>     $basket->{creationdate} = ""            unless ( $basket->{creationdate} );
>     $basket->{authorisedby} = $loggedinuser unless ( $basket->{authorisedby} );
>-    my $contract = &GetContract($basket->{contractnumber});
>+    my $contract = GetContract({
>+        contractnumber => $basket->{contractnumber}
>+    });
>     $template->param(
>         basketno             => $basketno,
>         basketname           => $basket->{'basketname'},
>@@ -370,7 +373,9 @@ if ( $op eq 'delete_confirm' ) {
>         push @cancelledorders_loop, $line;
>     }
> 
>-    my $contract = &GetContract($basket->{contractnumber});
>+    my $contract = GetContract({
>+        contractnumber => $basket->{contractnumber}
>+    });
>     my @orders = GetOrders($basketno);
> 
>     if ($basket->{basketgroupid}){
>diff --git a/acqui/basketheader.pl b/acqui/basketheader.pl
>index 93bd7f8..8b5fef9 100755
>--- a/acqui/basketheader.pl
>+++ b/acqui/basketheader.pl
>@@ -52,8 +52,9 @@ use C4::Context;
> use C4::Auth;
> use C4::Branch;
> use C4::Output;
>-use C4::Acquisition qw/GetBasket NewBasket GetContracts ModBasketHeader/;
>+use C4::Acquisition qw/GetBasket NewBasket ModBasketHeader/;
> use C4::Bookseller qw/GetBookSellerFromId GetBookSeller/;
>+use C4::Contract qw/GetContracts/;
> 
> 
> my $input = new CGI;
>@@ -84,7 +85,12 @@ if ( $op eq 'add_form' ) {
>         if (! $booksellerid) {
>             $booksellerid=$basket->{'booksellerid'};
>         }
>-        @contractloop = &GetContracts($booksellerid, 1);
>+        my $contracts = GetContracts({
>+            booksellerid => $booksellerid,
>+            activeonly => 1,
>+        });
>+
>+        @contractloop = @$contracts;
>         for (@contractloop) {
>             if ( $basket->{'contractnumber'} eq $_->{'contractnumber'} ) {
>                 $_->{'selected'} = 1;
>@@ -94,7 +100,11 @@ if ( $op eq 'add_form' ) {
>     } else {
>     #new basket
>         my $basket;
>-        push(@contractloop, &GetContracts($booksellerid, 1));
>+        my $contracts = GetContracts({
>+            booksellerid => $booksellerid,
>+            activeonly => 1,
>+        });
>+        push(@contractloop, @$contracts);
>     }
>     my $bookseller = GetBookSellerFromId($booksellerid);
>     my $count = scalar @contractloop;
>diff --git a/acqui/neworderempty.pl b/acqui/neworderempty.pl
>index 61df789..69784b2 100755
>--- a/acqui/neworderempty.pl
>+++ b/acqui/neworderempty.pl
>@@ -78,6 +78,7 @@ use C4::Input;
> 
> use C4::Bookseller  qw/ GetBookSellerFromId /;
> use C4::Acquisition;
>+use C4::Contract;
> use C4::Suggestions;	# GetSuggestion
> use C4::Biblio;			# GetBiblioData GetMarcPrice
> use C4::Items; #PrepareItemRecord
>@@ -132,7 +133,9 @@ our $basket = GetBasket($basketno);
> $booksellerid = $basket->{booksellerid} unless $booksellerid;
> my $bookseller = GetBookSellerFromId($booksellerid);
> 
>-my $contract = &GetContract($basket->{contractnumber});
>+my $contract = GetContract({
>+    contractnumber => $basket->{contractnumber}
>+});
> 
> #simple parameters reading (all in one :-)
> our $params = $input->Vars;
>diff --git a/acqui/supplier.pl b/acqui/supplier.pl
>index 86aad0a..a6ec861 100755
>--- a/acqui/supplier.pl
>+++ b/acqui/supplier.pl
>@@ -43,7 +43,7 @@ To know the bookseller this script has to display details.
> use strict;
> use warnings;
> use C4::Auth;
>-use C4::Contract qw/GetContract/;
>+use C4::Contract;
> use C4::Biblio;
> use C4::Output;
> use CGI;
>@@ -70,8 +70,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
> 
> #build array for currencies
> if ( $op eq 'display' ) {
>-
>-    my $contracts = GetContract( { booksellerid => $booksellerid } );
>+    my $contracts = GetContracts( { booksellerid => $booksellerid } );
> 
>     $template->param(
>         booksellerid  => $booksellerid,
>diff --git a/admin/aqcontract.pl b/admin/aqcontract.pl
>index ab513e7..d7166e3 100755
>--- a/admin/aqcontract.pl
>+++ b/admin/aqcontract.pl
>@@ -61,8 +61,9 @@ if ( $op eq 'add_form' ) {
> 
>    # if contractnumber exists, it's a modify action, so read values to modify...
>     if ($contractnumber) {
>-        my $contract =
>-          @{ GetContract( { contractnumber => $contractnumber } ) }[0];
>+        my $contract = GetContract({
>+            contractnumber => $contractnumber
>+        });
> 
>         $template->param(
>             contractnumber      => $contract->{contractnumber},
>@@ -118,7 +119,7 @@ elsif ( $op eq 'add_validate' ) {
> elsif ( $op eq 'delete_confirm' ) {
>     $template->param( delete_confirm => 1 );
> 
>-    my $contract = @{GetContract( { contractnumber => $contractnumber } )}[0];
>+    my $contract = GetContract( { contractnumber => $contractnumber } );
> 
>     $template->param(
>         contractnumber      => $$contract{contractnumber},
>@@ -146,7 +147,7 @@ else {
>     $template->param(else => 1);
> 
>     # get contracts
>-    my @contracts = @{GetContract( { booksellerid => $booksellerid } )};
>+    my @contracts = @{GetContracts( { booksellerid => $booksellerid } )};
> 
>     # format dates
>     for ( @contracts ) {
>-- 
>1.7.9.5

Actions: View | Diff | Splinter Review

Attachments on bug 12493: 29329 | 29353 | 29494 | 29495 | 29568 | 29831 | 29832 | 29833