From 73810c28e2ee65cc885bf31d071e2d45f48ac651 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 | 46 ++- Koha/CirculationRules.pm | 170 +++++++++-- Koha/REST/V1/CirculationRules.pm | 32 +++ Koha/RefundLostItemFeeRules.pm | 2 +- Koha/Schema/Result/Branch.pm | 19 +- Koha/Schema/Result/Category.pm | 19 +- Koha/Schema/Result/CirculationRule.pm | 82 +++++- Koha/Schema/Result/Itemtype.pm | 19 +- admin/smart-rules.pl | 96 +++---- 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 | 23 +- 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 | 311 +++++++++++++++++---- 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, 964 insertions(+), 418 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 b344af59de..bf985160b2 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -425,7 +425,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 @@ -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' ) "; } else { @@ -443,7 +443,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' ) "; } @@ -465,7 +465,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; @@ -1499,38 +1499,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 0d891818d9..4ee7e566ad 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -355,7 +355,7 @@ sub CanItemBeReserved { $holds_per_record = $rights->{holds_per_record}; } else { - $ruleitemtype = '*'; + $ruleitemtype = undef; } $item = Koha::Items->find( $itemnumber ); @@ -380,15 +380,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"; @@ -402,19 +402,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', } ); @@ -1819,21 +1810,16 @@ sub OPACItemHoldsAllowed { } } - my $query = "SELECT opacitemholds,categorycode,itemtype,branchcode FROM issuingrules WHERE - (issuingrules.categorycode = ? OR issuingrules.categorycode = '*') - AND - (issuingrules.itemtype = ? OR issuingrules.itemtype = '*') - AND - (issuingrules.branchcode = ? OR issuingrules.branchcode = '*') - ORDER BY - issuingrules.categorycode desc, - issuingrules.itemtype desc, - issuingrules.branchcode desc - LIMIT 1"; - my $sth = $dbh->prepare($query); - $sth->execute($borrower->{categorycode},$itype,$branchcode); - my $data = $sth->fetchrow_hashref; - my $opacitemholds = uc substr ($data->{opacitemholds}, 0, 1); + my $data = Koha::CirculationRules->get_effective_rule( { + branchcode => $branchcode, + categorycode => $borrower->{categorycode}, + itemtype => $itype, + rule_name => 'opacitemholds', + order_by => { + -desc => [ 'categorycode', 'itemtype', 'branchcode' ] + } + } ); + my $opacitemholds = $data ? uc substr ($data->{rule_value}, 0, 1) : ''; return '' if $opacitemholds eq 'N'; return $opacitemholds; } 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 76b4d8c331..ef1a5ea953 100644 --- a/Koha/Schema/Result/Branch.pm +++ b/Koha/Schema/Result/Branch.pm @@ -353,6 +353,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 @@ -589,8 +604,8 @@ Composing rels: L -> categorycode __PACKAGE__->many_to_many("categorycodes", "branchrelations", "categorycode"); -# Created by DBIx::Class::Schema::Loader v0.07042 @ 2017-07-03 15:35:15 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:9z/UhS1n8kZ2OEvevR8KZg +# Created by DBIx::Class::Schema::Loader v0.07042 @ 2017-09-30 08:26:50 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:SGWU+hnPENNNoIPRvxHWZw # You can replace this text with custom code or comments, and it will be preserved on regeneration diff --git a/Koha/Schema/Result/Category.pm b/Koha/Schema/Result/Category.pm index 7bae95510b..5c49a88d47 100644 --- a/Koha/Schema/Result/Category.pm +++ b/Koha/Schema/Result/Category.pm @@ -236,9 +236,24 @@ __PACKAGE__->has_many( { cascade_copy => 0, cascade_delete => 0 }, ); +=head2 circulation_rules -# Created by DBIx::Class::Schema::Loader v0.07042 @ 2017-07-03 15:35:15 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:Nyb7RzCc6y0W0izWTjksoA +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 @ 2017-09-30 08:26:50 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:fJF/fj7sCECysHEB3pDgyg # You can replace this text with custom content, and it will be preserved on regeneration 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 1c2f37920e..b0399f74de 100644 --- a/Koha/Schema/Result/Itemtype.pm +++ b/Koha/Schema/Result/Itemtype.pm @@ -149,6 +149,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 @@ -195,8 +210,8 @@ __PACKAGE__->has_many( ); -# Created by DBIx::Class::Schema::Loader v0.07042 @ 2017-07-03 15:35:15 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:tXTiZk4HKLEBWm/tBXJ9Dw +# Created by DBIx::Class::Schema::Loader v0.07042 @ 2017-09-30 08:26:50 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:D1B+jh+5hkb5U3OOQoepXA # Use the ItemtypeLocalization view to create the join on localization our $LANGUAGE; diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index c1b5ee2888..ebedfa35ec 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -70,9 +70,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, @@ -110,12 +110,19 @@ elsif ($op eq 'delete-branch-cat') { if ($categorycode eq "*") { Koha::CirculationRules->set_rules( { - categorycode => undef, branchcode => undef, - itemtype => 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, @@ -127,7 +134,6 @@ elsif ($op eq 'delete-branch-cat') { { categorycode => $categorycode, branchcode => undef, - itemtype => undef, rules => { max_holds => undef, patron_maxissueqty => undef, @@ -139,12 +145,18 @@ elsif ($op eq 'delete-branch-cat') { } 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, @@ -156,7 +168,6 @@ elsif ($op eq 'delete-branch-cat') { { categorycode => $categorycode, branchcode => $branch, - itemtype => undef, rules => { max_holds => undef, maxissueqty => undef, @@ -172,12 +183,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, @@ -187,7 +195,6 @@ elsif ($op eq 'delete-branch-item') { } else { Koha::CirculationRules->set_rules( { - categorycode => undef, branchcode => undef, itemtype => $itemtype, rules => { @@ -201,12 +208,9 @@ elsif ($op eq 'delete-branch-item') { } elsif ($itemtype eq "*") { Koha::CirculationRules->set_rules( { - categorycode => undef, branchcode => $branch, itemtype => undef, rules => { - patron_maxissueqty => undef, - patron_maxonsiteissueqty => undef, holdallowed => undef, hold_fulfillment_policy => undef, returnbranch => undef, @@ -216,7 +220,6 @@ elsif ($op eq 'delete-branch-item') { } else { Koha::CirculationRules->set_rules( { - categorycode => undef, branchcode => $branch, itemtype => $itemtype, rules => { @@ -271,12 +274,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, @@ -305,9 +305,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, @@ -337,33 +337,47 @@ 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, } } ); - } else { Koha::CirculationRules->set_rules( { categorycode => undef, - itemtype => undef, - branchcode => $branch, + branchcode => undef, rules => { patron_maxissueqty => $patron_maxissueqty, patron_maxonsiteissueqty => $patron_maxonsiteissueqty, + } + } + ); + } else { + Koha::CirculationRules->set_rules( + { + itemtype => undef, + branchcode => $branch, + rules => { 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, + } + } + ); } Koha::CirculationRules->set_rule( { @@ -392,7 +406,6 @@ elsif ($op eq "add-branch-cat") { Koha::CirculationRules->set_rules( { categorycode => undef, - itemtype => undef, branchcode => undef, rules => { max_holds => $max_holds, @@ -404,9 +417,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, @@ -419,7 +431,6 @@ elsif ($op eq "add-branch-cat") { Koha::CirculationRules->set_rules( { categorycode => undef, - itemtype => undef, branchcode => $branch, rules => { max_holds => $max_holds, @@ -432,7 +443,6 @@ elsif ($op eq "add-branch-cat") { Koha::CirculationRules->set_rules( { categorycode => $categorycode, - itemtype => undef, branchcode => $branch, rules => { max_holds => $max_holds, @@ -456,7 +466,6 @@ elsif ($op eq "add-branch-item") { if ($itemtype eq "*") { Koha::CirculationRules->set_rules( { - categorycode => undef, itemtype => undef, branchcode => undef, rules => { @@ -469,7 +478,6 @@ elsif ($op eq "add-branch-item") { } else { Koha::CirculationRules->set_rules( { - categorycode => undef, itemtype => $itemtype, branchcode => undef, rules => { @@ -483,7 +491,6 @@ elsif ($op eq "add-branch-item") { } elsif ($itemtype eq "*") { Koha::CirculationRules->set_rules( { - categorycode => undef, itemtype => undef, branchcode => $branch, rules => { @@ -496,7 +503,6 @@ elsif ($op eq "add-branch-item") { } else { Koha::CirculationRules->set_rules( { - categorycode => undef, itemtype => $itemtype, branchcode => $branch, rules => { @@ -517,8 +523,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 @@ -529,9 +533,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 } @@ -540,7 +542,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 @@ -555,7 +557,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 147e985386..96d62971c2 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 38610c9d44..b223baaf4a 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 bf7653f57b..00f27dc34d 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -4082,7 +4082,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 d12050411d..a31a0c2303 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 @@ -4,19 +4,19 @@ [% USE ItemTypes %] [% USE CirculationRules %] -[% 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 @@ -255,19 +255,23 @@ $(document).ready(function() { [% 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 %] @@ -354,7 +358,7 @@ $(document).ready(function() { [% rentaldiscount %] Edit - Delete + Delete [% END %] @@ -658,6 +662,7 @@ $(document).ready(function() {   [% 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' ) %] @@ -665,7 +670,7 @@ $(document).ready(function() { [% 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 01588cec27..ca1e6d2c4c 100755 --- a/t/db_dependent/ArticleRequests.t +++ b/t/db_dependent/ArticleRequests.t @@ -147,9 +147,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', } @@ -162,9 +162,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', } @@ -177,9 +177,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', } @@ -192,9 +192,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 555b951870..c7fd198676 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -18,6 +18,7 @@ use Modern::Perl; use Test::More tests => 102; +use Test::Deep qw( cmp_deeply ); use DateTime; @@ -170,9 +171,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, @@ -330,9 +331,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', } @@ -551,9 +552,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', } @@ -628,9 +629,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', @@ -644,9 +645,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', @@ -660,9 +661,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', @@ -676,9 +677,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', @@ -692,9 +693,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, @@ -709,9 +710,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', @@ -726,9 +727,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, @@ -759,9 +760,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', @@ -809,9 +810,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 => '', @@ -824,9 +825,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', @@ -845,9 +846,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', @@ -863,9 +864,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 => '', @@ -880,9 +881,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', @@ -902,9 +903,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, @@ -1126,9 +1127,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, @@ -1196,9 +1197,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - itemtype => '*', - branchcode => '*', + categorycode => undef, + itemtype => undef, + branchcode => undef, rules => { onshelfholds => 0, } @@ -1210,9 +1211,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - itemtype => '*', - branchcode => '*', + categorycode => undef, + itemtype => undef, + branchcode => undef, rules => { onshelfholds => 0, } @@ -1224,9 +1225,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - itemtype => '*', - branchcode => '*', + categorycode => undef, + itemtype => undef, + branchcode => undef, rules => { onshelfholds => 1, } @@ -1238,9 +1239,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); Koha::CirculationRules->set_rules( { - categorycode => '*', - itemtype => '*', - branchcode => '*', + categorycode => undef, + itemtype => undef, + branchcode => undef, rules => { onshelfholds => 1, } @@ -1754,23 +1755,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 { @@ -1813,9 +1830,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 @@ -1928,9 +1945,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 d19581d5d5..137915635d 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,14 +160,22 @@ Koha::CirculationRules->set_rules( } ); + Koha::CirculationRules->set_rules( { branchcode => $samplebranch2->{branchcode}, categorycode => undef, - itemtype => undef, rules => { patron_maxissueqty => 3, patron_maxonsiteissueqty => 2, + } + } +); +Koha::CirculationRules->set_rules( + { + branchcode => $samplebranch2->{branchcode}, + itemtype => undef, + rules => { holdallowed => 1, returnbranch => 'holdingbranch', } @@ -179,10 +186,17 @@ Koha::CirculationRules->set_rules( { branchcode => undef, categorycode => undef, - itemtype => undef, rules => { patron_maxissueqty => 4, patron_maxonsiteissueqty => 5, + } + } +); +Koha::CirculationRules->set_rules( + { + branchcode => undef, + itemtype => undef, + rules => { holdallowed => 3, returnbranch => 'homebranch', } @@ -192,7 +206,6 @@ Koha::CirculationRules->set_rules( Koha::CirculationRules->set_rules( { branchcode => $samplebranch1->{branchcode}, - categorycode => undef, itemtype => $sampleitemtype1->{itemtype}, rules => { holdallowed => 5, @@ -203,7 +216,6 @@ Koha::CirculationRules->set_rules( Koha::CirculationRules->set_rules( { branchcode => $samplebranch2->{branchcode}, - categorycode => undef, itemtype => $sampleitemtype1->{itemtype}, rules => { holdallowed => 5, @@ -214,7 +226,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 8a0adfa3a7..de262d0653 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 39a556f626..36403808ad 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 45a96ecea4..471a8c03f3 100644 --- a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t +++ b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t @@ -76,14 +76,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, } @@ -150,8 +147,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 d5f3b33adf..07297c9383 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, @@ -162,7 +162,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, @@ -202,7 +202,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, @@ -258,7 +258,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 ec5033fe6e..162249f639 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 fa117c2547..5ccce05dcd 100755 --- a/t/db_dependent/DecreaseLoanHighHolds.t +++ b/t/db_dependent/DecreaseLoanHighHolds.t @@ -90,13 +90,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 a556140d83..fbc49fbac6 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -235,9 +235,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, @@ -246,8 +246,8 @@ Koha::CirculationRules->set_rules( ); Koha::CirculationRules->set_rules( { - categorycode => '*', - branchcode => '*', + categorycode => undef, + branchcode => undef, itemtype => 'CANNOT', rules => { reservesallowed => 0, @@ -353,9 +353,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, @@ -366,7 +366,6 @@ Koha::CirculationRules->set_rules( { branchcode => $branch_1, itemtype => 'CANNOT', - categorycode => undef, rules => { holdallowed => 0, returnbranch => 'homebranch', @@ -377,7 +376,6 @@ Koha::CirculationRules->set_rules( { branchcode => $branch_1, itemtype => 'CAN', - categorycode => undef, rules => { holdallowed => 1, returnbranch => 'homebranch', @@ -416,8 +414,8 @@ $dbh->do('DELETE FROM biblio'); Koha::CirculationRules->set_rules( { - categorycode => '*', - branchcode => '*', + categorycode => undef, + branchcode => undef, itemtype => 'ONLY1', rules => { reservesallowed => 1, @@ -439,15 +437,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, @@ -469,7 +468,6 @@ subtest 'Test max_holds per library/patron category' => sub { { categorycode => $category->{categorycode}, branchcode => undef, - itemtype => undef, rule_name => 'max_holds', rule_value => 3, } @@ -479,7 +477,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 4a1ffcdceb..721ec20730 100755 --- a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t +++ b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t @@ -89,9 +89,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 ef99662cdd..0828f93452 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 => 1; +use Test::More tests => 2; +use Test::Deep qw( cmp_methods ); +use Test::Exception; use Benchmark; @@ -35,15 +37,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; @@ -58,25 +57,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; @@ -91,109 +98,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, @@ -207,8 +270,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 { @@ -265,11 +336,135 @@ subtest 'get_effective_issuing_rule' => sub { }; }; -sub _row_match { - my ($rule, $branchcode, $categorycode, $itemtype) = @_; +subtest 'set_rule' => sub { + plan tests => 3; + + my $branchcode = $builder->build({ source => 'Branch' })->{'branchcode'}; + my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'}; + my $itemtype = $builder->build({ source => 'Itemtype' })->{'itemtype'}; + + subtest 'Correct call' => sub { + plan tests => 4; + + 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 _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 3105d40deb..00bbe88b4d 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 a5a9112789..76a03ef5ad 100644 --- a/t/db_dependent/TestBuilder.t +++ b/t/db_dependent/TestBuilder.t @@ -351,12 +351,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 9a96bbaeaa..49282ae6a8 100644 --- a/t/db_dependent/api/v1/holds.t +++ b/t/db_dependent/api/v1/holds.t @@ -133,9 +133,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.11.0