Bug 12493: moving the subroutines GetContract and GetContracts from C4::Acquisition...

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm
index 0d1745c..29354f6 100644 (file)
--- 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.tt", "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} );
 
@@ -2471,72 +2475,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 (file)
--- 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 5366bf8..0ca8e3a 100755 (executable)
--- 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 deb77b7..b0ea2e7 100755 (executable)
--- 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 6ae3440..256175a 100755 (executable)
--- 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 1e6954a..e2c688f 100755 (executable)
--- 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 d173dfe..a286716 100755 (executable)
--- 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},
@@ -149,7 +150,7 @@ if ( $op eq 'list' ) {
     $template->param(else => 1);
 
     # get contracts
-    my @contracts = @{GetContract( { booksellerid => $booksellerid } )};
+    my @contracts = @{GetContracts( { booksellerid => $booksellerid } )};
 
     # format dates
     for ( @contracts ) {