Attachment 5329 Details for Bug 4530 – patch

[patch] patch

0001-bug_4530-Rework-issuing-rules.patch (text/plain), 24.10 KB, created by Srdjan Jankovic on 2011-09-07 02:26:02 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Srdjan Jankovic

Created: 2011-09-07 02:26:02 UTC

Size: 24.10 KB

patch

obsolete

>From f85f505815eb6db69b02d3c05c2a10801238f3e8 Mon Sep 17 00:00:00 2001
>From: Srdjan Jankovic <srdjan@catalyst.net.nz>
>Date: Wed, 7 Sep 2011 13:50:48 +1200
>Subject: [PATCH] bug_4530: Rework issuing rules
> Use only one set of functions
> Database efficient rules fetch - only one query
>
>---
> C4/Circulation.pm                          |  312 +++++++++-------------------
> C4/Members.pm                              |   41 ++--
> C4/Overdues.pm                             |   40 +----
> C4/Reserves.pm                             |   15 +--
> t/db_dependent/lib/KohaTest/Circulation.pm |    4 +-
> t/db_dependent/lib/KohaTest/Overdues.pm    |    1 -
> 6 files changed, 125 insertions(+), 288 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 47068f9..fb7c4a6 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -74,7 +74,6 @@ BEGIN {
> 		&GetItemIssues
> 		&GetBorrowerIssues
> 		&GetIssuingCharges
>-		&GetIssuingRule
>         &GetBranchBorrowerCircRule
>         &GetBranchItemRule
> 		&GetBiblioIssues
>@@ -356,12 +355,12 @@ sub TooMany {
>  
>     # given branch, patron category, and item type, determine
>     # applicable issuing rule
>-    my $issuing_rule = GetIssuingRule($cat_borrower, $type, $branch);
>+    my $issuing_rule = GetIssuingRuleRecord($cat_borrower, $type, $branch, 'maxissueqty');
> 
>     # if a rule is found and has a loan limit set, count
>     # how many loans the patron already has that meet that
>     # rule
>-    if (defined($issuing_rule) and defined($issuing_rule->{'maxissueqty'})) {
>+    if ( $issuing_rule ) {
>         my @bind_params;
>         my $count_query = "SELECT COUNT(*) FROM issues
>                            JOIN items USING (itemnumber) ";
>@@ -1119,56 +1118,8 @@ Get loan length for an itemtype, a borrower type and a branch
> 
> sub GetLoanLength {
>     my ( $borrowertype, $itemtype, $branchcode ) = @_;
>-    my $dbh = C4::Context->dbh;
>-    my $sth =
>-      $dbh->prepare(
>-"select issuelength from issuingrules where categorycode=? and itemtype=? and branchcode=? and issuelength is not null"
>-      );
>-# warn "in get loan lenght $borrowertype $itemtype $branchcode ";
>-# try to find issuelength & return the 1st available.
>-# check with borrowertype, itemtype and branchcode, then without one of those parameters
>-    $sth->execute( $borrowertype, $itemtype, $branchcode );
>-    my $loanlength = $sth->fetchrow_hashref;
>-    return $loanlength->{issuelength}
>-      if defined($loanlength) && $loanlength->{issuelength} ne 'NULL';
>-
>-    $sth->execute( $borrowertype, "*", $branchcode );
>-    $loanlength = $sth->fetchrow_hashref;
>-    return $loanlength->{issuelength}
>-      if defined($loanlength) && $loanlength->{issuelength} ne 'NULL';
>-
>-    $sth->execute( "*", $itemtype, $branchcode );
>-    $loanlength = $sth->fetchrow_hashref;
>-    return $loanlength->{issuelength}
>-      if defined($loanlength) && $loanlength->{issuelength} ne 'NULL';
>-
>-    $sth->execute( "*", "*", $branchcode );
>-    $loanlength = $sth->fetchrow_hashref;
>-    return $loanlength->{issuelength}
>-      if defined($loanlength) && $loanlength->{issuelength} ne 'NULL';
>-
>-    $sth->execute( $borrowertype, $itemtype, "*" );
>-    $loanlength = $sth->fetchrow_hashref;
>-    return $loanlength->{issuelength}
>-      if defined($loanlength) && $loanlength->{issuelength} ne 'NULL';
>-
>-    $sth->execute( $borrowertype, "*", "*" );
>-    $loanlength = $sth->fetchrow_hashref;
>-    return $loanlength->{issuelength}
>-      if defined($loanlength) && $loanlength->{issuelength} ne 'NULL';
>-
>-    $sth->execute( "*", $itemtype, "*" );
>-    $loanlength = $sth->fetchrow_hashref;
>-    return $loanlength->{issuelength}
>-      if defined($loanlength) && $loanlength->{issuelength} ne 'NULL';
>-
>-    $sth->execute( "*", "*", "*" );
>-    $loanlength = $sth->fetchrow_hashref;
>-    return $loanlength->{issuelength}
>-      if defined($loanlength) && $loanlength->{issuelength} ne 'NULL';
>-
>     # if no rule is set => 21 days (hardcoded)
>-    return 21;
>+    return GetIssuingRuleValue( $borrowertype, $itemtype, $branchcode, 'issuelength' ) || 21;
> }
> 
> 
>@@ -1182,108 +1133,100 @@ Get the Hard Due Date and it's comparison for an itemtype, a borrower type and a
> 
> sub GetHardDueDate {
>     my ( $borrowertype, $itemtype, $branchcode ) = @_;
>-    my $dbh = C4::Context->dbh;
>-    my $sth =
>-      $dbh->prepare(
>-"select hardduedate, hardduedatecompare from issuingrules where categorycode=? and itemtype=? and branchcode=?"
>-      );
>-    $sth->execute( $borrowertype, $itemtype, $branchcode );
>-    my $results = $sth->fetchrow_hashref;
>-    return (C4::Dates->new($results->{hardduedate}, 'iso'),$results->{hardduedatecompare})
>-      if defined($results) && $results->{hardduedate} ne 'NULL';
>-
>-    $sth->execute( $borrowertype, "*", $branchcode );
>-    $results = $sth->fetchrow_hashref;
>-    return (C4::Dates->new($results->{hardduedate}, 'iso'),$results->{hardduedatecompare})
>-      if defined($results) && $results->{hardduedate} ne 'NULL';
>-
>-    $sth->execute( "*", $itemtype, $branchcode );
>-    $results = $sth->fetchrow_hashref;
>-    return (C4::Dates->new($results->{hardduedate}, 'iso'),$results->{hardduedatecompare})
>-      if defined($results) && $results->{hardduedate} ne 'NULL';
>-
>-    $sth->execute( "*", "*", $branchcode );
>-    $results = $sth->fetchrow_hashref;
>-    return (C4::Dates->new($results->{hardduedate}, 'iso'),$results->{hardduedatecompare})
>-      if defined($results) && $results->{hardduedate} ne 'NULL';
>-
>-    $sth->execute( $borrowertype, $itemtype, "*" );
>-    $results = $sth->fetchrow_hashref;
>-    return (C4::Dates->new($results->{hardduedate}, 'iso'),$results->{hardduedatecompare})
>-      if defined($results) && $results->{hardduedate} ne 'NULL';
>-
>-    $sth->execute( $borrowertype, "*", "*" );
>-    $results = $sth->fetchrow_hashref;
>-    return (C4::Dates->new($results->{hardduedate}, 'iso'),$results->{hardduedatecompare})
>-      if defined($results) && $results->{hardduedate} ne 'NULL';
>-
>-    $sth->execute( "*", $itemtype, "*" );
>-    $results = $sth->fetchrow_hashref;
>-    return (C4::Dates->new($results->{hardduedate}, 'iso'),$results->{hardduedatecompare})
>-      if defined($results) && $results->{hardduedate} ne 'NULL';
>-
>-    $sth->execute( "*", "*", "*" );
>-    $results = $sth->fetchrow_hashref;
>-    return (C4::Dates->new($results->{hardduedate}, 'iso'),$results->{hardduedatecompare})
>-      if defined($results) && $results->{hardduedate} ne 'NULL';
>-
>-    # if no rule is set => return undefined
>-    return (undef, undef);
>+    my $issuing_rule = GetIssuingRuleRecord($borrowertype, $itemtype, $branchcode, 'hardduedate')
>+      or return;
>+    return (C4::Dates->new($issuing_rule->{hardduedate}, 'iso'),$issuing_rule->{hardduedatecompare});
>+}
>+   
>+
>+=head2 GetIssuingRuleValue
>+
>+  my $irule = &GetIssuingRuleValue($borrowertype,$itemtype,$branchcode, $rule_name)
>+
>+Get the most specific non-null value issuing rule for an itemtype, a borrower type and a branch
>+
>+=cut
>+
>+sub GetIssuingRuleValue {
>+    my ( $borrowertype, $itemtype, $branchcode, $rule ) = @_;
>+    my $irule = GetIssuingRuleRecord( $borrowertype, $itemtype, $branchcode, $rule )
>+      or return;
>+    return $irule->{$rule};
> }
> 
>-=head2 GetIssuingRule
>+=head2 GetIssuingRuleRecord
> 
>-  my $irule = &GetIssuingRule($borrowertype,$itemtype,branchcode)
>+  my $irule = &GetIssuingRuleRecord($borrowertype,$itemtype,$branchcode, $rule_name)
> 
>-FIXME - This is a copy-paste of GetLoanLength
>-as a stop-gap.  Do not wish to change API for GetLoanLength 
>-this close to release, however, Overdues::GetIssuingRules is broken.
>+Get the most specific non-null value issuing rule for an itemtype, a borrower type and a branch
>+If $rule_name is specified, only records with $rule_name not null are taken in account
> 
>-Get the issuing rule for an itemtype, a borrower type and a branch
>-Returns a hashref from the issuingrules table.
>+Order of preference is $branchcode, $borrowertype, $itemtype
> 
> =cut
> 
>-sub GetIssuingRule {
>-    my ( $borrowertype, $itemtype, $branchcode ) = @_;
>-    my $dbh = C4::Context->dbh;
>-    my $sth =  $dbh->prepare( "select * from issuingrules where categorycode=? and itemtype=? and branchcode=? and issuelength is not null"  );
>-    my $irule;
>+sub GetIssuingRuleRecord {
>+    my ( $borrowertype, $itemtype, $branchcode, $rule ) = @_;
>+
>+    my $irules = GetIssuingRules($borrowertype,$itemtype,$branchcode)
>+      or return;
>+
>+    return unless @$irules;
>+
>+    return $irules->[0] unless $rule;
>+
>+    die qq{Invalid rule (field) "$rule"} unless exists $irules->[0]->{$rule};
> 
>-	$sth->execute( $borrowertype, $itemtype, $branchcode );
>-    $irule = $sth->fetchrow_hashref;
>-    return $irule if defined($irule) ;
>+    foreach (@$irules) {
>+        return $_ if $_->{$rule}; # XXX defined $_->{$rule}
>+    }
> 
>-    $sth->execute( $borrowertype, "*", $branchcode );
>-    $irule = $sth->fetchrow_hashref;
>-    return $irule if defined($irule) ;
>+    return;
>+}
> 
>-    $sth->execute( "*", $itemtype, $branchcode );
>-    $irule = $sth->fetchrow_hashref;
>-    return $irule if defined($irule) ;
>+=head2 GetIssuingRules
> 
>-    $sth->execute( "*", "*", $branchcode );
>-    $irule = $sth->fetchrow_hashref;
>-    return $irule if defined($irule) ;
>+  my $irules = &GetIssuingRules($borrowertype,$itemtype,$branchcode)
> 
>-    $sth->execute( $borrowertype, $itemtype, "*" );
>-    $irule = $sth->fetchrow_hashref;
>-    return $irule if defined($irule) ;
>+Get issuing all rule records for an itemtype, a borrower type and a branch
>+in order of preference
> 
>-    $sth->execute( $borrowertype, "*", "*" );
>-    $irule = $sth->fetchrow_hashref;
>-    return $irule if defined($irule) ;
>+Order of preference is $branchcode, $borrowertype, $itemtype
> 
>-    $sth->execute( "*", $itemtype, "*" );
>-    $irule = $sth->fetchrow_hashref;
>-    return $irule if defined($irule) ;
>+=cut
> 
>-    $sth->execute( "*", "*", "*" );
>-    $irule = $sth->fetchrow_hashref;
>-    return $irule if defined($irule) ;
>+sub GetIssuingRules {
>+    my ( $borrowertype, $itemtype, $branchcode ) = @_;
>+
>+    my (@where, @params);
> 
>-    # if no rule matches,
>-    return undef;
>+    if ( $borrowertype ) {
>+        push @where, "categorycode=? OR categorycode='*'";
>+        push @params, $borrowertype;
>+    } else {
>+        push @where, "categorycode='*'";
>+    }
>+
>+    if ( $itemtype ) {
>+        push @where, "itemtype=? OR itemtype='*'";
>+        push @params, $itemtype;
>+    } else {
>+        push @where, "itemtype='*'";
>+    }
>+
>+    if ( $branchcode ) {
>+        push @where, "branchcode=? OR branchcode='*'";
>+        push @params, $branchcode;
>+    } else {
>+        push @where, "branchcode='*'";
>+    }
>+
>+    my $qry = "select * from issuingrules where "
>+            .join( ' AND ', map "($_)", @where )
>+            ." order by branchcode desc, categorycode desc, itemtype desc";
>+
>+    my $dbh = C4::Context->dbh;
>+    return  $dbh->selectall_arrayref($qry, { Slice => {} }, @params);
> }
> 
> =head2 GetBranchBorrowerCircRule
>@@ -2159,47 +2102,37 @@ sub CanBookBeRenewed {
>     
>     my $sthcount = $dbh->prepare("
>                    SELECT 
>-                    borrowers.categorycode, biblioitems.itemtype, issues.renewals, renewalsallowed, $controlbranch
>-                   FROM  issuingrules, 
>-                   issues 
>+                    borrowers.categorycode, biblioitems.itemtype, issues.renewals, $controlbranch
>+                   FROM issues 
>                    LEFT JOIN items USING (itemnumber) 
>                    LEFT JOIN borrowers USING (borrowernumber) 
>                    LEFT JOIN biblioitems USING (biblioitemnumber)
>                    
>                    WHERE
>-                    (issuingrules.categorycode = borrowers.categorycode OR issuingrules.categorycode = '*')
>-                   AND
>-                    (issuingrules.itemtype = $itype OR issuingrules.itemtype = '*')
>-                   AND
>-                    (issuingrules.branchcode = $controlbranch OR issuingrules.branchcode = '*') 
>-                   AND 
>                     borrowernumber = ? 
>                    AND
>                     itemnumber = ?
>-                   ORDER BY
>-                    issuingrules.categorycode desc,
>-                    issuingrules.itemtype desc,
>-                    issuingrules.branchcode desc
>                    LIMIT 1;
>                   ");
> 
>     $sthcount->execute( $borrowernumber, $itemnumber );
>     if ( my $data1 = $sthcount->fetchrow_hashref ) {
>-        
>-        if ( ( $data1->{renewalsallowed} && $data1->{renewalsallowed} > $data1->{renewals} ) || $override_limit ) {
>-            $renewokay = 1;
>-        }
>-        else {
>-			$error="too_many";
>-		}
>-		
>         my ( $resfound, $resrec ) = C4::Reserves::CheckReserves($itemnumber);
>         if ($resfound) {
>             $renewokay = 0;
> 			$error="on_reserve"
>         }
>+        elsif ( $override_limit ) {
>+            $renewokay = 1;
>+        }
>+        elsif ( my $renewalsallowed = GetIssuingRuleValue($data1->{categorycode}, $itype, $data1->{$controlbranch}, 'renewalsallowed') ) {
>+            if ( $renewalsallowed > $data1->{renewals} || $override_limit ) {
>+                $renewokay = 1;
>+            }
>+        }
>+	    $error="too_many" unless $renewokay;
>+	}
> 
>-    }
>     return ($renewokay,$error);
> }
> 
>@@ -2306,8 +2239,6 @@ sub GetRenewCount {
>     my ( $bornum, $itemno ) = @_;
>     my $dbh           = C4::Context->dbh;
>     my $renewcount    = 0;
>-    my $renewsallowed = 0;
>-    my $renewsleft    = 0;
> 
>     my $borrower = C4::Members::GetMemberDetails($bornum);
>     my $item     = GetItem($itemno); 
>@@ -2328,10 +2259,8 @@ sub GetRenewCount {
>     # $item and $borrower should be calculated
>     my $branchcode = _GetCircControlBranch($item, $borrower);
>     
>-    my $issuingrule = GetIssuingRule($borrower->{categorycode}, $item->{itype}, $branchcode);
>-    
>-    $renewsallowed = $issuingrule->{'renewalsallowed'};
>-    $renewsleft    = $renewsallowed - $renewcount;
>+    my $renewsallowed = GetIssuingRuleValue($borrower->{categorycode}, $item->{itype}, $branchcode, 'renewalsallowed') || 0;
>+    my $renewsleft    = $renewsallowed - $renewcount;
>     if($renewsleft < 0){ $renewsleft = 0; }
>     return ( $renewcount, $renewsallowed, $renewsleft );
> }
>@@ -2376,19 +2305,11 @@ sub GetIssuingCharges {
>         $item_type = $item_data->{itemtype};
>         $charge    = $item_data->{rentalcharge};
>         my $branch = C4::Branch::mybranch();
>-        my $discount_query = q|SELECT rentaldiscount,
>-            issuingrules.itemtype, issuingrules.branchcode
>+        my $borr_query = q|SELECT borrowers.categorycode
>             FROM borrowers
>-            LEFT JOIN issuingrules ON borrowers.categorycode = issuingrules.categorycode
>-            WHERE borrowers.borrowernumber = ?
>-            AND (issuingrules.itemtype = ? OR issuingrules.itemtype = '*')
>-            AND (issuingrules.branchcode = ? OR issuingrules.branchcode = '*')|;
>-        my $discount_sth = $dbh->prepare($discount_query);
>-        $discount_sth->execute( $borrowernumber, $item_type, $branch );
>-        my $discount_rules = $discount_sth->fetchall_arrayref({});
>-        if (@{$discount_rules}) {
>-            # We may have multiple rules so get the most specific
>-            my $discount = _get_discount_from_rule($discount_rules, $branch, $item_type);
>+            WHERE borrowers.borrowernumber = ?|;
>+        my $borr_cat = $dbh->selectrow_array($borr_query, undef, $borrowernumber);
>+        if ( my $discount = GetIssuingRuleValue($borr_cat, $item_type, $branch, 'rentaldiscount') ) {
>             $charge = ( $charge * ( 100 - $discount ) ) / 100;
>         }
>     }
>@@ -2397,43 +2318,6 @@ sub GetIssuingCharges {
>     return ( $charge, $item_type );
> }
> 
>-# Select most appropriate discount rule from those returned
>-sub _get_discount_from_rule {
>-    my ($rules_ref, $branch, $itemtype) = @_;
>-    my $discount;
>-
>-    if (@{$rules_ref} == 1) { # only 1 applicable rule use it
>-        $discount = $rules_ref->[0]->{rentaldiscount};
>-        return (defined $discount) ? $discount : 0;
>-    }
>-    # could have up to 4 does one match $branch and $itemtype
>-    my @d = grep { $_->{branchcode} eq $branch && $_->{itemtype} eq $itemtype } @{$rules_ref};
>-    if (@d) {
>-        $discount = $d[0]->{rentaldiscount};
>-        return (defined $discount) ? $discount : 0;
>-    }
>-    # do we have item type + all branches
>-    @d = grep { $_->{branchcode} eq q{*} && $_->{itemtype} eq $itemtype } @{$rules_ref};
>-    if (@d) {
>-        $discount = $d[0]->{rentaldiscount};
>-        return (defined $discount) ? $discount : 0;
>-    }
>-    # do we all item types + this branch
>-    @d = grep { $_->{branchcode} eq $branch && $_->{itemtype} eq q{*} } @{$rules_ref};
>-    if (@d) {
>-        $discount = $d[0]->{rentaldiscount};
>-        return (defined $discount) ? $discount : 0;
>-    }
>-    # so all and all (surely we wont get here)
>-    @d = grep { $_->{branchcode} eq q{*} && $_->{itemtype} eq q{*} } @{$rules_ref};
>-    if (@d) {
>-        $discount = $d[0]->{rentaldiscount};
>-        return (defined $discount) ? $discount : 0;
>-    }
>-    # none of the above
>-    return 0;
>-}
>-
> =head2 AddIssuingCharge
> 
>   &AddIssuingCharge( $itemno, $borrowernumber, $charge )
>diff --git a/C4/Members.pm b/C4/Members.pm
>index dcc168b..8cbfbe2 100644
>--- a/C4/Members.pm
>+++ b/C4/Members.pm
>@@ -660,28 +660,31 @@ sub IsMemberBlocked {
>     my $dbh            = C4::Context->dbh;
> 
>     # does patron have current fine days?
>-	my $strsth=qq{
>-            SELECT
>-            ADDDATE(returndate, finedays * DATEDIFF(returndate,date_due) ) AS blockingdate,
>-            DATEDIFF(ADDDATE(returndate, finedays * DATEDIFF(returndate,date_due)),NOW()) AS blockedcount
>-            FROM old_issues
>-	};
>+	my $strsth=<<EOQ;
>+SELECT
>+issuingrules.itemtype,
>+ADDDATE(returndate, finedays * DATEDIFF(returndate,date_due) ) AS blockingdate,
>+DATEDIFF(ADDDATE(returndate, finedays * DATEDIFF(returndate,date_due)),NOW()) AS blockedcount
>+FROM old_issues
>+LEFT JOIN items ON (items.itemnumber=old_issues.itemnumber)
>+EOQ
>     if(C4::Context->preference("item-level_itypes")){
>-        $strsth.=
>-		qq{ LEFT JOIN items ON (items.itemnumber=old_issues.itemnumber)
>-            LEFT JOIN issuingrules ON (issuingrules.itemtype=items.itype)}
>+        $strsth.=<<EOQ;
>+LEFT JOIN issuingrules ON (issuingrules.itemtype=items.itype OR issuingrules.itemtype='*')
>+EOQ
>     }else{
>-        $strsth .= 
>-		qq{ LEFT JOIN items ON (items.itemnumber=old_issues.itemnumber)
>-            LEFT JOIN biblioitems ON (biblioitems.biblioitemnumber=items.biblioitemnumber)
>-            LEFT JOIN issuingrules ON (issuingrules.itemtype=biblioitems.itemtype) };
>-    }
>-	$strsth.=
>-        qq{ WHERE finedays IS NOT NULL
>-            AND  date_due < returndate
>-            AND borrowernumber = ?
>-            ORDER BY blockingdate DESC, blockedcount DESC
>-            LIMIT 1};
>+        $strsth.=<<EOQ;
>+LEFT JOIN biblioitems ON (biblioitems.biblioitemnumber=items.biblioitemnumber)
>+LEFT JOIN issuingrules ON (issuingrules.itemtype=biblioitems.itemtype OR issuingrules.itemtype='*')
>+EOQ
>+    }
>+	$strsth.=<<EOQ;
>+WHERE finedays IS NOT NULL
>+AND  date_due < returndate
>+AND borrowernumber = ?
>+ORDER BY issuingrules.itemtype DESC, blockingdate DESC, blockedcount DESC
>+LIMIT 1
>+EOQ
> 	my $sth=$dbh->prepare($strsth);
>     $sth->execute($borrowernumber);
>     my $row = $sth->fetchrow_hashref;
>diff --git a/C4/Overdues.pm b/C4/Overdues.pm
>index 28b135c..a8bfe31 100644
>--- a/C4/Overdues.pm
>+++ b/C4/Overdues.pm
>@@ -72,9 +72,6 @@ BEGIN {
> 	push @EXPORT, qw(
>         &GetIssuesIteminfo
> 	);
>-    #
>-	# &GetIssuingRules - delete.
>-	# use C4::Circulation::GetIssuingRule instead.
> 	
> 	# subs to move to Members.pm
> 	push @EXPORT, qw(
>@@ -246,7 +243,6 @@ sub CalcFine {
> 	my $daystocharge;
> 	# get issuingrules (fines part will be used)
>     $debug and warn sprintf("CalcFine calling GetIssuingRule(%s, %s, %s)", $bortype, $item->{'itemtype'}, $branchcode);
>-    my $data = C4::Circulation::GetIssuingRule($bortype, $item->{'itemtype'}, $branchcode);
> 	if($difference) {
> 		# if $difference is supplied, the difference has already been calculated, but we still need to adjust for the calendar.
>     	# use copy-pasted functions from calendar module.  (deprecated -- these functions will be removed from C4::Overdues ).
>@@ -264,6 +260,7 @@ sub CalcFine {
> 		}
> 	}
> 	# correct for grace period.
>+    my $data = C4::Circulation::GetIssuingRuleRecord($bortype, $item->{'itemtype'}, $branchcode);
> 	my $days_minus_grace = $daystocharge - $data->{'firstremind'};
>     if ($data->{'chargeperiod'} > 0 && $days_minus_grace > 0 ) { 
>         $amount = int($daystocharge / $data->{'chargeperiod'}) * $data->{'fine'};
>@@ -624,41 +621,6 @@ sub GetFine {
>     return ( $data->{'sum(amountoutstanding)'} );
> }
> 
>-
>-=head2 GetIssuingRules
>-
>-FIXME - This sub should be deprecated and removed.
>-It ignores branch and defaults.
>-
>-    $data = &GetIssuingRules($itemtype,$categorycode);
>-
>-Looks up for all issuingrules an item info 
>-
>-C<$itemnumber> is a reference-to-hash whose keys are all of the fields
>-from the borrowers and categories tables of the Koha database. Thus,
>-
>-C<$categorycode> contains  information about borrowers category 
>-
>-C<$data> contains all information about both the borrower and
>-category he or she belongs to.
>-=cut 
>-
>-sub GetIssuingRules {
>-	warn "GetIssuingRules is deprecated: use GetIssuingRule from C4::Circulation instead.";
>-   my ($itemtype,$categorycode)=@_;
>-   my $dbh   = C4::Context->dbh();    
>-   my $query=qq|SELECT *
>-        FROM issuingrules
>-        WHERE issuingrules.itemtype=?
>-            AND issuingrules.categorycode=?
>-        |;
>-    my $sth = $dbh->prepare($query);
>-    #  print $query;
>-    $sth->execute($itemtype,$categorycode);
>-    return $sth->fetchrow_hashref;
>-}
>-
>-
> sub ReplacementCost2 {
>     my ( $itemnum, $borrowernumber ) = @_;
>     my $dbh   = C4::Context->dbh();
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 8373840..d9b1ea1 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -410,18 +410,6 @@ sub CanItemBeReserved{
>     my $item     = GetItem($itemnumber);
>     my $borrower = C4::Members::GetMember('borrowernumber'=>$borrowernumber);     
>     
>-    # we retrieve user rights on this itemtype and branchcode
>-    my $sth = $dbh->prepare("SELECT categorycode, itemtype, branchcode, reservesallowed 
>-                             FROM issuingrules 
>-                             WHERE (categorycode in (?,'*') ) 
>-                             AND (itemtype IN (?,'*')) 
>-                             AND (branchcode IN (?,'*')) 
>-                             ORDER BY 
>-                               categorycode DESC, 
>-                               itemtype     DESC, 
>-                               branchcode   DESC;"
>-                           );
>-                           
>     my $querycount ="SELECT 
>                             count(*) as count
>                             FROM reserves
>@@ -446,8 +434,7 @@ sub CanItemBeReserved{
>     }
>     
>     # we retrieve rights 
>-    $sth->execute($categorycode, $itemtype, $branchcode);
>-    if(my $rights = $sth->fetchrow_hashref()){
>+    if(my $rights = C4::Circulation::GetIssuingRuleRecord($categorycode, $itemtype, $branchcode, 'reservesallowed')){
>         $itemtype        = $rights->{itemtype};
>         $allowedreserves = $rights->{reservesallowed}; 
>     }else{
>diff --git a/t/db_dependent/lib/KohaTest/Circulation.pm b/t/db_dependent/lib/KohaTest/Circulation.pm
>index 7d5e69d..59238b1 100644
>--- a/t/db_dependent/lib/KohaTest/Circulation.pm
>+++ b/t/db_dependent/lib/KohaTest/Circulation.pm
>@@ -20,7 +20,9 @@ sub methods : Test( 1 ) {
>                       CanBookBeIssued 
>                       AddIssue 
>                       GetLoanLength 
>-                      GetIssuingRule 
>+                      GetIssuingRules
>+                      GetIssuingRuleRecord
>+                      GetIssuingRuleValue
>                       GetBranchBorrowerCircRule
>                       AddReturn 
>                       MarkIssueReturned 
>diff --git a/t/db_dependent/lib/KohaTest/Overdues.pm b/t/db_dependent/lib/KohaTest/Overdues.pm
>index 949c670..02b907e 100644
>--- a/t/db_dependent/lib/KohaTest/Overdues.pm
>+++ b/t/db_dependent/lib/KohaTest/Overdues.pm
>@@ -23,7 +23,6 @@ sub methods : Test( 1 ) {
>                        BorType 
>                        ReplacementCost 
>                        GetFine 
>-                       GetIssuingRules 
>                        ReplacementCost2 
>                        GetNextIdNotify 
>                        NumberNotifyId
>-- 
>1.6.5
>

Actions: View | Diff | Splinter Review

Attachments on bug 4530: 5329 | 6656 | 6915 | 6985 | 6986 | 7482