From f0d3b3bdfe2e83771b5ad21cfa8bbb7c2c9c6752 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Sun, 4 Aug 2019 14:09:21 -0500 Subject: [PATCH] Bug 18936: More fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Minna Kivinen Signed-off-by: Joonas Kylmälä --- C4/Reserves.pm | 2 +- Koha/Charges/Fees.pm | 12 ++-- Koha/CirculationRules.pm | 8 ++- Koha/REST/V1/Checkouts.pm | 3 +- admin/smart-rules.pl | 22 +++---- .../prog/en/modules/admin/smart-rules.tt | 67 ++++++++++----------- t/db_dependent/Circulation/CalcFine.t | 13 ++-- t/db_dependent/Circulation/TooMany.t | 17 +++--- t/db_dependent/Circulation/issue.t | 17 +++--- t/db_dependent/Holds.t | 55 +++++++++++++---- t/db_dependent/ILSDI_Services.t | 51 ++++++++-------- t/db_dependent/Koha/IssuingRules.t | 69 +++++++--------------- t/db_dependent/SIP/Transaction.t | 1 - t/db_dependent/selenium/administration_tasks.t | 2 +- 14 files changed, 175 insertions(+), 164 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 0dca5e7274..aa4c038f6d 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -1218,7 +1218,7 @@ sub IsAvailableForItemLevelRequest { $item->withdrawn || ($item->damaged && !C4::Context->preference('AllowHoldsOnDamagedItems')); - my $on_shelf_holds = Koha::IssuingRules->get_onshelfholds_policy( { item => $item, patron => $patron } ); + my $on_shelf_holds = Koha::CirculationRules->get_onshelfholds_policy( { item => $item, patron => $patron } ); if ($pickup_branchcode) { my $destination = Koha::Libraries->find($pickup_branchcode); diff --git a/Koha/Charges/Fees.pm b/Koha/Charges/Fees.pm index 9098a29cd0..22255d5aba 100644 --- a/Koha/Charges/Fees.pm +++ b/Koha/Charges/Fees.pm @@ -22,7 +22,6 @@ use Modern::Perl; use Carp qw( carp confess ); use Koha::Calendar; -use Koha::IssuingRules; use Koha::DateUtils qw( dt_from_string ); use Koha::Exceptions; @@ -90,17 +89,18 @@ sub new { sub accumulate_rentalcharge { my ($self) = @_; - my $itemtype = Koha::ItemTypes->find( $self->item->effective_itemtype ); - my $issuing_rule = Koha::IssuingRules->get_effective_issuing_rule( + my $itemtype = Koha::ItemTypes->find( $self->item->effective_itemtype ); + my $lengthunit_rule = Koha::CirculationRules->get_effective_rule( { categorycode => $self->patron->categorycode, itemtype => $itemtype->id, - branchcode => $self->library->id + branchcode => $self->library->id, + rule_name => 'lengthunit', } ); - return 0 unless $issuing_rule; + return 0 unless $lengthunit_rule; - my $units = $issuing_rule->lengthunit; + my $units = $lengthunit_rule->rule_value; my $rentalcharge_increment = ( $units eq 'days' ) ? $itemtype->rentalcharge_daily diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index fef76be068..7f1074c90e 100644 --- a/Koha/CirculationRules.pm +++ b/Koha/CirculationRules.pm @@ -154,6 +154,9 @@ our $RULE_KINDS = { suspension_chargeperiod => { scope => [ 'branchcode', 'categorycode', 'itemtype' ], }, + note => { # This is not really a rule. Maybe we will want to separate this later. + scope => [ 'branchcode', 'categorycode', 'itemtype' ], + }, # Not included (deprecated?): # * accountsent # * reservecharge @@ -180,8 +183,9 @@ sub get_effective_rule { my $itemtype = $params->{itemtype}; my $branchcode = $params->{branchcode}; + my @c = caller; Koha::Exceptions::MissingParameter->throw( - "Required parameter 'rule_name' missing") + "Required parameter 'rule_name' missing" . "@c") unless $rule_name; for my $v ( $branchcode, $categorycode, $itemtype ) { @@ -372,7 +376,7 @@ sub get_opacitemholds_policy { return unless $item or $patron; - my $rule = Koha::CirculationRules->get_effective_issuing_rule( + my $rule = Koha::CirculationRules->get_effective_rule( { categorycode => $patron->categorycode, itemtype => $item->effective_itemtype, diff --git a/Koha/REST/V1/Checkouts.pm b/Koha/REST/V1/Checkouts.pm index 6246f9d081..7fc6913f1e 100644 --- a/Koha/REST/V1/Checkouts.pm +++ b/Koha/REST/V1/Checkouts.pm @@ -24,7 +24,6 @@ use C4::Auth qw( haspermission ); use C4::Context; use C4::Circulation; use Koha::Checkouts; -use Koha::IssuingRules; use Koha::Old::Checkouts; use Try::Tiny; @@ -209,7 +208,7 @@ sub allows_renewal { my $renewable = Mojo::JSON->false; $renewable = Mojo::JSON->true if $can_renew; - my $rule = Koha::IssuingRules->get_effective_issuing_rule( + my $rule = Koha::CirculationRules->get_effective_rule( { categorycode => $checkout->patron->categorycode, itemtype => $checkout->item->effective_itemtype, diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index 631baf45f5..112efeb2ca 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -85,15 +85,16 @@ if ($op eq 'delete') { branchcode => $branch eq '*' ? undef : $branch, itemtype => $itemtype eq '*' ? undef : $itemtype, rules => { - restrictedtype => undef, + maxissueqty => undef, + maxonsiteissueqty => undef, rentaldiscount => undef, fine => undef, finedays => undef, maxsuspensiondays => undef, + suspension_chargeperiod => undef, firstremind => undef, chargeperiod => undef, chargeperiod_charge_at => undef, - accountsent => undef, issuelength => undef, lengthunit => undef, hardduedate => undef, @@ -106,11 +107,13 @@ if ($op eq 'delete') { no_auto_renewal_after_hard_limit => undef, reservesallowed => undef, holds_per_record => undef, - overduefinescap => undef, - cap_fine_to_replacement_price => undef, + holds_per_day => undef, onshelfholds => undef, opacitemholds => undef, + overduefinescap => undef, + cap_fine_to_replacement_price => undef, article_requests => undef, + note => undef, } } ); @@ -298,6 +301,7 @@ elsif ($op eq 'add') { my $rules = { maxissueqty => $maxissueqty, maxonsiteissueqty => $maxonsiteissueqty, + rentaldiscount => $rentaldiscount, fine => $fine, finedays => $finedays, maxsuspensiondays => $maxsuspensiondays, @@ -305,6 +309,10 @@ elsif ($op eq 'add') { firstremind => $firstremind, chargeperiod => $chargeperiod, chargeperiod_charge_at => $chargeperiod_charge_at, + issuelength => $issuelength, + lengthunit => $lengthunit, + hardduedate => $hardduedate, + hardduedatecompare => $hardduedatecompare, renewalsallowed => $renewalsallowed, renewalperiod => $renewalperiod, norenewalbefore => $norenewalbefore, @@ -314,11 +322,6 @@ elsif ($op eq 'add') { reservesallowed => $reservesallowed, holds_per_record => $holds_per_record, holds_per_day => $holds_per_day, - issuelength => $issuelength, - lengthunit => $lengthunit, - hardduedate => $hardduedate, - hardduedatecompare => $hardduedatecompare, - rentaldiscount => $rentaldiscount, onshelfholds => $onshelfholds, opacitemholds => $opacitemholds, overduefinescap => $overduefinescap, @@ -403,7 +406,6 @@ elsif ($op eq "set-branch-defaults") { { branchcode => $branch, categorycode => undef, - itemtype => undef, rule_name => 'max_holds', rule_value => $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 8bb1381eb5..7da729e0e6 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 @@ -124,40 +124,37 @@ [% SET row_count = 0 %] [% FOREACH c IN categorycodes %] [% FOREACH i IN itemtypes %] - [% SET note = CirculationRules.Get( branchcode, c, i, 'note' ) %] - [% SET maxissueqty = CirculationRules.Get( branchcode, c, i, 'maxissueqty' ) %] - [% SET maxonsiteissueqty = CirculationRules.Get( branchcode, c, i, 'maxonsiteissueqty' ) %] - [% SET issuelength = CirculationRules.Get( branchcode, c, i, 'issuelength' ) %] - [% SET lengthunit = CirculationRules.Get( branchcode, c, i, 'lengthunit' ) %] - [% SET hardduedate = CirculationRules.Get( branchcode, c, i, 'hardduedate' ) %] - [% SET hardduedatecompare = CirculationRules.Get( branchcode, c, i, 'hardduedatecompare' ) %] - [% SET fine = CirculationRules.Get( branchcode, c, i, 'fine' ) %] - [% SET chargeperiod = CirculationRules.Get( branchcode, c, i, 'chargeperiod' ) %] - [% SET chargeperiod_charge_at = CirculationRules.Get( branchcode, c, i, 'chargeperiod_charge_at' ) %] - [% SET firstremind = CirculationRules.Get( branchcode, c, i, 'firstremind' ) %] - [% SET overduefinescap = CirculationRules.Get( branchcode, c, i, 'overduefinescap' ) %] - [% SET cap_fine_to_replacement_price = CirculationRules.Get( branchcode, c, i, 'cap_fine_to_replacement_price' ) %] - [% SET finedays = CirculationRules.Get( branchcode, c, i, 'finedays' ) %] - [% SET maxsuspensiondays = CirculationRules.Get( branchcode, c, i, 'maxsuspensiondays' ) %] - [% SET suspension_chargeperiod = CirculationRules.Get( branchcode, c, i, 'suspension_chargeperiod' ) %] - [% SET renewalsallowed = CirculationRules.Get( branchcode, c, i, 'renewalsallowed' ) %] - [% SET renewalperiod = CirculationRules.Get( branchcode, c, i, 'renewalperiod' ) %] - [% SET norenewalbefore = CirculationRules.Get( branchcode, c, i, 'norenewalbefore' ) %] - [% SET auto_renew = CirculationRules.Get( branchcode, c, i, 'auto_renew' ) %] - [% SET no_auto_renewal_after = CirculationRules.Get( branchcode, c, i, 'no_auto_renewal_after' ) %] - [% SET no_auto_renewal_after_hard_limit = CirculationRules.Get( branchcode, c, i, 'no_auto_renewal_after_hard_limit' ) %] - [% SET reservesallowed = CirculationRules.Get( branchcode, c, i, 'reservesallowed' ) %] - [% SET holds_per_day = CirculationRules.Get( branchcode, c, i, 'holds_per_day' ) %] - [% SET holds_per_record = CirculationRules.Get( branchcode, c, i, 'holds_per_record' ) %] - [% SET onshelfholds = CirculationRules.Get( branchcode, c, i, 'onshelfholds' ) %] - [% SET opacitemholds = CirculationRules.Get( branchcode, c, i, 'opacitemholds' ) %] - [% SET article_requests = CirculationRules.Get( branchcode, c, i, 'article_requests' ) %] - [% SET rentaldiscount = CirculationRules.Get( branchcode, c, i, 'rentaldiscount' ) %] + [% SET note = CirculationRules.Search( branchcode, c, i, 'note' ) %] + [% SET maxissueqty = CirculationRules.Search( branchcode, c, i, 'maxissueqty' ) %] + [% SET maxonsiteissueqty = CirculationRules.Search( branchcode, c, i, 'maxonsiteissueqty' ) %] + [% SET issuelength = CirculationRules.Search( branchcode, c, i, 'issuelength' ) %] + [% SET lengthunit = CirculationRules.Search( branchcode, c, i, 'lengthunit' ) %] + [% SET hardduedate = CirculationRules.Search( branchcode, c, i, 'hardduedate' ) %] + [% SET hardduedatecompare = CirculationRules.Search( branchcode, c, i, 'hardduedatecompare' ) %] + [% SET fine = CirculationRules.Search( branchcode, c, i, 'fine' ) %] + [% SET chargeperiod = CirculationRules.Search( branchcode, c, i, 'chargeperiod' ) %] + [% SET chargeperiod_charge_at = CirculationRules.Search( branchcode, c, i, 'chargeperiod_charge_at' ) %] + [% SET firstremind = CirculationRules.Search( branchcode, c, i, 'firstremind' ) %] + [% SET overduefinescap = CirculationRules.Search( branchcode, c, i, 'overduefinescap' ) %] + [% SET cap_fine_to_replacement_price = CirculationRules.Search( branchcode, c, i, 'cap_fine_to_replacement_price' ) %] + [% SET finedays = CirculationRules.Search( branchcode, c, i, 'finedays' ) %] + [% SET maxsuspensiondays = CirculationRules.Search( branchcode, c, i, 'maxsuspensiondays' ) %] + [% SET suspension_chargeperiod = CirculationRules.Search( branchcode, c, i, 'suspension_chargeperiod' ) %] + [% SET renewalsallowed = CirculationRules.Search( branchcode, c, i, 'renewalsallowed' ) %] + [% SET renewalperiod = CirculationRules.Search( branchcode, c, i, 'renewalperiod' ) %] + [% SET norenewalbefore = CirculationRules.Search( branchcode, c, i, 'norenewalbefore' ) %] + [% SET auto_renew = CirculationRules.Search( branchcode, c, i, 'auto_renew' ) %] + [% SET no_auto_renewal_after = CirculationRules.Search( branchcode, c, i, 'no_auto_renewal_after' ) %] + [% SET no_auto_renewal_after_hard_limit = CirculationRules.Search( branchcode, c, i, 'no_auto_renewal_after_hard_limit' ) %] + [% SET reservesallowed = CirculationRules.Search( branchcode, c, i, 'reservesallowed' ) %] + [% SET holds_per_day = CirculationRules.Search( branchcode, c, i, 'holds_per_day' ) %] + [% SET holds_per_record = CirculationRules.Search( branchcode, c, i, 'holds_per_record' ) %] + [% SET onshelfholds = CirculationRules.Search( branchcode, c, i, 'onshelfholds' ) %] + [% SET opacitemholds = CirculationRules.Search( branchcode, c, i, 'opacitemholds' ) %] + [% SET article_requests = CirculationRules.Search( branchcode, c, i, 'article_requests' ) %] + [% SET rentaldiscount = CirculationRules.Search( branchcode, 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 || 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 %] [% IF show_rule %] [% SET row_count = row_count + 1 %] @@ -180,8 +177,8 @@ Delete - [% IF rule.note %] - View note + [% IF note.defined %] + View note [% ELSE %] [% END %] diff --git a/t/db_dependent/Circulation/CalcFine.t b/t/db_dependent/Circulation/CalcFine.t index fe1bd79744..f09a0e011d 100644 --- a/t/db_dependent/Circulation/CalcFine.t +++ b/t/db_dependent/Circulation/CalcFine.t @@ -165,13 +165,12 @@ subtest 'Test cap_fine_to_replacement_pricew with overduefinescap' => sub { plan tests => 2; t::lib::Mocks::mock_preference('useDefaultReplacementCost', '1'); - my $issuingrule = $builder->build_object( + Koha::CirculationRules->set_rules( { - class => 'Koha::IssuingRules', - value => { - branchcode => '*', - categorycode => '*', - itemtype => '*', + branchcode => undef, + categorycode => undef, + itemtype => undef, + rules => { fine => '1.00', lengthunit => 'days', finedays => 0, @@ -198,7 +197,7 @@ subtest 'Test cap_fine_to_replacement_pricew with overduefinescap' => sub { my ($amount) = CalcFine( $item, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt ); is( int($amount), 3, 'Got the lesser of overduefinescap and replacement price where overduefinescap < replacement price' ); - $issuingrule->overduefinescap(6)->store(); + Koha::CirculationRules->set_rule({ rule_name => 'overduefinescap', rule_value => 6, branchcode => undef, categorycode => undef, itemtype => undef }); ($amount) = CalcFine( $item, $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/TooMany.t b/t/db_dependent/Circulation/TooMany.t index 8a7cf3ae52..31ec01563b 100644 --- a/t/db_dependent/Circulation/TooMany.t +++ b/t/db_dependent/Circulation/TooMany.t @@ -500,18 +500,19 @@ subtest 'General vs specific rules limit quantity correctly' => sub { }); # Set up an issuing rule - my $rule = $builder->build({ - source => 'Issuingrule', - value => { + Koha::CirculationRules->set_rules( + { categorycode => '*', itemtype => $itemtype->{itemtype}, branchcode => '*', - issuelength => 1, - firstremind => 1, # 1 day of grace - finedays => 2, # 2 days of fine per day of overdue - lengthunit => 'days', + rules => { + issuelength => 1, + firstremind => 1, # 1 day of grace + finedays => 2, # 2 days of fine per day of overdue + lengthunit => 'days', + } } - }); + ); # Set an All->All for an itemtype Koha::CirculationRules->set_rules( diff --git a/t/db_dependent/Circulation/issue.t b/t/db_dependent/Circulation/issue.t index a0bfd2e9e6..3280071a43 100644 --- a/t/db_dependent/Circulation/issue.t +++ b/t/db_dependent/Circulation/issue.t @@ -87,16 +87,15 @@ my $categorycode = $builder->build({ })->{categorycode}; # A default issuingrule should always be present -my $issuingrule = $builder->build( +Koha::CirculationRules->set_rules( { - source => 'Issuingrule', - value => { - itemtype => '*', - categorycode => '*', - branchcode => '*', - lengthunit => 'days', - issuelength => 0, - renewalperiod => 0, + itemtype => '*', + categorycode => '*', + branchcode => '*', + rules => { + lengthunit => 'days', + issuelength => 0, + renewalperiod => 0, renewalsallowed => 0 } } diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 0227fef3db..aee25a8759 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -460,7 +460,7 @@ subtest 'Test max_holds per library/patron category' => sub { $dbh->do('DELETE FROM reserves'); $dbh->do('DELETE FROM circulation_rules'); - $biblio = $builder->build_sample_biblio({ itemtype => 'TEST' }); + $biblio = $builder->build_sample_biblio; ( $item_bibnum, $item_bibitemnum, $itemnumber ) = AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 }, $biblio->biblionumber ); @@ -468,7 +468,7 @@ subtest 'Test max_holds per library/patron category' => sub { { categorycode => undef, branchcode => undef, - itemtype => $testitemtype, + itemtype => $biblio->itemtype, rules => { reservesallowed => 99, holds_per_record => 99, @@ -535,11 +535,17 @@ subtest 'Pickup location availability tests' => sub { my ( $item_bibnum, $item_bibitemnum, $itemnumber ) = AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 }, $biblio->biblionumber ); #Add a default rule to allow some holds - $dbh->do( - q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record) - VALUES (?, ?, ?, ?, ?)}, - {}, - '*', '*', '*', 25, 99 + + Koha::CirculationRules->set_rules( + { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rules => { + reservesallowed => 25, + holds_per_record => 99, + } + } ); my $item = Koha::Items->find($itemnumber); my $branch_to = $builder->build({ source => 'Branch' })->{ branchcode }; @@ -641,7 +647,16 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { ); # Raise reservesallowed to avoid tooManyReserves from it - $issuingrule->set( { reservesallowed => 3 } )->store; + Koha::CirculationRules->set_rule( + { + + categorycode => '*', + branchcode => '*', + itemtype => $itemtype->itemtype, + rule_name => 'reservesallowed', + rule_value => 3, + } + ); is_deeply( CanItemBeReserved( $patron->borrowernumber, $itemnumber_2 ), @@ -669,7 +684,17 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { ); # Set holds_per_day to 0 - $issuingrule->set( { holds_per_day => 0 } )->store; + Koha::CirculationRules->set_rule( + { + + categorycode => '*', + branchcode => '*', + itemtype => $itemtype->itemtype, + rule_name => 'holds_per_day', + rule_value => 0, + } + ); + # Delete existing holds Koha::Holds->search->delete; @@ -679,7 +704,17 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub { 'Patron cannot reserve if holds_per_day is 0 (i.e. 0 is 0)' ); - $issuingrule->set( { holds_per_day => undef } )->store; + Koha::CirculationRules->set_rule( + { + + categorycode => '*', + branchcode => '*', + itemtype => $itemtype->itemtype, + rule_name => 'holds_per_day', + rule_value => undef, + } + ); + Koha::Holds->search->delete; is_deeply( CanItemBeReserved( $patron->borrowernumber, $itemnumber_2 ), diff --git a/t/db_dependent/ILSDI_Services.t b/t/db_dependent/ILSDI_Services.t index 430d900c77..d7fe08f65f 100644 --- a/t/db_dependent/ILSDI_Services.t +++ b/t/db_dependent/ILSDI_Services.t @@ -379,20 +379,21 @@ subtest 'Holds test' => sub { source => 'Item', value => { biblionumber => $biblio2->{biblionumber}, - damaged => 0 + damaged => 0, + itype => $builder->build_object({ class => 'Koha::ItemTypes' })->itemtype, } }); t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' ); - my $issuingrule = $builder->build({ - source => 'Issuingrule', - value => { + Koha::CirculationRules->set_rule( + { categorycode => $patron->{categorycode}, - itemtype => $item2->{itype}, - branchcode => $patron->{branchcode}, - reservesallowed => 0, + itemtype => $item2->{itype}, + branchcode => $patron->{branchcode}, + rule_name => 'reservesallowed', + rule_value => 0, } - }); + ); $query = new CGI; $query->param( 'patron_id', $patron->{borrowernumber}); @@ -419,6 +420,7 @@ subtest 'Holds test' => sub { value => { biblionumber => $biblio3->{biblionumber}, damaged => 0, + itype => $builder->build_object({ class => 'Koha::ItemTypes' })->itemtype, } }); @@ -427,18 +429,19 @@ subtest 'Holds test' => sub { value => { biblionumber => $biblio3->{biblionumber}, damaged => 1, + itype => $builder->build_object({ class => 'Koha::ItemTypes' })->itemtype, } }); - my $issuingrule2 = $builder->build({ - source => 'Issuingrule', - value => { + Koha::CirculationRules->set_rule( + { categorycode => $patron->{categorycode}, - itemtype => $item3->{itype}, - branchcode => $patron->{branchcode}, - reservesallowed => 10, + itemtype => $item3->{itype}, + branchcode => $patron->{branchcode}, + rule_name => 'reservesallowed', + rule_value => 10, } - }); + ); $query = new CGI; $query->param( 'patron_id', $patron->{borrowernumber}); @@ -499,19 +502,19 @@ subtest 'Holds test for branch transfer limits' => sub { biblionumber => $biblio->{biblionumber}, damaged => 0, itemlost => 0, + itype => $builder->build_object({ class => 'Koha::ItemTypes' })->itemtype, } }); - Koha::IssuingRules->search()->delete(); - my $issuingrule = $builder->build({ - source => 'Issuingrule', - value => { - categorycode => '*', - itemtype => '*', - branchcode => '*', - reservesallowed => 99, + Koha::CirculationRules->set_rule( + { + categorycode => undef, + itemtype => undef, + branchcode => undef, + rule_name => 'reservesallowed', + rule_value => 99, } - }); + ); my $limit = Koha::Item::Transfer::Limit->new({ toBranch => $pickup_branch->{branchcode}, diff --git a/t/db_dependent/Koha/IssuingRules.t b/t/db_dependent/Koha/IssuingRules.t index 044ed6986a..7af584d17d 100644 --- a/t/db_dependent/Koha/IssuingRules.t +++ b/t/db_dependent/Koha/IssuingRules.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 3; +use Test::More tests => 2; use Test::Deep qw( cmp_methods ); use Test::Exception; @@ -54,6 +54,7 @@ subtest 'get_effective_issuing_rule' => sub { categorycode => undef, itemtype => undef, rule_name => 'fine', + rule_value => 1, }); is($rule, undef, 'When I attempt to get effective issuing rule by' .' providing undefined values, then undef is returned.'); @@ -62,6 +63,7 @@ subtest 'get_effective_issuing_rule' => sub { categorycode => undef, itemtype => undef, rule_name => 'fine', + rule_value => 2, })->store, 'Given I added an issuing rule branchcode => undef,' .' categorycode => undef, itemtype => undef,'); $rule = Koha::CirculationRules->get_effective_rule({ @@ -69,6 +71,7 @@ subtest 'get_effective_issuing_rule' => sub { categorycode => undef, itemtype => undef, rule_name => 'fine', + rule_value => 3, }); _is_row_match( $rule, @@ -94,6 +97,7 @@ subtest 'get_effective_issuing_rule' => sub { categorycode => $categorycode, itemtype => $itemtype, rule_name => 'fine', + rule_value => 4, }); is($rule, undef, 'When I attempt to get effective issuing rule, then undef' .' is returned.'); @@ -103,12 +107,14 @@ subtest 'get_effective_issuing_rule' => sub { categorycode => undef, itemtype => undef, rule_name => 'fine', + rule_value => 5, })->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', + rule_value => 6, }); _is_row_match( $rule, @@ -126,12 +132,14 @@ subtest 'get_effective_issuing_rule' => sub { categorycode => undef, itemtype => $itemtype, rule_name => 'fine', + rule_value => 7, })->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', + rule_value => 8, }); _is_row_match( $rule, @@ -149,12 +157,14 @@ subtest 'get_effective_issuing_rule' => sub { categorycode => $categorycode, itemtype => undef, rule_name => 'fine', + rule_value => 9, })->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', + rule_value => 10, }); _is_row_match( $rule, @@ -172,12 +182,14 @@ subtest 'get_effective_issuing_rule' => sub { categorycode => $categorycode, itemtype => $itemtype, rule_name => 'fine', + rule_value => 11, })->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', + rule_value => 12, }); _is_row_match( $rule, @@ -195,12 +207,14 @@ subtest 'get_effective_issuing_rule' => sub { categorycode => undef, itemtype => undef, rule_name => 'fine', + rule_value => 13, })->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', + rule_value => 14, }); _is_row_match( $rule, @@ -218,12 +232,14 @@ subtest 'get_effective_issuing_rule' => sub { categorycode => undef, itemtype => $itemtype, rule_name => 'fine', + rule_value => 15, })->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', + rule_value => 16, }); _is_row_match( $rule, @@ -241,12 +257,14 @@ subtest 'get_effective_issuing_rule' => sub { categorycode => $categorycode, itemtype => undef, rule_name => 'fine', + rule_value => 17, })->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', + rule_value => 18, }); _is_row_match( $rule, @@ -264,12 +282,14 @@ subtest 'get_effective_issuing_rule' => sub { categorycode => $categorycode, itemtype => $itemtype, rule_name => 'fine', + rule_value => 19, })->store, "Given I added an issuing rule branchcode => $branchcode, categorycode => $categorycode, itemtype => $itemtype,"); $rule = Koha::CirculationRules->get_effective_rule({ branchcode => $branchcode, categorycode => $categorycode, itemtype => $itemtype, rule_name => 'fine', + rule_value => 20, }); _is_row_match( $rule, @@ -460,53 +480,6 @@ subtest 'set_rule' => sub { }; }; -subtest 'delete' => sub { - plan tests => 1; - - my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes' }); - my $library = $builder->build_object({ class => 'Koha::Libraries' }); - my $category = $builder->build_object({ class => 'Koha::Patron::Categories' }); - - # We make an issuing rule - my $issue_rule = $builder->build_object({ class => 'Koha::IssuingRules', value => { - categorycode => $category->categorycode, - itemtype => $itemtype->itemtype, - branchcode => $library->branchcode - } - }); - - my $count = Koha::CirculationRules->search()->count; - # Note how many circulation rules we start with - - # We make some circulation rules for the same thing - $builder->build_object({ class => 'Koha::CirculationRules', value => { - categorycode => $category->categorycode, - itemtype => $itemtype->itemtype, - branchcode => $library->branchcode, - rule_name => 'maxissueqty', - } - }); - $builder->build_object({ class => 'Koha::CirculationRules', value => { - categorycode => $category->categorycode, - itemtype => $itemtype->itemtype, - branchcode => $library->branchcode, - rule_name => 'maxonsiteissueqty', - } - }); - $builder->build_object({ class => 'Koha::CirculationRules', value => { - categorycode => $category->categorycode, - itemtype => $itemtype->itemtype, - branchcode => $library->branchcode, - rule_name => 'another_rule', # That must not be deleted - } - }); - - # Now we delete the issuing rule - $issue_rule->delete; - is( Koha::CirculationRules->search()->count ,$count + 1, "We remove related circ rules maxissueqty and maxonsiteissueqty with our issuing rule"); - -}; - sub _is_row_match { my ( $rule, $expected, $message ) = @_; diff --git a/t/db_dependent/SIP/Transaction.t b/t/db_dependent/SIP/Transaction.t index c7b30543ee..c33ba31670 100755 --- a/t/db_dependent/SIP/Transaction.t +++ b/t/db_dependent/SIP/Transaction.t @@ -16,7 +16,6 @@ use C4::SIP::ILS::Transaction::FeePayment; use C4::SIP::ILS::Transaction::Hold; use C4::Reserves; -use Koha::IssuingRules; my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; diff --git a/t/db_dependent/selenium/administration_tasks.t b/t/db_dependent/selenium/administration_tasks.t index 69145ad7fb..919a74712f 100644 --- a/t/db_dependent/selenium/administration_tasks.t +++ b/t/db_dependent/selenium/administration_tasks.t @@ -84,7 +84,7 @@ SKIP: { $elt = $driver->find_elements('//table[@id="default-circulation-rules"]/tbody/tr/td[contains(text(),"'.$itype->description.'")]/following-sibling::td/span[text() = "Unlimited"]'); is( @$elt,2,"We have unlimited checkouts"); #Clean up - Koha::IssuingRules->find({itemtype=>$itype->itemtype})->delete(); + Koha::CirculationRules->search( { itemtype => $itype->itemtype } )->delete; $itype->delete; # TODO Create more smart rules navigation here }; -- 2.11.0