Bugzilla – Attachment 98324 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: Fix some more tests
Bug-18936-Fix-some-more-tests.patch (text/plain), 19.56 KB, created by
Jonathan Druart
on 2020-02-03 16:07:59 UTC
(
hide
)
Description:
Bug 18936: Fix some more tests
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2020-02-03 16:07:59 UTC
Size:
19.56 KB
patch
obsolete
>From f8467a8eb7a11a2730badf562bb22d3327512db0 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Wed, 22 Jan 2020 12:27:22 +0100 >Subject: [PATCH] Bug 18936: Fix some more tests >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >* 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> >--- > C4/Reserves.pm | 15 ++--- > t/db_dependent/Circulation.t | 12 ++-- > t/db_dependent/Circulation/CalcDateDue.t | 25 +++++--- > .../Circulation/IssuingRules/maxsuspensiondays.t | 24 +++---- > t/db_dependent/Circulation/ReturnClaims.t | 13 ++-- > t/db_dependent/Holds.t | 73 ++++++---------------- > .../Holds/DisallowHoldIfItemsAvailable.t | 1 - > t/db_dependent/Koha/Charges/Fees.t | 12 ++-- > 8 files changed, 71 insertions(+), 104 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index aa4c038f6d..86ff1faebb 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -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( > { >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index f96cb9c7db..4b17bf5d61 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -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; > }; > >diff --git a/t/db_dependent/Circulation/CalcDateDue.t b/t/db_dependent/Circulation/CalcDateDue.t >index b154c1fd5e..4c0999ef0d 100644 >--- a/t/db_dependent/Circulation/CalcDateDue.t >+++ b/t/db_dependent/Circulation/CalcDateDue.t >@@ -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}); >diff --git a/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t b/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t >index 2cbc02e73d..f29df5f94c 100644 >--- a/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t >+++ b/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t >@@ -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', >diff --git a/t/db_dependent/Circulation/ReturnClaims.t b/t/db_dependent/Circulation/ReturnClaims.t >index 3681650464..464420ead2 100644 >--- a/t/db_dependent/Circulation/ReturnClaims.t >+++ b/t/db_dependent/Circulation/ReturnClaims.t >@@ -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}; > >diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t >index aee25a8759..d5a2f8b58a 100755 >--- a/t/db_dependent/Holds.t >+++ b/t/db_dependent/Holds.t >@@ -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', >diff --git a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t >index f49aa107a9..3df3d97daa 100755 >--- a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t >+++ b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t >@@ -239,7 +239,6 @@ sub set_holdallowed_rule { > Koha::CirculationRules->set_rules( > { > branchcode => $branchcode || undef, >- categorycode => undef, > itemtype => undef, > rules => { > holdallowed => $holdallowed, >diff --git a/t/db_dependent/Koha/Charges/Fees.t b/t/db_dependent/Koha/Charges/Fees.t >index 544d9050bd..431e292a54 100644 >--- a/t/db_dependent/Koha/Charges/Fees.t >+++ b/t/db_dependent/Koha/Charges/Fees.t >@@ -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(); > >-- >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