Bugzilla – Attachment 67629 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), 117.02 KB, created by
Jesse Weaver
on 2017-10-05 06:15:33 UTC
(
hide
)
Description:
Bug 18936: (follow-up) Add foreign key and scope enhancement to circ rules
Filename:
MIME Type:
Creator:
Jesse Weaver
Created:
2017-10-05 06:15:33 UTC
Size:
117.02 KB
patch
obsolete
>From 73810c28e2ee65cc885bf31d071e2d45f48ac651 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 | 46 ++- > Koha/CirculationRules.pm | 170 +++++++++-- > Koha/REST/V1/CirculationRules.pm | 32 +++ > Koha/RefundLostItemFeeRules.pm | 2 +- > Koha/Schema/Result/Branch.pm | 19 +- > Koha/Schema/Result/Category.pm | 19 +- > Koha/Schema/Result/CirculationRule.pm | 82 +++++- > Koha/Schema/Result/Itemtype.pm | 19 +- > admin/smart-rules.pl | 96 +++---- > api/v1/swagger/definitions.json | 3 + > api/v1/swagger/definitions/circ-rule-kind.json | 15 + > api/v1/swagger/paths.json | 3 + > api/v1/swagger/paths/circulation-rules.json | 35 +++ > installer/data/mysql/atomicupdate/bug_18887.perl | 5 +- > installer/data/mysql/kohastructure.sql | 5 +- > .../prog/en/modules/admin/smart-rules.tt | 19 +- > t/db_dependent/ArticleRequests.t | 24 +- > t/db_dependent/Circulation.t | 171 ++++++----- > t/db_dependent/Circulation/Branch.t | 23 +- > t/db_dependent/Circulation/CalcFine.t | 12 +- > t/db_dependent/Circulation/GetHardDueDate.t | 32 +-- > .../Circulation/IssuingRules/maxsuspensiondays.t | 12 +- > t/db_dependent/Circulation/Returns.t | 12 +- > t/db_dependent/Circulation/SwitchOnSiteCheckouts.t | 11 +- > t/db_dependent/Circulation/TooMany.t | 8 +- > t/db_dependent/Circulation/issue.t | 12 +- > t/db_dependent/DecreaseLoanHighHolds.t | 7 +- > t/db_dependent/Fines.t | 14 +- > t/db_dependent/Holds.t | 33 +-- > .../Holds/DisallowHoldIfItemsAvailable.t | 6 +- > t/db_dependent/Holds/HoldFulfillmentPolicy.t | 3 - > t/db_dependent/Holds/HoldItemtypeLimit.t | 1 - > t/db_dependent/HoldsQueue.t | 8 - > t/db_dependent/Koha/IssuingRules.t | 311 +++++++++++++++++---- > t/db_dependent/RefundLostItemFeeRule.t | 30 +- > t/db_dependent/Reserves.t | 8 +- > t/db_dependent/Reserves/MultiplePerRecord.t | 34 +-- > t/db_dependent/TestBuilder.t | 2 + > t/db_dependent/api/v1/holds.t | 6 +- > 40 files changed, 964 insertions(+), 418 deletions(-) > create mode 100644 Koha/REST/V1/CirculationRules.pm > create mode 100644 api/v1/swagger/definitions/circ-rule-kind.json > create mode 100644 api/v1/swagger/paths/circulation-rules.json > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index b344af59de..bf985160b2 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -425,7 +425,7 @@ sub TooMany { > |; > > my $rule_itemtype = $maxissueqty_rule->itemtype; >- if ($rule_itemtype eq "*") { >+ if (!defined $rule_itemtype) { > # matching rule has the default item type, so count only > # those existing loans that don't fall under a more > # specific rule >@@ -434,7 +434,7 @@ sub TooMany { > SELECT itemtype FROM circulation_rules > WHERE branchcode = ? > AND (categorycode = ? OR categorycode = ?) >- AND itemtype <> '*' >+ AND itemtype IS NOT NULL > AND rule_name = 'maxissueqty' > ) "; > } else { >@@ -443,7 +443,7 @@ sub TooMany { > SELECT itemtype FROM circulation_rules > WHERE branchcode = ? > AND (categorycode = ? OR categorycode = ?) >- AND itemtype <> '*' >+ AND itemtype IS NOT NULL > AND rule_name = 'maxissueqty' > ) "; > } >@@ -465,7 +465,7 @@ sub TooMany { > $count_query .= " AND borrowernumber = ? "; > push @bind_params, $borrower->{'borrowernumber'}; > my $rule_branch = $maxissueqty_rule->branchcode; >- if ($rule_branch ne "*") { >+ if (defined $rule_branch) { > if (C4::Context->preference('CircControl') eq 'PickupLibrary') { > $count_query .= " AND issues.branchcode = ? "; > push @bind_params, $branch; >@@ -1499,38 +1499,38 @@ sub GetLoanLength { > }, > { > categorycode => $categorycode, >- itemtype => '*', >+ itemtype => undef, > branchcode => $branchcode, > }, > { >- categorycode => '*', >+ categorycode => undef, > itemtype => $itemtype, > branchcode => $branchcode, > }, > { >- categorycode => '*', >- itemtype => '*', >+ categorycode => undef, >+ itemtype => undef, > branchcode => $branchcode, > }, > { > categorycode => $categorycode, > itemtype => $itemtype, >- branchcode => '*', >+ branchcode => undef, > }, > { > categorycode => $categorycode, >- itemtype => '*', >- branchcode => '*', >+ itemtype => undef, >+ branchcode => undef, > }, > { >- categorycode => '*', >+ categorycode => undef, > itemtype => $itemtype, >- branchcode => '*', >+ branchcode => undef, > }, > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > }, > ); > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 0d891818d9..4ee7e566ad 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -355,7 +355,7 @@ sub CanItemBeReserved { > $holds_per_record = $rights->{holds_per_record}; > } > else { >- $ruleitemtype = '*'; >+ $ruleitemtype = undef; > } > > $item = Koha::Items->find( $itemnumber ); >@@ -380,15 +380,15 @@ sub CanItemBeReserved { > C4::Context->preference('item-level_itypes') > ? " AND COALESCE( items.itype, biblioitems.itemtype ) = ?" > : " AND biblioitems.itemtype = ?" >- if ( $ruleitemtype ne "*" ); >+ if defined $ruleitemtype; > > my $sthcount = $dbh->prepare($querycount); > >- if ( $ruleitemtype eq "*" ) { >- $sthcount->execute( $borrowernumber, $branchcode ); >+ if ( defined $ruleitemtype ) { >+ $sthcount->execute( $borrowernumber, $branchcode, $ruleitemtype ); > } > else { >- $sthcount->execute( $borrowernumber, $branchcode, $ruleitemtype ); >+ $sthcount->execute( $borrowernumber, $branchcode ); > } > > my $reservecount = "0"; >@@ -402,19 +402,10 @@ sub CanItemBeReserved { > } > > # Now we need to check hold limits by patron category >- my $rule = Koha::CirculationRules->find( >+ my $rule = Koha::CirculationRules->get_effective_rule( > { >- categorycode => $borrower->{categorycode}, > branchcode => $borrower->{branchcode}, >- itemtype => undef, >- rule_name => 'max_holds', >- } >- ); >- $rule ||= Koha::CirculationRules->find( >- { > categorycode => $borrower->{categorycode}, >- branchcode => undef,, >- itemtype => undef, > rule_name => 'max_holds', > } > ); >@@ -1819,21 +1810,16 @@ sub OPACItemHoldsAllowed { > } > } > >- my $query = "SELECT opacitemholds,categorycode,itemtype,branchcode FROM issuingrules WHERE >- (issuingrules.categorycode = ? OR issuingrules.categorycode = '*') >- AND >- (issuingrules.itemtype = ? OR issuingrules.itemtype = '*') >- AND >- (issuingrules.branchcode = ? OR issuingrules.branchcode = '*') >- ORDER BY >- issuingrules.categorycode desc, >- issuingrules.itemtype desc, >- issuingrules.branchcode desc >- LIMIT 1"; >- my $sth = $dbh->prepare($query); >- $sth->execute($borrower->{categorycode},$itype,$branchcode); >- my $data = $sth->fetchrow_hashref; >- my $opacitemholds = uc substr ($data->{opacitemholds}, 0, 1); >+ my $data = Koha::CirculationRules->get_effective_rule( { >+ branchcode => $branchcode, >+ categorycode => $borrower->{categorycode}, >+ itemtype => $itype, >+ rule_name => 'opacitemholds', >+ order_by => { >+ -desc => [ 'categorycode', 'itemtype', 'branchcode' ] >+ } >+ } ); >+ my $opacitemholds = $data ? uc substr ($data->{rule_value}, 0, 1) : ''; > return '' if $opacitemholds eq 'N'; > return $opacitemholds; > } >diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm >index 8b6fd77bcd..e3a4b8c0d0 100644 >--- a/Koha/CirculationRules.pm >+++ b/Koha/CirculationRules.pm >@@ -35,6 +35,128 @@ Koha::CirculationRules - Koha CirculationRule Object set class > > =cut > >+=head3 rule_kinds >+ >+This structure describes the possible rules that may be set, and what scopes they can be set at. >+ >+Any attempt to set a rule with a nonsensical scope (for instance, setting the C<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 76b4d8c331..ef1a5ea953 100644 >--- a/Koha/Schema/Result/Branch.pm >+++ b/Koha/Schema/Result/Branch.pm >@@ -353,6 +353,21 @@ __PACKAGE__->has_many( > { cascade_copy => 0, cascade_delete => 0 }, > ); > >+=head2 circulation_rules >+ >+Type: has_many >+ >+Related object: L<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 >@@ -589,8 +604,8 @@ Composing rels: L</branchrelations> -> categorycode > __PACKAGE__->many_to_many("categorycodes", "branchrelations", "categorycode"); > > >-# Created by DBIx::Class::Schema::Loader v0.07042 @ 2017-07-03 15:35:15 >-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:9z/UhS1n8kZ2OEvevR8KZg >+# Created by DBIx::Class::Schema::Loader v0.07042 @ 2017-09-30 08:26:50 >+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:SGWU+hnPENNNoIPRvxHWZw > > > # You can replace this text with custom code or comments, and it will be preserved on regeneration >diff --git a/Koha/Schema/Result/Category.pm b/Koha/Schema/Result/Category.pm >index 7bae95510b..5c49a88d47 100644 >--- a/Koha/Schema/Result/Category.pm >+++ b/Koha/Schema/Result/Category.pm >@@ -236,9 +236,24 @@ __PACKAGE__->has_many( > { cascade_copy => 0, cascade_delete => 0 }, > ); > >+=head2 circulation_rules > >-# Created by DBIx::Class::Schema::Loader v0.07042 @ 2017-07-03 15:35:15 >-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:Nyb7RzCc6y0W0izWTjksoA >+Type: has_many >+ >+Related object: L<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 @ 2017-09-30 08:26:50 >+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:fJF/fj7sCECysHEB3pDgyg > > > # You can replace this text with custom content, and it will be preserved on regeneration >diff --git a/Koha/Schema/Result/CirculationRule.pm b/Koha/Schema/Result/CirculationRule.pm >index 360854cd5c..1eda0ab2dc 100644 >--- a/Koha/Schema/Result/CirculationRule.pm >+++ b/Koha/Schema/Result/CirculationRule.pm >@@ -32,18 +32,21 @@ __PACKAGE__->table("circulation_rules"); > =head2 branchcode > > data_type: 'varchar' >+ is_foreign_key: 1 > is_nullable: 1 > size: 10 > > =head2 categorycode > > data_type: 'varchar' >+ is_foreign_key: 1 > is_nullable: 1 > size: 10 > > =head2 itemtype > > data_type: 'varchar' >+ is_foreign_key: 1 > is_nullable: 1 > size: 10 > >@@ -65,11 +68,11 @@ __PACKAGE__->add_columns( > "id", > { data_type => "integer", is_auto_increment => 1, is_nullable => 0 }, > "branchcode", >- { data_type => "varchar", is_nullable => 1, size => 10 }, >+ { data_type => "varchar", is_foreign_key => 1, is_nullable => 1, size => 10 }, > "categorycode", >- { data_type => "varchar", is_nullable => 1, size => 10 }, >+ { data_type => "varchar", is_foreign_key => 1, is_nullable => 1, size => 10 }, > "itemtype", >- { data_type => "varchar", is_nullable => 1, size => 10 }, >+ { data_type => "varchar", is_foreign_key => 1, is_nullable => 1, size => 10 }, > "rule_name", > { data_type => "varchar", is_nullable => 0, size => 32 }, > "rule_value", >@@ -100,15 +103,82 @@ __PACKAGE__->set_primary_key("id"); > > =item * L</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 1c2f37920e..b0399f74de 100644 >--- a/Koha/Schema/Result/Itemtype.pm >+++ b/Koha/Schema/Result/Itemtype.pm >@@ -149,6 +149,21 @@ __PACKAGE__->has_many( > { cascade_copy => 0, cascade_delete => 0 }, > ); > >+=head2 circulation_rules >+ >+Type: has_many >+ >+Related object: L<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 >@@ -195,8 +210,8 @@ __PACKAGE__->has_many( > ); > > >-# Created by DBIx::Class::Schema::Loader v0.07042 @ 2017-07-03 15:35:15 >-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:tXTiZk4HKLEBWm/tBXJ9Dw >+# Created by DBIx::Class::Schema::Loader v0.07042 @ 2017-09-30 08:26:50 >+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:D1B+jh+5hkb5U3OOQoepXA > > # Use the ItemtypeLocalization view to create the join on localization > our $LANGUAGE; >diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl >index c1b5ee2888..ebedfa35ec 100755 >--- a/admin/smart-rules.pl >+++ b/admin/smart-rules.pl >@@ -70,9 +70,9 @@ if ($op eq 'delete') { > > Koha::CirculationRules->set_rules( > { >- categorycode => $categorycode, >- branchcode => $branch, >- itemtype => $itemtype, >+ categorycode => $categorycode eq '*' ? undef : $categorycode, >+ branchcode => $branch eq '*' ? undef : $branch, >+ itemtype => $itemtype eq '*' ? undef : $itemtype, > rules => { > restrictedtype => undef, > rentaldiscount => undef, >@@ -110,12 +110,19 @@ elsif ($op eq 'delete-branch-cat') { > if ($categorycode eq "*") { > Koha::CirculationRules->set_rules( > { >- categorycode => undef, > branchcode => undef, >- itemtype => undef, >+ categorycode => undef, > rules => { > patron_maxissueqty => undef, > patron_maxonsiteissueqty => undef, >+ } >+ } >+ ); >+ Koha::CirculationRules->set_rules( >+ { >+ branchcode => undef, >+ itemtype => undef, >+ rules => { > holdallowed => undef, > hold_fulfillment_policy => undef, > returnbranch => undef, >@@ -127,7 +134,6 @@ elsif ($op eq 'delete-branch-cat') { > { > categorycode => $categorycode, > branchcode => undef, >- itemtype => undef, > rules => { > max_holds => undef, > patron_maxissueqty => undef, >@@ -139,12 +145,18 @@ elsif ($op eq 'delete-branch-cat') { > } elsif ($categorycode eq "*") { > Koha::CirculationRules->set_rules( > { >+ branchcode => $branch, > categorycode => undef, >+ rules => { >+ patron_maxissueqty => undef, >+ patron_maxonsiteissueqty => undef, >+ } >+ } >+ ); >+ Koha::CirculationRules->set_rules( >+ { > branchcode => $branch, >- itemtype => undef, > rules => { >- patron_maxissueqty => undef, >- patron_maxonsiteissueqty => undef, > holdallowed => undef, > hold_fulfillment_policy => undef, > returnbranch => undef, >@@ -156,7 +168,6 @@ elsif ($op eq 'delete-branch-cat') { > { > categorycode => $categorycode, > branchcode => $branch, >- itemtype => undef, > rules => { > max_holds => undef, > maxissueqty => undef, >@@ -172,12 +183,9 @@ elsif ($op eq 'delete-branch-item') { > if ($itemtype eq "*") { > Koha::CirculationRules->set_rules( > { >- categorycode => undef, > branchcode => undef, > itemtype => undef, > rules => { >- patron_maxissueqty => undef, >- patron_maxonsiteissueqty => undef, > holdallowed => undef, > hold_fulfillment_policy => undef, > returnbranch => undef, >@@ -187,7 +195,6 @@ elsif ($op eq 'delete-branch-item') { > } else { > Koha::CirculationRules->set_rules( > { >- categorycode => undef, > branchcode => undef, > itemtype => $itemtype, > rules => { >@@ -201,12 +208,9 @@ elsif ($op eq 'delete-branch-item') { > } elsif ($itemtype eq "*") { > Koha::CirculationRules->set_rules( > { >- categorycode => undef, > branchcode => $branch, > itemtype => undef, > rules => { >- patron_maxissueqty => undef, >- patron_maxonsiteissueqty => undef, > holdallowed => undef, > hold_fulfillment_policy => undef, > returnbranch => undef, >@@ -216,7 +220,6 @@ elsif ($op eq 'delete-branch-item') { > } else { > Koha::CirculationRules->set_rules( > { >- categorycode => undef, > branchcode => $branch, > itemtype => $itemtype, > rules => { >@@ -271,12 +274,9 @@ elsif ($op eq 'add') { > my $article_requests = $input->param('article_requests') || 'no'; > my $overduefinescap = $input->param('overduefinescap') || undef; > my $cap_fine_to_replacement_price = $input->param('cap_fine_to_replacement_price') eq 'on'; >- $debug and warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty, $maxonsiteissueqty, $cap_fine_to_replacement_price"; >+ warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty, $maxonsiteissueqty, $cap_fine_to_replacement_price"; > > my $params = { >- branchcode => $br, >- categorycode => $bor, >- itemtype => $itemtype, > fine => $fine, > finedays => $finedays, > maxsuspensiondays => $maxsuspensiondays, >@@ -305,9 +305,9 @@ elsif ($op eq 'add') { > > Koha::CirculationRules->set_rules( > { >- categorycode => $bor, >- itemtype => $itemtype, >- branchcode => $br, >+ categorycode => $bor eq '*' ? undef : $bor, >+ itemtype => $itemtype eq '*' ? undef : $itemtype, >+ branchcode => $br eq '*' ? undef : $br, > rules => { > maxissueqty => $maxissueqty, > maxonsiteissueqty => $maxonsiteissueqty, >@@ -337,33 +337,47 @@ elsif ($op eq "set-branch-defaults") { > if ($branch eq "*") { > Koha::CirculationRules->set_rules( > { >- categorycode => undef, > itemtype => undef, > branchcode => undef, > rules => { >- patron_maxissueqty => $patron_maxissueqty, >- patron_maxonsiteissueqty => $patron_maxonsiteissueqty, > holdallowed => $holdallowed, > hold_fulfillment_policy => $hold_fulfillment_policy, > returnbranch => $returnbranch, > } > } > ); >- } else { > Koha::CirculationRules->set_rules( > { > categorycode => undef, >- itemtype => undef, >- branchcode => $branch, >+ branchcode => undef, > rules => { > patron_maxissueqty => $patron_maxissueqty, > patron_maxonsiteissueqty => $patron_maxonsiteissueqty, >+ } >+ } >+ ); >+ } else { >+ Koha::CirculationRules->set_rules( >+ { >+ itemtype => undef, >+ branchcode => $branch, >+ rules => { > holdallowed => $holdallowed, > hold_fulfillment_policy => $hold_fulfillment_policy, > returnbranch => $returnbranch, > } > } > ); >+ Koha::CirculationRules->set_rules( >+ { >+ categorycode => undef, >+ branchcode => $branch, >+ rules => { >+ patron_maxissueqty => $patron_maxissueqty, >+ patron_maxonsiteissueqty => $patron_maxonsiteissueqty, >+ } >+ } >+ ); > } > Koha::CirculationRules->set_rule( > { >@@ -392,7 +406,6 @@ elsif ($op eq "add-branch-cat") { > Koha::CirculationRules->set_rules( > { > categorycode => undef, >- itemtype => undef, > branchcode => undef, > rules => { > max_holds => $max_holds, >@@ -404,9 +417,8 @@ elsif ($op eq "add-branch-cat") { > } else { > Koha::CirculationRules->set_rules( > { >- branchcode => '*', > categorycode => $categorycode, >- itemtype => undef, >+ branchcode => undef, > rules => { > max_holds => $max_holds, > patron_maxissueqty => $patron_maxissueqty, >@@ -419,7 +431,6 @@ elsif ($op eq "add-branch-cat") { > Koha::CirculationRules->set_rules( > { > categorycode => undef, >- itemtype => undef, > branchcode => $branch, > rules => { > max_holds => $max_holds, >@@ -432,7 +443,6 @@ elsif ($op eq "add-branch-cat") { > Koha::CirculationRules->set_rules( > { > categorycode => $categorycode, >- itemtype => undef, > branchcode => $branch, > rules => { > max_holds => $max_holds, >@@ -456,7 +466,6 @@ elsif ($op eq "add-branch-item") { > if ($itemtype eq "*") { > Koha::CirculationRules->set_rules( > { >- categorycode => undef, > itemtype => undef, > branchcode => undef, > rules => { >@@ -469,7 +478,6 @@ elsif ($op eq "add-branch-item") { > } else { > Koha::CirculationRules->set_rules( > { >- categorycode => undef, > itemtype => $itemtype, > branchcode => undef, > rules => { >@@ -483,7 +491,6 @@ elsif ($op eq "add-branch-item") { > } elsif ($itemtype eq "*") { > Koha::CirculationRules->set_rules( > { >- categorycode => undef, > itemtype => undef, > branchcode => $branch, > rules => { >@@ -496,7 +503,6 @@ elsif ($op eq "add-branch-item") { > } else { > Koha::CirculationRules->set_rules( > { >- categorycode => undef, > itemtype => $itemtype, > branchcode => $branch, > rules => { >@@ -517,8 +523,6 @@ elsif ( $op eq 'mod-refund-lost-item-fee-rule' ) { > # only do something for $refund eq '*' if branch-specific > Koha::CirculationRules->set_rules( > { >- categorycode => undef, >- itemtype => undef, > branchcode => $branch, > rules => { > refund => undef >@@ -529,9 +533,7 @@ elsif ( $op eq 'mod-refund-lost-item-fee-rule' ) { > } else { > Koha::CirculationRules->set_rules( > { >- categorycode => undef, >- itemtype => undef, >- branchcode => $branch, >+ branchcode => undef, > rules => { > refund => $refund > } >@@ -540,7 +542,7 @@ elsif ( $op eq 'mod-refund-lost-item-fee-rule' ) { > } > } > >-my $refundLostItemFeeRule = Koha::RefundLostItemFeeRules->find({ branchcode => $branch }); >+my $refundLostItemFeeRule = Koha::RefundLostItemFeeRules->find({ branchcode => $branch eq '*' ? undef : $branch }); > $template->param( > refundLostItemFeeRule => $refundLostItemFeeRule, > defaultRefundRule => Koha::RefundLostItemFeeRules->_default_rule >@@ -555,7 +557,7 @@ $template->param(show_branch_cat_rule_form => 1); > $template->param( > patron_categories => $patron_categories, > itemtypeloop => $itemtypes, >- humanbranch => ( $branch ne '*' ? $branch : '' ), >+ humanbranch => ( $branch ne '*' ? $branch : undef ), > current_branch => $branch, > ); > output_html_with_http_headers $input, $cookie, $template->output; >diff --git a/api/v1/swagger/definitions.json b/api/v1/swagger/definitions.json >index 340b9a978b..93d8b09b4f 100644 >--- a/api/v1/swagger/definitions.json >+++ b/api/v1/swagger/definitions.json >@@ -1,4 +1,7 @@ > { >+ "circ-rule-kind": { >+ "$ref": "definitions/circ-rule-kind.json" >+ }, > "city": { > "$ref": "definitions/city.json" > }, >diff --git a/api/v1/swagger/definitions/circ-rule-kind.json b/api/v1/swagger/definitions/circ-rule-kind.json >new file mode 100644 >index 0000000000..90b956f9f6 >--- /dev/null >+++ b/api/v1/swagger/definitions/circ-rule-kind.json >@@ -0,0 +1,15 @@ >+{ >+ "type": "object", >+ "properties": { >+ "scope": { >+ "description": "levels that this rule kind can be set for", >+ "type": "array", >+ "items": { >+ "type": "string", >+ "enum": [ "branchcode", "categorycode", "itemtype" ] >+ } >+ } >+ }, >+ "additionalProperties": false, >+ "required": ["scope"] >+} >diff --git a/api/v1/swagger/paths.json b/api/v1/swagger/paths.json >index 147e985386..96d62971c2 100644 >--- a/api/v1/swagger/paths.json >+++ b/api/v1/swagger/paths.json >@@ -5,6 +5,9 @@ > "/acquisitions/vendors/{vendor_id}": { > "$ref": "paths/acquisitions_vendors.json#/~1acquisitions~1vendors~1{vendor_id}" > }, >+ "/circulation-rules/kinds": { >+ "$ref": "paths/circulation-rules.json#/~1circulation-rules~1kinds" >+ }, > "/cities": { > "$ref": "paths/cities.json#/~1cities" > }, >diff --git a/api/v1/swagger/paths/circulation-rules.json b/api/v1/swagger/paths/circulation-rules.json >new file mode 100644 >index 0000000000..e897cd7276 >--- /dev/null >+++ b/api/v1/swagger/paths/circulation-rules.json >@@ -0,0 +1,35 @@ >+{ >+ "/circulation-rules/kinds": { >+ "get": { >+ "x-mojo-controller": "Koha::REST::V1::CirculationRules", >+ "operationId": "get_kinds", >+ "tags": ["cities"], >+ "produces": [ >+ "application/json" >+ ], >+ "responses": { >+ "200": { >+ "description": "A map of rule kind information", >+ "schema": { >+ "type": "object", >+ "additionalProperties": { >+ "$ref": "../definitions.json#/circ-rule-kind" >+ } >+ } >+ }, >+ "403": { >+ "description": "Access forbidden", >+ "schema": { >+ "$ref": "../definitions.json#/error" >+ } >+ }, >+ "500": { >+ "description": "Internal error", >+ "schema": { >+ "$ref": "../definitions.json#/error" >+ } >+ } >+ } >+ } >+ } >+} >diff --git a/installer/data/mysql/atomicupdate/bug_18887.perl b/installer/data/mysql/atomicupdate/bug_18887.perl >index 38610c9d44..b223baaf4a 100644 >--- a/installer/data/mysql/atomicupdate/bug_18887.perl >+++ b/installer/data/mysql/atomicupdate/bug_18887.perl >@@ -14,7 +14,10 @@ if( CheckVersion( $DBversion ) ) { > KEY `categorycode` (`categorycode`), > KEY `itemtype` (`itemtype`), > KEY `rule_name` (`rule_name`), >- UNIQUE (`branchcode`,`categorycode`,`itemtype`,`rule_name`) >+ UNIQUE (`branchcode`,`categorycode`,`itemtype`,`rule_name`), >+ FOREIGN KEY (branchcode) REFERENCES branches (branchcode) ON DELETE CASCADE ON UPDATE CASCADE, >+ FOREIGN KEY (categorycode) REFERENCES categories (categorycode) ON DELETE CASCADE ON UPDATE CASCADE, >+ FOREIGN KEY (itemtype) REFERENCES itemtypes (itemtype) ON DELETE CASCADE ON UPDATE CASCADE > ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci; > }); > >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index bf7653f57b..00f27dc34d 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -4082,7 +4082,10 @@ CREATE TABLE `circulation_rules` ( > KEY `branchcode` (`branchcode`), > KEY `categorycode` (`categorycode`), > KEY `itemtype` (`itemtype`), >- UNIQUE (`branchcode`,`categorycode`,`itemtype`) >+ UNIQUE (`branchcode`,`categorycode`,`itemtype`,`rule_name`), >+ FOREIGN KEY (branchcode) REFERENCES branches (branchcode) ON DELETE CASCADE ON UPDATE CASCADE, >+ FOREIGN KEY (categorycode) REFERENCES categories (categorycode) ON DELETE CASCADE ON UPDATE CASCADE, >+ FOREIGN KEY (itemtype) REFERENCES itemtypes (itemtype) ON DELETE CASCADE ON UPDATE CASCADE > ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci; > > /*!40103 SET TIME_ZONE=@OLD_TIME_ZONE */; >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt >index d12050411d..a31a0c2303 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt >@@ -4,19 +4,19 @@ > [% USE ItemTypes %] > [% USE CirculationRules %] > >-[% SET branchcode = humanbranch || '*' %] >+[% SET branchcode = humanbranch || undef %] > > [% SET categorycodes = [] %] > [% FOREACH pc IN patron_categories %] > [% categorycodes.push( pc.id ) %] > [% END %] >-[% categorycodes.push('*') %] >+[% categorycodes.push(undef) %] > > [% SET itemtypes = [] %] > [% FOREACH i IN itemtypeloop %] > [% itemtypes.push( i.itemtype ) %] > [% END %] >-[% itemtypes.push('*') %] >+[% itemtypes.push(undef) %] > > [% INCLUDE 'doc-head-open.inc' %] > <title>Koha › Administration › Circulation and fine rules</title> >@@ -255,19 +255,23 @@ $(document).ready(function() { > [% 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 %] >@@ -354,7 +358,7 @@ $(document).ready(function() { > <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 %] >@@ -658,6 +662,7 @@ $(document).ready(function() { > <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' ) %] >@@ -665,7 +670,7 @@ $(document).ready(function() { > [% 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 01588cec27..ca1e6d2c4c 100755 >--- a/t/db_dependent/ArticleRequests.t >+++ b/t/db_dependent/ArticleRequests.t >@@ -147,9 +147,9 @@ is( $biblio->article_requests_finished()->count(), 1, 'Canceled request not retu > > my $rule = Koha::CirculationRules->set_rule( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rule_name => 'article_requests', > rule_value => 'yes', > } >@@ -162,9 +162,9 @@ $rule->delete(); > > $rule = Koha::CirculationRules->set_rule( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rule_name => 'article_requests', > rule_value => 'bib_only', > } >@@ -177,9 +177,9 @@ $rule->delete(); > > $rule = Koha::CirculationRules->set_rule( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rule_name => 'article_requests', > rule_value => 'item_only', > } >@@ -192,9 +192,9 @@ $rule->delete(); > > $rule = Koha::CirculationRules->set_rule( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rule_name => 'article_requests', > rule_value => 'no', > } >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index 555b951870..c7fd198676 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -18,6 +18,7 @@ > use Modern::Perl; > > use Test::More tests => 102; >+use Test::Deep qw( cmp_deeply ); > > use DateTime; > >@@ -170,9 +171,9 @@ is( > $dbh->do('DELETE FROM circulation_rules'); > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > reservesallowed => 25, > issuelength => 14, >@@ -330,9 +331,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > # Testing of feature to allow the renewal of reserved items if other items on the record can fill all needed holds > Koha::CirculationRules->set_rule( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rule_name => 'onshelfholds', > rule_value => '1', > } >@@ -551,9 +552,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > # Test premature manual renewal > Koha::CirculationRules->set_rule( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rule_name => 'norenewalbefore', > rule_value => '7', > } >@@ -628,9 +629,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > norenewalbefore => '7', > no_auto_renewal_after => '9', >@@ -644,9 +645,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > norenewalbefore => '7', > no_auto_renewal_after => '10', >@@ -660,9 +661,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > norenewalbefore => '7', > no_auto_renewal_after => '11', >@@ -676,9 +677,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > norenewalbefore => '10', > no_auto_renewal_after => '11', >@@ -692,9 +693,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > norenewalbefore => '10', > no_auto_renewal_after => undef, >@@ -709,9 +710,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > norenewalbefore => '7', > no_auto_renewal_after => '15', >@@ -726,9 +727,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > norenewalbefore => '10', > no_auto_renewal_after => undef, >@@ -759,9 +760,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > norenewalbefore => '10', > no_auto_renewal_after => '11', >@@ -809,9 +810,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > AddIssue( $renewing_borrower, $item_to_auto_renew->{barcode}, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > norenewalbefore => '7', > no_auto_renewal_after => '', >@@ -824,9 +825,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > my $five_days_before = dt_from_string->add( days => -5 ); > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > norenewalbefore => '10', > no_auto_renewal_after => '5', >@@ -845,9 +846,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > $dbh->do(q{UPDATE circulation_rules SET rule_value = NULL WHERE rule_name = 'no_auto_renewal_after_hard_limit'}); > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > norenewalbefore => '10', > no_auto_renewal_after => '15', >@@ -863,9 +864,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > my $two_days_ahead = dt_from_string->add( days => 2 ); > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > norenewalbefore => '10', > no_auto_renewal_after => '', >@@ -880,9 +881,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > ); > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > norenewalbefore => '10', > no_auto_renewal_after => '15', >@@ -902,9 +903,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > # set policy to forbid renewals > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > norenewalbefore => undef, > renewalsallowed => 0, >@@ -1126,9 +1127,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > $dbh->do('DELETE FROM circulation_rules'); > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > reservesallowed => 25, > issuelength => 14, >@@ -1196,9 +1197,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > onshelfholds => 0, > } >@@ -1210,9 +1211,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > onshelfholds => 0, > } >@@ -1224,9 +1225,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > onshelfholds => 1, > } >@@ -1238,9 +1239,9 @@ C4::Context->dbh->do("DELETE FROM accountlines"); > > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > onshelfholds => 1, > } >@@ -1754,23 +1755,39 @@ subtest 'CanBookBeIssued + AllowMultipleIssuesOnABiblio' => sub { > > t::lib::Mocks::mock_preference('AllowMultipleIssuesOnABiblio', 0); > ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} ); >- is( keys(%$error) + keys(%$alerts), 0, 'No error or alert should be raised' . str($error, $question, $alerts) ); >- is( $question->{BIBLIO_ALREADY_ISSUED}, 1, 'BIBLIO_ALREADY_ISSUED question flag should be set if AllowMultipleIssuesOnABiblio=0 and issue already exists' . str($error, $question, $alerts) ); >+ cmp_deeply( >+ { error => $error, alerts => $alerts }, >+ { error => {}, alerts => {} }, >+ 'No error or alert should be raised' >+ ); >+ is( $question->{BIBLIO_ALREADY_ISSUED}, 1, 'BIBLIO_ALREADY_ISSUED question flag should be set if AllowMultipleIssuesOnABiblio=0 and issue already exists' ); > > t::lib::Mocks::mock_preference('AllowMultipleIssuesOnABiblio', 1); > ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} ); >- is( keys(%$error) + keys(%$question) + keys(%$alerts), 0, 'No BIBLIO_ALREADY_ISSUED flag should be set if AllowMultipleIssuesOnABiblio=1' . str($error, $question, $alerts) ); >+ cmp_deeply( >+ { error => $error, question => $question, alerts => $alerts }, >+ { error => {}, question => {}, alerts => {} }, >+ 'No BIBLIO_ALREADY_ISSUED flag should be set if AllowMultipleIssuesOnABiblio=1' >+ ); > > # Add a subscription > Koha::Subscription->new({ biblionumber => $biblionumber })->store; > > t::lib::Mocks::mock_preference('AllowMultipleIssuesOnABiblio', 0); > ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} ); >- is( keys(%$error) + keys(%$question) + keys(%$alerts), 0, 'No BIBLIO_ALREADY_ISSUED flag should be set if it is a subscription' . str($error, $question, $alerts) ); >+ cmp_deeply( >+ { error => $error, question => $question, alerts => $alerts }, >+ { error => {}, question => {}, alerts => {} }, >+ 'No BIBLIO_ALREADY_ISSUED flag should be set if it is a subscription' >+ ); > > t::lib::Mocks::mock_preference('AllowMultipleIssuesOnABiblio', 1); > ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} ); >- is( keys(%$error) + keys(%$question) + keys(%$alerts), 0, 'No BIBLIO_ALREADY_ISSUED flag should be set if it is a subscription' . str($error, $question, $alerts) ); >+ cmp_deeply( >+ { error => $error, question => $question, alerts => $alerts }, >+ { error => {}, question => {}, alerts => {} }, >+ 'No BIBLIO_ALREADY_ISSUED flag should be set if it is a subscription' >+ ); > }; > > subtest 'AddReturn + CumulativeRestrictionPeriods' => sub { >@@ -1813,9 +1830,9 @@ subtest 'AddReturn + CumulativeRestrictionPeriods' => sub { > Koha::CirculationRules->search->delete; > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > issuelength => 1, > firstremind => 1, # 1 day of grace >@@ -1928,9 +1945,9 @@ subtest 'AddReturn | is_overdue' => sub { > Koha::CirculationRules->search->delete; > my $rule = Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > maxissueqty => 99, > issuelength => 6, >diff --git a/t/db_dependent/Circulation/Branch.t b/t/db_dependent/Circulation/Branch.t >index d19581d5d5..137915635d 100644 >--- a/t/db_dependent/Circulation/Branch.t >+++ b/t/db_dependent/Circulation/Branch.t >@@ -153,7 +153,6 @@ Koha::CirculationRules->set_rules( > { > branchcode => $samplebranch1->{branchcode}, > categorycode => $samplecat->{categorycode}, >- itemtype => undef, > rules => { > patron_maxissueqty => 5, > patron_maxonsiteissueqty => 6, >@@ -161,14 +160,22 @@ Koha::CirculationRules->set_rules( > } > ); > >+ > Koha::CirculationRules->set_rules( > { > branchcode => $samplebranch2->{branchcode}, > categorycode => undef, >- itemtype => undef, > rules => { > patron_maxissueqty => 3, > patron_maxonsiteissueqty => 2, >+ } >+ } >+); >+Koha::CirculationRules->set_rules( >+ { >+ branchcode => $samplebranch2->{branchcode}, >+ itemtype => undef, >+ rules => { > holdallowed => 1, > returnbranch => 'holdingbranch', > } >@@ -179,10 +186,17 @@ Koha::CirculationRules->set_rules( > { > branchcode => undef, > categorycode => undef, >- itemtype => undef, > rules => { > patron_maxissueqty => 4, > patron_maxonsiteissueqty => 5, >+ } >+ } >+); >+Koha::CirculationRules->set_rules( >+ { >+ branchcode => undef, >+ itemtype => undef, >+ rules => { > holdallowed => 3, > returnbranch => 'homebranch', > } >@@ -192,7 +206,6 @@ Koha::CirculationRules->set_rules( > Koha::CirculationRules->set_rules( > { > branchcode => $samplebranch1->{branchcode}, >- categorycode => undef, > itemtype => $sampleitemtype1->{itemtype}, > rules => { > holdallowed => 5, >@@ -203,7 +216,6 @@ Koha::CirculationRules->set_rules( > Koha::CirculationRules->set_rules( > { > branchcode => $samplebranch2->{branchcode}, >- categorycode => undef, > itemtype => $sampleitemtype1->{itemtype}, > rules => { > holdallowed => 5, >@@ -214,7 +226,6 @@ Koha::CirculationRules->set_rules( > Koha::CirculationRules->set_rules( > { > branchcode => $samplebranch2->{branchcode}, >- categorycode => undef, > itemtype => $sampleitemtype2->{itemtype}, > rules => { > holdallowed => 5, >diff --git a/t/db_dependent/Circulation/CalcFine.t b/t/db_dependent/Circulation/CalcFine.t >index 89dbb2d34d..ff7d44ac40 100644 >--- a/t/db_dependent/Circulation/CalcFine.t >+++ b/t/db_dependent/Circulation/CalcFine.t >@@ -68,9 +68,9 @@ subtest 'Test basic functionality' => sub { > > Koha::CirculationRules->set_rules( > { >- branchcode => '*', >- categorycode => '*', >- itemtype => '*', >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, > rules => { > fine => '1.00', > lengthunit => 'days', >@@ -106,9 +106,9 @@ subtest 'Test cap_fine_to_replacement_price' => sub { > plan tests => 1; > Koha::CirculationRules->set_rules( > { >- branchcode => '*', >- categorycode => '*', >- itemtype => '*', >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, > rules => { > fine => '1.00', > lengthunit => 'days', >diff --git a/t/db_dependent/Circulation/GetHardDueDate.t b/t/db_dependent/Circulation/GetHardDueDate.t >index 8a0adfa3a7..de262d0653 100644 >--- a/t/db_dependent/Circulation/GetHardDueDate.t >+++ b/t/db_dependent/Circulation/GetHardDueDate.t >@@ -7,6 +7,8 @@ use Koha::DateUtils; > use Koha::CirculationRules; > use Koha::Library; > >+use t::lib::TestBuilder; >+ > use Test::More tests => 10; > > BEGIN { >@@ -103,6 +105,10 @@ $dbh->do( > $samplecat->{category_type} > ); > >+my $builder = t::lib::TestBuilder->new; >+my $sampleitemtype1 = $builder->build({ source => 'Itemtype' })->{itemtype}; >+my $sampleitemtype2 = $builder->build({ source => 'Itemtype' })->{itemtype}; >+ > #Begin Tests > > my $default = { >@@ -115,12 +121,8 @@ my $default = { > my $sampleissuingrule1 = { > branchcode => $samplebranch1->{branchcode}, > categorycode => $samplecat->{categorycode}, >- itemtype => 'BOOK', >+ itemtype => $sampleitemtype1, > rules => { >- reservecharge => '0.000000', >- chargename => 'Null', >- restrictedtype => 0, >- accountsent => 0, > finedays => 0, > lengthunit => 'days', > renewalperiod => 5, >@@ -148,7 +150,7 @@ my $sampleissuingrule1 = { > my $sampleissuingrule2 = { > branchcode => $samplebranch2->{branchcode}, > categorycode => $samplecat->{categorycode}, >- itemtype => 'BOOK', >+ itemtype => $sampleitemtype1, > rules => { > renewalsallowed => 'Null', > renewalperiod => 2, >@@ -166,10 +168,6 @@ my $sampleissuingrule2 = { > chargeperiod_charge_at => 0, > rentaldiscount => 2.00, > overduefinescap => 'Null', >- accountsent => 'Null', >- reservecharge => 'Null', >- chargename => 'Null', >- restrictedtype => 'Null', > maxsuspensiondays => 0, > onshelfholds => 1, > opacitemholds => 'Y', >@@ -181,7 +179,7 @@ my $sampleissuingrule2 = { > my $sampleissuingrule3 = { > branchcode => $samplebranch1->{branchcode}, > categorycode => $samplecat->{categorycode}, >- itemtype => 'DVD', >+ itemtype => $sampleitemtype2, > rules => { > renewalsallowed => 'Null', > renewalperiod => 3, >@@ -199,10 +197,6 @@ my $sampleissuingrule3 = { > chargeperiod_charge_at => 0, > rentaldiscount => 3.00, > overduefinescap => 'Null', >- accountsent => 'Null', >- reservecharge => 'Null', >- chargename => 'Null', >- restrictedtype => 'Null', > maxsuspensiondays => 0, > onshelfholds => 1, > opacitemholds => 'F', >@@ -235,7 +229,7 @@ is_deeply( > is_deeply( > C4::Circulation::GetLoanLength( > $samplecat->{categorycode}, >- 'BOOK', $samplebranch1->{branchcode} >+ $sampleitemtype1, $samplebranch1->{branchcode} > ), > { issuelength => 5, lengthunit => 'days', renewalperiod => 5 }, > "GetLoanLength" >@@ -257,12 +251,12 @@ is_deeply( > "With only one parameter, GetLoanLength returns hardcoded values" > ); #NOTE : is that really what is expected? > is_deeply( >- C4::Circulation::GetLoanLength( $samplecat->{categorycode}, 'BOOK' ), >+ C4::Circulation::GetLoanLength( $samplecat->{categorycode}, $sampleitemtype1 ), > $default, > "With only two parameters, GetLoanLength returns hardcoded values" > ); #NOTE : is that really what is expected? > is_deeply( >- C4::Circulation::GetLoanLength( $samplecat->{categorycode}, 'BOOK', $samplebranch1->{branchcode} ), >+ C4::Circulation::GetLoanLength( $samplecat->{categorycode}, $sampleitemtype1, $samplebranch1->{branchcode} ), > { > issuelength => 5, > renewalperiod => 5, >@@ -273,7 +267,7 @@ is_deeply( > > #Test GetHardDueDate > my @hardduedate = C4::Circulation::GetHardDueDate( $samplecat->{categorycode}, >- 'BOOK', $samplebranch1->{branchcode} ); >+ $sampleitemtype1, $samplebranch1->{branchcode} ); > is_deeply( > \@hardduedate, > [ >diff --git a/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t b/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t >index 8257c9e280..d79c6aa0ca 100644 >--- a/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t >+++ b/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t >@@ -33,9 +33,9 @@ my $userenv->{branch} = $branchcode; > Koha::CirculationRules->search->delete; > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > firstremind => 0, > finedays => 2, >@@ -88,9 +88,9 @@ DelDebarment( $debarments->[0]->{borrower_debarment_id} ); > # Test with maxsuspensiondays = 10 days > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > maxsuspensiondays => 10, > } >diff --git a/t/db_dependent/Circulation/Returns.t b/t/db_dependent/Circulation/Returns.t >index 39a556f626..36403808ad 100644 >--- a/t/db_dependent/Circulation/Returns.t >+++ b/t/db_dependent/Circulation/Returns.t >@@ -51,9 +51,9 @@ my $builder = t::lib::TestBuilder->new(); > Koha::CirculationRules->search->delete; > Koha::CirculationRules->set_rule( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rule_name => 'issuelength', > rule_value => 1, > } >@@ -285,9 +285,9 @@ subtest 'Handle ids duplication' => sub { > t::lib::Mocks::mock_preference( 'finesMode', 'production' ); > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > chargeperiod => 1, > fine => 1, >diff --git a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t >index 45a96ecea4..471a8c03f3 100644 >--- a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t >+++ b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t >@@ -76,14 +76,11 @@ Koha::CirculationRules->search()->delete(); > Koha::CirculationRules->set_rules( > { > branchcode => $branch->{branchcode}, >- categorycode => '*', >- itemtype => '*', >+ categorycode => undef, >+ itemtype => undef, > rules => { > maxissueqty => 2, > maxonsiteissueqty => 1, >- branchcode => $branch->{branchcode}, >- categorycode => '*', >- itemtype => '*', > lengthunit => 'days', > issuelength => 5, > } >@@ -150,8 +147,8 @@ Koha::CirculationRules->search()->delete(); > Koha::CirculationRules->set_rules( > { > branchcode => $branch->{branchcode}, >- categorycode => '*', >- itemtype => '*', >+ categorycode => undef, >+ itemtype => undef, > rules => { > maxissueqty => 2, > maxonsiteissueqty => 1, >diff --git a/t/db_dependent/Circulation/TooMany.t b/t/db_dependent/Circulation/TooMany.t >index d5f3b33adf..07297c9383 100644 >--- a/t/db_dependent/Circulation/TooMany.t >+++ b/t/db_dependent/Circulation/TooMany.t >@@ -106,7 +106,7 @@ subtest '1 Issuingrule exist 0 0: no issue allowed' => sub { > { > branchcode => $branch->{branchcode}, > categorycode => $category->{categorycode}, >- itemtype => '*', >+ itemtype => undef, > rules => { > maxissueqty => 0, > maxonsiteissueqty => 0, >@@ -162,7 +162,7 @@ subtest '1 Issuingrule exist 1 1: issue is allowed' => sub { > { > branchcode => $branch->{branchcode}, > categorycode => $category->{categorycode}, >- itemtype => '*', >+ itemtype => undef, > rules => { > maxissueqty => 1, > maxonsiteissueqty => 1, >@@ -202,7 +202,7 @@ subtest '1 Issuingrule exist: 1 CO allowed, 1 OSCO allowed. Do a CO' => sub { > { > branchcode => $branch->{branchcode}, > categorycode => $category->{categorycode}, >- itemtype => '*', >+ itemtype => undef, > rules => { > maxissueqty => 1, > maxonsiteissueqty => 1, >@@ -258,7 +258,7 @@ subtest '1 Issuingrule exist: 1 CO allowed, 1 OSCO allowed, Do a OSCO' => sub { > { > branchcode => $branch->{branchcode}, > categorycode => $category->{categorycode}, >- itemtype => '*', >+ itemtype => undef, > rules => { > maxissueqty => 1, > maxonsiteissueqty => 1, >diff --git a/t/db_dependent/Circulation/issue.t b/t/db_dependent/Circulation/issue.t >index ec5033fe6e..162249f639 100644 >--- a/t/db_dependent/Circulation/issue.t >+++ b/t/db_dependent/Circulation/issue.t >@@ -290,9 +290,9 @@ is_deeply( > # Add a default rule: No renewal allowed > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > issuelength => 10, > renewalsallowed => 0, >@@ -321,9 +321,9 @@ is_deeply( > # Add a default rule: renewal is allowed > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > renewalsallowed => 3, > } >diff --git a/t/db_dependent/DecreaseLoanHighHolds.t b/t/db_dependent/DecreaseLoanHighHolds.t >index fa117c2547..5ccce05dcd 100755 >--- a/t/db_dependent/DecreaseLoanHighHolds.t >+++ b/t/db_dependent/DecreaseLoanHighHolds.t >@@ -90,13 +90,14 @@ for my $i ( 0 .. 5 ) { > > Koha::CirculationRules->set_rules( > { >- branchcode => '*', >- categorycode => '*', >- itemtype => '*', >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, > rules => { > issuelength => '14', > lengthunit => 'days', > reservesallowed => '99', >+ holds_per_record => '99', > } > } > ); >diff --git a/t/db_dependent/Fines.t b/t/db_dependent/Fines.t >index 20d34cebc2..da5e193b4b 100644 >--- a/t/db_dependent/Fines.t >+++ b/t/db_dependent/Fines.t >@@ -20,9 +20,9 @@ $dbh->do(q|DELETE FROM circulation_rules|); > > my $issuingrule = Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > fine => 1, > finedays => 0, >@@ -47,11 +47,11 @@ $period_end = dt_from_string('2000-01-10'); > is( $fine, 1, '9 days overdue, charge period 7 days, charge at end of interval gives fine of $1' ); > > # Test charging fine at the *beginning* of each charge period >-my $issuingrule = Koha::CirculationRules->set_rules( >+$issuingrule = Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > chargeperiod_charge_at => 1, > } >diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t >index a556140d83..fbc49fbac6 100755 >--- a/t/db_dependent/Holds.t >+++ b/t/db_dependent/Holds.t >@@ -235,9 +235,9 @@ my ($foreign_item_bibnum, $foreign_item_bibitemnum, $foreign_itemnumber) > $dbh->do('DELETE FROM circulation_rules'); > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > reservesallowed => 25, > holds_per_record => 99, >@@ -246,8 +246,8 @@ Koha::CirculationRules->set_rules( > ); > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >+ categorycode => undef, >+ branchcode => undef, > itemtype => 'CANNOT', > rules => { > reservesallowed => 0, >@@ -353,9 +353,9 @@ ok( > $dbh->do('DELETE FROM circulation_rules'); > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > reservesallowed => 25, > holds_per_record => 99, >@@ -366,7 +366,6 @@ Koha::CirculationRules->set_rules( > { > branchcode => $branch_1, > itemtype => 'CANNOT', >- categorycode => undef, > rules => { > holdallowed => 0, > returnbranch => 'homebranch', >@@ -377,7 +376,6 @@ Koha::CirculationRules->set_rules( > { > branchcode => $branch_1, > itemtype => 'CAN', >- categorycode => undef, > rules => { > holdallowed => 1, > returnbranch => 'homebranch', >@@ -416,8 +414,8 @@ $dbh->do('DELETE FROM biblio'); > > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >+ categorycode => undef, >+ branchcode => undef, > itemtype => 'ONLY1', > rules => { > reservesallowed => 1, >@@ -439,15 +437,16 @@ subtest 'Test max_holds per library/patron category' => sub { > $dbh->do('DELETE FROM reserves'); > $dbh->do('DELETE FROM circulation_rules'); > >- ( $bibnum, $title, $bibitemnum ) = create_helper_biblio('TEST'); >+ my $testitemtype = $builder->build({ source => 'Itemtype' })->{itemtype}; >+ ( $bibnum, $title, $bibitemnum ) = create_helper_biblio($testitemtype); > ( $item_bibnum, $item_bibitemnum, $itemnumber ) = > AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 }, > $bibnum ); > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => 'TEST', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => $testitemtype, > rules => { > reservesallowed => 99, > holds_per_record => 99, >@@ -469,7 +468,6 @@ subtest 'Test max_holds per library/patron category' => sub { > { > categorycode => $category->{categorycode}, > branchcode => undef, >- itemtype => undef, > rule_name => 'max_holds', > rule_value => 3, > } >@@ -479,7 +477,6 @@ subtest 'Test max_holds per library/patron category' => sub { > { > branchcode => $branch_1, > categorycode => $category->{categorycode}, >- itemtype => undef, > rule_name => 'max_holds', > rule_value => 5, > } >diff --git a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t >index 4a1ffcdceb..721ec20730 100755 >--- a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t >+++ b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t >@@ -89,9 +89,9 @@ my $item2 = GetItem( $itemnumber2 ); > $dbh->do("DELETE FROM circulation_rules"); > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > issuelength => 7, > lengthunit => 8, >diff --git a/t/db_dependent/Holds/HoldFulfillmentPolicy.t b/t/db_dependent/Holds/HoldFulfillmentPolicy.t >index e82cdd8b88..803ebc44ec 100755 >--- a/t/db_dependent/Holds/HoldFulfillmentPolicy.t >+++ b/t/db_dependent/Holds/HoldFulfillmentPolicy.t >@@ -74,7 +74,6 @@ my $itemnumber = > $dbh->do("DELETE FROM circulation_rules"); > Koha::CirculationRules->set_rules( > { >- categorycode => undef, > branchcode => undef, > itemtype => undef, > rules => { >@@ -106,7 +105,6 @@ Koha::Holds->find( $reserve_id )->cancel; > $dbh->do("DELETE FROM circulation_rules"); > Koha::CirculationRules->set_rules( > { >- categorycode => undef, > branchcode => undef, > itemtype => undef, > rules => { >@@ -138,7 +136,6 @@ Koha::Holds->find( $reserve_id )->cancel; > $dbh->do("DELETE FROM circulation_rules"); > Koha::CirculationRules->set_rules( > { >- categorycode => undef, > branchcode => undef, > itemtype => undef, > rules => { >diff --git a/t/db_dependent/Holds/HoldItemtypeLimit.t b/t/db_dependent/Holds/HoldItemtypeLimit.t >index ce206de531..751047dc14 100644 >--- a/t/db_dependent/Holds/HoldItemtypeLimit.t >+++ b/t/db_dependent/Holds/HoldItemtypeLimit.t >@@ -90,7 +90,6 @@ $dbh->do("DELETE FROM circulation_rules"); > Koha::CirculationRules->set_rules( > { > itemtype => undef, >- categorycode => undef, > branchcode => undef, > rules => { > holdallowed => 2, >diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t >index dca97af829..c461a7cd2b 100755 >--- a/t/db_dependent/HoldsQueue.t >+++ b/t/db_dependent/HoldsQueue.t >@@ -293,7 +293,6 @@ Koha::CirculationRules->set_rule( > rule_name => 'holdallowed', > rule_value => 1, > branchcode => undef, >- categorycode => undef, > itemtype => undef, > } > ); >@@ -331,7 +330,6 @@ Koha::CirculationRules->set_rule( > rule_name => 'holdallowed', > rule_value => 2, > branchcode => undef, >- categorycode => undef, > itemtype => undef, > } > ); >@@ -359,7 +357,6 @@ Koha::CirculationRules->set_rule( > rule_name => 'holdallowed', > rule_value => 2, > branchcode => undef, >- categorycode => undef, > itemtype => undef, > } > ); >@@ -479,7 +476,6 @@ Koha::CirculationRules->set_rules( > { > branchcode => $library_A, > itemtype => $itemtype, >- categorycode => undef, > rules => { > holdallowed => 2, > returnbranch => 'homebranch', >@@ -578,7 +574,6 @@ Koha::CirculationRules->set_rules( > { > branchcode => undef, > itemtype => undef, >- categorycode => undef, > rules => { > holdallowed => 2, > hold_fulfillment_policy => 'homebranch', >@@ -613,7 +608,6 @@ Koha::CirculationRules->set_rules( > { > branchcode => undef, > itemtype => undef, >- categorycode => undef, > rules => { > holdallowed => 2, > hold_fulfillment_policy => 'holdingbranch', >@@ -648,7 +642,6 @@ Koha::CirculationRules->set_rules( > { > branchcode => undef, > itemtype => undef, >- categorycode => undef, > rules => { > holdallowed => 2, > hold_fulfillment_policy => 'any', >@@ -716,7 +709,6 @@ Koha::CirculationRules->set_rules( > { > branchcode => undef, > itemtype => undef, >- categorycode => undef, > rules => { > holdallowed => 2, > hold_fulfillment_policy => 'any', >diff --git a/t/db_dependent/Koha/IssuingRules.t b/t/db_dependent/Koha/IssuingRules.t >index ef99662cdd..0828f93452 100644 >--- a/t/db_dependent/Koha/IssuingRules.t >+++ b/t/db_dependent/Koha/IssuingRules.t >@@ -19,7 +19,9 @@ > > use Modern::Perl; > >-use Test::More tests => 1; >+use Test::More tests => 2; >+use Test::Deep qw( cmp_methods ); >+use Test::Exception; > > use Benchmark; > >@@ -35,15 +37,12 @@ my $builder = t::lib::TestBuilder->new; > subtest 'get_effective_issuing_rule' => sub { > plan tests => 3; > >- my $patron = $builder->build({ source => 'Borrower' }); >- my $item = $builder->build({ source => 'Item' }); >- >- my $categorycode = $patron->{'categorycode'}; >- my $itemtype = $item->{'itype'}; >- my $branchcode = $item->{'homebranch'}; >+ my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'}; >+ my $itemtype = $builder->build({ source => 'Itemtype' })->{'itemtype'}; >+ my $branchcode = $builder->build({ source => 'Branch' })->{'branchcode'}; > > subtest 'Call with undefined values' => sub { >- plan tests => 4; >+ plan tests => 5; > > my $rule; > Koha::CirculationRules->delete; >@@ -58,25 +57,33 @@ subtest 'get_effective_issuing_rule' => sub { > is($rule, undef, 'When I attempt to get effective issuing rule by' > .' providing undefined values, then undef is returned.'); > ok(Koha::CirculationRule->new({ >- branchcode => '*', >- categorycode => '*', >- itemtype => '*', >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, > rule_name => 'fine', >- })->store, 'Given I added an issuing rule branchcode => *,' >- .' categorycode => *, itemtype => *,'); >+ })->store, 'Given I added an issuing rule branchcode => undef,' >+ .' categorycode => undef, itemtype => undef,'); > $rule = Koha::CirculationRules->get_effective_rule({ >- branchcode => '*', >- categorycode => '*', >- itemtype => '*', >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, > rule_name => 'fine', > }); >- ok(_row_match($rule, '*', '*', '*'), 'When I attempt to get effective' >+ _is_row_match( >+ $rule, >+ { >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef >+ }, >+ 'When I attempt to get effective' > .' issuing rule by providing undefined values, then the above one is' >- .' returned.'); >+ .' returned.' >+ ); > }; > > subtest 'Get effective issuing rule in correct order' => sub { >- plan tests => 18; >+ plan tests => 26; > > my $rule; > Koha::CirculationRules->delete; >@@ -91,109 +98,165 @@ subtest 'get_effective_issuing_rule' => sub { > .' is returned.'); > > ok(Koha::CirculationRule->new({ >- branchcode => '*', >- categorycode => '*', >- itemtype => '*', >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, > rule_name => 'fine', >- })->store, 'Given I added an issuing rule branchcode => *, categorycode => *, itemtype => *,'); >+ })->store, 'Given I added an issuing rule branchcode => undef, categorycode => undef, itemtype => undef,'); > $rule = Koha::CirculationRules->get_effective_rule({ > branchcode => $branchcode, > categorycode => $categorycode, > itemtype => $itemtype, > rule_name => 'fine', > }); >- ok(_row_match($rule, '*', '*', '*'), 'When I attempt to get effective issuing rule,' >- .' then the above one is returned.'); >+ _is_row_match( >+ $rule, >+ { >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef >+ }, >+ 'When I attempt to get effective issuing rule,' >+ .' then the above one is returned.' >+ ); > > ok(Koha::CirculationRule->new({ >- branchcode => '*', >- categorycode => '*', >+ branchcode => undef, >+ categorycode => undef, > itemtype => $itemtype, > rule_name => 'fine', >- })->store, "Given I added an issuing rule branchcode => *, categorycode => *, itemtype => $itemtype,"); >+ })->store, "Given I added an issuing rule branchcode => undef, categorycode => undef, itemtype => $itemtype,"); > $rule = Koha::CirculationRules->get_effective_rule({ > branchcode => $branchcode, > categorycode => $categorycode, > itemtype => $itemtype, > rule_name => 'fine', > }); >- ok(_row_match($rule, '*', '*', $itemtype), 'When I attempt to get effective issuing rule,' >- .' then the above one is returned.'); >+ _is_row_match( >+ $rule, >+ { >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => $itemtype >+ }, >+ 'When I attempt to get effective issuing rule,' >+ .' then the above one is returned.' >+ ); > > ok(Koha::CirculationRule->new({ >- branchcode => '*', >+ branchcode => undef, > categorycode => $categorycode, >- itemtype => '*', >+ itemtype => undef, > rule_name => 'fine', >- })->store, "Given I added an issuing rule branchcode => *, categorycode => $categorycode, itemtype => *,"); >+ })->store, "Given I added an issuing rule branchcode => undef, categorycode => $categorycode, itemtype => undef,"); > $rule = Koha::CirculationRules->get_effective_rule({ > branchcode => $branchcode, > categorycode => $categorycode, > itemtype => $itemtype, > rule_name => 'fine', > }); >- ok(_row_match($rule, '*', $categorycode, '*'), 'When I attempt to get effective issuing rule,' >- .' then the above one is returned.'); >+ _is_row_match( >+ $rule, >+ { >+ branchcode => undef, >+ categorycode => $categorycode, >+ itemtype => undef >+ }, >+ 'When I attempt to get effective issuing rule,' >+ .' then the above one is returned.' >+ ); > > ok(Koha::CirculationRule->new({ >- branchcode => '*', >+ branchcode => undef, > categorycode => $categorycode, > itemtype => $itemtype, > rule_name => 'fine', >- })->store, "Given I added an issuing rule branchcode => *, categorycode => $categorycode, itemtype => $itemtype,"); >+ })->store, "Given I added an issuing rule branchcode => undef, categorycode => $categorycode, itemtype => $itemtype,"); > $rule = Koha::CirculationRules->get_effective_rule({ > branchcode => $branchcode, > categorycode => $categorycode, > itemtype => $itemtype, > rule_name => 'fine', > }); >- ok(_row_match($rule, '*', $categorycode, $itemtype), 'When I attempt to get effective issuing rule,' >- .' then the above one is returned.'); >+ _is_row_match( >+ $rule, >+ { >+ branchcode => undef, >+ categorycode => $categorycode, >+ itemtype => $itemtype >+ }, >+ 'When I attempt to get effective issuing rule,' >+ .' then the above one is returned.' >+ ); > > ok(Koha::CirculationRule->new({ > branchcode => $branchcode, >- categorycode => '*', >- itemtype => '*', >+ categorycode => undef, >+ itemtype => undef, > rule_name => 'fine', >- })->store, "Given I added an issuing rule branchcode => $branchcode, categorycode => '*', itemtype => '*',"); >+ })->store, "Given I added an issuing rule branchcode => $branchcode, categorycode => undef, itemtype => undef,"); > $rule = Koha::CirculationRules->get_effective_rule({ > branchcode => $branchcode, > categorycode => $categorycode, > itemtype => $itemtype, > rule_name => 'fine', > }); >- ok(_row_match($rule, $branchcode, '*', '*'), 'When I attempt to get effective issuing rule,' >- .' then the above one is returned.'); >+ _is_row_match( >+ $rule, >+ { >+ branchcode => $branchcode, >+ categorycode => undef, >+ itemtype => undef >+ }, >+ 'When I attempt to get effective issuing rule,' >+ .' then the above one is returned.' >+ ); > > ok(Koha::CirculationRule->new({ > branchcode => $branchcode, >- categorycode => '*', >+ categorycode => undef, > itemtype => $itemtype, > rule_name => 'fine', >- })->store, "Given I added an issuing rule branchcode => $branchcode, categorycode => '*', itemtype => $itemtype,"); >+ })->store, "Given I added an issuing rule branchcode => $branchcode, categorycode => undef, itemtype => $itemtype,"); > $rule = Koha::CirculationRules->get_effective_rule({ > branchcode => $branchcode, > categorycode => $categorycode, > itemtype => $itemtype, > rule_name => 'fine', > }); >- ok(_row_match($rule, $branchcode, '*', $itemtype), 'When I attempt to get effective issuing rule,' >- .' then the above one is returned.'); >+ _is_row_match( >+ $rule, >+ { >+ branchcode => $branchcode, >+ categorycode => undef, >+ itemtype => $itemtype >+ }, >+ 'When I attempt to get effective issuing rule,' >+ .' then the above one is returned.' >+ ); > > ok(Koha::CirculationRule->new({ > branchcode => $branchcode, > categorycode => $categorycode, >- itemtype => '*', >+ itemtype => undef, > rule_name => 'fine', >- })->store, "Given I added an issuing rule branchcode => $branchcode, categorycode => $categorycode, itemtype => '*',"); >+ })->store, "Given I added an issuing rule branchcode => $branchcode, categorycode => $categorycode, itemtype => undef,"); > $rule = Koha::CirculationRules->get_effective_rule({ > branchcode => $branchcode, > categorycode => $categorycode, > itemtype => $itemtype, > rule_name => 'fine', > }); >- ok(_row_match($rule, $branchcode, $categorycode, '*'), 'When I attempt to get effective issuing rule,' >- .' then the above one is returned.'); >+ _is_row_match( >+ $rule, >+ { >+ branchcode => $branchcode, >+ categorycode => $categorycode, >+ itemtype => undef >+ }, >+ 'When I attempt to get effective issuing rule,' >+ .' then the above one is returned.' >+ ); > > ok(Koha::CirculationRule->new({ > branchcode => $branchcode, >@@ -207,8 +270,16 @@ subtest 'get_effective_issuing_rule' => sub { > itemtype => $itemtype, > rule_name => 'fine', > }); >- ok(_row_match($rule, $branchcode, $categorycode, $itemtype), 'When I attempt to get effective issuing rule,' >- .' then the above one is returned.'); >+ _is_row_match( >+ $rule, >+ { >+ branchcode => $branchcode, >+ categorycode => $categorycode, >+ itemtype => $itemtype >+ }, >+ 'When I attempt to get effective issuing rule,' >+ .' then the above one is returned.' >+ ); > }; > > subtest 'Performance' => sub { >@@ -265,11 +336,135 @@ subtest 'get_effective_issuing_rule' => sub { > }; > }; > >-sub _row_match { >- my ($rule, $branchcode, $categorycode, $itemtype) = @_; >+subtest 'set_rule' => sub { >+ plan tests => 3; >+ >+ my $branchcode = $builder->build({ source => 'Branch' })->{'branchcode'}; >+ my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'}; >+ my $itemtype = $builder->build({ source => 'Itemtype' })->{'itemtype'}; >+ >+ subtest 'Correct call' => sub { >+ plan tests => 4; >+ >+ Koha::CirculationRules->delete; >+ >+ lives_ok( sub { >+ Koha::CirculationRules->set_rule( { >+ branchcode => $branchcode, >+ rule_name => 'refund', >+ rule_value => '', >+ } ); >+ }, 'setting refund with branch' ); >+ >+ lives_ok( sub { >+ Koha::CirculationRules->set_rule( { >+ branchcode => $branchcode, >+ categorycode => $categorycode, >+ rule_name => 'patron_maxissueqty', >+ rule_value => '', >+ } ); >+ }, 'setting patron_maxissueqty with branch/category succeeds' ); >+ >+ lives_ok( sub { >+ Koha::CirculationRules->set_rule( { >+ branchcode => $branchcode, >+ itemtype => $itemtype, >+ rule_name => 'holdallowed', >+ rule_value => '', >+ } ); >+ }, 'setting holdallowed with branch/itemtype succeeds' ); >+ >+ lives_ok( sub { >+ Koha::CirculationRules->set_rule( { >+ branchcode => $branchcode, >+ categorycode => $categorycode, >+ itemtype => $itemtype, >+ rule_name => 'fine', >+ rule_value => '', >+ } ); >+ }, 'setting fine with branch/category/itemtype succeeds' ); >+ }; >+ >+ subtest 'Call with missing params' => sub { >+ plan tests => 4; >+ >+ Koha::CirculationRules->delete; >+ >+ throws_ok( sub { >+ Koha::CirculationRules->set_rule( { >+ rule_name => 'refund', >+ rule_value => '', >+ } ); >+ }, qr/branchcode/, 'setting refund without branch fails' ); >+ >+ throws_ok( sub { >+ Koha::CirculationRules->set_rule( { >+ branchcode => $branchcode, >+ rule_name => 'patron_maxissueqty', >+ rule_value => '', >+ } ); >+ }, qr/categorycode/, 'setting patron_maxissueqty without categorycode fails' ); >+ >+ throws_ok( sub { >+ Koha::CirculationRules->set_rule( { >+ branchcode => $branchcode, >+ rule_name => 'holdallowed', >+ rule_value => '', >+ } ); >+ }, qr/itemtype/, 'setting holdallowed without itemtype fails' ); >+ >+ throws_ok( sub { >+ Koha::CirculationRules->set_rule( { >+ branchcode => $branchcode, >+ categorycode => $categorycode, >+ rule_name => 'fine', >+ rule_value => '', >+ } ); >+ }, qr/itemtype/, 'setting fine without itemtype fails' ); >+ }; >+ >+ subtest 'Call with extra params' => sub { >+ plan tests => 3; >+ >+ Koha::CirculationRules->delete; >+ >+ throws_ok( sub { >+ Koha::CirculationRules->set_rule( { >+ branchcode => $branchcode, >+ categorycode => $categorycode, >+ rule_name => 'refund', >+ rule_value => '', >+ } ); >+ }, qr/categorycode/, 'setting refund with categorycode fails' ); >+ >+ throws_ok( sub { >+ Koha::CirculationRules->set_rule( { >+ branchcode => $branchcode, >+ categorycode => $categorycode, >+ itemtype => $itemtype, >+ rule_name => 'patron_maxissueqty', >+ rule_value => '', >+ } ); >+ }, qr/itemtype/, 'setting patron_maxissueqty with itemtype fails' ); >+ >+ throws_ok( sub { >+ Koha::CirculationRules->set_rule( { >+ branchcode => $branchcode, >+ rule_name => 'holdallowed', >+ categorycode => $categorycode, >+ itemtype => $itemtype, >+ rule_value => '', >+ } ); >+ }, qr/categorycode/, 'setting holdallowed with categorycode fails' ); >+ }; >+}; >+ >+sub _is_row_match { >+ my ( $rule, $expected, $message ) = @_; > >- return $rule->branchcode eq $branchcode && $rule->categorycode eq $categorycode >- && $rule->itemtype eq $itemtype; >+ ok( $rule, $message ) ? >+ cmp_methods( $rule, [ %$expected ], $message ) : >+ fail( $message ); > } > > $schema->storage->txn_rollback; >diff --git a/t/db_dependent/RefundLostItemFeeRule.t b/t/db_dependent/RefundLostItemFeeRule.t >index 3c7b235f17..18ff46d01a 100755 >--- a/t/db_dependent/RefundLostItemFeeRule.t >+++ b/t/db_dependent/RefundLostItemFeeRule.t >@@ -47,17 +47,19 @@ subtest 'Koha::RefundLostItemFeeRule::delete() tests' => sub { > { > source => 'CirculationRule', > value => { >- branchcode => '*', >+ branchcode => undef, > categorycode => undef, > itemtype => undef, > rule_name => 'refund', > } > } > ); >+ my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; > my $generated_other_rule = $builder->build( > { > source => 'CirculationRule', > value => { >+ branchcode => $branchcode, > categorycode => undef, > itemtype => undef, > rule_name => 'refund', >@@ -67,7 +69,7 @@ subtest 'Koha::RefundLostItemFeeRule::delete() tests' => sub { > > my $default_rule = Koha::CirculationRules->search( > { >- branchcode => '*', >+ branchcode => undef, > categorycode => undef, > itemtype => undef, > rule_name => 'refund', >@@ -109,7 +111,7 @@ subtest 'Koha::RefundLostItemFeeRules::_default_rule() tests' => sub { > { > source => 'CirculationRule', > value => { >- branchcode => '*', >+ branchcode => undef, > categorycode => undef, > itemtype => undef, > rule_name => 'refund', >@@ -117,10 +119,12 @@ subtest 'Koha::RefundLostItemFeeRules::_default_rule() tests' => sub { > } > } > ); >+ my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; > my $generated_other_rule = $builder->build( > { > source => 'CirculationRule', > value => { >+ branchcode => $branchcode, > categorycode => undef, > itemtype => undef, > rule_name => 'refund', >@@ -130,7 +134,7 @@ subtest 'Koha::RefundLostItemFeeRules::_default_rule() tests' => sub { > > my $default_rule = Koha::CirculationRules->search( > { >- branchcode => '*', >+ branchcode => undef, > categorycode => undef, > itemtype => undef, > rule_name => 'refund', >@@ -146,7 +150,7 @@ subtest 'Koha::RefundLostItemFeeRules::_default_rule() tests' => sub { > # Re-read from DB, to be sure > $default_rule = Koha::CirculationRules->search( > { >- branchcode => '*', >+ branchcode => undef, > categorycode => undef, > itemtype => undef, > rule_name => 'refund', >@@ -177,7 +181,7 @@ subtest 'Koha::RefundLostItemFeeRules::_effective_branch_rule() tests' => sub { > { > source => 'CirculationRule', > value => { >- branchcode => '*', >+ branchcode => undef, > categorycode => undef, > itemtype => undef, > rule_name => 'refund', >@@ -185,10 +189,12 @@ subtest 'Koha::RefundLostItemFeeRules::_effective_branch_rule() tests' => sub { > } > } > ); >+ my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; > my $specific_rule_false = $builder->build( > { > source => 'CirculationRule', > value => { >+ branchcode => $branchcode, > categorycode => undef, > itemtype => undef, > rule_name => 'refund', >@@ -196,10 +202,12 @@ subtest 'Koha::RefundLostItemFeeRules::_effective_branch_rule() tests' => sub { > } > } > ); >+ my $branchcode2 = $builder->build( { source => 'Branch' } )->{branchcode}; > my $specific_rule_true = $builder->build( > { > source => 'CirculationRule', > value => { >+ branchcode => $branchcode2, > categorycode => undef, > itemtype => undef, > rule_name => 'refund', >@@ -293,7 +301,7 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { > { > source => 'CirculationRule', > value => { >- branchcode => '*', >+ branchcode => undef, > categorycode => undef, > itemtype => undef, > rule_name => 'refund', >@@ -301,10 +309,12 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { > } > } > ); >+ my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; > my $specific_rule_false = $builder->build( > { > source => 'CirculationRule', > value => { >+ branchcode => $branchcode, > categorycode => undef, > itemtype => undef, > rule_name => 'refund', >@@ -312,10 +322,12 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { > } > } > ); >+ my $branchcode2 = $builder->build( { source => 'Branch' } )->{branchcode}; > my $specific_rule_true = $builder->build( > { > source => 'CirculationRule', > value => { >+ branchcode => $branchcode2, > categorycode => undef, > itemtype => undef, > rule_name => 'refund', >@@ -324,10 +336,12 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { > } > ); > # Make sure we have an unused branchcode >+ my $branchcode3 = $builder->build( { source => 'Branch' } )->{branchcode}; > my $specific_rule_dummy = $builder->build( > { > source => 'CirculationRule', > value => { >+ branchcode => $branchcode3, > categorycode => undef, > itemtype => undef, > rule_name => 'refund', >@@ -363,7 +377,7 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { > 1,'No rule for branch, global rule applied (true)'); > > # Change the default value just to try >- Koha::CirculationRules->search({ branchcode => '*', rule_name => 'refund' })->next->rule_value(0)->store; >+ Koha::CirculationRules->search({ branchcode => undef, rule_name => 'refund' })->next->rule_value(0)->store; > t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'ItemHoldingBranch' ); > is( Koha::RefundLostItemFeeRules->should_refund( $params ), > 0,'No rule for branch, global rule applied (false)'); >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index 3105d40deb..00bbe88b4d 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -201,9 +201,9 @@ $requesters{$branch_3} = AddMember( > $dbh->do('DELETE FROM circulation_rules'); > Koha::CirculationRules->set_rules( > { >- branchcode => '*', >- categorycode => '*', >- itemtype => '*', >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, > rules => { > reservesallowed => 25, > holds_per_record => 1, >@@ -215,7 +215,6 @@ Koha::CirculationRules->set_rules( > Koha::CirculationRules->set_rules( > { > branchcode => $branch_1, >- categorycode => undef, > itemtype => undef, > rules => { > holdallowed => 1, >@@ -228,7 +227,6 @@ Koha::CirculationRules->set_rules( > Koha::CirculationRules->set_rules( > { > branchcode => $branch_2, >- categorycode => undef, > itemtype => undef, > rules => { > holdallowed => 2, >diff --git a/t/db_dependent/Reserves/MultiplePerRecord.t b/t/db_dependent/Reserves/MultiplePerRecord.t >index ff64caba32..4d1e1772aa 100755 >--- a/t/db_dependent/Reserves/MultiplePerRecord.t >+++ b/t/db_dependent/Reserves/MultiplePerRecord.t >@@ -120,9 +120,9 @@ Koha::CirculationRules->delete(); > # Test GetMaxPatronHoldsForRecord and GetHoldRule > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > reservesallowed => 1, > holds_per_record => 1, >@@ -139,17 +139,17 @@ my $rule = C4::Reserves::GetHoldRule( > $itemtype1->{itemtype}, > $library->{branchcode} > ); >-is( $rule->{categorycode}, '*', 'Got rule with universal categorycode' ); >-is( $rule->{itemtype}, '*', 'Got rule with universal itemtype' ); >-is( $rule->{branchcode}, '*', 'Got rule with universal branchcode' ); >+is( $rule->{categorycode}, undef, 'Got rule with universal categorycode' ); >+is( $rule->{itemtype}, undef, 'Got rule with universal itemtype' ); >+is( $rule->{branchcode}, undef, 'Got rule with universal branchcode' ); > is( $rule->{reservesallowed}, 1, 'Got reservesallowed of 1' ); > is( $rule->{holds_per_record}, 1, 'Got holds_per_record of 1' ); > > Koha::CirculationRules->set_rules( > { > categorycode => $category->{categorycode}, >- itemtype => '*', >- branchcode => '*', >+ itemtype => undef, >+ branchcode => undef, > rules => { > reservesallowed => 2, > holds_per_record => 2, >@@ -165,8 +165,8 @@ $rule = C4::Reserves::GetHoldRule( > $library->{branchcode} > ); > is( $rule->{categorycode}, $category->{categorycode}, 'Got rule with specific categorycode' ); >-is( $rule->{itemtype}, '*', 'Got rule with universal itemtype' ); >-is( $rule->{branchcode}, '*', 'Got rule with universal branchcode' ); >+is( $rule->{itemtype}, undef, 'Got rule with universal itemtype' ); >+is( $rule->{branchcode}, undef, 'Got rule with universal branchcode' ); > is( $rule->{reservesallowed}, 2, 'Got reservesallowed of 2' ); > is( $rule->{holds_per_record}, 2, 'Got holds_per_record of 2' ); > >@@ -174,7 +174,7 @@ Koha::CirculationRules->set_rules( > { > categorycode => $category->{categorycode}, > itemtype => $itemtype1->{itemtype}, >- branchcode => '*', >+ branchcode => undef, > rules => { > reservesallowed => 3, > holds_per_record => 3, >@@ -191,7 +191,7 @@ $rule = C4::Reserves::GetHoldRule( > ); > is( $rule->{categorycode}, $category->{categorycode}, 'Got rule with specific categorycode' ); > is( $rule->{itemtype}, $itemtype1->{itemtype}, 'Got rule with universal itemtype' ); >-is( $rule->{branchcode}, '*', 'Got rule with universal branchcode' ); >+is( $rule->{branchcode}, undef, 'Got rule with universal branchcode' ); > is( $rule->{reservesallowed}, 3, 'Got reservesallowed of 3' ); > is( $rule->{holds_per_record}, 3, 'Got holds_per_record of 3' ); > >@@ -199,7 +199,7 @@ Koha::CirculationRules->set_rules( > { > categorycode => $category->{categorycode}, > itemtype => $itemtype2->{itemtype}, >- branchcode => '*', >+ branchcode => undef, > rules => { > reservesallowed => 4, > holds_per_record => 4, >@@ -216,7 +216,7 @@ $rule = C4::Reserves::GetHoldRule( > ); > is( $rule->{categorycode}, $category->{categorycode}, 'Got rule with specific categorycode' ); > is( $rule->{itemtype}, $itemtype2->{itemtype}, 'Got rule with universal itemtype' ); >-is( $rule->{branchcode}, '*', 'Got rule with universal branchcode' ); >+is( $rule->{branchcode}, undef, 'Got rule with universal branchcode' ); > is( $rule->{reservesallowed}, 4, 'Got reservesallowed of 4' ); > is( $rule->{holds_per_record}, 4, 'Got holds_per_record of 4' ); > >@@ -273,9 +273,9 @@ $hold->delete(); > # Test multi-hold via AddReserve > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- itemtype => '*', >- branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, > rules => { > reservesallowed => 2, > holds_per_record => 2, >diff --git a/t/db_dependent/TestBuilder.t b/t/db_dependent/TestBuilder.t >index a5a9112789..76a03ef5ad 100644 >--- a/t/db_dependent/TestBuilder.t >+++ b/t/db_dependent/TestBuilder.t >@@ -351,12 +351,14 @@ subtest 'build_object() tests' => sub { > > $builder = t::lib::TestBuilder->new(); > >+ my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; > my $categorycode = $builder->build( { source => 'Category' } )->{categorycode}; > my $itemtype = $builder->build( { source => 'Itemtype' } )->{itemtype}; > > my $issuing_rule = $builder->build_object( > { class => 'Koha::CirculationRules', > value => { >+ branchcode => $branchcode, > categorycode => $categorycode, > itemtype => $itemtype > } >diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t >index 9a96bbaeaa..49282ae6a8 100644 >--- a/t/db_dependent/api/v1/holds.t >+++ b/t/db_dependent/api/v1/holds.t >@@ -133,9 +133,9 @@ $dbh->do('DELETE FROM reserves'); > Koha::CirculationRules->search()->delete(); > Koha::CirculationRules->set_rules( > { >- categorycode => '*', >- branchcode => '*', >- itemtype => '*', >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => undef, > rules => { > reservesallowed => 1, > holds_per_record => 99 >-- >2.11.0
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