From f2dfa17f0e7b3e6e1a035af18a16b0378a1b16f2 Mon Sep 17 00:00:00 2001 From: Jesse Weaver Date: Thu, 14 Sep 2017 13:32:26 -0600 Subject: [PATCH] Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This necessitates moving the circ rules from using '*' to using undef/NULL. Signed-off-by: Minna Kivinen Signed-off-by: Joonas Kylmälä Signed-off-by: Martin Renvoize --- C4/Circulation.pm | 28 +- C4/Reserves.pm | 10 +- Koha/CirculationRules.pm | 153 +++++++- Koha/REST/V1/CirculationRules.pm | 32 ++ admin/smart-rules.pl | 154 ++++---- api/v1/swagger/definitions.json | 3 + .../swagger/definitions/circ-rule-kind.json | 15 + api/v1/swagger/paths.json | 3 + api/v1/swagger/paths/circulation-rules.json | 35 ++ .../prog/en/modules/admin/smart-rules.tt | 19 +- t/db_dependent/ArticleRequests.t | 24 +- t/db_dependent/Circulation.t | 171 +++++---- t/db_dependent/Circulation/Branch.t | 31 +- t/db_dependent/Circulation/CalcFine.t | 12 +- t/db_dependent/Circulation/GetHardDueDate.t | 31 +- .../IssuingRules/maxsuspensiondays.t | 12 +- t/db_dependent/Circulation/Returns.t | 12 +- .../Circulation/SwitchOnSiteCheckouts.t | 11 +- t/db_dependent/Circulation/TooMany.t | 8 +- t/db_dependent/Circulation/issue.t | 6 +- t/db_dependent/DecreaseLoanHighHolds.t | 5 +- t/db_dependent/Fines.t | 14 +- t/db_dependent/Holds.t | 30 +- .../Holds/DisallowHoldIfItemsAvailable.t | 4 +- t/db_dependent/Holds/HoldFulfillmentPolicy.t | 3 - t/db_dependent/Holds/HoldItemtypeLimit.t | 1 - t/db_dependent/HoldsQueue.t | 8 - t/db_dependent/Koha/IssuingRules.t | 363 ++++++++++++------ t/db_dependent/RefundLostItemFeeRule.t | 14 + t/db_dependent/Reserves.t | 8 +- t/db_dependent/Reserves/MultiplePerRecord.t | 34 +- t/db_dependent/TestBuilder.t | 2 + t/db_dependent/api/v1/holds.t | 6 +- 33 files changed, 823 insertions(+), 439 deletions(-) create mode 100644 Koha/REST/V1/CirculationRules.pm create mode 100644 api/v1/swagger/definitions/circ-rule-kind.json create mode 100644 api/v1/swagger/paths/circulation-rules.json diff --git a/C4/Circulation.pm b/C4/Circulation.pm index d9fad947df..fcf48f83ad 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -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, }, ); diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 39e61a9d50..0dca5e7274 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -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"; diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index 82ac198087..7f8d36e762 100644 --- a/Koha/CirculationRules.pm +++ b/Koha/CirculationRules.pm @@ -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 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 index 0000000000..a04be03a94 --- /dev/null +++ b/Koha/REST/V1/CirculationRules.pm @@ -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; diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index a8dc91e9d9..67f72e6c83 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -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; diff --git a/api/v1/swagger/definitions.json b/api/v1/swagger/definitions.json index 40e10c9f94..a27fa0de22 100644 --- a/api/v1/swagger/definitions.json +++ b/api/v1/swagger/definitions.json @@ -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 index 0000000000..90b956f9f6 --- /dev/null +++ b/api/v1/swagger/definitions/circ-rule-kind.json @@ -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"] +} diff --git a/api/v1/swagger/paths.json b/api/v1/swagger/paths.json index ecf357abb8..ed9ed0627c 100644 --- a/api/v1/swagger/paths.json +++ b/api/v1/swagger/paths.json @@ -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 index 0000000000..e897cd7276 --- /dev/null +++ b/api/v1/swagger/paths/circulation-rules.json @@ -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" + } + } + } + } + } +} diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt index 738c2ad3f5..7ad46a2d62 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -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' %] Koha › Administration › Circulation and fine rules @@ -162,19 +162,23 @@ [% SET row_count = row_count + 1 %] - [% IF c == '*' %] + [% IF c == undef %] All [% ELSE %] [% Categories.GetName(c) %] [% END %] - [% IF i == '*' %] + [% IF i == undef %] All [% ELSE %] [% ItemTypes.GetDescription(i) %] [% END %] + + Edit + Delete + [% IF rule.note %] View note @@ -288,7 +292,7 @@ [% rentaldiscount %] Edit - Delete + Delete [% END %] @@ -625,6 +629,7 @@   [% 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' ) %] @@ -632,7 +637,7 @@ [% IF ( patron_maxissueqty.defined && patron_maxissueqty != '' ) || ( patron_maxonsiteissueqty.defined && patron_maxonsiteissueqty != '' ) || ( max_holds.defined && max_holds != '' ) %] - [% IF c == '*'%] + [% IF c == undef %] Default [% ELSE %] [% Categories.GetName(c) | html %] diff --git a/t/db_dependent/ArticleRequests.t b/t/db_dependent/ArticleRequests.t index 0acc9eb596..f9aee4138a 100755 --- a/t/db_dependent/ArticleRequests.t +++ b/t/db_dependent/ArticleRequests.t @@ -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', } diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index a71bb1be62..cebe7dcb5d 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -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', diff --git a/t/db_dependent/Circulation/Branch.t b/t/db_dependent/Circulation/Branch.t index dfd65d33d2..4da6d09c05 100644 --- a/t/db_dependent/Circulation/Branch.t +++ b/t/db_dependent/Circulation/Branch.t @@ -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, diff --git a/t/db_dependent/Circulation/CalcFine.t b/t/db_dependent/Circulation/CalcFine.t index daff962ed6..fe1bd79744 100644 --- a/t/db_dependent/Circulation/CalcFine.t +++ b/t/db_dependent/Circulation/CalcFine.t @@ -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', diff --git a/t/db_dependent/Circulation/GetHardDueDate.t b/t/db_dependent/Circulation/GetHardDueDate.t index 0968213234..c61587f38e 100644 --- a/t/db_dependent/Circulation/GetHardDueDate.t +++ b/t/db_dependent/Circulation/GetHardDueDate.t @@ -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, [ diff --git a/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t b/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t index 1662d30143..2cbc02e73d 100644 --- a/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t +++ b/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t @@ -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, } diff --git a/t/db_dependent/Circulation/Returns.t b/t/db_dependent/Circulation/Returns.t index c931d36b3d..5d345594c5 100644 --- a/t/db_dependent/Circulation/Returns.t +++ b/t/db_dependent/Circulation/Returns.t @@ -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, diff --git a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t index 0a654a824f..779c19d84e 100644 --- a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t +++ b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t @@ -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, diff --git a/t/db_dependent/Circulation/TooMany.t b/t/db_dependent/Circulation/TooMany.t index f813ed0e5c..7e4584435d 100644 --- a/t/db_dependent/Circulation/TooMany.t +++ b/t/db_dependent/Circulation/TooMany.t @@ -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, diff --git a/t/db_dependent/Circulation/issue.t b/t/db_dependent/Circulation/issue.t index 7d391a4280..a0bfd2e9e6 100644 --- a/t/db_dependent/Circulation/issue.t +++ b/t/db_dependent/Circulation/issue.t @@ -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, } diff --git a/t/db_dependent/DecreaseLoanHighHolds.t b/t/db_dependent/DecreaseLoanHighHolds.t index a72754f9a9..3337cfcf1c 100755 --- a/t/db_dependent/DecreaseLoanHighHolds.t +++ b/t/db_dependent/DecreaseLoanHighHolds.t @@ -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', } } ); diff --git a/t/db_dependent/Fines.t b/t/db_dependent/Fines.t index 1960a1fcbb..76cc0638a9 100644 --- a/t/db_dependent/Fines.t +++ b/t/db_dependent/Fines.t @@ -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, } diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index fde87fe85e..d959406f53 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -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, } diff --git a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t index 72012ca84e..fdc8f81f64 100755 --- a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t +++ b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t @@ -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, diff --git a/t/db_dependent/Holds/HoldFulfillmentPolicy.t b/t/db_dependent/Holds/HoldFulfillmentPolicy.t index 1916d08972..189c3c635c 100755 --- a/t/db_dependent/Holds/HoldFulfillmentPolicy.t +++ b/t/db_dependent/Holds/HoldFulfillmentPolicy.t @@ -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 => { diff --git a/t/db_dependent/Holds/HoldItemtypeLimit.t b/t/db_dependent/Holds/HoldItemtypeLimit.t index ce206de531..751047dc14 100644 --- a/t/db_dependent/Holds/HoldItemtypeLimit.t +++ b/t/db_dependent/Holds/HoldItemtypeLimit.t @@ -90,7 +90,6 @@ $dbh->do("DELETE FROM circulation_rules"); Koha::CirculationRules->set_rules( { itemtype => undef, - categorycode => undef, branchcode => undef, rules => { holdallowed => 2, diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t index f103ac7b0c..2c52c87ca4 100755 --- a/t/db_dependent/HoldsQueue.t +++ b/t/db_dependent/HoldsQueue.t @@ -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', diff --git a/t/db_dependent/Koha/IssuingRules.t b/t/db_dependent/Koha/IssuingRules.t index 6c6d4ad4a6..044ed6986a 100644 --- a/t/db_dependent/Koha/IssuingRules.t +++ b/t/db_dependent/Koha/IssuingRules.t @@ -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; diff --git a/t/db_dependent/RefundLostItemFeeRule.t b/t/db_dependent/RefundLostItemFeeRule.t index 4599165961..da53f3ab1b 100755 --- a/t/db_dependent/RefundLostItemFeeRule.t +++ b/t/db_dependent/RefundLostItemFeeRule.t @@ -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', diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 5c2f570e3c..f7f66fd6ec 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -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, diff --git a/t/db_dependent/Reserves/MultiplePerRecord.t b/t/db_dependent/Reserves/MultiplePerRecord.t index b64fd44403..bd4529fb70 100755 --- a/t/db_dependent/Reserves/MultiplePerRecord.t +++ b/t/db_dependent/Reserves/MultiplePerRecord.t @@ -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, diff --git a/t/db_dependent/TestBuilder.t b/t/db_dependent/TestBuilder.t index 97ff96ca66..e2fbba7a9b 100644 --- a/t/db_dependent/TestBuilder.t +++ b/t/db_dependent/TestBuilder.t @@ -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 } diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t index ba5aee4ea4..ba90b29f75 100644 --- a/t/db_dependent/api/v1/holds.t +++ b/t/db_dependent/api/v1/holds.t @@ -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 -- 2.39.5