Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules
authorJesse Weaver <pianohacker@gmail.com>
Thu, 14 Sep 2017 19:32:26 +0000 (13:32 -0600)
committerMartin Renvoize <martin.renvoize@ptfs-europe.com>
Tue, 4 Feb 2020 09:56:24 +0000 (09:56 +0000)
This necessitates moving the circ rules from using '*' to using
undef/NULL.

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

33 files changed:
C4/Circulation.pm
C4/Reserves.pm
Koha/CirculationRules.pm
Koha/REST/V1/CirculationRules.pm [new file with mode: 0644]
admin/smart-rules.pl
api/v1/swagger/definitions.json
api/v1/swagger/definitions/circ-rule-kind.json [new file with mode: 0644]
api/v1/swagger/paths.json
api/v1/swagger/paths/circulation-rules.json [new file with mode: 0644]
koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
t/db_dependent/ArticleRequests.t
t/db_dependent/Circulation.t
t/db_dependent/Circulation/Branch.t
t/db_dependent/Circulation/CalcFine.t
t/db_dependent/Circulation/GetHardDueDate.t
t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t
t/db_dependent/Circulation/Returns.t
t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
t/db_dependent/Circulation/TooMany.t
t/db_dependent/Circulation/issue.t
t/db_dependent/DecreaseLoanHighHolds.t
t/db_dependent/Fines.t
t/db_dependent/Holds.t
t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
t/db_dependent/Holds/HoldFulfillmentPolicy.t
t/db_dependent/Holds/HoldItemtypeLimit.t
t/db_dependent/HoldsQueue.t
t/db_dependent/Koha/IssuingRules.t
t/db_dependent/RefundLostItemFeeRule.t
t/db_dependent/Reserves.t
t/db_dependent/Reserves/MultiplePerRecord.t
t/db_dependent/TestBuilder.t
t/db_dependent/api/v1/holds.t

index d9fad94..fcf48f8 100644 (file)
@@ -425,7 +425,7 @@ sub TooMany {
                                     SELECT itemtype FROM circulation_rules
                                     WHERE branchcode = ?
                                     AND   (categorycode = ? OR categorycode = ?)
-                                    AND   itemtype <> '*'
+                                    AND   itemtype IS NOT NULL
                                     AND   rule_name = 'maxissueqty'
                                   ) ";
             } else {
@@ -434,7 +434,7 @@ sub TooMany {
                                     SELECT itemtype FROM circulation_rules
                                     WHERE branchcode = ?
                                     AND   (categorycode = ? OR categorycode = ?)
-                                    AND   itemtype <> '*'
+                                    AND   itemtype IS NOT NULL
                                     AND   rule_name = 'maxissueqty'
                                   ) ";
             }
@@ -1553,38 +1553,38 @@ sub GetLoanLength {
         },
         {
             categorycode => $categorycode,
-            itemtype     => '*',
+            itemtype     => undef,
             branchcode   => $branchcode,
         },
         {
-            categorycode => '*',
+            categorycode => undef,
             itemtype     => $itemtype,
             branchcode   => $branchcode,
         },
         {
-            categorycode => '*',
-            itemtype     => '*',
+            categorycode => undef,
+            itemtype     => undef,
             branchcode   => $branchcode,
         },
         {
             categorycode => $categorycode,
             itemtype     => $itemtype,
-            branchcode   => '*',
+            branchcode   => undef,
         },
         {
             categorycode => $categorycode,
-            itemtype     => '*',
-            branchcode   => '*',
+            itemtype     => undef,
+            branchcode   => undef,
         },
         {
-            categorycode => '*',
+            categorycode => undef,
             itemtype     => $itemtype,
-            branchcode   => '*',
+            branchcode   => undef,
         },
         {
-            categorycode => '*',
-            itemtype     => '*',
-            branchcode   => '*',
+            categorycode => undef,
+            itemtype     => undef,
+            branchcode   => undef,
         },
     );
 
index 39e61a9..0dca5e7 100644 (file)
@@ -384,7 +384,7 @@ sub CanItemBeReserved {
         $holds_per_day    = $rights->{holds_per_day};
     }
     else {
-        $ruleitemtype = '*';
+        $ruleitemtype = undef;
     }
 
     my $holds = Koha::Holds->search(
@@ -419,15 +419,15 @@ sub CanItemBeReserved {
       C4::Context->preference('item-level_itypes')
       ? " AND COALESCE( items.itype, biblioitems.itemtype ) = ?"
       : " AND biblioitems.itemtype = ?"
-      if ( $ruleitemtype ne "*" );
+      if defined $ruleitemtype;
 
     my $sthcount = $dbh->prepare($querycount);
 
-    if ( $ruleitemtype eq "*" ) {
-        $sthcount->execute( $borrowernumber, $branchcode );
+    if ( defined $ruleitemtype ) {
+        $sthcount->execute( $borrowernumber, $branchcode, $ruleitemtype );
     }
     else {
-        $sthcount->execute( $borrowernumber, $branchcode, $ruleitemtype );
+        $sthcount->execute( $borrowernumber, $branchcode );
     }
 
     my $reservecount = "0";
index 82ac198..7f8d36e 100644 (file)
@@ -34,6 +34,128 @@ Koha::CirculationRules - Koha CirculationRule Object set class
 
 =cut
 
+=head3 rule_kinds
+
+This structure describes the possible rules that may be set, and what scopes they can be set at.
+
+Any attempt to set a rule with a nonsensical scope (for instance, setting the C<patron_maxissueqty> for a branchcode and itemtype), is an error.
+
+=cut
+
+our $RULE_KINDS = {
+    refund => {
+        scope => [ 'branchcode' ],
+    },
+
+    patron_maxissueqty => {
+        scope => [ 'branchcode', 'categorycode' ],
+    },
+    patron_maxonsiteissueqty => {
+        scope => [ 'branchcode', 'categorycode' ],
+    },
+    max_holds => {
+        scope => [ 'branchcode', 'categorycode' ],
+    },
+
+    holdallowed => {
+        scope => [ 'branchcode', 'itemtype' ],
+    },
+    hold_fulfillment_policy => {
+        scope => [ 'branchcode', 'itemtype' ],
+    },
+    returnbranch => {
+        scope => [ 'branchcode', 'itemtype' ],
+    },
+
+    article_requests => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    auto_renew => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    cap_fine_to_replacement_price => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    chargeperiod => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    chargeperiod_charge_at => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    fine => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    finedays => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    firstremind => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    hardduedate => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    hardduedatecompare => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    holds_per_record => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    issuelength => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    lengthunit => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    maxissueqty => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    maxonsiteissueqty => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    maxsuspensiondays => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    no_auto_renewal_after => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    no_auto_renewal_after_hard_limit => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    norenewalbefore => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    onshelfholds => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    opacitemholds => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    overduefinescap => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    renewalperiod => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    renewalsallowed => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    rentaldiscount => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    reservesallowed => {
+        scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+    },
+    # Not included (deprecated?):
+    #   * accountsent
+    #   * chargename
+    #   * reservecharge
+    #   * restrictedtype
+};
+
+sub rule_kinds {
+    return $RULE_KINDS;
+}
+
 =head3 get_effective_rule
 
 =cut
@@ -41,9 +163,9 @@ Koha::CirculationRules - Koha CirculationRule Object set class
 sub get_effective_rule {
     my ( $self, $params ) = @_;
 
-    $params->{categorycode} = '*' if exists($params->{categorycode}) && !defined($params->{categorycode});
-    $params->{branchcode}   = '*' if exists($params->{branchcode})   && !defined($params->{branchcode});
-    $params->{itemtype}     = '*' if exists($params->{itemtype})     && !defined($params->{itemtype});
+    $params->{categorycode} //= undef;
+    $params->{branchcode}   //= undef;
+    $params->{itemtype}     //= undef;
 
     my $rule_name    = $params->{rule_name};
     my $categorycode = $params->{categorycode};
@@ -119,6 +241,20 @@ sub set_rule {
         Koha::Exceptions::MissingParameter->throw(
             "Required parameter 'branchcode' missing")
           unless exists $params->{$mandatory_parameter};
+
+    my $kind_info = $RULE_KINDS->{ $params->{rule_name} };
+    croak "set_rule given unknown rule '$params->{rule_name}'!"
+        unless defined $kind_info;
+
+    # Enforce scope; a rule should be set for its defined scope, no more, no less.
+    foreach my $scope_level ( qw( branchcode categorycode itemtype ) ) {
+        if ( grep /$scope_level/, @{ $kind_info->{scope} } ) {
+            croak "set_rule needs '$scope_level' to set '$params->{rule_name}'!"
+                unless exists $params->{$scope_level};
+        } else {
+            croak "set_rule cannot set '$params->{rule_name}' for a '$scope_level'!"
+                if exists $params->{$scope_level};
+        }
     }
 
     my $branchcode   = $params->{branchcode};
@@ -173,18 +309,17 @@ sub set_rule {
 sub set_rules {
     my ( $self, $params ) = @_;
 
-    my $branchcode   = $params->{branchcode};
-    my $categorycode = $params->{categorycode};
-    my $itemtype     = $params->{itemtype};
+    my %set_params;
+    $set_params{branchcode} = $params->{branchcode} if exists $params->{branchcode};
+    $set_params{categorycode} = $params->{categorycode} if exists $params->{categorycode};
+    $set_params{itemtype} = $params->{itemtype} if exists $params->{itemtype};
     my $rules        = $params->{rules};
 
     my $rule_objects = [];
     while ( my ( $rule_name, $rule_value ) = each %$rules ) {
         my $rule_object = Koha::CirculationRules->set_rule(
             {
-                branchcode   => $branchcode,
-                categorycode => $categorycode,
-                itemtype     => $itemtype,
+                %set_params,
                 rule_name    => $rule_name,
                 rule_value   => $rule_value,
             }
diff --git a/Koha/REST/V1/CirculationRules.pm b/Koha/REST/V1/CirculationRules.pm
new file mode 100644 (file)
index 0000000..a04be03
--- /dev/null
@@ -0,0 +1,32 @@
+package Koha::REST::V1::CirculationRules;
+
+# This file is part of Koha.
+#
+# Koha is free software; you can redistribute it and/or modify it under the
+# terms of the GNU General Public License as published by the Free Software
+# Foundation; either version 3 of the License, or (at your option) any later
+# version.
+#
+# Koha is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# 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 Mojo::Base 'Mojolicious::Controller';
+
+use Koha::CirculationRules;
+
+use Try::Tiny;
+
+sub get_kinds {
+    my ( $c, $args, $cb ) = @_;
+
+    return $c->$cb( Koha::CirculationRules->rule_kinds, 200 );
+}
+
+1;
index a8dc91e..67f72e6 100755 (executable)
@@ -81,9 +81,9 @@ if ($op eq 'delete') {
 
     Koha::CirculationRules->set_rules(
         {
-            categorycode => $categorycode,
-            branchcode   => $branch,
-            itemtype     => $itemtype,
+            categorycode => $categorycode eq '*' ? undef : $categorycode,
+            branchcode   => $branch eq '*' ? undef : $branch,
+            itemtype     => $itemtype eq '*' ? undef : $itemtype,
             rules        => {
                 restrictedtype                   => undef,
                 rentaldiscount                   => undef,
@@ -119,44 +119,56 @@ elsif ($op eq 'delete-branch-cat') {
     my $categorycode  = $input->param('categorycode');
     if ($branch eq "*") {
         if ($categorycode eq "*") {
-             Koha::CirculationRules->set_rules(
-                 {
-                     categorycode => undef,
-                     branchcode   => undef,
-                     itemtype     => undef,
-                     rules        => {
-                         patron_maxissueqty             => undef,
-                         patron_maxonsiteissueqty       => undef,
-                         holdallowed             => undef,
-                         hold_fulfillment_policy => undef,
-                         returnbranch            => undef,
-                         max_holds               => undef,
-                     }
-                 }
-             );
-         } else {
-             Koha::CirculationRules->set_rules(
-                 {
-                     categorycode => $categorycode,
-                     branchcode   => undef,
-                     itemtype     => undef,
-                     rules        => {
-                         max_holds         => undef,
-                         patron_maxissueqty       => undef,
-                         patron_maxonsiteissueqty => undef,
-                     }
-                 }
-             );
+            Koha::CirculationRules->set_rules(
+                {
+                    branchcode   => undef,
+                    categorycode => undef,
+                    rules        => {
+                        max_holds                      => undef,
+                        patron_maxissueqty             => undef,
+                        patron_maxonsiteissueqty       => undef,
+                    }
+                }
+            );
+            Koha::CirculationRules->set_rules(
+                {
+                    branchcode   => undef,
+                    itemtype     => undef,
+                    rules        => {
+                        holdallowed             => undef,
+                        hold_fulfillment_policy => undef,
+                        returnbranch            => undef,
+                    }
+                }
+            );
+        } else {
+            Koha::CirculationRules->set_rules(
+                {
+                    categorycode => $categorycode,
+                    branchcode   => undef,
+                    rules        => {
+                        max_holds                => undef,
+                        patron_maxissueqty       => undef,
+                        patron_maxonsiteissueqty => undef,
+                    }
+                }
+            );
         }
     } elsif ($categorycode eq "*") {
         Koha::CirculationRules->set_rules(
             {
+                branchcode   => $branch,
                 categorycode => undef,
+                rules        => {
+                    patron_maxissueqty       => undef,
+                    patron_maxonsiteissueqty => undef,
+                }
+            }
+        );
+        Koha::CirculationRules->set_rules(
+            {
                 branchcode   => $branch,
-                itemtype     => undef,
                 rules        => {
-                    patron_maxissueqty             => undef,
-                    patron_maxonsiteissueqty       => undef,
                     holdallowed             => undef,
                     hold_fulfillment_policy => undef,
                     returnbranch            => undef,
@@ -169,7 +181,6 @@ elsif ($op eq 'delete-branch-cat') {
             {
                 categorycode => $categorycode,
                 branchcode   => $branch,
-                itemtype     => undef,
                 rules        => {
                     max_holds         => undef,
                     patron_maxissueqty       => undef,
@@ -185,12 +196,9 @@ elsif ($op eq 'delete-branch-item') {
         if ($itemtype eq "*") {
             Koha::CirculationRules->set_rules(
                 {
-                    categorycode => undef,
                     branchcode   => undef,
                     itemtype     => undef,
                     rules        => {
-                        patron_maxissueqty             => undef,
-                        patron_maxonsiteissueqty       => undef,
                         holdallowed             => undef,
                         hold_fulfillment_policy => undef,
                         returnbranch            => undef,
@@ -200,7 +208,6 @@ elsif ($op eq 'delete-branch-item') {
         } else {
             Koha::CirculationRules->set_rules(
                 {
-                    categorycode => undef,
                     branchcode   => undef,
                     itemtype     => $itemtype,
                     rules        => {
@@ -214,12 +221,9 @@ elsif ($op eq 'delete-branch-item') {
     } elsif ($itemtype eq "*") {
         Koha::CirculationRules->set_rules(
             {
-                categorycode => undef,
                 branchcode   => $branch,
                 itemtype     => undef,
                 rules        => {
-                    maxissueqty             => undef,
-                    maxonsiteissueqty       => undef,
                     holdallowed             => undef,
                     hold_fulfillment_policy => undef,
                     returnbranch            => undef,
@@ -229,7 +233,6 @@ elsif ($op eq 'delete-branch-item') {
     } else {
         Koha::CirculationRules->set_rules(
             {
-                categorycode => undef,
                 branchcode   => $branch,
                 itemtype     => $itemtype,
                 rules        => {
@@ -325,9 +328,9 @@ elsif ($op eq 'add') {
 
     Koha::CirculationRules->set_rules(
         {
-            categorycode => $bor,
-            itemtype     => $itemtype,
-            branchcode   => $br,
+            categorycode => $bor eq '*' ? undef : $bor,
+            itemtype     => $itemtype eq '*' ? undef : $itemtype,
+            branchcode   => $br eq '*' ? undef : $br,
             rules        => $rules,
         }
     );
@@ -353,30 +356,44 @@ elsif ($op eq "set-branch-defaults") {
     if ($branch eq "*") {
         Koha::CirculationRules->set_rules(
             {
-                categorycode => undef,
                 itemtype     => undef,
                 branchcode   => undef,
                 rules        => {
-                    patron_maxissueqty       => $patron_maxissueqty,
-                    patron_maxonsiteissueqty => $patron_maxonsiteissueqty,
-                    holdallowed              => $holdallowed,
-                    hold_fulfillment_policy  => $hold_fulfillment_policy,
-                    returnbranch             => $returnbranch,
+                    holdallowed             => $holdallowed,
+                    hold_fulfillment_policy => $hold_fulfillment_policy,
+                    returnbranch            => $returnbranch,
                 }
             }
         );
-    } else {
         Koha::CirculationRules->set_rules(
             {
                 categorycode => undef,
+                branchcode   => undef,
+                rules        => {
+                    patron_maxissueqty             => $patron_maxissueqty,
+                    patron_maxonsiteissueqty       => $patron_maxonsiteissueqty,
+                }
+            }
+        );
+    } else {
+        Koha::CirculationRules->set_rules(
+            {
                 itemtype     => undef,
                 branchcode   => $branch,
                 rules        => {
-                    patron_maxissueqty       => $patron_maxissueqty,
-                    patron_maxonsiteissueqty => $patron_maxonsiteissueqty,
-                    holdallowed              => $holdallowed,
-                    hold_fulfillment_policy  => $hold_fulfillment_policy,
-                    returnbranch             => $returnbranch,
+                    holdallowed             => $holdallowed,
+                    hold_fulfillment_policy => $hold_fulfillment_policy,
+                    returnbranch            => $returnbranch,
+                }
+            }
+        );
+        Koha::CirculationRules->set_rules(
+            {
+                categorycode => undef,
+                branchcode   => $branch,
+                rules        => {
+                    patron_maxissueqty             => $patron_maxissueqty,
+                    patron_maxonsiteissueqty       => $patron_maxonsiteissueqty,
                 }
             }
         );
@@ -408,7 +425,6 @@ elsif ($op eq "add-branch-cat") {
             Koha::CirculationRules->set_rules(
                 {
                     categorycode => undef,
-                    itemtype     => undef,
                     branchcode   => undef,
                     rules        => {
                         max_holds         => $max_holds,
@@ -420,9 +436,8 @@ elsif ($op eq "add-branch-cat") {
         } else {
             Koha::CirculationRules->set_rules(
                 {
-                    branchcode   => undef,
                     categorycode => $categorycode,
-                    itemtype     => undef,
+                    branchcode   => undef,
                     rules        => {
                         max_holds         => $max_holds,
                         patron_maxissueqty       => $patron_maxissueqty,
@@ -435,7 +450,6 @@ elsif ($op eq "add-branch-cat") {
         Koha::CirculationRules->set_rules(
             {
                 categorycode => undef,
-                itemtype     => undef,
                 branchcode   => $branch,
                 rules        => {
                     max_holds         => $max_holds,
@@ -448,7 +462,6 @@ elsif ($op eq "add-branch-cat") {
         Koha::CirculationRules->set_rules(
             {
                 categorycode => $categorycode,
-                itemtype     => undef,
                 branchcode   => $branch,
                 rules        => {
                     max_holds         => $max_holds,
@@ -472,7 +485,6 @@ elsif ($op eq "add-branch-item") {
         if ($itemtype eq "*") {
             Koha::CirculationRules->set_rules(
                 {
-                    categorycode => undef,
                     itemtype     => undef,
                     branchcode   => undef,
                     rules        => {
@@ -485,7 +497,6 @@ elsif ($op eq "add-branch-item") {
         } else {
             Koha::CirculationRules->set_rules(
                 {
-                    categorycode => undef,
                     itemtype     => $itemtype,
                     branchcode   => undef,
                     rules        => {
@@ -499,7 +510,6 @@ elsif ($op eq "add-branch-item") {
     } elsif ($itemtype eq "*") {
             Koha::CirculationRules->set_rules(
                 {
-                    categorycode => undef,
                     itemtype     => undef,
                     branchcode   => $branch,
                     rules        => {
@@ -512,7 +522,6 @@ elsif ($op eq "add-branch-item") {
     } else {
         Koha::CirculationRules->set_rules(
             {
-                categorycode => undef,
                 itemtype     => $itemtype,
                 branchcode   => $branch,
                 rules        => {
@@ -533,8 +542,6 @@ elsif ( $op eq 'mod-refund-lost-item-fee-rule' ) {
             # only do something for $refund eq '*' if branch-specific
             Koha::CirculationRules->set_rules(
                 {
-                    categorycode => undef,
-                    itemtype     => undef,
                     branchcode   => $branch,
                     rules        => {
                         refund => undef
@@ -545,9 +552,7 @@ elsif ( $op eq 'mod-refund-lost-item-fee-rule' ) {
     } else {
         Koha::CirculationRules->set_rules(
             {
-                categorycode => undef,
-                itemtype     => undef,
-                branchcode   => $branch,
+                branchcode   => undef,
                 rules        => {
                     refund => $refund
                 }
@@ -556,8 +561,7 @@ elsif ( $op eq 'mod-refund-lost-item-fee-rule' ) {
     }
 }
 
-my $refundLostItemFeeRule = Koha::RefundLostItemFeeRules->find({ branchcode => ($branch eq '*') ? undef:$branch });
-
+my $refundLostItemFeeRule = Koha::RefundLostItemFeeRules->find({ branchcode => ($branch eq '*') ? undef : $branch });
 $template->param(
     refundLostItemFeeRule => $refundLostItemFeeRule,
     defaultRefundRule     => Koha::RefundLostItemFeeRules->_default_rule
@@ -572,7 +576,7 @@ $template->param(show_branch_cat_rule_form => 1);
 $template->param(
     patron_categories => $patron_categories,
     itemtypeloop      => $itemtypes,
-    humanbranch       => ( $branch ne '*' ? $branch : '' ),
+    humanbranch       => ( $branch ne '*' ? $branch : undef ),
     current_branch    => $branch,
 );
 output_html_with_http_headers $input, $cookie, $template->output;
index 40e10c9..a27fa0d 100644 (file)
@@ -5,6 +5,9 @@
   "basket": {
     "$ref": "definitions/basket.json"
   },
+  "circ-rule-kind": {
+    "$ref": "definitions/circ-rule-kind.json"
+  },
   "city": {
     "$ref": "definitions/city.json"
   },
diff --git a/api/v1/swagger/definitions/circ-rule-kind.json b/api/v1/swagger/definitions/circ-rule-kind.json
new file mode 100644 (file)
index 0000000..90b956f
--- /dev/null
@@ -0,0 +1,15 @@
+{
+  "type": "object",
+  "properties": {
+    "scope": {
+      "description": "levels that this rule kind can be set for",
+      "type": "array",
+      "items": {
+        "type": "string",
+        "enum": [ "branchcode", "categorycode", "itemtype" ]
+      }
+    }
+  },
+  "additionalProperties": false,
+  "required": ["scope"]
+}
index ecf357a..ed9ed06 100644 (file)
@@ -26,6 +26,9 @@
   "/checkouts/{checkout_id}/renewal": {
     "$ref": "paths/checkouts.json#/~1checkouts~1{checkout_id}~1renewal"
   },
+  "/circulation-rules/kinds": {
+    "$ref": "paths/circulation-rules.json#/~1circulation-rules~1kinds"
+  },
   "/cities": {
     "$ref": "paths/cities.json#/~1cities"
   },
diff --git a/api/v1/swagger/paths/circulation-rules.json b/api/v1/swagger/paths/circulation-rules.json
new file mode 100644 (file)
index 0000000..e897cd7
--- /dev/null
@@ -0,0 +1,35 @@
+{
+  "/circulation-rules/kinds": {
+    "get": {
+      "x-mojo-controller": "Koha::REST::V1::CirculationRules",
+      "operationId": "get_kinds",
+      "tags": ["cities"],
+      "produces": [
+        "application/json"
+      ],
+      "responses": {
+        "200": {
+          "description": "A map of rule kind information",
+          "schema": {
+            "type": "object",
+            "additionalProperties": {
+              "$ref": "../definitions.json#/circ-rule-kind"
+            }
+          }
+        },
+        "403": {
+          "description": "Access forbidden",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        },
+        "500": {
+          "description": "Internal error",
+          "schema": {
+            "$ref": "../definitions.json#/error"
+          }
+        }
+      }
+    }
+  }
+}
index 738c2ad..7ad46a2 100644 (file)
@@ -8,19 +8,19 @@
 [% USE CirculationRules %]
 [% SET footerjs = 1 %]
 
-[% SET branchcode = humanbranch || '*' %]
+[% SET branchcode = humanbranch || undef %]
 
 [% SET categorycodes = [] %]
 [% FOREACH pc IN patron_categories %]
     [% categorycodes.push( pc.id ) %]
 [% END %]
-[% categorycodes.push('*') %]
+[% categorycodes.push(undef) %]
 
 [% SET itemtypes = [] %]
 [% FOREACH i IN itemtypeloop %]
     [% itemtypes.push( i.itemtype ) %]
 [% END %]
-[% itemtypes.push('*') %]
+[% itemtypes.push(undef) %]
 
 [% INCLUDE 'doc-head-open.inc' %]
 <title>Koha &rsaquo; Administration &rsaquo; Circulation and fine rules</title>
                             [% SET row_count = row_count + 1 %]
                             <tr row_countd="row_[% row_count %]">
                                     <td>
-                                        [% IF c == '*' %]
+                                        [% IF c == undef %]
                                             <em>All</em>
                                         [% ELSE %]
                                             [% Categories.GetName(c) %]
                                         [% END %]
                                     </td>
                                     <td>
-                                        [% IF i == '*' %]
+                                        [% IF i == undef %]
                                             <em>All</em>
                                         [% ELSE %]
                                             [% ItemTypes.GetDescription(i) %]
                                         [% END %]
                                     </td>
+                                    <td class="actions">
+                                      <a href="#" class="editrule btn btn-default btn-xs"><i class="fa fa-pencil"></i> Edit</a>
+                                      <a class="btn btn-default btn-xs delete" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete&amp;itemtype=[% rule.itemtype || '*' %]&amp;categorycode=[% rule.categorycode || '*' %]&amp;branch=[% current_branch %]"><i class="fa fa-trash"></i> Delete</a>
+                                    </td>
                                     <td>
                                         [% IF rule.note %]
                                             <a name="viewnote" data-toggle="popover" title="Note" data-content="[% rule.note | html %]" data-placement="top" data-trigger="hover">View note</a>
                                     <td>[% rentaldiscount %]</td>
                                     <td class="actions">
                                       <a href="#" class="editrule btn btn-default btn-xs"><i class="fa fa-pencil"></i> Edit</a>
-                                      <a class="btn btn-default btn-xs delete" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete&amp;itemtype=[% rule.itemtype | uri %]&amp;categorycode=[% rule.categorycode | uri %]&amp;branch=[% rule.current_branch | uri %]"><i class="fa fa-trash"></i> Delete</a>
+                                      <a class="btn btn-default btn-xs delete" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete&amp;itemtype=[% rule.itemtype || '*' | uri %]&amp;categorycode=[% rule.categorycode || '*' | uri %]&amp;branch=[% rule.current_branch | uri %]"><i class="fa fa-trash"></i> Delete</a>
                                     </td>
                             </tr>
                         [% END %]
                     <th>&nbsp;</th>
                 </tr>
                 [% FOREACH c IN categorycodes %]
+                    [% NEXT UNLESS c %]
                     [% SET patron_maxissueqty = CirculationRules.Search( branchcode, c, undef, 'patron_maxissueqty' ) %]
                     [% SET patron_maxonsiteissueqty = CirculationRules.Search( branchcode, c, undef, 'patron_maxonsiteissueqty' ) %]
                     [% SET max_holds = CirculationRules.Search( branchcode, c, undef, 'max_holds' ) %]
                     [% IF  ( patron_maxissueqty.defined && patron_maxissueqty != '' ) || ( patron_maxonsiteissueqty.defined && patron_maxonsiteissueqty != '' ) || ( max_holds.defined && max_holds != '' ) %]
                     <tr>
                         <td>
-                            [% IF c == '*'%]
+                            [% IF c == undef %]
                                 <em>Default</em>
                             [% ELSE %]
                                 [% Categories.GetName(c) | html %]
index 0acc9eb..f9aee41 100755 (executable)
@@ -174,9 +174,9 @@ is( $biblio->article_requests_finished()->count(), 1, 'Canceled request not retu
 
 my $rule = Koha::CirculationRules->set_rule(
     {
-        categorycode => '*',
-        itemtype     => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        itemtype     => undef,
+        branchcode   => undef,
         rule_name    => 'article_requests',
         rule_value   => 'yes',
     }
@@ -189,9 +189,9 @@ $rule->delete();
 
 $rule = Koha::CirculationRules->set_rule(
     {
-        categorycode => '*',
-        itemtype     => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        itemtype     => undef,
+        branchcode   => undef,
         rule_name    => 'article_requests',
         rule_value   => 'bib_only',
     }
@@ -204,9 +204,9 @@ $rule->delete();
 
 $rule = Koha::CirculationRules->set_rule(
     {
-        categorycode => '*',
-        itemtype     => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        itemtype     => undef,
+        branchcode   => undef,
         rule_name    => 'article_requests',
         rule_value   => 'item_only',
     }
@@ -219,9 +219,9 @@ $rule->delete();
 
 $rule = Koha::CirculationRules->set_rule(
     {
-        categorycode => '*',
-        itemtype     => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        itemtype     => undef,
+        branchcode   => undef,
         rule_name    => 'article_requests',
         rule_value   => 'no',
     }
index a71bb1b..cebe7dc 100755 (executable)
@@ -20,6 +20,7 @@ use utf8;
 
 use Test::More tests => 46;
 use Test::MockModule;
+use Test::Deep qw( cmp_deeply );
 
 use Data::Dumper;
 use DateTime;
@@ -254,9 +255,9 @@ is(
 $dbh->do('DELETE FROM circulation_rules');
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        branchcode   => '*',
-        itemtype     => '*',
+        categorycode => undef,
+        branchcode   => undef,
+        itemtype     => undef,
         rules        => {
             reservesallowed => 25,
             issuelength     => 14,
@@ -392,9 +393,9 @@ subtest "CanBookBeRenewed tests" => sub {
     # Testing of feature to allow the renewal of reserved items if other items on the record can fill all needed holds
     Koha::CirculationRules->set_rule(
         {
-            categorycode => '*',
-            branchcode   => '*',
-            itemtype     => '*',
+            categorycode => undef,
+            branchcode   => undef,
+            itemtype     => undef,
             rule_name    => 'onshelfholds',
             rule_value   => '1',
         }
@@ -615,9 +616,9 @@ subtest "CanBookBeRenewed tests" => sub {
     # Test premature manual renewal
     Koha::CirculationRules->set_rule(
         {
-            categorycode => '*',
-            branchcode   => '*',
-            itemtype     => '*',
+            categorycode => undef,
+            branchcode   => undef,
+            itemtype     => undef,
             rule_name    => 'norenewalbefore',
             rule_value   => '7',
         }
@@ -692,9 +693,9 @@ subtest "CanBookBeRenewed tests" => sub {
 
         Koha::CirculationRules->set_rules(
             {
-                categorycode => '*',
-                branchcode   => '*',
-                itemtype     => '*',
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
                 rules        => {
                     norenewalbefore       => '7',
                     no_auto_renewal_after => '9',
@@ -708,9 +709,9 @@ subtest "CanBookBeRenewed tests" => sub {
 
         Koha::CirculationRules->set_rules(
             {
-                categorycode => '*',
-                branchcode   => '*',
-                itemtype     => '*',
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
                 rules        => {
                     norenewalbefore       => '7',
                     no_auto_renewal_after => '10',
@@ -724,9 +725,9 @@ subtest "CanBookBeRenewed tests" => sub {
 
         Koha::CirculationRules->set_rules(
             {
-                categorycode => '*',
-                branchcode   => '*',
-                itemtype     => '*',
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
                 rules        => {
                     norenewalbefore       => '7',
                     no_auto_renewal_after => '11',
@@ -740,9 +741,9 @@ subtest "CanBookBeRenewed tests" => sub {
 
         Koha::CirculationRules->set_rules(
             {
-                categorycode => '*',
-                branchcode   => '*',
-                itemtype     => '*',
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
                 rules        => {
                     norenewalbefore       => '10',
                     no_auto_renewal_after => '11',
@@ -756,9 +757,9 @@ subtest "CanBookBeRenewed tests" => sub {
 
         Koha::CirculationRules->set_rules(
             {
-                categorycode => '*',
-                branchcode   => '*',
-                itemtype     => '*',
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
                 rules        => {
                     norenewalbefore       => '10',
                     no_auto_renewal_after => undef,
@@ -773,9 +774,9 @@ subtest "CanBookBeRenewed tests" => sub {
 
         Koha::CirculationRules->set_rules(
             {
-                categorycode => '*',
-                branchcode   => '*',
-                itemtype     => '*',
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
                 rules        => {
                     norenewalbefore       => '7',
                     no_auto_renewal_after => '15',
@@ -790,9 +791,9 @@ subtest "CanBookBeRenewed tests" => sub {
 
         Koha::CirculationRules->set_rules(
             {
-                categorycode => '*',
-                branchcode   => '*',
-                itemtype     => '*',
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
                 rules        => {
                     norenewalbefore       => '10',
                     no_auto_renewal_after => undef,
@@ -823,9 +824,9 @@ subtest "CanBookBeRenewed tests" => sub {
 
         Koha::CirculationRules->set_rules(
             {
-                categorycode => '*',
-                branchcode   => '*',
-                itemtype     => '*',
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
                 rules        => {
                     norenewalbefore       => '10',
                     no_auto_renewal_after => '11',
@@ -971,9 +972,9 @@ subtest "CanBookBeRenewed tests" => sub {
         AddIssue( $renewing_borrower, $item_to_auto_renew->{barcode}, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } );
         Koha::CirculationRules->set_rules(
             {
-                categorycode => '*',
-                branchcode   => '*',
-                itemtype     => '*',
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
                 rules        => {
                     norenewalbefore       => '7',
                     no_auto_renewal_after => '',
@@ -986,9 +987,9 @@ subtest "CanBookBeRenewed tests" => sub {
         my $five_days_before = dt_from_string->add( days => -5 );
         Koha::CirculationRules->set_rules(
             {
-                categorycode => '*',
-                branchcode   => '*',
-                itemtype     => '*',
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
                 rules        => {
                     norenewalbefore       => '10',
                     no_auto_renewal_after => '5',
@@ -1007,9 +1008,9 @@ subtest "CanBookBeRenewed tests" => sub {
         $dbh->do(q{UPDATE circulation_rules SET rule_value = NULL WHERE rule_name = 'no_auto_renewal_after_hard_limit'});
         Koha::CirculationRules->set_rules(
             {
-                categorycode => '*',
-                branchcode   => '*',
-                itemtype     => '*',
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
                 rules        => {
                     norenewalbefore       => '10',
                     no_auto_renewal_after => '15',
@@ -1025,9 +1026,9 @@ subtest "CanBookBeRenewed tests" => sub {
         my $two_days_ahead = dt_from_string->add( days => 2 );
         Koha::CirculationRules->set_rules(
             {
-                categorycode => '*',
-                branchcode   => '*',
-                itemtype     => '*',
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
                 rules        => {
                     norenewalbefore       => '10',
                     no_auto_renewal_after => '',
@@ -1042,9 +1043,9 @@ subtest "CanBookBeRenewed tests" => sub {
         );
         Koha::CirculationRules->set_rules(
             {
-                categorycode => '*',
-                branchcode   => '*',
-                itemtype     => '*',
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
                 rules        => {
                     norenewalbefore       => '10',
                     no_auto_renewal_after => '15',
@@ -1064,9 +1065,9 @@ subtest "CanBookBeRenewed tests" => sub {
     # set policy to forbid renewals
     Koha::CirculationRules->set_rules(
         {
-            categorycode => '*',
-            branchcode   => '*',
-            itemtype     => '*',
+            categorycode => undef,
+            branchcode   => undef,
+            itemtype     => undef,
             rules        => {
                 norenewalbefore => undef,
                 renewalsallowed => 0,
@@ -1310,9 +1311,9 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub {
     $dbh->do('DELETE FROM circulation_rules');
     Koha::CirculationRules->set_rules(
         {
-            categorycode => '*',
-            itemtype     => '*',
-            branchcode   => '*',
+            categorycode => undef,
+            itemtype     => undef,
+            branchcode   => undef,
             rules        => {
                 reservesallowed => 25,
                 issuelength     => 14,
@@ -1374,9 +1375,9 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub {
 
     Koha::CirculationRules->set_rules(
         {
-            categorycode => '*',
-            itemtype     => '*',
-            branchcode   => '*',
+            categorycode => undef,
+            itemtype     => undef,
+            branchcode   => undef,
             rules        => {
                 onshelfholds => 0,
             }
@@ -1388,9 +1389,9 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub {
 
     Koha::CirculationRules->set_rules(
         {
-            categorycode => '*',
-            itemtype     => '*',
-            branchcode   => '*',
+            categorycode => undef,
+            itemtype     => undef,
+            branchcode   => undef,
             rules        => {
                 onshelfholds => 0,
             }
@@ -1402,9 +1403,9 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub {
 
     Koha::CirculationRules->set_rules(
         {
-            categorycode => '*',
-            itemtype     => '*',
-            branchcode   => '*',
+            categorycode => undef,
+            itemtype     => undef,
+            branchcode   => undef,
             rules        => {
                 onshelfholds => 1,
             }
@@ -1416,9 +1417,9 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub {
 
     Koha::CirculationRules->set_rules(
         {
-            categorycode => '*',
-            itemtype     => '*',
-            branchcode   => '*',
+            categorycode => undef,
+            itemtype     => undef,
+            branchcode   => undef,
             rules        => {
                 onshelfholds => 1,
             }
@@ -1856,23 +1857,39 @@ subtest 'CanBookBeIssued + AllowMultipleIssuesOnABiblio' => sub {
 
     t::lib::Mocks::mock_preference('AllowMultipleIssuesOnABiblio', 0);
     ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} );
-    is( keys(%$error) + keys(%$alerts),  0, 'No error or alert should be raised' . str($error, $question, $alerts) );
-    is( $question->{BIBLIO_ALREADY_ISSUED}, 1, 'BIBLIO_ALREADY_ISSUED question flag should be set if AllowMultipleIssuesOnABiblio=0 and issue already exists' . str($error, $question, $alerts) );
+    cmp_deeply(
+        { error => $error, alerts => $alerts },
+        { error => {}, alerts => {} },
+        'No error or alert should be raised'
+    );
+    is( $question->{BIBLIO_ALREADY_ISSUED}, 1, 'BIBLIO_ALREADY_ISSUED question flag should be set if AllowMultipleIssuesOnABiblio=0 and issue already exists' );
 
     t::lib::Mocks::mock_preference('AllowMultipleIssuesOnABiblio', 1);
     ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} );
-    is( keys(%$error) + keys(%$question) + keys(%$alerts),  0, 'No BIBLIO_ALREADY_ISSUED flag should be set if AllowMultipleIssuesOnABiblio=1' . str($error, $question, $alerts) );
+    cmp_deeply(
+        { error => $error, question => $question, alerts => $alerts },
+        { error => {}, question => {}, alerts => {} },
+        'No BIBLIO_ALREADY_ISSUED flag should be set if AllowMultipleIssuesOnABiblio=1'
+    );
 
     # Add a subscription
     Koha::Subscription->new({ biblionumber => $biblionumber })->store;
 
     t::lib::Mocks::mock_preference('AllowMultipleIssuesOnABiblio', 0);
     ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} );
-    is( keys(%$error) + keys(%$question) + keys(%$alerts),  0, 'No BIBLIO_ALREADY_ISSUED flag should be set if it is a subscription' . str($error, $question, $alerts) );
+    cmp_deeply(
+        { error => $error, question => $question, alerts => $alerts },
+        { error => {}, question => {}, alerts => {} },
+        'No BIBLIO_ALREADY_ISSUED flag should be set if it is a subscription'
+    );
 
     t::lib::Mocks::mock_preference('AllowMultipleIssuesOnABiblio', 1);
     ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} );
-    is( keys(%$error) + keys(%$question) + keys(%$alerts),  0, 'No BIBLIO_ALREADY_ISSUED flag should be set if it is a subscription' . str($error, $question, $alerts) );
+    cmp_deeply(
+        { error => $error, question => $question, alerts => $alerts },
+        { error => {}, question => {}, alerts => {} },
+        'No BIBLIO_ALREADY_ISSUED flag should be set if it is a subscription'
+    );
 };
 
 subtest 'AddReturn + CumulativeRestrictionPeriods' => sub {
@@ -1904,9 +1921,9 @@ subtest 'AddReturn + CumulativeRestrictionPeriods' => sub {
     Koha::CirculationRules->search->delete;
     Koha::CirculationRules->set_rules(
         {
-            categorycode => '*',
-            itemtype     => '*',
-            branchcode   => '*',
+            categorycode => undef,
+            itemtype     => undef,
+            branchcode   => undef,
             rules        => {
                 issuelength => 1,
                 firstremind => 1,        # 1 day of grace
@@ -2215,9 +2232,9 @@ subtest 'AddReturn | is_overdue' => sub {
     Koha::CirculationRules->search->delete;
     my $rule = Koha::CirculationRules->set_rules(
         {
-            categorycode => '*',
-            itemtype     => '*',
-            branchcode   => '*',
+            categorycode => undef,
+            itemtype     => undef,
+            branchcode   => undef,
             rules        => {
                 issuelength  => 6,
                 lengthunit   => 'days',
index dfd65d3..4da6d09 100644 (file)
@@ -154,7 +154,6 @@ Koha::CirculationRules->set_rules(
     {
         branchcode   => $samplebranch1->{branchcode},
         categorycode => $samplecat->{categorycode},
-        itemtype     => undef,
         rules        => {
             patron_maxissueqty       => 5,
             patron_maxonsiteissueqty => 6,
@@ -162,16 +161,24 @@ Koha::CirculationRules->set_rules(
     }
 );
 
+
 Koha::CirculationRules->set_rules(
     {
         branchcode   => $samplebranch2->{branchcode},
         categorycode => undef,
-        itemtype     => undef,
         rules        => {
             patron_maxissueqty       => 3,
             patron_maxonsiteissueqty => 2,
-            holdallowed              => 1,
-            returnbranch             => 'holdingbranch',
+        }
+    }
+);
+Koha::CirculationRules->set_rules(
+    {
+        branchcode   => $samplebranch2->{branchcode},
+        itemtype     => undef,
+        rules        => {
+            holdallowed       => 1,
+            returnbranch      => 'holdingbranch',
         }
     }
 );
@@ -180,12 +187,19 @@ Koha::CirculationRules->set_rules(
     {
         branchcode   => undef,
         categorycode => undef,
-        itemtype     => undef,
         rules        => {
             patron_maxissueqty       => 4,
             patron_maxonsiteissueqty => 5,
-            holdallowed              => 3,
-            returnbranch             => 'homebranch',
+        }
+    }
+);
+Koha::CirculationRules->set_rules(
+    {
+        branchcode   => undef,
+        itemtype     => undef,
+        rules        => {
+            holdallowed       => 3,
+            returnbranch      => 'homebranch',
         }
     }
 );
@@ -193,7 +207,6 @@ Koha::CirculationRules->set_rules(
 Koha::CirculationRules->set_rules(
     {
         branchcode   => $samplebranch1->{branchcode},
-        categorycode => undef,
         itemtype     => $sampleitemtype1->{itemtype},
         rules        => {
             holdallowed       => 5,
@@ -204,7 +217,6 @@ Koha::CirculationRules->set_rules(
 Koha::CirculationRules->set_rules(
     {
         branchcode   => $samplebranch2->{branchcode},
-        categorycode => undef,
         itemtype     => $sampleitemtype1->{itemtype},
         rules        => {
             holdallowed       => 5,
@@ -215,7 +227,6 @@ Koha::CirculationRules->set_rules(
 Koha::CirculationRules->set_rules(
     {
         branchcode   => $samplebranch2->{branchcode},
-        categorycode => undef,
         itemtype     => $sampleitemtype2->{itemtype},
         rules        => {
             holdallowed       => 5,
index daff962..fe1bd79 100644 (file)
@@ -80,9 +80,9 @@ subtest 'Test basic functionality' => sub {
 
     Koha::CirculationRules->set_rules(
         {
-            branchcode   => '*',
-            categorycode => '*',
-            itemtype     => '*',
+            branchcode   => undef,
+            categorycode => undef,
+            itemtype     => undef,
             rules        => {
                 fine                          => '1.00',
                 lengthunit                    => 'days',
@@ -120,9 +120,9 @@ subtest 'Test cap_fine_to_replacement_price' => sub {
     t::lib::Mocks::mock_preference('useDefaultReplacementCost', '1');
     Koha::CirculationRules->set_rules(
         {
-            branchcode   => '*',
-            categorycode => '*',
-            itemtype     => '*',
+            branchcode   => undef,
+            categorycode => undef,
+            itemtype     => undef,
             rules        => {
                 fine                          => '1.00',
                 lengthunit                    => 'days',
index 0968213..c61587f 100644 (file)
@@ -8,7 +8,9 @@ use Koha::DateUtils;
 use Koha::CirculationRules;
 use Koha::Library;
 
-use Test::More tests => 8;
+use t::lib::TestBuilder;
+
+use Test::More tests => 10;
 
 BEGIN {
     use_ok('C4::Circulation');
@@ -103,6 +105,10 @@ $dbh->do(
     $samplecat->{category_type}
 );
 
+my $builder = t::lib::TestBuilder->new;
+my $sampleitemtype1 = $builder->build({ source => 'Itemtype' })->{itemtype};
+my $sampleitemtype2 = $builder->build({ source => 'Itemtype' })->{itemtype};
+
 #Begin Tests
 
 my $default = {
@@ -115,11 +121,8 @@ my $default = {
 my $sampleissuingrule1 = {
     branchcode   => $samplebranch1->{branchcode},
     categorycode => $samplecat->{categorycode},
-    itemtype     => 'BOOK',
+    itemtype     => $sampleitemtype1,
     rules        => {
-        reservecharge                    => 0,
-        restrictedtype                   => 0,
-        accountsent                      => 0,
         finedays                         => 0,
         lengthunit                       => 'days',
         renewalperiod                    => 5,
@@ -151,7 +154,7 @@ my $sampleissuingrule1 = {
 my $sampleissuingrule2 = {
     branchcode   => $samplebranch2->{branchcode},
     categorycode => $samplecat->{categorycode},
-    itemtype     => 'BOOK',
+    itemtype     => $sampleitemtype1,
     rules        => {
         renewalsallowed               => 0,
         renewalperiod                 => 2,
@@ -169,9 +172,6 @@ my $sampleissuingrule2 = {
         chargeperiod_charge_at        => 0,
         rentaldiscount                => 2.00,
         overduefinescap               => undef,
-        accountsent                   => undef,
-        reservecharge                 => undef,
-        restrictedtype                => undef,
         maxsuspensiondays             => 0,
         onshelfholds                  => 1,
         opacitemholds                 => 'Y',
@@ -183,7 +183,7 @@ my $sampleissuingrule2 = {
 my $sampleissuingrule3 = {
     branchcode   => $samplebranch1->{branchcode},
     categorycode => $samplecat->{categorycode},
-    itemtype     => 'DVD',
+    itemtype     => $sampleitemtype2,
     rules        => {
         renewalsallowed               => 0,
         renewalperiod                 => 3,
@@ -201,9 +201,6 @@ my $sampleissuingrule3 = {
         chargeperiod_charge_at        => 0,
         rentaldiscount                => 3.00,
         overduefinescap               => undef,
-        accountsent                   => undef,
-        reservecharge                 => undef,
-        restrictedtype                => undef,
         maxsuspensiondays             => 0,
         onshelfholds                  => 1,
         opacitemholds                 => 'F',
@@ -221,7 +218,7 @@ Koha::CirculationRules->set_rules( $sampleissuingrule3 );
 is_deeply(
     C4::Circulation::GetLoanLength(
         $samplecat->{categorycode},
-        'BOOK', $samplebranch1->{branchcode}
+        $sampleitemtype1, $samplebranch1->{branchcode}
     ),
     { issuelength => 5, lengthunit => 'days', renewalperiod => 5 },
     "GetLoanLength"
@@ -243,12 +240,12 @@ is_deeply(
     "With only one parameter, GetLoanLength returns hardcoded values"
 );    #NOTE : is that really what is expected?
 is_deeply(
-    C4::Circulation::GetLoanLength( $samplecat->{categorycode}, 'BOOK' ),
+    C4::Circulation::GetLoanLength( $samplecat->{categorycode}, $sampleitemtype1 ),
     $default,
     "With only two parameters, GetLoanLength returns hardcoded values"
 );    #NOTE : is that really what is expected?
 is_deeply(
-    C4::Circulation::GetLoanLength( $samplecat->{categorycode}, 'BOOK', $samplebranch1->{branchcode} ),
+    C4::Circulation::GetLoanLength( $samplecat->{categorycode}, $sampleitemtype1, $samplebranch1->{branchcode} ),
     {
         issuelength   => 5,
         renewalperiod => 5,
@@ -259,7 +256,7 @@ is_deeply(
 
 #Test GetHardDueDate
 my @hardduedate = C4::Circulation::GetHardDueDate( $samplecat->{categorycode},
-    'BOOK', $samplebranch1->{branchcode} );
+    $sampleitemtype1, $samplebranch1->{branchcode} );
 is_deeply(
     \@hardduedate,
     [
index 1662d30..2cbc02e 100644 (file)
@@ -32,9 +32,9 @@ t::lib::Mocks::mock_userenv({ branchcode => $branchcode });
 Koha::CirculationRules->search->delete;
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        itemtype     => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        itemtype     => undef,
+        branchcode   => undef,
         rules        => {
             firstremind => 0,
             finedays    => 2,
@@ -88,9 +88,9 @@ DelDebarment( $debarments->[0]->{borrower_debarment_id} );
 # Test with maxsuspensiondays = 10 days
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        itemtype     => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        itemtype     => undef,
+        branchcode   => undef,
         rules        => {
             maxsuspensiondays => 10,
         }
index c931d36..5d34559 100644 (file)
@@ -60,9 +60,9 @@ my $builder = t::lib::TestBuilder->new();
 Koha::CirculationRules->search->delete;
 Koha::CirculationRules->set_rule(
     {
-        categorycode => '*',
-        itemtype     => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        itemtype     => undef,
+        branchcode   => undef,
         rule_name    => 'issuelength',
         rule_value   => 1,
     }
@@ -256,9 +256,9 @@ subtest 'Handle ids duplication' => sub {
     t::lib::Mocks::mock_preference( 'finesMode', 'production' );
     Koha::CirculationRules->set_rules(
         {
-            categorycode => '*',
-            itemtype     => '*',
-            branchcode   => '*',
+            categorycode => undef,
+            itemtype     => undef,
+            branchcode   => undef,
             rules        => {
                 chargeperiod => 1,
                 fine         => 1,
index 0a654a8..779c19d 100644 (file)
@@ -85,14 +85,11 @@ Koha::CirculationRules->search()->delete();
 Koha::CirculationRules->set_rules(
     {
         branchcode   => $branch->{branchcode},
-        categorycode => '*',
-        itemtype     => '*',
+        categorycode => undef,
+        itemtype     => undef,
         rules        => {
             maxissueqty       => 2,
             maxonsiteissueqty => 1,
-            branchcode        => $branch->{branchcode},
-            categorycode      => '*',
-            itemtype          => '*',
             lengthunit        => 'days',
             issuelength       => 5,
             hardduedate        => undef,
@@ -160,8 +157,8 @@ Koha::CirculationRules->search()->delete();
 Koha::CirculationRules->set_rules(
     {
         branchcode   => $branch->{branchcode},
-        categorycode => '*',
-        itemtype     => '*',
+        categorycode => undef,
+        itemtype     => undef,
         rules        => {
             maxissueqty       => 2,
             maxonsiteissueqty => 1,
index f813ed0..7e45844 100644 (file)
@@ -107,7 +107,7 @@ subtest '1 Issuingrule exist 0 0: no issue allowed' => sub {
         {
             branchcode   => $branch->{branchcode},
             categorycode => $category->{categorycode},
-            itemtype     => '*',
+            itemtype     => undef,
             rules        => {
                 maxissueqty       => 0,
                 maxonsiteissueqty => 0,
@@ -219,7 +219,7 @@ subtest '1 Issuingrule exist 1 1: issue is allowed' => sub {
         {
             branchcode   => $branch->{branchcode},
             categorycode => $category->{categorycode},
-            itemtype     => '*',
+            itemtype     => undef,
             rules        => {
                 maxissueqty       => 1,
                 maxonsiteissueqty => 1,
@@ -259,7 +259,7 @@ subtest '1 Issuingrule exist: 1 CO allowed, 1 OSCO allowed. Do a CO' => sub {
         {
             branchcode   => $branch->{branchcode},
             categorycode => $category->{categorycode},
-            itemtype     => '*',
+            itemtype     => undef,
             rules        => {
                 maxissueqty       => 1,
                 maxonsiteissueqty => 1,
@@ -315,7 +315,7 @@ subtest '1 Issuingrule exist: 1 CO allowed, 1 OSCO allowed, Do a OSCO' => sub {
         {
             branchcode   => $branch->{branchcode},
             categorycode => $category->{categorycode},
-            itemtype     => '*',
+            itemtype     => undef,
             rules        => {
                 maxissueqty       => 1,
                 maxonsiteissueqty => 1,
index 7d391a4..a0bfd2e 100644 (file)
@@ -319,9 +319,9 @@ is_deeply(
 # Add a default rule: renewal is allowed
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        itemtype     => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        itemtype     => undef,
+        branchcode   => undef,
         rules        => {
             renewalsallowed => 3,
         }
index a72754f..3337cfc 100755 (executable)
@@ -99,13 +99,14 @@ my $patron = pop(@patrons);
 
 Koha::CirculationRules->set_rules(
     {
-        branchcode   => '*',
-        categorycode => '*',
+        branchcode   => undef,
+        categorycode => undef,
         itemtype     => $item->itype,
         rules        => {
             issuelength     => '14',
             lengthunit      => 'days',
             reservesallowed => '99',
+            holds_per_record => '99',
         }
     }
 );
index 1960a1f..76cc063 100644 (file)
@@ -17,9 +17,9 @@ $dbh->do(q|DELETE FROM circulation_rules|);
 
 my $issuingrule = Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        itemtype     => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        itemtype     => undef,
+        branchcode   => undef,
         rules        => {
             fine                   => 1,
             finedays               => 0,
@@ -44,11 +44,11 @@ $period_end = dt_from_string('2000-01-10');
 is( $fine, 1, '9 days overdue, charge period 7 days, charge at end of interval gives fine of $1' );
 
 # Test charging fine at the *beginning* of each charge period
-my $issuingrule = Koha::CirculationRules->set_rules(
+$issuingrule = Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        itemtype     => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        itemtype     => undef,
+        branchcode   => undef,
         rules        => {
             chargeperiod_charge_at => 1,
         }
index fde87fe..d959406 100755 (executable)
@@ -246,9 +246,9 @@ my ($foreign_item_bibnum, $foreign_item_bibitemnum, $foreign_itemnumber)
   = AddItem({ homebranch => $branch_2, holdingbranch => $branch_2 } , $foreign_biblio->biblionumber);
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        branchcode   => '*',
-        itemtype     => '*',
+        categorycode => undef,
+        branchcode   => undef,
+        itemtype     => undef,
         rules        => {
             reservesallowed  => 25,
             holds_per_record => 99,
@@ -257,8 +257,8 @@ Koha::CirculationRules->set_rules(
 );
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        branchcode   => undef,
         itemtype     => 'CANNOT',
         rules        => {
             reservesallowed  => 0,
@@ -365,9 +365,9 @@ ok(
 $dbh->do('DELETE FROM circulation_rules');
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        branchcode   => '*',
-        itemtype     => '*',
+        categorycode => undef,
+        branchcode   => undef,
+        itemtype     => undef,
         rules        => {
             reservesallowed  => 25,
             holds_per_record => 99,
@@ -378,7 +378,6 @@ Koha::CirculationRules->set_rules(
     {
         branchcode => $branch_1,
         itemtype   => 'CANNOT',
-        categorycode => undef,
         rules => {
             holdallowed => 0,
             returnbranch => 'homebranch',
@@ -389,7 +388,6 @@ Koha::CirculationRules->set_rules(
     {
         branchcode => $branch_1,
         itemtype   => 'CAN',
-        categorycode => undef,
         rules => {
             holdallowed => 1,
             returnbranch => 'homebranch',
@@ -433,8 +431,8 @@ $biblio = $builder->build_sample_biblio({ itemtype => 'ONLY1' });
 
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        branchcode   => undef,
         itemtype     => 'ONLY1',
         rules        => {
             reservesallowed  => 1,
@@ -469,9 +467,9 @@ subtest 'Test max_holds per library/patron category' => sub {
         $biblio->biblionumber );
     Koha::CirculationRules->set_rules(
         {
-            categorycode => '*',
-            branchcode   => '*',
-            itemtype     => 'TEST',
+            categorycode => undef,
+            branchcode   => undef,
+            itemtype     => $testitemtype,
             rules        => {
                 reservesallowed  => 99,
                 holds_per_record => 99,
@@ -493,7 +491,6 @@ subtest 'Test max_holds per library/patron category' => sub {
         {
             categorycode => $category->{categorycode},
             branchcode   => undef,
-            itemtype     => undef,
             rule_name    => 'max_holds',
             rule_value   => 3,
         }
@@ -503,7 +500,6 @@ subtest 'Test max_holds per library/patron category' => sub {
         {
             branchcode   => $branch_1,
             categorycode => $category->{categorycode},
-            itemtype     => undef,
             rule_name    => 'max_holds',
             rule_value   => 5,
         }
index 72012ca..fdc8f81 100755 (executable)
@@ -84,9 +84,9 @@ my $item2  = $builder->build_sample_item({
 $dbh->do("DELETE FROM circulation_rules");
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
+        categorycode => undef,
         itemtype     => $itemtype,
-        branchcode   => '*',
+        branchcode   => undef,
         rules        => {
             issuelength     => 7,
             lengthunit      => 8,
index 1916d08..189c3c6 100755 (executable)
@@ -75,7 +75,6 @@ my $itemnumber =
 $dbh->do("DELETE FROM circulation_rules");
 Koha::CirculationRules->set_rules(
     {
-        categorycode => undef,
         branchcode   => undef,
         itemtype     => undef,
         rules        => {
@@ -107,7 +106,6 @@ Koha::Holds->find( $reserve_id )->cancel;
 $dbh->do("DELETE FROM circulation_rules");
 Koha::CirculationRules->set_rules(
     {
-        categorycode => undef,
         branchcode   => undef,
         itemtype     => undef,
         rules        => {
@@ -139,7 +137,6 @@ Koha::Holds->find( $reserve_id )->cancel;
 $dbh->do("DELETE FROM circulation_rules");
 Koha::CirculationRules->set_rules(
     {
-        categorycode => undef,
         branchcode   => undef,
         itemtype     => undef,
         rules        => {
index ce206de..751047d 100644 (file)
@@ -90,7 +90,6 @@ $dbh->do("DELETE FROM circulation_rules");
 Koha::CirculationRules->set_rules(
     {
         itemtype     => undef,
-        categorycode => undef,
         branchcode   => undef,
         rules        => {
             holdallowed             => 2,
index f103ac7..2c52c87 100755 (executable)
@@ -296,7 +296,6 @@ Koha::CirculationRules->set_rule(
         rule_name    => 'holdallowed',
         rule_value   => 1,
         branchcode   => undef,
-        categorycode => undef,
         itemtype     => undef,
     }
 );
@@ -334,7 +333,6 @@ Koha::CirculationRules->set_rule(
         rule_name    => 'holdallowed',
         rule_value   => 2,
         branchcode   => undef,
-        categorycode => undef,
         itemtype     => undef,
     }
 );
@@ -362,7 +360,6 @@ Koha::CirculationRules->set_rule(
         rule_name    => 'holdallowed',
         rule_value   => 2,
         branchcode   => undef,
-        categorycode => undef,
         itemtype     => undef,
     }
 );
@@ -525,7 +522,6 @@ Koha::CirculationRules->set_rules(
     {
         branchcode   => $library_A,
         itemtype     => $itemtype,
-        categorycode => undef,
         rules        => {
             holdallowed  => 2,
             returnbranch => 'homebranch',
@@ -624,7 +620,6 @@ Koha::CirculationRules->set_rules(
     {
         branchcode   => undef,
         itemtype     => undef,
-        categorycode => undef,
         rules        => {
             holdallowed             => 2,
             hold_fulfillment_policy => 'homebranch',
@@ -659,7 +654,6 @@ Koha::CirculationRules->set_rules(
     {
         branchcode   => undef,
         itemtype     => undef,
-        categorycode => undef,
         rules        => {
             holdallowed             => 2,
             hold_fulfillment_policy => 'holdingbranch',
@@ -694,7 +688,6 @@ Koha::CirculationRules->set_rules(
     {
         branchcode   => undef,
         itemtype     => undef,
-        categorycode => undef,
         rules        => {
             holdallowed             => 2,
             hold_fulfillment_policy => 'any',
@@ -762,7 +755,6 @@ Koha::CirculationRules->set_rules(
     {
         branchcode   => undef,
         itemtype     => undef,
-        categorycode => undef,
         rules        => {
             holdallowed             => 2,
             hold_fulfillment_policy => 'any',
index 6c6d4ad..044ed69 100644 (file)
@@ -19,7 +19,9 @@
 
 use Modern::Perl;
 
-use Test::More tests => 4;
+use Test::More tests => 3;
+use Test::Deep qw( cmp_methods );
+use Test::Exception;
 
 use Benchmark;
 
@@ -36,15 +38,12 @@ my $builder      = t::lib::TestBuilder->new;
 subtest 'get_effective_issuing_rule' => sub {
     plan tests => 3;
 
-    my $patron       = $builder->build({ source => 'Borrower' });
-    my $item     = $builder->build({ source => 'Item' });
-
-    my $categorycode = $patron->{'categorycode'};
-    my $itemtype     = $item->{'itype'};
-    my $branchcode   = $item->{'homebranch'};
+    my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'};
+    my $itemtype     = $builder->build({ source => 'Itemtype' })->{'itemtype'};
+    my $branchcode   = $builder->build({ source => 'Branch' })->{'branchcode'};
 
     subtest 'Call with undefined values' => sub {
-        plan tests => 4;
+        plan tests => 5;
 
         my $rule;
         Koha::CirculationRules->delete;
@@ -59,25 +58,33 @@ subtest 'get_effective_issuing_rule' => sub {
         is($rule, undef, 'When I attempt to get effective issuing rule by'
            .' providing undefined values, then undef is returned.');
         ok(Koha::CirculationRule->new({
-            branchcode => '*',
-            categorycode => '*',
-            itemtype => '*',
+            branchcode => undef,
+            categorycode => undef,
+            itemtype => undef,
             rule_name => 'fine',
-        })->store, 'Given I added an issuing rule branchcode => *,'
-           .' categorycode => *, itemtype => *,');
+        })->store, 'Given I added an issuing rule branchcode => undef,'
+           .' categorycode => undef, itemtype => undef,');
         $rule = Koha::CirculationRules->get_effective_rule({
-            branchcode   => '*',
-            categorycode => '*',
-            itemtype     => '*',
+            branchcode   => undef,
+            categorycode => undef,
+            itemtype     => undef,
             rule_name    => 'fine',
         });
-        ok(_row_match($rule, '*', '*', '*'), 'When I attempt to get effective'
+        _is_row_match(
+            $rule,
+            {
+                branchcode => undef,
+                categorycode => undef,
+                itemtype => undef
+            },
+            'When I attempt to get effective'
            .' issuing rule by providing undefined values, then the above one is'
-           .' returned.');
+           .' returned.'
+        );
     };
 
     subtest 'Get effective issuing rule in correct order' => sub {
-        plan tests => 18;
+        plan tests => 26;
 
         my $rule;
         Koha::CirculationRules->delete;
@@ -92,109 +99,165 @@ subtest 'get_effective_issuing_rule' => sub {
                         .' is returned.');
 
         ok(Koha::CirculationRule->new({
-            branchcode => '*',
-            categorycode => '*',
-            itemtype => '*',
+            branchcode => undef,
+            categorycode => undef,
+            itemtype => undef,
             rule_name => 'fine',
-        })->store, 'Given I added an issuing rule branchcode => *, categorycode => *, itemtype => *,');
+        })->store, 'Given I added an issuing rule branchcode => undef, categorycode => undef, itemtype => undef,');
         $rule = Koha::CirculationRules->get_effective_rule({
             branchcode   => $branchcode,
             categorycode => $categorycode,
             itemtype     => $itemtype,
             rule_name    => 'fine',
         });
-        ok(_row_match($rule, '*', '*', '*'), 'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.');
+        _is_row_match(
+            $rule,
+            {
+                branchcode => undef,
+                categorycode => undef,
+                itemtype => undef
+            },
+            'When I attempt to get effective issuing rule,'
+           .' then the above one is returned.'
+        );
 
         ok(Koha::CirculationRule->new({
-            branchcode => '*',
-            categorycode => '*',
+            branchcode => undef,
+            categorycode => undef,
             itemtype => $itemtype,
             rule_name => 'fine',
-        })->store, "Given I added an issuing rule branchcode => *, categorycode => *, itemtype => $itemtype,");
+        })->store, "Given I added an issuing rule branchcode => undef, categorycode => undef, itemtype => $itemtype,");
         $rule = Koha::CirculationRules->get_effective_rule({
             branchcode   => $branchcode,
             categorycode => $categorycode,
             itemtype     => $itemtype,
             rule_name    => 'fine',
         });
-        ok(_row_match($rule, '*', '*', $itemtype), 'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.');
+        _is_row_match(
+            $rule,
+            {
+                branchcode => undef,
+                categorycode => undef,
+                itemtype => $itemtype
+            },
+            'When I attempt to get effective issuing rule,'
+           .' then the above one is returned.'
+        );
 
         ok(Koha::CirculationRule->new({
-            branchcode => '*',
+            branchcode => undef,
             categorycode => $categorycode,
-            itemtype => '*',
+            itemtype => undef,
             rule_name => 'fine',
-        })->store, "Given I added an issuing rule branchcode => *, categorycode => $categorycode, itemtype => *,");
+        })->store, "Given I added an issuing rule branchcode => undef, categorycode => $categorycode, itemtype => undef,");
         $rule = Koha::CirculationRules->get_effective_rule({
             branchcode   => $branchcode,
             categorycode => $categorycode,
             itemtype     => $itemtype,
             rule_name    => 'fine',
         });
-        ok(_row_match($rule, '*', $categorycode, '*'), 'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.');
+        _is_row_match(
+            $rule,
+            {
+                branchcode => undef,
+                categorycode => $categorycode,
+                itemtype => undef
+            },
+            'When I attempt to get effective issuing rule,'
+           .' then the above one is returned.'
+        );
 
         ok(Koha::CirculationRule->new({
-            branchcode => '*',
+            branchcode => undef,
             categorycode => $categorycode,
             itemtype => $itemtype,
             rule_name => 'fine',
-        })->store, "Given I added an issuing rule branchcode => *, categorycode => $categorycode, itemtype => $itemtype,");
+        })->store, "Given I added an issuing rule branchcode => undef, categorycode => $categorycode, itemtype => $itemtype,");
         $rule = Koha::CirculationRules->get_effective_rule({
             branchcode   => $branchcode,
             categorycode => $categorycode,
             itemtype     => $itemtype,
             rule_name    => 'fine',
         });
-        ok(_row_match($rule, '*', $categorycode, $itemtype), 'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.');
+        _is_row_match(
+            $rule,
+            {
+                branchcode => undef,
+                categorycode => $categorycode,
+                itemtype => $itemtype
+            },
+            'When I attempt to get effective issuing rule,'
+           .' then the above one is returned.'
+        );
 
         ok(Koha::CirculationRule->new({
             branchcode => $branchcode,
-            categorycode => '*',
-            itemtype => '*',
+            categorycode => undef,
+            itemtype => undef,
             rule_name => 'fine',
-        })->store, "Given I added an issuing rule branchcode => $branchcode, categorycode => '*', itemtype => '*',");
+        })->store, "Given I added an issuing rule branchcode => $branchcode, categorycode => undef, itemtype => undef,");
         $rule = Koha::CirculationRules->get_effective_rule({
             branchcode   => $branchcode,
             categorycode => $categorycode,
             itemtype     => $itemtype,
             rule_name    => 'fine',
         });
-        ok(_row_match($rule, $branchcode, '*', '*'), 'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.');
+        _is_row_match(
+            $rule,
+            {
+                branchcode => $branchcode,
+                categorycode => undef,
+                itemtype => undef
+            },
+            'When I attempt to get effective issuing rule,'
+           .' then the above one is returned.'
+        );
 
         ok(Koha::CirculationRule->new({
             branchcode => $branchcode,
-            categorycode => '*',
+            categorycode => undef,
             itemtype => $itemtype,
             rule_name => 'fine',
-        })->store, "Given I added an issuing rule branchcode => $branchcode, categorycode => '*', itemtype => $itemtype,");
+        })->store, "Given I added an issuing rule branchcode => $branchcode, categorycode => undef, itemtype => $itemtype,");
         $rule = Koha::CirculationRules->get_effective_rule({
             branchcode   => $branchcode,
             categorycode => $categorycode,
             itemtype     => $itemtype,
             rule_name    => 'fine',
         });
-        ok(_row_match($rule, $branchcode, '*', $itemtype), 'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.');
+        _is_row_match(
+            $rule,
+            {
+                branchcode => $branchcode,
+                categorycode => undef,
+                itemtype => $itemtype
+            },
+            'When I attempt to get effective issuing rule,'
+           .' then the above one is returned.'
+        );
 
         ok(Koha::CirculationRule->new({
             branchcode => $branchcode,
             categorycode => $categorycode,
-            itemtype => '*',
+            itemtype => undef,
             rule_name => 'fine',
-        })->store, "Given I added an issuing rule branchcode => $branchcode, categorycode => $categorycode, itemtype => '*',");
+        })->store, "Given I added an issuing rule branchcode => $branchcode, categorycode => $categorycode, itemtype => undef,");
         $rule = Koha::CirculationRules->get_effective_rule({
             branchcode   => $branchcode,
             categorycode => $categorycode,
             itemtype     => $itemtype,
             rule_name    => 'fine',
         });
-        ok(_row_match($rule, $branchcode, $categorycode, '*'), 'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.');
+        _is_row_match(
+            $rule,
+            {
+                branchcode => $branchcode,
+                categorycode => $categorycode,
+                itemtype => undef
+            },
+            'When I attempt to get effective issuing rule,'
+           .' then the above one is returned.'
+        );
 
         ok(Koha::CirculationRule->new({
             branchcode => $branchcode,
@@ -208,8 +271,16 @@ subtest 'get_effective_issuing_rule' => sub {
             itemtype     => $itemtype,
             rule_name    => 'fine',
         });
-        ok(_row_match($rule, $branchcode, $categorycode, $itemtype), 'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.');
+        _is_row_match(
+            $rule,
+            {
+                branchcode => $branchcode,
+                categorycode => $categorycode,
+                itemtype => $itemtype
+            },
+            'When I attempt to get effective issuing rule,'
+           .' then the above one is returned.'
+        );
     };
 
     subtest 'Performance' => sub {
@@ -266,68 +337,127 @@ subtest 'get_effective_issuing_rule' => sub {
     };
 };
 
-subtest 'get_opacitemholds_policy' => sub {
-    plan tests => 4;
-    my $itype = $builder->build_object({ class => 'Koha::ItemTypes' });
-    my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes' });
-    my $library = $builder->build_object({ class => 'Koha::Libraries' });
-    my $patron = $builder->build_object({ class => 'Koha::Patrons' });
-    my $biblio = $builder->build_object({ class => 'Koha::Biblios' });
-    my $biblioitem = $builder->build_object( { class => 'Koha::Biblioitems', value => { itemtype => $itemtype->itemtype, biblionumber => $biblio->biblionumber } } );
-    my $item = $builder->build_object(
-        {   class  => 'Koha::Items',
-            value  => {
-                homebranch    => $library->branchcode,
-                holdingbranch => $library->branchcode,
-                notforloan    => 0,
-                itemlost      => 0,
-                withdrawn     => 0,
-                biblionumber  => $biblio->biblionumber,
-                biblioitemnumber => $biblioitem->biblioitemnumber,
-                itype         => $itype->itemtype,
-            }
-        }
-    );
-
-    Koha::IssuingRules->delete;
-    Koha::IssuingRule->new({categorycode => '*', itemtype => '*',                 branchcode => '*', opacitemholds => "N"})->store;
-    Koha::IssuingRule->new({categorycode => '*', itemtype => $itype->itemtype,    branchcode => '*', opacitemholds => "Y"})->store;
-    Koha::IssuingRule->new({categorycode => '*', itemtype => $itemtype->itemtype, branchcode => '*', opacitemholds => "N"})->store;
-    t::lib::Mocks::mock_preference('item-level_itypes', 1);
-    my $opacitemholds = Koha::IssuingRules->get_opacitemholds_policy( { item => $item, patron => $patron } );
-    is ( $opacitemholds, 'Y', 'Patrons can place a hold on this itype');
-    t::lib::Mocks::mock_preference('item-level_itypes', 0);
-    $opacitemholds = Koha::IssuingRules->get_opacitemholds_policy( { item => $item, patron => $patron } );
-    is ( $opacitemholds, 'N', 'Patrons cannot place a hold on this itemtype');
-
-    Koha::IssuingRules->delete;
-    Koha::IssuingRule->new({categorycode => '*', itemtype => '*',                 branchcode => '*', opacitemholds => "N"})->store;
-    Koha::IssuingRule->new({categorycode => '*', itemtype => $itype->itemtype,    branchcode => '*', opacitemholds => "N"})->store;
-    Koha::IssuingRule->new({categorycode => '*', itemtype => $itemtype->itemtype, branchcode => '*', opacitemholds => "Y"})->store;
-    t::lib::Mocks::mock_preference('item-level_itypes', 1);
-    $opacitemholds = Koha::IssuingRules->get_opacitemholds_policy( { item => $item, patron => $patron } );
-    is ( $opacitemholds, 'N', 'Patrons cannot place a hold on this itype');
-    t::lib::Mocks::mock_preference('item-level_itypes', 0);
-    $opacitemholds = Koha::IssuingRules->get_opacitemholds_policy( { item => $item, patron => $patron } );
-    is ( $opacitemholds, 'Y', 'Patrons can place a hold on this itemtype');
-
-    $patron->delete;
-};
-
-subtest 'get_onshelfholds_policy' => sub {
+subtest 'set_rule' => sub {
     plan tests => 3;
 
-    t::lib::Mocks::mock_preference('item-level_itypes', 1);
-    Koha::IssuingRules->delete;
+    my $branchcode   = $builder->build({ source => 'Branch' })->{'branchcode'};
+    my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'};
+    my $itemtype     = $builder->build({ source => 'Itemtype' })->{'itemtype'};
 
-    my $patron = $builder->build_object({ class => 'Koha::Patrons' });
-    my $item = $builder->build_object({ class => 'Koha::Items' });
+    subtest 'Correct call' => sub {
+        plan tests => 4;
 
-    is( Koha::IssuingRules->get_onshelfholds_policy({ item => $item, patron => $patron }), undef, 'Should return undef when no rules can be found' );
-    Koha::IssuingRule->new({ categorycode => $patron->categorycode, itemtype => $item->itype, branchcode => '*', onshelfholds => "0" })->store;
-    is( Koha::IssuingRules->get_onshelfholds_policy({ item => $item, patron => $patron }), 0, 'Should be zero' );
-    Koha::IssuingRule->new({ categorycode => $patron->categorycode, itemtype => $item->itype, branchcode => $item->holdingbranch, onshelfholds => "2" })->store;
-    is( Koha::IssuingRules->get_onshelfholds_policy({ item => $item, patron => $patron }), 2, 'Should be two now' );
+        Koha::CirculationRules->delete;
+
+        lives_ok( sub {
+            Koha::CirculationRules->set_rule( {
+                branchcode => $branchcode,
+                rule_name => 'refund',
+                rule_value => '',
+            } );
+        }, 'setting refund with branch' );
+
+        lives_ok( sub {
+            Koha::CirculationRules->set_rule( {
+                branchcode => $branchcode,
+                categorycode => $categorycode,
+                rule_name => 'patron_maxissueqty',
+                rule_value => '',
+            } );
+        }, 'setting patron_maxissueqty with branch/category succeeds' );
+
+        lives_ok( sub {
+            Koha::CirculationRules->set_rule( {
+                branchcode => $branchcode,
+                itemtype => $itemtype,
+                rule_name => 'holdallowed',
+                rule_value => '',
+            } );
+        }, 'setting holdallowed with branch/itemtype succeeds' );
+
+        lives_ok( sub {
+            Koha::CirculationRules->set_rule( {
+                branchcode => $branchcode,
+                categorycode => $categorycode,
+                itemtype => $itemtype,
+                rule_name => 'fine',
+                rule_value => '',
+            } );
+        }, 'setting fine with branch/category/itemtype succeeds' );
+    };
+
+    subtest 'Call with missing params' => sub {
+        plan tests => 4;
+
+        Koha::CirculationRules->delete;
+
+        throws_ok( sub {
+            Koha::CirculationRules->set_rule( {
+                rule_name => 'refund',
+                rule_value => '',
+            } );
+        }, qr/branchcode/, 'setting refund without branch fails' );
+
+        throws_ok( sub {
+            Koha::CirculationRules->set_rule( {
+                branchcode => $branchcode,
+                rule_name => 'patron_maxissueqty',
+                rule_value => '',
+            } );
+        }, qr/categorycode/, 'setting patron_maxissueqty without categorycode fails' );
+
+        throws_ok( sub {
+            Koha::CirculationRules->set_rule( {
+                branchcode => $branchcode,
+                rule_name => 'holdallowed',
+                rule_value => '',
+            } );
+        }, qr/itemtype/, 'setting holdallowed without itemtype fails' );
+
+        throws_ok( sub {
+            Koha::CirculationRules->set_rule( {
+                branchcode => $branchcode,
+                categorycode => $categorycode,
+                rule_name => 'fine',
+                rule_value => '',
+            } );
+        }, qr/itemtype/, 'setting fine without itemtype fails' );
+    };
+
+    subtest 'Call with extra params' => sub {
+        plan tests => 3;
+
+        Koha::CirculationRules->delete;
+
+        throws_ok( sub {
+            Koha::CirculationRules->set_rule( {
+                branchcode => $branchcode,
+                categorycode => $categorycode,
+                rule_name => 'refund',
+                rule_value => '',
+            } );
+        }, qr/categorycode/, 'setting refund with categorycode fails' );
+
+        throws_ok( sub {
+            Koha::CirculationRules->set_rule( {
+                branchcode => $branchcode,
+                categorycode => $categorycode,
+                itemtype => $itemtype,
+                rule_name => 'patron_maxissueqty',
+                rule_value => '',
+            } );
+        }, qr/itemtype/, 'setting patron_maxissueqty with itemtype fails' );
+
+        throws_ok( sub {
+            Koha::CirculationRules->set_rule( {
+                branchcode => $branchcode,
+                rule_name => 'holdallowed',
+                categorycode => $categorycode,
+                itemtype => $itemtype,
+                rule_value => '',
+            } );
+        }, qr/categorycode/, 'setting holdallowed with categorycode fails' );
+    };
 };
 
 subtest 'delete' => sub {
@@ -377,11 +507,12 @@ subtest 'delete' => sub {
 
 };
 
-sub _row_match {
-    my ($rule, $branchcode, $categorycode, $itemtype) = @_;
+sub _is_row_match {
+    my ( $rule, $expected, $message ) = @_;
 
-    return $rule->branchcode eq $branchcode && $rule->categorycode eq $categorycode
-            && $rule->itemtype eq $itemtype;
+    ok( $rule, $message ) ?
+        cmp_methods( $rule, [ %$expected ], $message ) :
+        fail( $message );
 }
 
 $schema->storage->txn_rollback;
index 4599165..da53f3a 100755 (executable)
@@ -54,10 +54,12 @@ subtest 'Koha::RefundLostItemFeeRule::delete() tests' => sub {
             }
         }
     );
+    my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode};
     my $generated_other_rule = $builder->build(
         {
             source => 'CirculationRule',
             value  => {
+                branchcode   => $branchcode,
                 categorycode => undef,
                 itemtype     => undef,
                 rule_name    => 'refund',
@@ -117,10 +119,12 @@ subtest 'Koha::RefundLostItemFeeRules::_default_rule() tests' => sub {
             }
         }
     );
+    my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode};
     my $generated_other_rule = $builder->build(
         {
             source => 'CirculationRule',
             value  => {
+                branchcode   => $branchcode,
                 categorycode => undef,
                 itemtype     => undef,
                 rule_name    => 'refund',
@@ -185,10 +189,12 @@ subtest 'Koha::RefundLostItemFeeRules::_effective_branch_rule() tests' => sub {
             }
         }
     );
+    my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode};
     my $specific_rule_false = $builder->build(
         {
             source => 'CirculationRule',
             value  => {
+                branchcode   => $branchcode,
                 categorycode => undef,
                 itemtype     => undef,
                 rule_name    => 'refund',
@@ -196,10 +202,12 @@ subtest 'Koha::RefundLostItemFeeRules::_effective_branch_rule() tests' => sub {
             }
         }
     );
+    my $branchcode2 = $builder->build( { source => 'Branch' } )->{branchcode};
     my $specific_rule_true = $builder->build(
         {
             source => 'CirculationRule',
             value  => {
+                branchcode   => $branchcode2,
                 categorycode => undef,
                 itemtype     => undef,
                 rule_name    => 'refund',
@@ -301,10 +309,12 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub {
             }
         }
     );
+    my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode};
     my $specific_rule_false = $builder->build(
         {
             source => 'CirculationRule',
             value  => {
+                branchcode   => $branchcode,
                 categorycode => undef,
                 itemtype     => undef,
                 rule_name    => 'refund',
@@ -312,10 +322,12 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub {
             }
         }
     );
+    my $branchcode2 = $builder->build( { source => 'Branch' } )->{branchcode};
     my $specific_rule_true = $builder->build(
         {
             source => 'CirculationRule',
             value  => {
+                branchcode   => $branchcode2,
                 categorycode => undef,
                 itemtype     => undef,
                 rule_name    => 'refund',
@@ -324,10 +336,12 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub {
         }
     );
     # Make sure we have an unused branchcode
+    my $branchcode3 = $builder->build( { source => 'Branch' } )->{branchcode};
     my $specific_rule_dummy = $builder->build(
         {
             source => 'CirculationRule',
             value  => {
+                branchcode   => $branchcode3,
                 categorycode => undef,
                 itemtype     => undef,
                 rule_name    => 'refund',
index 5c2f570..f7f66fd 100755 (executable)
@@ -193,9 +193,9 @@ $requesters{$branch_3} = Koha::Patron->new({
 $dbh->do('DELETE FROM circulation_rules');
 Koha::CirculationRules->set_rules(
     {
-        branchcode   => '*',
-        categorycode => '*',
-        itemtype     => '*',
+        branchcode   => undef,
+        categorycode => undef,
+        itemtype     => undef,
         rules        => {
             reservesallowed => 25,
             holds_per_record => 1,
@@ -207,7 +207,6 @@ Koha::CirculationRules->set_rules(
 Koha::CirculationRules->set_rules(
     {
         branchcode   => $branch_1,
-        categorycode => undef,
         itemtype     => undef,
         rules        => {
             holdallowed  => 1,
@@ -220,7 +219,6 @@ Koha::CirculationRules->set_rules(
 Koha::CirculationRules->set_rules(
     {
         branchcode   => $branch_2,
-        categorycode => undef,
         itemtype     => undef,
         rules        => {
             holdallowed  => 2,
index b64fd44..bd4529f 100755 (executable)
@@ -120,9 +120,9 @@ Koha::CirculationRules->delete();
 # Test GetMaxPatronHoldsForRecord and GetHoldRule
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        itemtype     => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        itemtype     => undef,
+        branchcode   => undef,
         rules        => {
             reservesallowed  => 1,
             holds_per_record => 1,
@@ -139,17 +139,17 @@ my $rule = C4::Reserves::GetHoldRule(
     $itemtype1->{itemtype},
     $library->{branchcode}
 );
-is( $rule->{categorycode},     '*', 'Got rule with universal categorycode' );
-is( $rule->{itemtype},         '*', 'Got rule with universal itemtype' );
-is( $rule->{branchcode},       '*', 'Got rule with universal branchcode' );
+is( $rule->{categorycode},     undef, 'Got rule with universal categorycode' );
+is( $rule->{itemtype},         undef, 'Got rule with universal itemtype' );
+is( $rule->{branchcode},       undef, 'Got rule with universal branchcode' );
 is( $rule->{reservesallowed},  1,   'Got reservesallowed of 1' );
 is( $rule->{holds_per_record}, 1,   'Got holds_per_record of 1' );
 
 Koha::CirculationRules->set_rules(
     {
         categorycode => $category->{categorycode},
-        itemtype     => '*',
-        branchcode   => '*',
+        itemtype     => undef,
+        branchcode   => undef,
         rules        => {
             reservesallowed  => 2,
             holds_per_record => 2,
@@ -165,8 +165,8 @@ $rule = C4::Reserves::GetHoldRule(
     $library->{branchcode}
 );
 is( $rule->{categorycode},     $category->{categorycode}, 'Got rule with specific categorycode' );
-is( $rule->{itemtype},         '*',                       'Got rule with universal itemtype' );
-is( $rule->{branchcode},       '*',                       'Got rule with universal branchcode' );
+is( $rule->{itemtype},         undef,                       'Got rule with universal itemtype' );
+is( $rule->{branchcode},       undef,                       'Got rule with universal branchcode' );
 is( $rule->{reservesallowed},  2,                         'Got reservesallowed of 2' );
 is( $rule->{holds_per_record}, 2,                         'Got holds_per_record of 2' );
 
@@ -174,7 +174,7 @@ Koha::CirculationRules->set_rules(
     {
         categorycode => $category->{categorycode},
         itemtype     => $itemtype1->{itemtype},
-        branchcode   => '*',
+        branchcode   => undef,
         rules        => {
             reservesallowed  => 3,
             holds_per_record => 3,
@@ -191,7 +191,7 @@ $rule = C4::Reserves::GetHoldRule(
 );
 is( $rule->{categorycode},     $category->{categorycode}, 'Got rule with specific categorycode' );
 is( $rule->{itemtype},         $itemtype1->{itemtype},    'Got rule with universal itemtype' );
-is( $rule->{branchcode},       '*',                       'Got rule with universal branchcode' );
+is( $rule->{branchcode},       undef,                       'Got rule with universal branchcode' );
 is( $rule->{reservesallowed},  3,                         'Got reservesallowed of 3' );
 is( $rule->{holds_per_record}, 3,                         'Got holds_per_record of 3' );
 
@@ -199,7 +199,7 @@ Koha::CirculationRules->set_rules(
     {
         categorycode => $category->{categorycode},
         itemtype     => $itemtype2->{itemtype},
-        branchcode   => '*',
+        branchcode   => undef,
         rules        => {
             reservesallowed  => 4,
             holds_per_record => 4,
@@ -216,7 +216,7 @@ $rule = C4::Reserves::GetHoldRule(
 );
 is( $rule->{categorycode},     $category->{categorycode}, 'Got rule with specific categorycode' );
 is( $rule->{itemtype},         $itemtype2->{itemtype},    'Got rule with universal itemtype' );
-is( $rule->{branchcode},       '*',                       'Got rule with universal branchcode' );
+is( $rule->{branchcode},       undef,                       'Got rule with universal branchcode' );
 is( $rule->{reservesallowed},  4,                         'Got reservesallowed of 4' );
 is( $rule->{holds_per_record}, 4,                         'Got holds_per_record of 4' );
 
@@ -273,9 +273,9 @@ $hold->delete();
 # Test multi-hold via AddReserve
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        itemtype     => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        itemtype     => undef,
+        branchcode   => undef,
         rules        => {
             reservesallowed  => 2,
             holds_per_record => 2,
index 97ff96c..e2fbba7 100644 (file)
@@ -364,12 +364,14 @@ subtest 'build_object() tests' => sub {
 
     $builder = t::lib::TestBuilder->new();
 
+    my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode};
     my $categorycode = $builder->build( { source => 'Category' } )->{categorycode};
     my $itemtype = $builder->build( { source => 'Itemtype' } )->{itemtype};
 
     my $issuing_rule = $builder->build_object(
         {   class => 'Koha::CirculationRules',
             value => {
+                branchcode   => $branchcode,
                 categorycode => $categorycode,
                 itemtype     => $itemtype
             }
index ba5aee4..ba90b29 100644 (file)
@@ -117,9 +117,9 @@ $dbh->do('DELETE FROM reserves');
 Koha::CirculationRules->search()->delete();
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        branchcode   => '*',
-        itemtype     => '*',
+        categorycode => undef,
+        branchcode   => undef,
+        itemtype     => undef,
         rules        => {
             reservesallowed => 1,
             holds_per_record => 99