From 540b4284338046980f6c13fc437053b95a0318dc Mon Sep 17 00:00:00 2001
From: Lari Taskula <lari.taskula@hypernova.fi>
Date: Tue, 31 Mar 2020 16:25:59 +0000
Subject: [PATCH] Bug 25089: Remove on-site specific circulation rules
To apply:
1. perl installer/data/mysql/updatedatabase.pl
To test:
1. Search for patron_maxonsiteissueqty rule
$ grep -rn 'patron_maxonsiteissueqty'
2. Observe no results in Koha source files
3. Search for maxonsiteissueqty rule
$ grep -rn 'maxonsiteissueqty'
4. Observe no results (apart from .git)
5. Check modification made to C4::Circulation::GetBranchBorrowerCircRule().
Make sure the new parameter is being used everywhere. You can search
the usage of this subroutine with similar grep commands as above.
6. Run the following tests:
prove t/db_dependent/Circulation/Branch.t
prove t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
prove t/db_dependent/Circulation/TooMany.t
Sponsored-by: The National Library of Finland
---
C4/Circulation.pm | 49 +++++++------------
Koha/CirculationRules.pm | 6 ---
admin/smart-rules.pl | 19 +------
...bug_24101_2_convert_circulation_rules.perl | 33 +++++++++++++
installer/onboarding.pl | 2 -
.../prog/en/modules/admin/smart-rules.tt | 38 ++------------
t/db_dependent/Circulation/Branch.t | 47 ++++++++++++------
.../Circulation/SwitchOnSiteCheckouts.t | 26 +++++++++-
t/db_dependent/Circulation/TooMany.t | 36 +++++++++-----
9 files changed, 138 insertions(+), 118 deletions(-)
create mode 100644 installer/data/mysql/atomicupdate/bug_24101_2_convert_circulation_rules.perl
diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index efdc2b15a0..acc1f14786 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -385,6 +385,11 @@ sub TooMany {
# Get which branchcode we need
$branch = _GetCircControlBranch($item_object->unblessed,$borrower);
my $type = $item_object->effective_itemtype;
+ my $checkout_type = $onsite_checkout
+ ? $switch_onsite_checkout
+ ? $Koha::Checkouts::type->{checkout}
+ : $Koha::Checkouts::type->{onsite_checkout}
+ : $Koha::Checkouts::type->{checkout};
# given branch, patron category, and item type, determine
# applicable issuing rule
@@ -396,14 +401,6 @@ sub TooMany {
rule_name => 'maxissueqty',
}
);
- my $maxonsiteissueqty_rule = Koha::CirculationRules->get_effective_rule(
- {
- categorycode => $cat_borrower,
- itemtype => $type,
- branchcode => $branch,
- rule_name => 'maxonsiteissueqty',
- }
- );
# if a rule is found and has a loan limit set, count
@@ -473,14 +470,13 @@ sub TooMany {
my ( $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $count_query, {}, @bind_params );
my $max_checkouts_allowed = $maxissueqty_rule ? $maxissueqty_rule->rule_value : undef;
- my $max_onsite_checkouts_allowed = $maxonsiteissueqty_rule ? $maxonsiteissueqty_rule->rule_value : undef;
- if ( $onsite_checkout and $max_onsite_checkouts_allowed ne '' ) {
- if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed ) {
+ if ( $onsite_checkout and $max_checkouts_allowed ne '' ) {
+ if ( $onsite_checkout_count >= $max_checkouts_allowed ) {
return {
reason => 'TOO_MANY_ONSITE_CHECKOUTS',
count => $onsite_checkout_count,
- max_allowed => $max_onsite_checkouts_allowed,
+ max_allowed => $max_checkouts_allowed,
}
}
}
@@ -505,7 +501,7 @@ sub TooMany {
}
# Now count total loans against the limit for the branch
- my $branch_borrower_circ_rule = GetBranchBorrowerCircRule($branch, $cat_borrower);
+ my $branch_borrower_circ_rule = GetBranchBorrowerCircRule($branch, $cat_borrower, $checkout_type );
if (defined($branch_borrower_circ_rule->{patron_maxissueqty}) and $branch_borrower_circ_rule->{patron_maxissueqty} ne '') {
my @bind_params = ();
my $branch_count_query = qq|
@@ -527,14 +523,13 @@ sub TooMany {
}
my ( $checkout_count, $onsite_checkout_count ) = $dbh->selectrow_array( $branch_count_query, {}, @bind_params );
my $max_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxissueqty};
- my $max_onsite_checkouts_allowed = $branch_borrower_circ_rule->{patron_maxonsiteissueqty};
- if ( $onsite_checkout and $max_onsite_checkouts_allowed ne '' ) {
- if ( $onsite_checkout_count >= $max_onsite_checkouts_allowed ) {
+ if ( $onsite_checkout and $max_checkouts_allowed ne '' ) {
+ if ( $onsite_checkout_count >= $max_checkouts_allowed ) {
return {
reason => 'TOO_MANY_ONSITE_CHECKOUTS',
count => $onsite_checkout_count,
- max_allowed => $max_onsite_checkouts_allowed,
+ max_allowed => $max_checkouts_allowed,
}
}
}
@@ -1601,7 +1596,7 @@ sub GetHardDueDate {
=head2 GetBranchBorrowerCircRule
- my $branch_cat_rule = GetBranchBorrowerCircRule($branchcode, $categorycode);
+ my $branch_cat_rule = GetBranchBorrowerCircRule($branchcode, $categorycode, $checkout_type);
Retrieves circulation rule attributes that apply to the given
branch and patron category, regardless of item type.
@@ -1611,21 +1606,13 @@ patron_maxissueqty - maximum number of loans that a
patron of the given category can have at the given
branch. If the value is undef, no limit.
-patron_maxonsiteissueqty - maximum of on-site checkouts that a
-patron of the given category can have at the given
-branch. If the value is undef, no limit.
-
-This will check for different branch/category combinations in the following order:
-branch and category
-branch only
-category only
-default branch and category
+The order in which rules are searched is defined in circulation
+rules user interface.
If no rule has been found in the database, it will default to
the buillt in rule:
patron_maxissueqty - undef
-patron_maxonsiteissueqty - undef
C<$branchcode> and C<$categorycode> should contain the
literal branch code and patron category code, respectively - no
@@ -1634,21 +1621,21 @@ wildcards.
=cut
sub GetBranchBorrowerCircRule {
- my ( $branchcode, $categorycode ) = @_;
+ my ( $branchcode, $categorycode, $checkout_type ) = @_;
# Initialize default values
my $rules = {
patron_maxissueqty => undef,
- patron_maxonsiteissueqty => undef,
};
# Search for rules!
- foreach my $rule_name (qw( patron_maxissueqty patron_maxonsiteissueqty )) {
+ foreach my $rule_name (qw( patron_maxissueqty )) {
my $rule = Koha::CirculationRules->get_effective_rule(
{
categorycode => $categorycode,
itemtype => undef,
branchcode => $branchcode,
+ checkout_type => $checkout_type,
rule_name => $rule_name,
}
);
diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm
index 4b8d8d8675..c267930bda 100644
--- a/Koha/CirculationRules.pm
+++ b/Koha/CirculationRules.pm
@@ -53,9 +53,6 @@ our $RULE_KINDS = {
patron_maxissueqty => {
scope => [ 'branchcode', 'categorycode', 'checkout_type' ],
},
- patron_maxonsiteissueqty => {
- scope => [ 'branchcode', 'categorycode' ],
- },
max_holds => {
scope => [ 'branchcode', 'categorycode' ],
},
@@ -115,9 +112,6 @@ our $RULE_KINDS = {
maxissueqty => {
scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ],
},
- maxonsiteissueqty => {
- scope => [ 'branchcode', 'categorycode', 'itemtype' ],
- },
maxsuspensiondays => {
scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ],
},
diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl
index 6bea920dd9..941abdae73 100755
--- a/admin/smart-rules.pl
+++ b/admin/smart-rules.pl
@@ -84,7 +84,6 @@ if ($op eq 'delete') {
itemtype => $itemtype eq '*' ? undef : $itemtype,
rules => {
maxissueqty => undef,
- maxonsiteissueqty => undef,
rentaldiscount => undef,
fine => undef,
finedays => undef,
@@ -127,7 +126,6 @@ elsif ($op eq 'delete-branch-cat') {
rules => {
max_holds => undef,
patron_maxissueqty => undef,
- patron_maxonsiteissueqty => undef,
}
}
);
@@ -150,7 +148,6 @@ elsif ($op eq 'delete-branch-cat') {
rules => {
max_holds => undef,
patron_maxissueqty => undef,
- patron_maxonsiteissueqty => undef,
}
}
);
@@ -162,7 +159,6 @@ elsif ($op eq 'delete-branch-cat') {
categorycode => undef,
rules => {
patron_maxissueqty => undef,
- patron_maxonsiteissueqty => undef,
}
}
);
@@ -185,7 +181,6 @@ elsif ($op eq 'delete-branch-cat') {
rules => {
max_holds => undef,
patron_maxissueqty => undef,
- patron_maxonsiteissueqty => undef,
}
}
);
@@ -260,7 +255,6 @@ elsif ($op eq 'add') {
my $chargeperiod = $input->param('chargeperiod');
my $chargeperiod_charge_at = $input->param('chargeperiod_charge_at');
my $maxissueqty = strip_non_numeric($input->param('maxissueqty'));
- my $maxonsiteissueqty = strip_non_numeric($input->param('maxonsiteissueqty'));
my $renewalsallowed = $input->param('renewalsallowed');
my $renewalperiod = $input->param('renewalperiod');
my $norenewalbefore = $input->param('norenewalbefore');
@@ -288,11 +282,10 @@ elsif ($op eq 'add') {
my $overduefinescap = $input->param('overduefinescap') || '';
my $cap_fine_to_replacement_price = $input->param('cap_fine_to_replacement_price') eq 'on';
my $note = $input->param('note');
- $debug and warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty, $maxonsiteissueqty, $cap_fine_to_replacement_price";
+ $debug and warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty, $cap_fine_to_replacement_price";
my $rules = {
maxissueqty => $maxissueqty,
- maxonsiteissueqty => $maxonsiteissueqty,
rentaldiscount => $rentaldiscount,
fine => $fine,
finedays => $finedays,
@@ -335,8 +328,6 @@ elsif ($op eq 'add') {
elsif ($op eq "set-branch-defaults") {
my $categorycode = $input->param('categorycode');
my $patron_maxissueqty = strip_non_numeric($input->param('patron_maxissueqty'));
- my $patron_maxonsiteissueqty = $input->param('patron_maxonsiteissueqty');
- $patron_maxonsiteissueqty = strip_non_numeric($patron_maxonsiteissueqty);
my $holdallowed = $input->param('holdallowed');
my $hold_fulfillment_policy = $input->param('hold_fulfillment_policy');
my $returnbranch = $input->param('returnbranch');
@@ -362,7 +353,6 @@ elsif ($op eq "set-branch-defaults") {
branchcode => undef,
rules => {
patron_maxissueqty => $patron_maxissueqty,
- patron_maxonsiteissueqty => $patron_maxonsiteissueqty,
}
}
);
@@ -384,7 +374,6 @@ elsif ($op eq "set-branch-defaults") {
branchcode => $branch,
rules => {
patron_maxissueqty => $patron_maxissueqty,
- patron_maxonsiteissueqty => $patron_maxonsiteissueqty,
}
}
);
@@ -401,8 +390,6 @@ elsif ($op eq "set-branch-defaults") {
elsif ($op eq "add-branch-cat") {
my $categorycode = $input->param('categorycode');
my $patron_maxissueqty = strip_non_numeric($input->param('patron_maxissueqty'));
- my $patron_maxonsiteissueqty = $input->param('patron_maxonsiteissueqty');
- $patron_maxonsiteissueqty = strip_non_numeric($patron_maxonsiteissueqty);
my $max_holds = $input->param('max_holds');
$max_holds =~ s/\s//g;
$max_holds = undef if $max_holds !~ /^\d+/;
@@ -416,7 +403,6 @@ elsif ($op eq "add-branch-cat") {
rules => {
max_holds => $max_holds,
patron_maxissueqty => $patron_maxissueqty,
- patron_maxonsiteissueqty => $patron_maxonsiteissueqty,
}
}
);
@@ -428,7 +414,6 @@ elsif ($op eq "add-branch-cat") {
rules => {
max_holds => $max_holds,
patron_maxissueqty => $patron_maxissueqty,
- patron_maxonsiteissueqty => $patron_maxonsiteissueqty,
}
}
);
@@ -441,7 +426,6 @@ elsif ($op eq "add-branch-cat") {
rules => {
max_holds => $max_holds,
patron_maxissueqty => $patron_maxissueqty,
- patron_maxonsiteissueqty => $patron_maxonsiteissueqty,
}
}
);
@@ -453,7 +437,6 @@ elsif ($op eq "add-branch-cat") {
rules => {
max_holds => $max_holds,
patron_maxissueqty => $patron_maxissueqty,
- patron_maxonsiteissueqty => $patron_maxonsiteissueqty,
}
}
);
diff --git a/installer/data/mysql/atomicupdate/bug_24101_2_convert_circulation_rules.perl b/installer/data/mysql/atomicupdate/bug_24101_2_convert_circulation_rules.perl
new file mode 100644
index 0000000000..7b7df4922e
--- /dev/null
+++ b/installer/data/mysql/atomicupdate/bug_24101_2_convert_circulation_rules.perl
@@ -0,0 +1,33 @@
+$DBversion = 'XXX'; # will be replaced by the RM
+if( CheckVersion( $DBversion ) ) {
+
+ # First, convert maxonsiteissueqty to maxissueqty
+ my $update_sth = $dbh->prepare(q|
+ UPDATE circulation_rules
+ SET checkout_type = ?,
+ rule_name = ?
+ WHERE rule_name=?
+ |);
+ $update_sth->execute(
+ 'OS', # checkout_type => 'OS' (on-site checkout)
+ 'maxissueqty',
+ 'maxonsiteissueqty'
+ );
+
+ # Then, convert patron_maxonsiteissueqty to patron_maxissueqty
+ $update_sth = $dbh->prepare(q|
+ UPDATE circulation_rules
+ SET checkout_type = ?,
+ rule_name = ?
+ WHERE rule_name=?
+ |);
+ $update_sth->execute(
+ 'OS', # checkout_type => 'OS' (on-site checkout)
+ 'patron_maxissueqty',
+ 'patron_maxonsiteissueqty'
+ );
+
+ # Always end with this (adjust the bug info)
+ SetVersion( $DBversion );
+ print "Upgrade to $DBversion done (Bug 24101 - Convert maxonsiteissueqty to maxissueqty )\n";
+}
diff --git a/installer/onboarding.pl b/installer/onboarding.pl
index 547df29e0d..dab7980ee9 100755
--- a/installer/onboarding.pl
+++ b/installer/onboarding.pl
@@ -273,7 +273,6 @@ if ( $step == 5 ) {
holds_per_day => $holds_per_day,
holds_per_record => $holds_per_record,
maxissueqty => "",
- maxonsiteissueqty => "",
maxsuspensiondays => "",
no_auto_renewal_after => "",
no_auto_renewal_after_hard_limit => "",
@@ -291,7 +290,6 @@ if ( $step == 5 ) {
categorycode => $categorycode,
rules => {
patron_maxissueqty => "",
- patron_maxonsiteissueqty => "",
max_holds => "",
}
};
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 5ae3bb94e1..86f3edee0d 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
@@ -91,7 +91,6 @@
<th>Actions</th>
<th>Note</th>
<th>Current checkouts allowed</th>
- <th>Current on-site checkouts allowed</th>
<th>Loan period</th>
<th>Unit</th>
<th>Hard due date</th>
@@ -128,7 +127,6 @@
[% SET i = '' UNLESS i.defined %]
[% SET note = all_rules.$c.$i.note %]
[% SET maxissueqty = all_rules.$c.$i.maxissueqty %]
- [% SET maxonsiteissueqty = all_rules.$c.$i.maxonsiteissueqty %]
[% SET issuelength = all_rules.$c.$i.issuelength %]
[% SET lengthunit = all_rules.$c.$i.lengthunit %]
[% SET hardduedate = all_rules.$c.$i.hardduedate %]
@@ -156,7 +154,7 @@
[% SET article_requests = all_rules.$c.$i.article_requests %]
[% SET rentaldiscount = all_rules.$c.$i.rentaldiscount %]
- [% SET show_rule = maxissueqty || maxonsiteissueqty || issuelength || lengthunit || hardduedate || hardduedatebefore || hardduedateexact || fine || chargeperiod || chargeperiod_charge_at || firstremind || overduefinescap || cap_fine_to_replacement_price || finedays || maxsuspensiondays || suspension_chargeperiod || renewalsallowed || renewalsallowed || norenewalbefore || auto_renew || no_auto_renewal_after || no_auto_renewal_after_hard_limit || reservesallowed || holds_per_day || holds_per_record || onshelfholds || opacitemholds || article_requests || article_requests %]
+ [% SET show_rule = maxissueqty || issuelength || lengthunit || hardduedate || hardduedatebefore || hardduedateexact || fine || chargeperiod || chargeperiod_charge_at || firstremind || overduefinescap || cap_fine_to_replacement_price || finedays || maxsuspensiondays || suspension_chargeperiod || renewalsallowed || renewalsallowed || norenewalbefore || auto_renew || no_auto_renewal_after || no_auto_renewal_after_hard_limit || reservesallowed || holds_per_day || holds_per_record || onshelfholds || opacitemholds || article_requests || article_requests %]
[% IF show_rule %]
[% SET row_count = row_count + 1 %]
<tr row_countd="row_[% row_count | html %]">
@@ -190,13 +188,6 @@
<span>Unlimited</span>
[% END %]
</td>
- <td>
- [% IF maxonsiteissueqty.defined && maxonsiteissueqty != '' %]
- [% maxonsiteissueqty | html %]
- [% ELSE %]
- <span>Unlimited</span>
- [% END %]
- </td>
<td>[% issuelength | html %]</td>
<td>
[% IF ( lengthunit == 'days' ) %]
@@ -333,7 +324,6 @@
</td>
<td><input type="text" name="note" id="note" size="15" value="" maxlength="100"></td>
<td><input type="text" name="maxissueqty" id="maxissueqty" size="3" /></td>
- <td><input type="text" name="maxonsiteissueqty" id="maxonsiteissueqty" size="3" /></td>
<td><input type="text" name="issuelength" id="issuelength" size="3" /> </td>
<td>
<select name="lengthunit" id="lengthunit">
@@ -417,7 +407,6 @@
<th> </th>
<th>Note</th>
<th>Current checkouts allowed</th>
- <th>Current on-site checkouts allowed</th>
<th>Loan period</th>
<th>Unit</th>
<th>Hard due date</th>
@@ -460,7 +449,6 @@
<tr>
<th> </th>
<th>Total current checkouts allowed</th>
- <th>Total current on-site checkouts allowed</th>
<th>Maximum total holds allowed (count)</th>
<th>Hold policy</th>
<th>Hold pickup library match</th>
@@ -473,10 +461,6 @@
[% SET patron_maxissueqty = CirculationRules.Search( current_branch, undef, undef, 'patron_maxissueqty' ) %]
<input type="text" name="patron_maxissueqty" size="3" value="[% patron_maxissueqty | html %]"/>
</td>
- <td>
- [% SET patron_maxonsiteissueqty = CirculationRules.Search( current_branch, undef, undef, 'patron_maxonsiteissueqty' ) %]
- <input type="text" name="patron_maxonsiteissueqty" size="3" value="[% patron_maxonsiteissueqty | html %]"/>
- </td>
<td>
[% SET rule_value = CirculationRules.Search( current_branch, undef , undef, 'max_holds' ) %]
<input name="max_holds" size="3" value="[% rule_value | html %]" />
@@ -635,17 +619,15 @@
<tr>
<th>Patron category</th>
<th>Total current checkouts allowed</th>
- <th>Total current on-site checkouts allowed</th>
<th>Total holds allowed</th>
<th> </th>
</tr>
[% FOREACH c IN categorycodes %]
[% NEXT UNLESS c %]
[% SET patron_maxissueqty = CirculationRules.Search( branchcode, c, undef, 'patron_maxissueqty' ) %]
- [% SET patron_maxonsiteissueqty = CirculationRules.Search( branchcode, c, undef, 'patron_maxonsiteissueqty' ) %]
[% SET max_holds = CirculationRules.Search( branchcode, c, undef, 'max_holds' ) %]
- [% IF ( patron_maxissueqty.defined && patron_maxissueqty != '' ) || ( patron_maxonsiteissueqty.defined && patron_maxonsiteissueqty != '' ) || ( max_holds.defined && max_holds != '' ) %]
+ [% IF ( patron_maxissueqty.defined && patron_maxissueqty != '' ) || ( max_holds.defined && max_holds != '' ) %]
<tr>
<td>
[% IF c == undef %]
@@ -661,13 +643,6 @@
<span>Unlimited</span>
[% END %]
</td>
- <td>
- [% IF patron_maxonsiteissueqty.defined && patron_maxonsiteissueqty != '' %]
- [% patron_maxonsiteissueqty | html %]
- [% ELSE %]
- <span>Unlimited</span>
- [% END %]
- </td>
<td>
[% IF max_holds.defined && max_holds != '' %]
[% max_holds | html %]
@@ -690,8 +665,6 @@
[% END %]
</select>
</td>
- <td><input name="patron_maxissueqty" size="3" type="text" /></td>
- <td><input name="patron_maxonsiteissueqty" size="3" type="text" /></td>
<td><input name="max_holds" size="3" type="text" /></td>
<td class="actions"><button type="submit" class="btn btn-default btn-xs"><i class="fa fa-plus"></i> Add</td>
</tr>
@@ -988,7 +961,7 @@
// specific processing for the Note column
var note = $(this).find("a[name='viewnote']").data("content");
$(current_column).find("input[type='text']").val(note);
- } else if ( i == 8 ) {
+ } else if ( i == 7 ) {
// specific processing for the Hard due date column
var select_value = $(this).find("input[type='hidden'][name='hardduedatecomparebackup']").val();
var input_value = '';
@@ -999,7 +972,7 @@
}
$(current_column).find("input[type='text']").val(input_value);
$(current_column).find("select").val(select_value);
- } else if ( i == 14 ) {
+ } else if ( i == 13 ) {
// specific processing for cap_fine_to_replacement_price
var cap_fine_to_replacement_price = $(this).find("input[type='checkbox']");
$('#cap_fine_to_replacement_price').prop('checked', cap_fine_to_replacement_price.is(':checked') );
@@ -1024,10 +997,9 @@
// Remove potential previous input added
$(current_column).find("input").remove();
$(current_column).append("<input type='hidden' name='"+name+"' value='"+val+"' />");
- } else if ( i == 4 || i == 5 || i == 24 || i == 25 || i == 26 ) {
+ } else if ( i == 4 || i == 23 || i == 24 || i == 25 ) {
// If the value is not an integer for
// - "Current checkouts allowed"
- // - "Current on-site checkouts allowed"
// - "Holds allowed (total)"
// - "Holds allowed (daily)"
// - "Holds per record (count)"
diff --git a/t/db_dependent/Circulation/Branch.t b/t/db_dependent/Circulation/Branch.t
index 16e590fcc3..2a706780a3 100644
--- a/t/db_dependent/Circulation/Branch.t
+++ b/t/db_dependent/Circulation/Branch.t
@@ -25,7 +25,7 @@ use Koha::CirculationRules;
use Koha::Patrons;
-use Test::More tests => 14;
+use Test::More tests => 15;
use t::lib::Mocks;
use t::lib::TestBuilder;
@@ -143,29 +143,39 @@ my $borrower_id1 = Koha::Patron->new({
is_deeply(
GetBranchBorrowerCircRule(),
- { patron_maxissueqty => undef, patron_maxonsiteissueqty => undef },
-"Without parameter, GetBranchBorrower returns undef (unilimited) for patron_maxissueqty and patron_maxonsiteissueqty if no rules defined"
+ { patron_maxissueqty => undef },
+"Without parameter, GetBranchBorrower returns undef (unilimited) for patron_maxissueqty if no rules defined"
);
Koha::CirculationRules->set_rules(
{
branchcode => $samplebranch1->{branchcode},
categorycode => $samplecat->{categorycode},
+ checkout_type => undef,
rules => {
patron_maxissueqty => 5,
- patron_maxonsiteissueqty => 6,
}
}
);
+Koha::CirculationRules->set_rules(
+ {
+ branchcode => $samplebranch1->{branchcode},
+ categorycode => $samplecat->{categorycode},
+ checkout_type => $Koha::Checkouts::type->{onsite_checkout},
+ rules => {
+ patron_maxissueqty => 9001,
+ }
+ }
+);
Koha::CirculationRules->set_rules(
{
branchcode => $samplebranch2->{branchcode},
categorycode => undef,
+ checkout_type => undef,
rules => {
patron_maxissueqty => 3,
- patron_maxonsiteissueqty => 2,
}
}
);
@@ -184,9 +194,9 @@ Koha::CirculationRules->set_rules(
{
branchcode => undef,
categorycode => undef,
+ checkout_type => undef,
rules => {
patron_maxissueqty => 4,
- patron_maxonsiteissueqty => 5,
}
}
);
@@ -235,26 +245,35 @@ Koha::CirculationRules->set_rules(
#Test GetBranchBorrowerCircRule
is_deeply(
GetBranchBorrowerCircRule(),
- { patron_maxissueqty => 4, patron_maxonsiteissueqty => 5 },
-"Without parameter, GetBranchBorrower returns the patron_maxissueqty and patron_maxonsiteissueqty of default_circ_rules"
+ { patron_maxissueqty => 4 },
+"Without parameter, GetBranchBorrower returns the patron_maxissueqty of default_circ_rules"
);
is_deeply(
GetBranchBorrowerCircRule( $samplebranch2->{branchcode} ),
- { patron_maxissueqty => 3, patron_maxonsiteissueqty => 2 },
-"Without only the branchcode specified, GetBranchBorrower returns the patron_maxissueqty and patron_maxonsiteissueqty corresponding"
+ { patron_maxissueqty => 3 },
+"Without only the branchcode specified, GetBranchBorrower returns the patron_maxissueqty corresponding"
);
is_deeply(
GetBranchBorrowerCircRule(
$samplebranch1->{branchcode},
$samplecat->{categorycode}
),
- { patron_maxissueqty => 5, patron_maxonsiteissueqty => 6 },
- "GetBranchBorrower returns the patron_maxissueqty and patron_maxonsiteissueqty of the branch1 and the category1"
+ { patron_maxissueqty => 5 },
+ "GetBranchBorrower returns the patron_maxissueqty of the branch1 and the category1"
+);
+is_deeply(
+ GetBranchBorrowerCircRule(
+ $samplebranch1->{branchcode},
+ $samplecat->{categorycode},
+ $Koha::Checkouts::type->{onsite_checkout}
+ ),
+ { patron_maxissueqty => 9001 },
+ "GetBranchBorrower returns the patron_maxissueqty of the branch1 and the category1 and checkout_type on-site"
);
is_deeply(
GetBranchBorrowerCircRule( -1, -1 ),
- { patron_maxissueqty => 4, patron_maxonsiteissueqty => 5 },
-"GetBranchBorrower with wrong parameters returns the patron_maxissueqty and patron_maxonsiteissueqty of default_circ_rules"
+ { patron_maxissueqty => 4 },
+"GetBranchBorrower with wrong parameters returns the patron_maxissueqty of default_circ_rules"
);
#Test GetBranchItemRule
diff --git a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
index 41e4eb8433..7989c5e5eb 100644
--- a/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
+++ b/t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
@@ -87,9 +87,9 @@ Koha::CirculationRules->set_rules(
branchcode => $branch->{branchcode},
categorycode => undef,
itemtype => undef,
+ checkout_type => undef,
rules => {
maxissueqty => 2,
- maxonsiteissueqty => 1,
lengthunit => 'days',
issuelength => 5,
hardduedate => undef,
@@ -97,6 +97,17 @@ Koha::CirculationRules->set_rules(
}
}
);
+Koha::CirculationRules->set_rules(
+ {
+ branchcode => $branch->{branchcode},
+ categorycode => undef,
+ itemtype => undef,
+ checkout_type => $Koha::Checkouts::type->{onsite_checkout},
+ rules => {
+ maxissueqty => 1,
+ }
+ }
+);
t::lib::Mocks::mock_userenv({ patron => $patron });
@@ -159,9 +170,20 @@ Koha::CirculationRules->set_rules(
branchcode => $branch->{branchcode},
categorycode => undef,
itemtype => undef,
+ checkout_type => undef,
+ rules => {
+ maxissueqty => 2,
+ }
+ }
+);
+Koha::CirculationRules->set_rules(
+ {
+ branchcode => $branch->{branchcode},
+ categorycode => undef,
+ itemtype => undef,
+ checkout_type => $Koha::Checkouts::type->{onsite_checkout},
rules => {
maxissueqty => 2,
- maxonsiteissueqty => 1,
}
}
);
diff --git a/t/db_dependent/Circulation/TooMany.t b/t/db_dependent/Circulation/TooMany.t
index 31ec01563b..46f1f55531 100644
--- a/t/db_dependent/Circulation/TooMany.t
+++ b/t/db_dependent/Circulation/TooMany.t
@@ -108,9 +108,9 @@ subtest '1 Issuingrule exist 0 0: no issue allowed' => sub {
branchcode => $branch->{branchcode},
categorycode => $category->{categorycode},
itemtype => undef,
+ checkout_type => undef,
rules => {
maxissueqty => 0,
- maxonsiteissueqty => 0,
}
},
);
@@ -165,9 +165,20 @@ subtest '1 Issuingrule exist with onsiteissueqty=unlimited' => sub {
branchcode => $branch->{branchcode},
categorycode => $category->{categorycode},
itemtype => undef,
+ checkout_type => undef,
rules => {
maxissueqty => 1,
- maxonsiteissueqty => undef,
+ }
+ },
+ );
+ Koha::CirculationRules->set_rules(
+ {
+ branchcode => $branch->{branchcode},
+ categorycode => $category->{categorycode},
+ itemtype => undef,
+ checkout_type => $Koha::Checkouts::type->{onsite_checkout},
+ rules => {
+ maxissueqty => undef,
}
},
);
@@ -220,9 +231,9 @@ subtest '1 Issuingrule exist 1 1: issue is allowed' => sub {
branchcode => $branch->{branchcode},
categorycode => $category->{categorycode},
itemtype => undef,
+ checkout_type => undef,
rules => {
maxissueqty => 1,
- maxonsiteissueqty => 1,
}
}
);
@@ -260,9 +271,9 @@ subtest '1 Issuingrule exist: 1 CO allowed, 1 OSCO allowed. Do a CO' => sub {
branchcode => $branch->{branchcode},
categorycode => $category->{categorycode},
itemtype => undef,
+ checkout_type => undef,
rules => {
maxissueqty => 1,
- maxonsiteissueqty => 1,
}
}
);
@@ -316,9 +327,9 @@ subtest '1 Issuingrule exist: 1 CO allowed, 1 OSCO allowed, Do a OSCO' => sub {
branchcode => $branch->{branchcode},
categorycode => $category->{categorycode},
itemtype => undef,
+ checkout_type => undef,
rules => {
maxissueqty => 1,
- maxonsiteissueqty => 1,
}
}
);
@@ -375,9 +386,9 @@ subtest '1 BranchBorrowerCircRule exist: 1 CO allowed, 1 OSCO allowed' => sub {
branchcode => $branch->{branchcode},
categorycode => $category->{categorycode},
itemtype => undef,
+ checkout_type => undef,
rules => {
maxissueqty => 1,
- maxonsiteissueqty => 1,
}
}
);
@@ -427,9 +438,9 @@ subtest '1 BranchBorrowerCircRule exist: 1 CO allowed, 1 OSCO allowed' => sub {
branchcode => $branch->{branchcode},
categorycode => $category->{categorycode},
itemtype => undef,
+ checkout_type => undef,
rules => {
maxissueqty => 1,
- maxonsiteissueqty => 1,
}
}
);
@@ -505,6 +516,7 @@ subtest 'General vs specific rules limit quantity correctly' => sub {
categorycode => '*',
itemtype => $itemtype->{itemtype},
branchcode => '*',
+ checkout_type => '*',
rules => {
issuelength => 1,
firstremind => 1, # 1 day of grace
@@ -519,10 +531,10 @@ subtest 'General vs specific rules limit quantity correctly' => sub {
{
branchcode => '*',
categorycode => '*',
+ checkout_type => '*',
itemtype => $itemtype->{itemtype},
rules => {
maxissueqty => 1,
- maxonsiteissueqty => 1,
}
}
);
@@ -573,9 +585,9 @@ subtest 'General vs specific rules limit quantity correctly' => sub {
branchcode => $branch->{branchcode},
categorycode => $category->{categorycode},
itemtype => $itemtype->{itemtype},
+ checkout_type => undef,
rules => {
maxissueqty => 1,
- maxonsiteissueqty => 1,
}
}
);
@@ -669,9 +681,9 @@ subtest 'General vs specific rules limit quantity correctly' => sub {
branchcode => $branch2->{branchcode},
categorycode => $category->{categorycode},
itemtype => $itemtype->{itemtype},
+ checkout_type => undef,
rules => {
maxissueqty => 1,
- maxonsiteissueqty => 1,
}
}
);
@@ -691,9 +703,9 @@ subtest 'empty string means unlimited' => sub {
branchcode => '*',
categorycode => '*',
itemtype => '*',
+ checkout_type => '*',
rules => {
maxissueqty => '',
- maxonsiteissueqty => '',
}
},
);
@@ -706,7 +718,7 @@ subtest 'empty string means unlimited' => sub {
is(
C4::Circulation::TooMany( $patron, $item_object, { onsite_checkout => 1 } ),
undef,
- 'maxonsiteissueqty="" should mean unlimited'
+ 'maxissueqty="" should mean unlimited'
);
teardown();
--
2.17.1