From 6fc665dd91437a4114f649733da9e24a7cf7e5d7 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 This necessitates moving the circ rules from using '*' to using undef/NULL. --- C4/Circulation.pm | 32 +- C4/Reserves.pm | 21 +- Koha/CirculationRules.pm | 170 ++++++++-- Koha/REST/V1/CirculationRules.pm | 32 ++ Koha/RefundLostItemFeeRules.pm | 2 +- Koha/Schema/Result/Branch.pm | 15 + Koha/Schema/Result/Category.pm | 15 + Koha/Schema/Result/CirculationRule.pm | 82 ++++- Koha/Schema/Result/Itemtype.pm | 15 + admin/smart-rules.pl | 156 ++++----- api/v1/swagger/definitions.json | 3 + api/v1/swagger/definitions/circ-rule-kind.json | 15 + api/v1/swagger/paths.json | 3 + api/v1/swagger/paths/circulation-rules.json | 35 ++ installer/data/mysql/atomicupdate/bug_18887.perl | 5 +- installer/data/mysql/kohastructure.sql | 5 +- .../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 | 32 +- .../Circulation/IssuingRules/maxsuspensiondays.t | 12 +- t/db_dependent/Circulation/Returns.t | 12 +- t/db_dependent/Circulation/SwitchOnSiteCheckouts.t | 11 +- t/db_dependent/Circulation/TooMany.t | 8 +- t/db_dependent/Circulation/issue.t | 12 +- t/db_dependent/DecreaseLoanHighHolds.t | 7 +- t/db_dependent/Fines.t | 14 +- t/db_dependent/Holds.t | 33 +- .../Holds/DisallowHoldIfItemsAvailable.t | 6 +- 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 | 30 +- 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 +- 40 files changed, 976 insertions(+), 489 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 8d12c0a9ff..7b3d4bca3d 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -414,7 +414,7 @@ sub TooMany { |; my $rule_itemtype = $maxissueqty_rule->itemtype; - if ($rule_itemtype eq "*") { + if (!defined $rule_itemtype) { # matching rule has the default item type, so count only # those existing loans that don't fall under a more # specific rule @@ -423,7 +423,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 { @@ -432,7 +432,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' ) "; } @@ -454,7 +454,7 @@ sub TooMany { $count_query .= " AND borrowernumber = ? "; push @bind_params, $borrower->{'borrowernumber'}; my $rule_branch = $maxissueqty_rule->branchcode; - if ($rule_branch ne "*") { + if (defined $rule_branch) { if (C4::Context->preference('CircControl') eq 'PickupLibrary') { $count_query .= " AND issues.branchcode = ? "; push @bind_params, $branch; @@ -1485,38 +1485,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 2ea7610cd9..6ebbb0d93f 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -352,7 +352,7 @@ sub CanItemBeReserved { $holds_per_record = $rights->{holds_per_record}; } else { - $ruleitemtype = '*'; + $ruleitemtype = undef; } $item = Koha::Items->find( $itemnumber ); @@ -377,15 +377,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"; @@ -399,19 +399,10 @@ sub CanItemBeReserved { } # Now we need to check hold limits by patron category - my $rule = Koha::CirculationRules->find( + my $rule = Koha::CirculationRules->get_effective_rule( { - categorycode => $borrower->{categorycode}, branchcode => $borrower->{branchcode}, - itemtype => undef, - rule_name => 'max_holds', - } - ); - $rule ||= Koha::CirculationRules->find( - { categorycode => $borrower->{categorycode}, - branchcode => undef,, - itemtype => undef, rule_name => 'max_holds', } ); diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index 8b6fd77bcd..e3a4b8c0d0 100644 --- a/Koha/CirculationRules.pm +++ b/Koha/CirculationRules.pm @@ -35,6 +35,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 @@ -42,9 +164,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}; @@ -59,9 +181,9 @@ sub get_effective_rule { my $search_params; $search_params->{rule_name} = $rule_name; - $search_params->{categorycode} = defined $categorycode ? { 'in' => [ $categorycode, '*' ] } : undef; - $search_params->{itemtype} = defined $itemtype ? { 'in' => [ $itemtype, '*' ] } : undef; - $search_params->{branchcode} = defined $branchcode ? { 'in' => [ $branchcode, '*' ] } : undef; + $search_params->{categorycode} = defined $categorycode ? [ $categorycode, undef ] : undef; + $search_params->{itemtype} = defined $itemtype ? [ $itemtype, undef ] : undef; + $search_params->{branchcode} = defined $branchcode ? [ $branchcode, undef ] : undef; my $rule = $self->search( $search_params, @@ -110,16 +232,25 @@ sub get_effective_rules { sub set_rule { my ( $self, $params ) = @_; - croak q{set_rule requires the parameter 'branchcode'!} - unless exists $params->{branchcode}; - croak q{set_rule requires the parameter 'categorycode'!} - unless exists $params->{categorycode}; - croak q{set_rule requires the parameter 'itemtype'!} - unless exists $params->{itemtype}; croak q{set_rule requires the parameter 'rule_name'!} - unless exists $params->{rule_name}; + unless exists $params->{rule_name}; croak q{set_rule requires the parameter 'rule_value'!} - unless exists $params->{rule_value}; + unless exists $params->{rule_value}; + + 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}; my $categorycode = $params->{categorycode}; @@ -170,18 +301,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/Koha/RefundLostItemFeeRules.pm b/Koha/RefundLostItemFeeRules.pm index b08ac1179d..4a1760cf1b 100644 --- a/Koha/RefundLostItemFeeRules.pm +++ b/Koha/RefundLostItemFeeRules.pm @@ -148,7 +148,7 @@ sub _default_rule { my $self = shift; my $default_rule = $self->search( { - branchcode => '*', + branchcode => undef, categorycode => undef, itemtype => undef, rule_name => 'refund', diff --git a/Koha/Schema/Result/Branch.pm b/Koha/Schema/Result/Branch.pm index 91ec691c92..fd0110148d 100644 --- a/Koha/Schema/Result/Branch.pm +++ b/Koha/Schema/Result/Branch.pm @@ -338,6 +338,21 @@ __PACKAGE__->has_many( { cascade_copy => 0, cascade_delete => 0 }, ); +=head2 circulation_rules + +Type: has_many + +Related object: L + +=cut + +__PACKAGE__->has_many( + "circulation_rules", + "Koha::Schema::Result::CirculationRule", + { "foreign.branchcode" => "self.branchcode" }, + { cascade_copy => 0, cascade_delete => 0 }, +); + =head2 club_enrollments Type: has_many diff --git a/Koha/Schema/Result/Category.pm b/Koha/Schema/Result/Category.pm index e34b2cc4cf..3f54b020e3 100644 --- a/Koha/Schema/Result/Category.pm +++ b/Koha/Schema/Result/Category.pm @@ -236,6 +236,21 @@ __PACKAGE__->has_many( { cascade_copy => 0, cascade_delete => 0 }, ); +=head2 circulation_rules + +Type: has_many + +Related object: L + +=cut + +__PACKAGE__->has_many( + "circulation_rules", + "Koha::Schema::Result::CirculationRule", + { "foreign.categorycode" => "self.categorycode" }, + { cascade_copy => 0, cascade_delete => 0 }, +); + # Created by DBIx::Class::Schema::Loader v0.07042 @ 2018-02-16 17:54:53 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:TGAiiFICSjlKNvnfa87kFQ diff --git a/Koha/Schema/Result/CirculationRule.pm b/Koha/Schema/Result/CirculationRule.pm index 360854cd5c..1eda0ab2dc 100644 --- a/Koha/Schema/Result/CirculationRule.pm +++ b/Koha/Schema/Result/CirculationRule.pm @@ -32,18 +32,21 @@ __PACKAGE__->table("circulation_rules"); =head2 branchcode data_type: 'varchar' + is_foreign_key: 1 is_nullable: 1 size: 10 =head2 categorycode data_type: 'varchar' + is_foreign_key: 1 is_nullable: 1 size: 10 =head2 itemtype data_type: 'varchar' + is_foreign_key: 1 is_nullable: 1 size: 10 @@ -65,11 +68,11 @@ __PACKAGE__->add_columns( "id", { data_type => "integer", is_auto_increment => 1, is_nullable => 0 }, "branchcode", - { data_type => "varchar", is_nullable => 1, size => 10 }, + { data_type => "varchar", is_foreign_key => 1, is_nullable => 1, size => 10 }, "categorycode", - { data_type => "varchar", is_nullable => 1, size => 10 }, + { data_type => "varchar", is_foreign_key => 1, is_nullable => 1, size => 10 }, "itemtype", - { data_type => "varchar", is_nullable => 1, size => 10 }, + { data_type => "varchar", is_foreign_key => 1, is_nullable => 1, size => 10 }, "rule_name", { data_type => "varchar", is_nullable => 0, size => 32 }, "rule_value", @@ -100,15 +103,82 @@ __PACKAGE__->set_primary_key("id"); =item * L +=item * L + =back =cut -__PACKAGE__->add_unique_constraint("branchcode_2", ["branchcode", "categorycode", "itemtype"]); +__PACKAGE__->add_unique_constraint( + "branchcode_2", + ["branchcode", "categorycode", "itemtype", "rule_name"], +); + +=head1 RELATIONS + +=head2 branchcode + +Type: belongs_to + +Related object: L + +=cut + +__PACKAGE__->belongs_to( + "branchcode", + "Koha::Schema::Result::Branch", + { branchcode => "branchcode" }, + { + is_deferrable => 1, + join_type => "LEFT", + on_delete => "CASCADE", + on_update => "CASCADE", + }, +); + +=head2 categorycode + +Type: belongs_to + +Related object: L + +=cut + +__PACKAGE__->belongs_to( + "categorycode", + "Koha::Schema::Result::Category", + { categorycode => "categorycode" }, + { + is_deferrable => 1, + join_type => "LEFT", + on_delete => "CASCADE", + on_update => "CASCADE", + }, +); + +=head2 itemtype + +Type: belongs_to + +Related object: L + +=cut + +__PACKAGE__->belongs_to( + "itemtype", + "Koha::Schema::Result::Itemtype", + { itemtype => "itemtype" }, + { + is_deferrable => 1, + join_type => "LEFT", + on_delete => "CASCADE", + on_update => "CASCADE", + }, +); -# Created by DBIx::Class::Schema::Loader v0.07042 @ 2017-07-03 15:35:15 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:ok1AzADM8/wcfU9xS7LgNQ +# Created by DBIx::Class::Schema::Loader v0.07042 @ 2017-09-30 08:26:50 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:sK9+Iwm8emAhqv9ETdCQAg # You can replace this text with custom code or comments, and it will be preserved on regeneration diff --git a/Koha/Schema/Result/Itemtype.pm b/Koha/Schema/Result/Itemtype.pm index 0e36247b50..95ff7af580 100644 --- a/Koha/Schema/Result/Itemtype.pm +++ b/Koha/Schema/Result/Itemtype.pm @@ -165,6 +165,21 @@ __PACKAGE__->has_many( { cascade_copy => 0, cascade_delete => 0 }, ); +=head2 circulation_rules + +Type: has_many + +Related object: L + +=cut + +__PACKAGE__->has_many( + "circulation_rules", + "Koha::Schema::Result::CirculationRule", + { "foreign.itemtype" => "self.itemtype" }, + { cascade_copy => 0, cascade_delete => 0 }, +); + =head2 default_branch_item_rule Type: might_have diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index d127ce7ae8..b9ee140c2d 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -69,9 +69,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, @@ -107,43 +107,55 @@ 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, - } - } - ); - } 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 => { + 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, @@ -155,7 +167,6 @@ elsif ($op eq 'delete-branch-cat') { { categorycode => $categorycode, branchcode => $branch, - itemtype => undef, rules => { max_holds => undef, patron_maxissueqty => undef, @@ -171,12 +182,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, @@ -186,7 +194,6 @@ elsif ($op eq 'delete-branch-item') { } else { Koha::CirculationRules->set_rules( { - categorycode => undef, branchcode => undef, itemtype => $itemtype, rules => { @@ -200,12 +207,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, @@ -215,7 +219,6 @@ elsif ($op eq 'delete-branch-item') { } else { Koha::CirculationRules->set_rules( { - categorycode => undef, branchcode => $branch, itemtype => $itemtype, rules => { @@ -270,12 +273,9 @@ elsif ($op eq 'add') { my $article_requests = $input->param('article_requests') || 'no'; my $overduefinescap = $input->param('overduefinescap') || undef; my $cap_fine_to_replacement_price = $input->param('cap_fine_to_replacement_price') eq 'on'; - $debug and warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty, $maxonsiteissueqty, $cap_fine_to_replacement_price"; + warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty, $maxonsiteissueqty, $cap_fine_to_replacement_price"; my $params = { - branchcode => $br, - categorycode => $bor, - itemtype => $itemtype, fine => $fine, finedays => $finedays, maxsuspensiondays => $maxsuspensiondays, @@ -304,9 +304,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 => { maxissueqty => $maxissueqty, maxonsiteissueqty => $maxonsiteissueqty, @@ -336,30 +336,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, } } ); @@ -391,7 +405,6 @@ elsif ($op eq "add-branch-cat") { Koha::CirculationRules->set_rules( { categorycode => undef, - itemtype => undef, branchcode => undef, rules => { max_holds => $max_holds, @@ -403,9 +416,8 @@ elsif ($op eq "add-branch-cat") { } else { Koha::CirculationRules->set_rules( { - branchcode => '*', categorycode => $categorycode, - itemtype => undef, + branchcode => undef, rules => { max_holds => $max_holds, patron_maxissueqty => $patron_maxissueqty, @@ -418,7 +430,6 @@ elsif ($op eq "add-branch-cat") { Koha::CirculationRules->set_rules( { categorycode => undef, - itemtype => undef, branchcode => $branch, rules => { max_holds => $max_holds, @@ -431,7 +442,6 @@ elsif ($op eq "add-branch-cat") { Koha::CirculationRules->set_rules( { categorycode => $categorycode, - itemtype => undef, branchcode => $branch, rules => { max_holds => $max_holds, @@ -455,7 +465,6 @@ elsif ($op eq "add-branch-item") { if ($itemtype eq "*") { Koha::CirculationRules->set_rules( { - categorycode => undef, itemtype => undef, branchcode => undef, rules => { @@ -468,7 +477,6 @@ elsif ($op eq "add-branch-item") { } else { Koha::CirculationRules->set_rules( { - categorycode => undef, itemtype => $itemtype, branchcode => undef, rules => { @@ -482,7 +490,6 @@ elsif ($op eq "add-branch-item") { } elsif ($itemtype eq "*") { Koha::CirculationRules->set_rules( { - categorycode => undef, itemtype => undef, branchcode => $branch, rules => { @@ -495,7 +502,6 @@ elsif ($op eq "add-branch-item") { } else { Koha::CirculationRules->set_rules( { - categorycode => undef, itemtype => $itemtype, branchcode => $branch, rules => { @@ -516,8 +522,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 @@ -528,9 +532,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 } @@ -539,7 +541,7 @@ elsif ( $op eq 'mod-refund-lost-item-fee-rule' ) { } } -my $refundLostItemFeeRule = Koha::RefundLostItemFeeRules->find({ branchcode => $branch }); +my $refundLostItemFeeRule = Koha::RefundLostItemFeeRules->find({ branchcode => $branch eq '*' ? undef : $branch }); $template->param( refundLostItemFeeRule => $refundLostItemFeeRule, defaultRefundRule => Koha::RefundLostItemFeeRules->_default_rule @@ -554,7 +556,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 340b9a978b..93d8b09b4f 100644 --- a/api/v1/swagger/definitions.json +++ b/api/v1/swagger/definitions.json @@ -1,4 +1,7 @@ { + "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 df4f55a999..d6701bd658 100644 --- a/api/v1/swagger/paths.json +++ b/api/v1/swagger/paths.json @@ -5,6 +5,9 @@ "/acquisitions/vendors/{vendor_id}": { "$ref": "paths/acquisitions_vendors.json#/~1acquisitions~1vendors~1{vendor_id}" }, + "/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/installer/data/mysql/atomicupdate/bug_18887.perl b/installer/data/mysql/atomicupdate/bug_18887.perl index 07ee2d357f..75d9c4f3e8 100644 --- a/installer/data/mysql/atomicupdate/bug_18887.perl +++ b/installer/data/mysql/atomicupdate/bug_18887.perl @@ -14,7 +14,10 @@ if( CheckVersion( $DBversion ) ) { KEY `categorycode` (`categorycode`), KEY `itemtype` (`itemtype`), KEY `rule_name` (`rule_name`), - UNIQUE (`branchcode`,`categorycode`,`itemtype`,`rule_name`) + UNIQUE (`branchcode`,`categorycode`,`itemtype`,`rule_name`), + FOREIGN KEY (branchcode) REFERENCES branches (branchcode) ON DELETE CASCADE ON UPDATE CASCADE, + FOREIGN KEY (categorycode) REFERENCES categories (categorycode) ON DELETE CASCADE ON UPDATE CASCADE, + FOREIGN KEY (itemtype) REFERENCES itemtypes (itemtype) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci; }); diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 65dc4bd7c0..ed882f6e9f 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -4135,7 +4135,10 @@ CREATE TABLE `circulation_rules` ( KEY `branchcode` (`branchcode`), KEY `categorycode` (`categorycode`), KEY `itemtype` (`itemtype`), - UNIQUE (`branchcode`,`categorycode`,`itemtype`) + UNIQUE (`branchcode`,`categorycode`,`itemtype`,`rule_name`), + FOREIGN KEY (branchcode) REFERENCES branches (branchcode) ON DELETE CASCADE ON UPDATE CASCADE, + FOREIGN KEY (categorycode) REFERENCES categories (categorycode) ON DELETE CASCADE ON UPDATE CASCADE, + FOREIGN KEY (itemtype) REFERENCES itemtypes (itemtype) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci; /*!40103 SET TIME_ZONE=@OLD_TIME_ZONE */; 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 de226c526f..0a21aac374 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 @@ -5,19 +5,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 @@ -151,19 +151,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 maxissueqty %] [% maxissueqty %] @@ -250,7 +254,7 @@ [% rentaldiscount %] Edit - Delete + Delete [% END %] @@ -552,6 +556,7 @@   [% FOREACH c IN categorycodes %] + [% NEXT UNLESS c %] [% SET patron_maxissueqty = CirculationRules.Get( branchcode, c, undef, 'patron_maxissueqty' ) %] [% SET patron_maxonsiteissueqty = CirculationRules.Get( branchcode, c, undef, 'patron_maxonsiteissueqty' ) %] [% SET max_holds = CirculationRules.Get( branchcode, c, undef, 'max_holds' ) %] @@ -559,7 +564,7 @@ [% IF patron_maxissueqty || patron_maxonsiteissueqty || max_holds %] - [% IF c == '*'%] + [% IF c == undef %] Default [% ELSE %] [% Categories.GetName(c) %] diff --git a/t/db_dependent/ArticleRequests.t b/t/db_dependent/ArticleRequests.t index 33f26f7346..f0012e8c96 100755 --- a/t/db_dependent/ArticleRequests.t +++ b/t/db_dependent/ArticleRequests.t @@ -175,9 +175,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', } @@ -190,9 +190,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', } @@ -205,9 +205,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', } @@ -220,9 +220,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 1f15776b0d..d03452d617 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -18,6 +18,7 @@ use Modern::Perl; use Test::More tests => 113; +use Test::Deep qw( cmp_deeply ); use DateTime; @@ -182,9 +183,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, @@ -343,9 +344,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); # 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', } @@ -564,9 +565,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); # Test premature manual renewal Koha::CirculationRules->set_rule( { - categorycode => '*', - branchcode => '*', - itemtype => '*', + categorycode => undef, + branchcode => undef, + itemtype => undef, rule_name => 'norenewalbefore', rule_value => '7', } @@ -641,9 +642,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - branchcode => '*', - itemtype => '*', + categorycode => undef, + branchcode => undef, + itemtype => undef, rules => { norenewalbefore => '7', no_auto_renewal_after => '9', @@ -657,9 +658,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - branchcode => '*', - itemtype => '*', + categorycode => undef, + branchcode => undef, + itemtype => undef, rules => { norenewalbefore => '7', no_auto_renewal_after => '10', @@ -673,9 +674,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - branchcode => '*', - itemtype => '*', + categorycode => undef, + branchcode => undef, + itemtype => undef, rules => { norenewalbefore => '7', no_auto_renewal_after => '11', @@ -689,9 +690,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - branchcode => '*', - itemtype => '*', + categorycode => undef, + branchcode => undef, + itemtype => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => '11', @@ -705,9 +706,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - branchcode => '*', - itemtype => '*', + categorycode => undef, + branchcode => undef, + itemtype => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => undef, @@ -722,9 +723,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - branchcode => '*', - itemtype => '*', + categorycode => undef, + branchcode => undef, + itemtype => undef, rules => { norenewalbefore => '7', no_auto_renewal_after => '15', @@ -739,9 +740,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - branchcode => '*', - itemtype => '*', + categorycode => undef, + branchcode => undef, + itemtype => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => undef, @@ -772,9 +773,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - branchcode => '*', - itemtype => '*', + categorycode => undef, + branchcode => undef, + itemtype => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => '11', @@ -874,9 +875,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); 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 => '', @@ -889,9 +890,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); 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', @@ -910,9 +911,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); $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', @@ -928,9 +929,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); 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 => '', @@ -945,9 +946,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); ); Koha::CirculationRules->set_rules( { - categorycode => '*', - branchcode => '*', - itemtype => '*', + categorycode => undef, + branchcode => undef, + itemtype => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => '15', @@ -967,9 +968,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); # set policy to forbid renewals Koha::CirculationRules->set_rules( { - categorycode => '*', - branchcode => '*', - itemtype => '*', + categorycode => undef, + branchcode => undef, + itemtype => undef, rules => { norenewalbefore => undef, renewalsallowed => 0, @@ -1196,9 +1197,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); $dbh->do('DELETE FROM circulation_rules'); Koha::CirculationRules->set_rules( { - categorycode => '*', - itemtype => '*', - branchcode => '*', + categorycode => undef, + itemtype => undef, + branchcode => undef, rules => { reservesallowed => 25, issuelength => 14, @@ -1265,9 +1266,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - itemtype => '*', - branchcode => '*', + categorycode => undef, + itemtype => undef, + branchcode => undef, rules => { onshelfholds => 0, } @@ -1279,9 +1280,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - itemtype => '*', - branchcode => '*', + categorycode => undef, + itemtype => undef, + branchcode => undef, rules => { onshelfholds => 0, } @@ -1293,9 +1294,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - itemtype => '*', - branchcode => '*', + categorycode => undef, + itemtype => undef, + branchcode => undef, rules => { onshelfholds => 1, } @@ -1307,9 +1308,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - itemtype => '*', - branchcode => '*', + categorycode => undef, + itemtype => undef, + branchcode => undef, rules => { onshelfholds => 1, } @@ -1811,23 +1812,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 { @@ -1870,9 +1887,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 @@ -1985,9 +2002,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 => { maxissueqty => 99, issuelength => 6, diff --git a/t/db_dependent/Circulation/Branch.t b/t/db_dependent/Circulation/Branch.t index d9e64800bf..0e5649d9c6 100644 --- a/t/db_dependent/Circulation/Branch.t +++ b/t/db_dependent/Circulation/Branch.t @@ -153,7 +153,6 @@ Koha::CirculationRules->set_rules( { branchcode => $samplebranch1->{branchcode}, categorycode => $samplecat->{categorycode}, - itemtype => undef, rules => { patron_maxissueqty => 5, patron_maxonsiteissueqty => 6, @@ -161,16 +160,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', } } ); @@ -185,12 +192,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', } } ); @@ -198,7 +212,6 @@ Koha::CirculationRules->set_rules( Koha::CirculationRules->set_rules( { branchcode => $samplebranch1->{branchcode}, - categorycode => undef, itemtype => $sampleitemtype1->{itemtype}, rules => { holdallowed => 5, @@ -209,7 +222,6 @@ Koha::CirculationRules->set_rules( Koha::CirculationRules->set_rules( { branchcode => $samplebranch2->{branchcode}, - categorycode => undef, itemtype => $sampleitemtype1->{itemtype}, rules => { holdallowed => 5, @@ -220,7 +232,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 89dbb2d34d..ff7d44ac40 100644 --- a/t/db_dependent/Circulation/CalcFine.t +++ b/t/db_dependent/Circulation/CalcFine.t @@ -68,9 +68,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', @@ -106,9 +106,9 @@ subtest 'Test cap_fine_to_replacement_price' => sub { plan tests => 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 9700741a40..3c42f7a9d5 100644 --- a/t/db_dependent/Circulation/GetHardDueDate.t +++ b/t/db_dependent/Circulation/GetHardDueDate.t @@ -7,6 +7,8 @@ use Koha::DateUtils; use Koha::CirculationRules; use Koha::Library; +use t::lib::TestBuilder; + use Test::More tests => 10; BEGIN { @@ -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,12 +121,8 @@ my $default = { my $sampleissuingrule1 = { branchcode => $samplebranch1->{branchcode}, categorycode => $samplecat->{categorycode}, - itemtype => 'BOOK', + itemtype => $sampleitemtype1, rules => { - reservecharge => '0.000000', - chargename => 'Null', - restrictedtype => 0, - accountsent => 0, finedays => 0, lengthunit => 'days', renewalperiod => 5, @@ -148,7 +150,7 @@ my $sampleissuingrule1 = { my $sampleissuingrule2 = { branchcode => $samplebranch2->{branchcode}, categorycode => $samplecat->{categorycode}, - itemtype => 'BOOK', + itemtype => $sampleitemtype1, rules => { renewalsallowed => 'Null', renewalperiod => 2, @@ -166,10 +168,6 @@ my $sampleissuingrule2 = { chargeperiod_charge_at => 0, rentaldiscount => 2.00, overduefinescap => 'Null', - accountsent => 'Null', - reservecharge => 'Null', - chargename => 'Null', - restrictedtype => 'Null', maxsuspensiondays => 0, onshelfholds => 1, opacitemholds => 'Y', @@ -181,7 +179,7 @@ my $sampleissuingrule2 = { my $sampleissuingrule3 = { branchcode => $samplebranch1->{branchcode}, categorycode => $samplecat->{categorycode}, - itemtype => 'DVD', + itemtype => $sampleitemtype2, rules => { renewalsallowed => 'Null', renewalperiod => 3, @@ -199,10 +197,6 @@ my $sampleissuingrule3 = { chargeperiod_charge_at => 0, rentaldiscount => 3.00, overduefinescap => 'Null', - accountsent => 'Null', - reservecharge => 'Null', - chargename => 'Null', - restrictedtype => 'Null', maxsuspensiondays => 0, onshelfholds => 1, opacitemholds => 'F', @@ -235,7 +229,7 @@ is_deeply( is_deeply( C4::Circulation::GetLoanLength( $samplecat->{categorycode}, - 'BOOK', $samplebranch1->{branchcode} + $sampleitemtype1, $samplebranch1->{branchcode} ), { issuelength => 5, lengthunit => 'days', renewalperiod => 5 }, "GetLoanLength" @@ -257,12 +251,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, @@ -273,7 +267,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 8257c9e280..d79c6aa0ca 100644 --- a/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t +++ b/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t @@ -33,9 +33,9 @@ my $userenv->{branch} = $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 b554d0dc38..e0db28be4c 100644 --- a/t/db_dependent/Circulation/Returns.t +++ b/t/db_dependent/Circulation/Returns.t @@ -51,9 +51,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, } @@ -285,9 +285,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 198cf15536..0501797f18 100644 --- a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t +++ b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t @@ -79,14 +79,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, } @@ -153,8 +150,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 564cc3a20d..11a08945e6 100644 --- a/t/db_dependent/Circulation/TooMany.t +++ b/t/db_dependent/Circulation/TooMany.t @@ -106,7 +106,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, @@ -215,7 +215,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, @@ -255,7 +255,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, @@ -311,7 +311,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 e2489a51c6..49e6fe39ef 100644 --- a/t/db_dependent/Circulation/issue.t +++ b/t/db_dependent/Circulation/issue.t @@ -290,9 +290,9 @@ is_deeply( # Add a default rule: No renewal allowed Koha::CirculationRules->set_rules( { - categorycode => '*', - itemtype => '*', - branchcode => '*', + categorycode => undef, + itemtype => undef, + branchcode => undef, rules => { issuelength => 10, renewalsallowed => 0, @@ -321,9 +321,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 1f09f8963b..3dda43ffc4 100755 --- a/t/db_dependent/DecreaseLoanHighHolds.t +++ b/t/db_dependent/DecreaseLoanHighHolds.t @@ -96,13 +96,14 @@ for my $i ( 0 .. 5 ) { Koha::CirculationRules->set_rules( { - branchcode => '*', - categorycode => '*', - itemtype => '*', + branchcode => undef, + categorycode => undef, + itemtype => undef, 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 20d34cebc2..da5e193b4b 100644 --- a/t/db_dependent/Fines.t +++ b/t/db_dependent/Fines.t @@ -20,9 +20,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, @@ -47,11 +47,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 06f48673e0..b4f5c7b5de 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -245,9 +245,9 @@ my ($foreign_item_bibnum, $foreign_item_bibitemnum, $foreign_itemnumber) $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, @@ -256,8 +256,8 @@ Koha::CirculationRules->set_rules( ); Koha::CirculationRules->set_rules( { - categorycode => '*', - branchcode => '*', + categorycode => undef, + branchcode => undef, itemtype => 'CANNOT', rules => { reservesallowed => 0, @@ -363,9 +363,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, @@ -376,7 +376,6 @@ Koha::CirculationRules->set_rules( { branchcode => $branch_1, itemtype => 'CANNOT', - categorycode => undef, rules => { holdallowed => 0, returnbranch => 'homebranch', @@ -387,7 +386,6 @@ Koha::CirculationRules->set_rules( { branchcode => $branch_1, itemtype => 'CAN', - categorycode => undef, rules => { holdallowed => 1, returnbranch => 'homebranch', @@ -426,8 +424,8 @@ $dbh->do('DELETE FROM biblio'); Koha::CirculationRules->set_rules( { - categorycode => '*', - branchcode => '*', + categorycode => undef, + branchcode => undef, itemtype => 'ONLY1', rules => { reservesallowed => 1, @@ -449,15 +447,16 @@ subtest 'Test max_holds per library/patron category' => sub { $dbh->do('DELETE FROM reserves'); $dbh->do('DELETE FROM circulation_rules'); - ( $bibnum, $title, $bibitemnum ) = create_helper_biblio('TEST'); + my $testitemtype = $builder->build({ source => 'Itemtype' })->{itemtype}; + ( $bibnum, $title, $bibitemnum ) = create_helper_biblio($testitemtype); ( $item_bibnum, $item_bibitemnum, $itemnumber ) = AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 }, $bibnum ); Koha::CirculationRules->set_rules( { - categorycode => '*', - branchcode => '*', - itemtype => 'TEST', + categorycode => undef, + branchcode => undef, + itemtype => $testitemtype, rules => { reservesallowed => 99, holds_per_record => 99, @@ -479,7 +478,6 @@ subtest 'Test max_holds per library/patron category' => sub { { categorycode => $category->{categorycode}, branchcode => undef, - itemtype => undef, rule_name => 'max_holds', rule_value => 3, } @@ -489,7 +487,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 4d8802c6f4..c4842b4689 100755 --- a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t +++ b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t @@ -95,9 +95,9 @@ my $item2 = GetItem( $itemnumber2 ); $dbh->do("DELETE FROM circulation_rules"); Koha::CirculationRules->set_rules( { - categorycode => '*', - itemtype => '*', - branchcode => '*', + categorycode => undef, + itemtype => undef, + branchcode => undef, rules => { issuelength => 7, lengthunit => 8, diff --git a/t/db_dependent/Holds/HoldFulfillmentPolicy.t b/t/db_dependent/Holds/HoldFulfillmentPolicy.t index e82cdd8b88..803ebc44ec 100755 --- a/t/db_dependent/Holds/HoldFulfillmentPolicy.t +++ b/t/db_dependent/Holds/HoldFulfillmentPolicy.t @@ -74,7 +74,6 @@ my $itemnumber = $dbh->do("DELETE FROM circulation_rules"); Koha::CirculationRules->set_rules( { - categorycode => undef, branchcode => undef, itemtype => undef, rules => { @@ -106,7 +105,6 @@ Koha::Holds->find( $reserve_id )->cancel; $dbh->do("DELETE FROM circulation_rules"); Koha::CirculationRules->set_rules( { - categorycode => undef, branchcode => undef, itemtype => undef, rules => { @@ -138,7 +136,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 dca97af829..c461a7cd2b 100755 --- a/t/db_dependent/HoldsQueue.t +++ b/t/db_dependent/HoldsQueue.t @@ -293,7 +293,6 @@ Koha::CirculationRules->set_rule( rule_name => 'holdallowed', rule_value => 1, branchcode => undef, - categorycode => undef, itemtype => undef, } ); @@ -331,7 +330,6 @@ Koha::CirculationRules->set_rule( rule_name => 'holdallowed', rule_value => 2, branchcode => undef, - categorycode => undef, itemtype => undef, } ); @@ -359,7 +357,6 @@ Koha::CirculationRules->set_rule( rule_name => 'holdallowed', rule_value => 2, branchcode => undef, - categorycode => undef, itemtype => undef, } ); @@ -479,7 +476,6 @@ Koha::CirculationRules->set_rules( { branchcode => $library_A, itemtype => $itemtype, - categorycode => undef, rules => { holdallowed => 2, returnbranch => 'homebranch', @@ -578,7 +574,6 @@ Koha::CirculationRules->set_rules( { branchcode => undef, itemtype => undef, - categorycode => undef, rules => { holdallowed => 2, hold_fulfillment_policy => 'homebranch', @@ -613,7 +608,6 @@ Koha::CirculationRules->set_rules( { branchcode => undef, itemtype => undef, - categorycode => undef, rules => { holdallowed => 2, hold_fulfillment_policy => 'holdingbranch', @@ -648,7 +642,6 @@ Koha::CirculationRules->set_rules( { branchcode => undef, itemtype => undef, - categorycode => undef, rules => { holdallowed => 2, hold_fulfillment_policy => 'any', @@ -716,7 +709,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 987ad0d9bd..eda239cb2b 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 => 3; +use Test::More tests => 2; +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,75 +337,135 @@ 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' ); + }; }; -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 3c7b235f17..18ff46d01a 100755 --- a/t/db_dependent/RefundLostItemFeeRule.t +++ b/t/db_dependent/RefundLostItemFeeRule.t @@ -47,17 +47,19 @@ subtest 'Koha::RefundLostItemFeeRule::delete() tests' => sub { { source => 'CirculationRule', value => { - branchcode => '*', + branchcode => undef, categorycode => undef, itemtype => undef, rule_name => 'refund', } } ); + 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', @@ -67,7 +69,7 @@ subtest 'Koha::RefundLostItemFeeRule::delete() tests' => sub { my $default_rule = Koha::CirculationRules->search( { - branchcode => '*', + branchcode => undef, categorycode => undef, itemtype => undef, rule_name => 'refund', @@ -109,7 +111,7 @@ subtest 'Koha::RefundLostItemFeeRules::_default_rule() tests' => sub { { source => 'CirculationRule', value => { - branchcode => '*', + branchcode => undef, 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', @@ -130,7 +134,7 @@ subtest 'Koha::RefundLostItemFeeRules::_default_rule() tests' => sub { my $default_rule = Koha::CirculationRules->search( { - branchcode => '*', + branchcode => undef, categorycode => undef, itemtype => undef, rule_name => 'refund', @@ -146,7 +150,7 @@ subtest 'Koha::RefundLostItemFeeRules::_default_rule() tests' => sub { # Re-read from DB, to be sure $default_rule = Koha::CirculationRules->search( { - branchcode => '*', + branchcode => undef, categorycode => undef, itemtype => undef, rule_name => 'refund', @@ -177,7 +181,7 @@ subtest 'Koha::RefundLostItemFeeRules::_effective_branch_rule() tests' => sub { { source => 'CirculationRule', value => { - branchcode => '*', + branchcode => undef, 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', @@ -293,7 +301,7 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { { source => 'CirculationRule', value => { - branchcode => '*', + branchcode => undef, 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', @@ -363,7 +377,7 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { 1,'No rule for branch, global rule applied (true)'); # Change the default value just to try - Koha::CirculationRules->search({ branchcode => '*', rule_name => 'refund' })->next->rule_value(0)->store; + Koha::CirculationRules->search({ branchcode => undef, rule_name => 'refund' })->next->rule_value(0)->store; t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'ItemHoldingBranch' ); is( Koha::RefundLostItemFeeRules->should_refund( $params ), 0,'No rule for branch, global rule applied (false)'); diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 3cc3527437..44a973757b 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -201,9 +201,9 @@ $requesters{$branch_3} = AddMember( $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, @@ -215,7 +215,6 @@ Koha::CirculationRules->set_rules( Koha::CirculationRules->set_rules( { branchcode => $branch_1, - categorycode => undef, itemtype => undef, rules => { holdallowed => 1, @@ -228,7 +227,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 ff64caba32..4d1e1772aa 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 10ce689596..3b113ea3a7 100644 --- a/t/db_dependent/TestBuilder.t +++ b/t/db_dependent/TestBuilder.t @@ -355,12 +355,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 1dce19de95..3d01749a49 100644 --- a/t/db_dependent/api/v1/holds.t +++ b/t/db_dependent/api/v1/holds.t @@ -140,9 +140,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.15.1