Bug 18936: Fix some more tests

* CanItemBeReserved
Prior to "Bug 18936: Convert issuingrules fields to circulation_rules",
GetHoldRule returned holds_per_record even if no reservesallowed was
defined. This change restores this behavior.
FIXME Note: In GetHoldRule we return itemtype only if reservesallowed is set,
not sure it is correct.

* t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
When setting returnbranch, holdallowed and hold_fulfillment_policy, we
should not provide categorycode.

* t/db_dependent/Holds.t
Prefer to keep the existing rules instead of removing them. It got quite
hard to understand what was going on here because of the mixup with
the rule reservesallowed that was in issuingrules, and the other rules
we used for the tests. Also, categorycode should not be passed to set
those 3 rules (holdallowed, hold_fulfillment_policy and returnbranch)

* t/db_dependent/Circulation.t
Setting lengthunit to 'hours', no need to make sure the rule has been
correctly be saved

* t/db_dependent/Circulation/CalcDateDue.t
It uses hardcoded data that is not in the sample data (categorycode=C).
Let use K that exists and postpone a refactore of the whole script (to
make it create the data it needs).

* t/db_dependent/Circulation/ReturnClaims.t
* t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t
Simple replace Koha::IssuingRule with Koha::CirculationRules

* t/db_dependent/Koha/Charges/Fees.t
=> FIXME Still failing, stuck here, need help

Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This commit is contained in:
Jonathan Druart 2020-01-22 12:27:22 +01:00 committed by Martin Renvoize
parent e4dfe623d9
commit 25848e5af3
No known key found for this signature in database
GPG Key ID: 422B469130441A0F
8 changed files with 71 additions and 104 deletions

View File

@ -379,8 +379,8 @@ sub CanItemBeReserved {
# we retrieve rights
if ( my $rights = GetHoldRule( $borrower->{'categorycode'}, $item->effective_itemtype, $branchcode ) ) {
$ruleitemtype = $rights->{itemtype};
$allowedreserves = $rights->{reservesallowed};
$holds_per_record = $rights->{holds_per_record};
$allowedreserves = $rights->{reservesallowed} // $allowedreserves;
$holds_per_record = $rights->{holds_per_record} // $holds_per_record;
$holds_per_day = $rights->{holds_per_day};
}
else {
@ -2216,13 +2216,14 @@ sub GetHoldRule {
}
}
);
return unless $reservesallowed;;
my $rules;
$rules->{reservesallowed} = $reservesallowed->rule_value;
$rules->{itemtype} = $reservesallowed->itemtype;
$rules->{categorycode} = $reservesallowed->categorycode;
$rules->{branchcode} = $reservesallowed->branchcode;
if ( $reservesallowed ) {
$rules->{reservesallowed} = $reservesallowed->rule_value;
$rules->{itemtype} = $reservesallowed->itemtype;
$rules->{categorycode} = $reservesallowed->categorycode;
$rules->{branchcode} = $reservesallowed->branchcode;
}
my $holds_per_x_rules = Koha::CirculationRules->get_effective_rules(
{

View File

@ -3358,7 +3358,7 @@ subtest 'ProcessOfflinePayment() tests' => sub {
};
subtest 'Incremented fee tests' => sub {
plan tests => 20;
plan tests => 19;
my $dt = dt_from_string();
Time::Fake->offset( $dt->epoch );
@ -3486,16 +3486,15 @@ subtest 'Incremented fee tests' => sub {
'Rental charge reset and retreived correctly' );
# Hourly
my $issuingrule = Koha::IssuingRules->get_effective_issuing_rule(
Koha::CirculationRules->set_rule(
{
categorycode => $patron->categorycode,
itemtype => $itemtype->id,
branchcode => $library->id
branchcode => $library->id,
rule_name => 'lengthunit',
rule_value => 'hours',
}
);
$issuingrule->lengthunit('hours')->store();
is( $issuingrule->lengthunit, 'hours',
'Issuingrule updated and retrieved correctly' );
$itemtype->rentalcharge_hourly('0.25')->store();
is( $itemtype->rentalcharge_hourly,
@ -3545,7 +3544,6 @@ subtest 'Incremented fee tests' => sub {
"Hourly rental charge calculated correctly with finesCalendar = noFinesWhenClosed, for renewal (312h - 168h - 0h * 0.25u)" );
$accountline->delete();
$issue->delete();
$issuingrule->lengthunit('days')->store();
Time::Fake->reset;
};

View File

@ -157,22 +157,27 @@ $calendar->delete_holiday(
# Now we test it does the right thing if the loan and renewal periods
# are a multiple of 7 days
my $dayweek_categorycode = 'C';
my $dayweek_categorycode = 'K';
my $dayweek_itemtype = 'MX';
my $dayweek_branchcode = 'FPL';
my $dayweek_issuelength = 14;
my $dayweek_renewalperiod = 7;
my $dayweek_lengthunit = 'days';
Koha::Database->schema->resultset('Issuingrule')->create({
categorycode => $dayweek_categorycode,
itemtype => $dayweek_itemtype,
branchcode => $dayweek_branchcode,
issuelength => $dayweek_issuelength,
renewalperiod => $dayweek_renewalperiod,
lengthunit => $dayweek_lengthunit,
});
my $dayweek_borrower = {categorycode => 'C', dateexpiry => $dateexpiry};
Koha::CirculationRules->set_rules(
{
categorycode => $dayweek_categorycode,
itemtype => $dayweek_itemtype,
branchcode => $dayweek_branchcode,
rules => {
issuelength => $dayweek_issuelength,
renewalperiod => $dayweek_renewalperiod,
lengthunit => $dayweek_lengthunit,
}
}
);
my $dayweek_borrower = {categorycode => 'K', dateexpiry => $dateexpiry};
# For issues...
$start_date = DateTime->new({year => 2013, month => 2, day => 9});

View File

@ -109,14 +109,12 @@ is(
DelDebarment( $debarments->[0]->{borrower_debarment_id} );
subtest "suspension_chargeperiod" => sub {
Koha::IssuingRules->search->delete;
$builder->build(
Koha::CirculationRules->set_rules(
{
source => 'Issuingrule',
value => {
categorycode => '*',
itemtype => '*',
branchcode => '*',
categorycode => undef,
itemtype => undef,
branchcode => undef,
rules => {
firstremind => 0,
finedays => 7,
lengthunit => 'days',
@ -137,14 +135,12 @@ subtest "suspension_chargeperiod" => sub {
};
subtest "maxsuspensiondays" => sub {
Koha::IssuingRules->search->delete;
$builder->build(
Koha::CirculationRules->set_rules(
{
source => 'Issuingrule',
value => {
categorycode => '*',
itemtype => '*',
branchcode => '*',
categorycode => undef,
itemtype => undef,
branchcode => undef,
rules => {
firstremind => 0,
finedays => 15,
lengthunit => 'days',

View File

@ -46,16 +46,15 @@ my $schema = Koha::Database->schema;
$schema->storage->txn_begin;
my $builder = t::lib::TestBuilder->new();
Koha::IssuingRules->search->delete;
my $rule = Koha::IssuingRule->new(
Koha::CirculationRules->set_rule(
{
categorycode => '*',
itemtype => '*',
branchcode => '*',
issuelength => 1,
branchcode => undef,
categorycode => undef,
itemtype => undef,
rule_name => 'issuelength',
rule_value => 1
}
);
$rule->store();
$branch = $builder->build( { source => 'Branch' } )->{branchcode};

View File

@ -458,7 +458,6 @@ subtest 'Test max_holds per library/patron category' => sub {
plan tests => 6;
$dbh->do('DELETE FROM reserves');
$dbh->do('DELETE FROM circulation_rules');
$biblio = $builder->build_sample_biblio;
( $item_bibnum, $item_bibitemnum, $itemnumber ) =
@ -586,8 +585,6 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub {
Koha::Holds->search->delete;
$dbh->do('DELETE FROM issues');
$dbh->do('DELETE FROM issuingrules');
$dbh->do('DELETE FROM circulation_rules');
Koha::Items->search->delete;
Koha::Biblios->search->delete;
@ -753,14 +750,15 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
# Cleanup database
Koha::Holds->search->delete;
$dbh->do('DELETE FROM issues');
$dbh->do('DELETE FROM issuingrules');
$dbh->do(
q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
VALUES (?, ?, ?, ?)},
{},
'*', '*', '*', 25
Koha::CirculationRules->set_rule(
{
branchcode => undef,
categorycode => undef,
itemtype => undef,
rule_name => 'reservesallowed',
rule_value => 25,
}
);
$dbh->do('DELETE FROM circulation_rules');
Koha::Items->search->delete;
Koha::Biblios->search->delete;
@ -818,7 +816,6 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
{
branchcode => undef,
itemtype => undef,
categorycode => undef,
rules => {
holdallowed => 3,
hold_fulfillment_policy => 'any',
@ -841,15 +838,11 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
'Patron cannot place hold because patron\'s home library is not part of hold group'
);
# Cleanup default_cirt_rules
$dbh->do('DELETE FROM circulation_rules');
# Insert default circ rule to "any" for library 2
Koha::CirculationRules->set_rules(
{
branchcode => $library2->branchcode,
itemtype => undef,
categorycode => undef,
rules => {
holdallowed => 2,
hold_fulfillment_policy => 'any',
@ -870,7 +863,6 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
{
branchcode => $library2->branchcode,
itemtype => undef,
categorycode => undef,
rules => {
holdallowed => 3,
hold_fulfillment_policy => 'any',
@ -886,15 +878,11 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
'Patron cannot place hold if holdallowed is set to "hold group" for library 2'
);
# Cleanup default_branch_cirt_rules
$dbh->do('DELETE FROM circulation_rules');
# Insert default item rule to "any" for itemtype 2
Koha::CirculationRules->set_rules(
{
branchcode => undef,
branchcode => $library2->branchcode,
itemtype => $itemtype2->itemtype,
categorycode => undef,
rules => {
holdallowed => 2,
hold_fulfillment_policy => 'any',
@ -913,9 +901,8 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
# Update default item rule to "hold group" for itemtype 2
Koha::CirculationRules->set_rules(
{
branchcode => undef,
branchcode => $library2->branchcode,
itemtype => $itemtype2->itemtype,
categorycode => undef,
rules => {
holdallowed => 3,
hold_fulfillment_policy => 'any',
@ -931,15 +918,11 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
'Patron cannot place hold if holdallowed is set to "hold group" for itemtype 2'
);
# Cleanup default_branch_item_rules
$dbh->do('DELETE FROM circulation_rules');
# Insert branch item rule to "any" for itemtype 2 and library 2
Koha::CirculationRules->set_rules(
{
branchcode => $library2->branchcode,
itemtype => $itemtype2->itemtype,
categorycode => undef,
rules => {
holdallowed => 2,
hold_fulfillment_policy => 'any',
@ -960,7 +943,6 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
{
branchcode => $library2->branchcode,
itemtype => $itemtype2->itemtype,
categorycode => undef,
rules => {
holdallowed => 3,
hold_fulfillment_policy => 'any',
@ -988,14 +970,15 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
# Cleanup database
Koha::Holds->search->delete;
$dbh->do('DELETE FROM issues');
$dbh->do('DELETE FROM issuingrules');
$dbh->do(
q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
VALUES (?, ?, ?, ?)},
{},
'*', '*', '*', 25
Koha::CirculationRules->set_rule(
{
branchcode => undef,
categorycode => undef,
itemtype => undef,
rule_name => 'reservesallowed',
rule_value => 25,
}
);
$dbh->do('DELETE FROM circulation_rules');
Koha::Items->search->delete;
Koha::Biblios->search->delete;
@ -1053,7 +1036,6 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
{
branchcode => undef,
itemtype => undef,
categorycode => undef,
rules => {
holdallowed => 2,
hold_fulfillment_policy => 'holdgroup',
@ -1076,15 +1058,11 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
'Patron cannot place hold because pickup location is not part of hold group'
);
# Cleanup default_cirt_rules
$dbh->do('DELETE FROM circulation_rules');
# Insert default circ rule to "any" for library 2
Koha::CirculationRules->set_rules(
{
branchcode => $library2->branchcode,
itemtype => undef,
categorycode => undef,
rules => {
holdallowed => 2,
hold_fulfillment_policy => 'any',
@ -1105,7 +1083,6 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
{
branchcode => $library2->branchcode,
itemtype => undef,
categorycode => undef,
rules => {
holdallowed => 2,
hold_fulfillment_policy => 'holdgroup',
@ -1121,15 +1098,11 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
'Patron cannot place hold if hold_fulfillment_policy is set to "hold group" for library 2'
);
# Cleanup default_branch_cirt_rules
$dbh->do('DELETE FROM circulation_rules');
# Insert default item rule to "any" for itemtype 2
Koha::CirculationRules->set_rules(
{
branchcode => undef,
branchcode => $library2->branchcode,
itemtype => $itemtype2->itemtype,
categorycode => undef,
rules => {
holdallowed => 2,
hold_fulfillment_policy => 'any',
@ -1148,9 +1121,8 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
# Update default item rule to "hold group" for itemtype 2
Koha::CirculationRules->set_rules(
{
branchcode => undef,
branchcode => $library2->branchcode,
itemtype => $itemtype2->itemtype,
categorycode => undef,
rules => {
holdallowed => 2,
hold_fulfillment_policy => 'holdgroup',
@ -1166,15 +1138,11 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
'Patron cannot place hold if hold_fulfillment_policy is set to "hold group" for itemtype 2'
);
# Cleanup default_branch_item_rules
$dbh->do('DELETE FROM circulation_rules');
# Insert branch item rule to "any" for itemtype 2 and library 2
Koha::CirculationRules->set_rules(
{
branchcode => $library2->branchcode,
itemtype => $itemtype2->itemtype,
categorycode => undef,
rules => {
holdallowed => 2,
hold_fulfillment_policy => 'any',
@ -1195,7 +1163,6 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
{
branchcode => $library2->branchcode,
itemtype => $itemtype2->itemtype,
categorycode => undef,
rules => {
holdallowed => 2,
hold_fulfillment_policy => 'holdgroup',

View File

@ -239,7 +239,6 @@ sub set_holdallowed_rule {
Koha::CirculationRules->set_rules(
{
branchcode => $branchcode || undef,
categorycode => undef,
itemtype => undef,
rules => {
holdallowed => $holdallowed,

View File

@ -364,15 +364,17 @@ subtest 'accumulate_rentalcharge tests' => sub {
);
# Hourly tests
my $issuingrule = Koha::IssuingRules->get_effective_issuing_rule(
{
Koha::CirculationRules->set_rules({
categorycode => $patron->categorycode,
itemtype => $itemtype->id,
branchcode => $library->id
branchcode => $library->id,
rules => {
lengthunit => 'hours',
}
}
);
$issuingrule->lengthunit('hours');
$issuingrule->store();
$itemtype->rentalcharge_hourly("0.25");
$itemtype->store();