Bugzilla – Attachment 114421 Details for
Bug 26814
Add onsite_checkout to circulation rules
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 26814: Add onsite_checkout to set_rule
Bug-26814-Add-onsitecheckout-to-setrule.patch (text/plain), 13.48 KB, created by
Lari Taskula
on 2020-12-15 23:31:40 UTC
(
hide
)
Description:
Bug 26814: Add onsite_checkout to set_rule
Filename:
MIME Type:
Creator:
Lari Taskula
Created:
2020-12-15 23:31:40 UTC
Size:
13.48 KB
patch
obsolete
>From 9f6c4fbf096a15eadfbc9a56d99e57c2c1d75659 Mon Sep 17 00:00:00 2001 >From: Lari Taskula <lari.taskula@hypernova.fi> >Date: Fri, 10 Apr 2020 00:02:24 +0000 >Subject: [PATCH] Bug 26814: Add onsite_checkout to set_rule > >To test: >1. Find all occurrences of set_rule that are missing onsite_checkout > where required > >grep --exclude-dir '.git' --exclude-dir 'misc/translator' \ >--exclude-dir 'koha-tmpl' -Przo '(?s)(::|->)set_rule(?!s).*?\)' | \ >grep -avz 'onsite_checkout' | grep -Pavz 'hold|reserves|article_requests|lostreturn' \ >&& echo "" > > The only occasions this should return anything are cases where > a HASH or HASHref is given to set_rule(), or that we are > explicitly testing missing parameters in an unit test. > > 1.2 Verify the HASH/HASHref cases. The hash should contain > a onsite_checkout (unless the rule is related to holds) > >2. Find all subroutines using set_rule() > >git grep --no-index -n -p -P 'set_rule\s*\(' | grep -v 'sub {' \ >| grep -P 'sub .*' > > 2.1 The only return should be > Koha/CirculationRules.pm=331=sub set_rules { > >3. The boring step. Find all occurrences of set_rule() with > the following command: > >grep --exclude-dir='.git' -Prn 'set_rule' | grep -v set_rules > > Go through this list and make sure all neccessary locations are > updated. A change is required when the rule scope includes > onsite_checkout. See Koha/CirculationRules.pm for scopes. > Hold/reserve related rules do not require it. > >4. Check changes to CirculationRule objects built with TestBuilder > >cd t && grep -Prn "CirculationRules?'" > > Make sure all these cases are updated with onsite_checkout => undef, > otherwise tests will fail later on. > >Sponsored-by: The National Library of Finland >--- > t/db_dependent/Circulation.t | 9 +++++++ > t/db_dependent/Circulation/CalcFine.t | 5 +++- > t/db_dependent/Circulation/ReturnClaims.t | 1 + > t/db_dependent/Circulation/Returns.t | 1 + > t/db_dependent/Koha/IssuingRules.t | 33 ++++++++++++++++++++--- > t/db_dependent/Koha/Items.t | 3 +++ > 6 files changed, 48 insertions(+), 4 deletions(-) > >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index f29c175aef..be320000af 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -686,6 +686,7 @@ subtest "CanBookBeRenewed tests" => sub { > categorycode => undef, > branchcode => undef, > itemtype => undef, >+ onsite_checkout => undef, > rule_name => 'norenewalbefore', > rule_value => '7', > } >@@ -2227,6 +2228,7 @@ subtest 'AddReturn + suspension_chargeperiod' => sub { > categorycode => undef, > branchcode => undef, > itemtype => undef, >+ onsite_checkout => undef, > rule_name => 'suspension_chargeperiod', > rule_value => '2', > } >@@ -2746,6 +2748,7 @@ subtest 'AddReturn | is_overdue' => sub { > branchcode => $branchcode_false, > categorycode => undef, > itemtype => undef, >+ onsite_checkout => undef, > rule_name => 'lostreturn', > rule_value => 0 > } >@@ -2760,6 +2763,7 @@ subtest 'AddReturn | is_overdue' => sub { > branchcode => $branchcode_refund, > categorycode => undef, > itemtype => undef, >+ onsite_checkout => undef, > rule_name => 'lostreturn', > rule_value => 'refund' > } >@@ -2774,6 +2778,7 @@ subtest 'AddReturn | is_overdue' => sub { > branchcode => $branchcode_restore, > categorycode => undef, > itemtype => undef, >+ onsite_checkout => undef, > rule_name => 'lostreturn', > rule_value => 'restore' > } >@@ -2788,6 +2793,7 @@ subtest 'AddReturn | is_overdue' => sub { > branchcode => $branchcode_charge, > categorycode => undef, > itemtype => undef, >+ onsite_checkout => undef, > rule_name => 'lostreturn', > rule_value => 'charge' > } >@@ -3956,6 +3962,7 @@ subtest 'Incremented fee tests' => sub { > categorycode => $patron->categorycode, > itemtype => $itemtype->id, > branchcode => $library->id, >+ onsite_checkout => undef, > rule_name => 'lengthunit', > rule_value => 'hours', > } >@@ -4206,6 +4213,7 @@ subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddReturn' => sub { > branchcode => undef, > categorycode => undef, > itemtype => undef, >+ onsite_checkout => undef, > rule_name => 'lostreturn', > rule_value => 'refund' > } >@@ -4399,6 +4407,7 @@ subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddIssue' => sub { > branchcode => undef, > categorycode => undef, > itemtype => undef, >+ onsite_checkout => undef, > rule_name => 'lostreturn', > rule_value => 'refund' > } >diff --git a/t/db_dependent/Circulation/CalcFine.t b/t/db_dependent/Circulation/CalcFine.t >index 85f38ea42e..c443cd16a9 100755 >--- a/t/db_dependent/Circulation/CalcFine.t >+++ b/t/db_dependent/Circulation/CalcFine.t >@@ -69,6 +69,7 @@ subtest 'Test basic functionality' => sub { > branchcode => undef, > categorycode => undef, > itemtype => undef, >+ onsite_checkout => undef, > rules => { > fine => '1.00', > lengthunit => 'days', >@@ -109,6 +110,7 @@ subtest 'Test cap_fine_to_replacement_price' => sub { > branchcode => undef, > categorycode => undef, > itemtype => undef, >+ onsite_checkout => undef, > rules => { > fine => '1.00', > lengthunit => 'days', >@@ -162,6 +164,7 @@ subtest 'Test cap_fine_to_replacement_pricew with overduefinescap' => sub { > branchcode => undef, > categorycode => undef, > itemtype => undef, >+ onsite_checkout => undef, > rules => { > fine => '1.00', > lengthunit => 'days', >@@ -189,7 +192,7 @@ subtest 'Test cap_fine_to_replacement_pricew with overduefinescap' => sub { > my ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt ); > is( int($amount), 3, 'Got the lesser of overduefinescap and replacement price where overduefinescap < replacement price' ); > >- Koha::CirculationRules->set_rule({ rule_name => 'overduefinescap', rule_value => 6, branchcode => undef, categorycode => undef, itemtype => undef }); >+ Koha::CirculationRules->set_rule({ rule_name => 'overduefinescap', rule_value => 6, branchcode => undef, onsite_checkout => undef, categorycode => undef, itemtype => undef }); > ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt ); > is( int($amount), 5, 'Get the lesser of overduefinescap and replacement price where overduefinescap > replacement price' ); > >diff --git a/t/db_dependent/Circulation/ReturnClaims.t b/t/db_dependent/Circulation/ReturnClaims.t >index 7d3b52128e..c5f6622b6d 100755 >--- a/t/db_dependent/Circulation/ReturnClaims.t >+++ b/t/db_dependent/Circulation/ReturnClaims.t >@@ -51,6 +51,7 @@ Koha::CirculationRules->set_rule( > branchcode => undef, > categorycode => undef, > itemtype => undef, >+ onsite_checkout => undef, > rule_name => 'issuelength', > rule_value => 1 > } >diff --git a/t/db_dependent/Circulation/Returns.t b/t/db_dependent/Circulation/Returns.t >index 79ae8555bf..5a2c4ded1b 100755 >--- a/t/db_dependent/Circulation/Returns.t >+++ b/t/db_dependent/Circulation/Returns.t >@@ -63,6 +63,7 @@ Koha::CirculationRules->set_rule( > categorycode => undef, > itemtype => undef, > branchcode => undef, >+ onsite_checkout => undef, > rule_name => 'issuelength', > rule_value => 1, > } >diff --git a/t/db_dependent/Koha/IssuingRules.t b/t/db_dependent/Koha/IssuingRules.t >index afcb2867b7..074f75e6af 100755 >--- a/t/db_dependent/Koha/IssuingRules.t >+++ b/t/db_dependent/Koha/IssuingRules.t >@@ -154,9 +154,10 @@ subtest 'set_rule' => sub { > my $branchcode = $builder->build({ source => 'Branch' })->{'branchcode'}; > my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'}; > my $itemtype = $builder->build({ source => 'Itemtype' })->{'itemtype'}; >+ my $onsite_checkout = 0; > > subtest 'Correct call' => sub { >- plan tests => 4; >+ plan tests => 5; > > Koha::CirculationRules->delete; > >@@ -172,6 +173,7 @@ subtest 'set_rule' => sub { > Koha::CirculationRules->set_rule( { > branchcode => $branchcode, > categorycode => $categorycode, >+ onsite_checkout => $onsite_checkout, > rule_name => 'patron_maxissueqty', > rule_value => '', > } ); >@@ -191,10 +193,22 @@ subtest 'set_rule' => sub { > branchcode => $branchcode, > categorycode => $categorycode, > itemtype => $itemtype, >+ onsite_checkout => $onsite_checkout, > rule_name => 'fine', > rule_value => '', > } ); >- }, 'setting fine with branch/category/itemtype succeeds' ); >+ }, 'setting fine with branch/category/itemtype/onsite_checkout succeeds' ); >+ >+ lives_ok( sub { >+ Koha::CirculationRules->set_rule( { >+ branchcode => $branchcode, >+ categorycode => $categorycode, >+ itemtype => $itemtype, >+ onsite_checkout => $onsite_checkout, >+ rule_name => 'maxissueqty', >+ rule_value => 5, >+ } ); >+ }, 'setting maxissueqty with branch/category/itemtype/onsite_checkout succeeds' ); > }; > > subtest 'Call with missing params' => sub { >@@ -212,6 +226,7 @@ subtest 'set_rule' => sub { > throws_ok( sub { > Koha::CirculationRules->set_rule( { > branchcode => $branchcode, >+ onsite_checkout => $onsite_checkout, > rule_name => 'patron_maxissueqty', > rule_value => '', > } ); >@@ -229,6 +244,7 @@ subtest 'set_rule' => sub { > Koha::CirculationRules->set_rule( { > branchcode => $branchcode, > categorycode => $categorycode, >+ onsite_checkout => undef, > rule_name => 'fine', > rule_value => '', > } ); >@@ -236,7 +252,7 @@ subtest 'set_rule' => sub { > }; > > subtest 'Call with extra params' => sub { >- plan tests => 3; >+ plan tests => 4; > > Koha::CirculationRules->delete; > >@@ -252,6 +268,7 @@ subtest 'set_rule' => sub { > throws_ok( sub { > Koha::CirculationRules->set_rule( { > branchcode => $branchcode, >+ onsite_checkout => undef, > categorycode => $categorycode, > itemtype => $itemtype, > rule_name => 'patron_maxissueqty', >@@ -268,6 +285,16 @@ subtest 'set_rule' => sub { > rule_value => '', > } ); > }, qr/categorycode/, 'setting holdallowed with categorycode fails' ); >+ >+ throws_ok( sub { >+ Koha::CirculationRules->set_rule( { >+ branchcode => $branchcode, >+ onsite_checkout => undef, >+ rule_name => 'holdallowed', >+ itemtype => $itemtype, >+ rule_value => '', >+ } ); >+ }, qr/onsite_checkout/, 'setting holdallowed with onsite_checkout fails' ); > }; > }; > >diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t >index 0737881609..4ffbb38e6c 100755 >--- a/t/db_dependent/Koha/Items.t >+++ b/t/db_dependent/Koha/Items.t >@@ -778,6 +778,7 @@ subtest 'store' => sub { > branchcode => $manager->branchcode, > categorycode => undef, > itemtype => undef, >+ onsite_checkout => undef, > rule_name => 'lostreturn', > rule_value => 'restore' > } >@@ -870,6 +871,7 @@ subtest 'store' => sub { > branchcode => $manager->branchcode, > categorycode => undef, > itemtype => undef, >+ onsite_checkout => undef, > rule_name => 'lostreturn', > rule_value => 'restore' > } >@@ -981,6 +983,7 @@ subtest 'store' => sub { > branchcode => $manager->branchcode, > categorycode => undef, > itemtype => undef, >+ onsite_checkout => undef, > rule_name => 'lostreturn', > rule_value => 'restore' > } >-- >2.17.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 26814
:
112376
|
112377
|
112378
|
112379
|
112380
|
112381
|
112382
|
112383
|
112384
|
112385
|
112608
|
112609
|
112610
|
112611
|
112623
|
112624
|
112625
|
112626
|
112627
|
112628
|
112630
|
112631
|
112632
|
112651
|
112652
|
112653
|
114417
|
114418
|
114419
|
114420
| 114421 |
114422
|
114423
|
114424
|
114425
|
114426
|
114427