Bugzilla – Attachment 72233 Details for
Bug 18936
Move issuingrules into circulation_rules
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules
Bug-18936-follow-up-Add-foreign-key-and-scope-enha.patch (text/plain), 120.15 KB, created by
Jesse Weaver
on 2018-02-26 19:18:35 UTC
(
hide
)
Description:
Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules
Filename:
MIME Type:
Creator:
Jesse Weaver
Created:
2018-02-26 19:18:35 UTC
Size:
120.15 KB
patch
obsolete
>From 6fc665dd91437a4114f649733da9e24a7cf7e5d7 Mon Sep 17 00:00:00 2001 >From: Jesse Weaver <pianohacker@gmail.com> >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<patron_maxissueqty> for a branchcode and itemtype), is an error. >+ >+=cut >+ >+our $RULE_KINDS = { >+ refund => { >+ scope => [ 'branchcode' ], >+ }, >+ >+ patron_maxissueqty => { >+ scope => [ 'branchcode', 'categorycode' ], >+ }, >+ patron_maxonsiteissueqty => { >+ scope => [ 'branchcode', 'categorycode' ], >+ }, >+ max_holds => { >+ scope => [ 'branchcode', 'categorycode' ], >+ }, >+ >+ holdallowed => { >+ scope => [ 'branchcode', 'itemtype' ], >+ }, >+ hold_fulfillment_policy => { >+ scope => [ 'branchcode', 'itemtype' ], >+ }, >+ returnbranch => { >+ scope => [ 'branchcode', 'itemtype' ], >+ }, >+ >+ article_requests => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ auto_renew => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ cap_fine_to_replacement_price => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ chargeperiod => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ chargeperiod_charge_at => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ fine => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ finedays => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ firstremind => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ hardduedate => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ hardduedatecompare => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ holds_per_record => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ issuelength => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ lengthunit => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ maxissueqty => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ maxonsiteissueqty => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ maxsuspensiondays => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ no_auto_renewal_after => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ no_auto_renewal_after_hard_limit => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ norenewalbefore => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ onshelfholds => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ opacitemholds => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ overduefinescap => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ renewalperiod => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ renewalsallowed => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ rentaldiscount => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ reservesallowed => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, >+ # Not included (deprecated?): >+ # * accountsent >+ # * chargename >+ # * reservecharge >+ # * restrictedtype >+}; >+ >+sub rule_kinds { >+ return $RULE_KINDS; >+} >+ > =head3 get_effective_rule > > =cut >@@ -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<Koha::Schema::Result::CirculationRule> >+ >+=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<Koha::Schema::Result::CirculationRule> >+ >+=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</itemtype> > >+=item * L</rule_name> >+ > =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<Koha::Schema::Result::Branch> >+ >+=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<Koha::Schema::Result::Category> >+ >+=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<Koha::Schema::Result::Itemtype> >+ >+=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<Koha::Schema::Result::CirculationRule> >+ >+=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' %] > <title>Koha › Administration › Circulation and fine rules</title> >@@ -151,19 +151,23 @@ > [% SET row_count = row_count + 1 %] > <tr row_countd="row_[% row_count %]"> > <td> >- [% IF c == '*' %] >+ [% IF c == undef %] > <em>All</em> > [% ELSE %] > [% Categories.GetName(c) %] > [% END %] > </td> > <td> >- [% IF i == '*' %] >+ [% IF i == undef %] > <em>All</em> > [% ELSE %] > [% ItemTypes.GetDescription(i) %] > [% END %] > </td> >+ <td class="actions"> >+ <a href="#" class="editrule btn btn-default btn-xs"><i class="fa fa-pencil"></i> Edit</a> >+ <a class="btn btn-default btn-xs delete" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete&itemtype=[% rule.itemtype || '*' %]&categorycode=[% rule.categorycode || '*' %]&branch=[% current_branch %]"><i class="fa fa-trash"></i> Delete</a> >+ </td> > <td> > [% IF maxissueqty %] > [% maxissueqty %] >@@ -250,7 +254,7 @@ > <td>[% rentaldiscount %]</td> > <td class="actions"> > <a href="#" class="editrule btn btn-default btn-xs"><i class="fa fa-pencil"></i> Edit</a> >- <a class="btn btn-default btn-xs delete" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete&itemtype=[% rule.itemtype %]&categorycode=[% rule.categorycode %]&branch=[% rule.current_branch %]"><i class="fa fa-trash"></i> Delete</a> >+ <a class="btn btn-default btn-xs delete" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete&itemtype=[% rule.itemtype || '*' %]&categorycode=[% rule.categorycode || '*' %]&branch=[% current_branch %]"><i class="fa fa-trash"></i> Delete</a> > </td> > </tr> > [% END %] >@@ -552,6 +556,7 @@ > <th> </th> > </tr> > [% 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 %] > <tr> > <td> >- [% IF c == '*'%] >+ [% IF c == undef %] > <em>Default</em> > [% 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 18936
:
65247
|
65248
|
65249
|
67629
|
67758
|
68236
|
71034
|
71035
|
71036
|
71037
|
71038
|
71039
|
71040
|
71041
|
72230
|
72231
|
72232
|
72233
|
72234
|
72235
|
72236
|
72237
|
72238
|
91953
|
91954
|
91955
|
91956
|
91957
|
91958
|
91959
|
91960
|
91961
|
91962
|
91963
|
91964
|
91965
|
91966
|
91967
|
91968
|
91969
|
91970
|
91971
|
91972
|
91973
|
91974
|
91975
|
94179
|
94180
|
94181
|
94182
|
94183
|
94184
|
94185
|
94186
|
94187
|
94188
|
94189
|
94190
|
94191
|
94192
|
94193
|
94194
|
94195
|
94196
|
94197
|
94198
|
94199
|
94200
|
98299
|
98300
|
98301
|
98302
|
98303
|
98304
|
98305
|
98306
|
98307
|
98308
|
98309
|
98310
|
98311
|
98312
|
98313
|
98314
|
98315
|
98316
|
98317
|
98318
|
98319
|
98320
|
98321
|
98322
|
98323
|
98324
|
98325
|
98326
|
98327
|
98328
|
98329
|
98330
|
98331
|
98332
|
98333
|
98334
|
98335
|
98336
|
98337
|
98338
|
98339
|
98340
|
98341
|
98342
|
98343
|
98344
|
98345
|
98346
|
98347
|
98407
|
98408