Bugzilla – Attachment 105526 Details for
Bug 25663
Koha::RefundLostItemFeeRules should be merged into Koha::CirculationRules
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses
Bug-25663-Remove-KohaRefundLostItemFeeRule-and-use.patch (text/plain), 19.83 KB, created by
Martin Renvoize (ashimema)
on 2020-06-03 13:55:15 UTC
(
hide
)
Description:
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2020-06-03 13:55:15 UTC
Size:
19.83 KB
patch
obsolete
>From b0fac2ec8c625689cb13cca8839f4c9e69b99fb3 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Wed, 3 Jun 2020 14:51:42 +0100 >Subject: [PATCH] Bug 25663: Remove Koha::RefundLostItemFeeRule and uses > >This patch replaces all calls to RefundLostItemFeeRules with >Koha::CirculationRules->get_lostreturn_policy and removes the module it >makes redundant. > >Test plan >1/ Confirm that there are no longer any uses of RefundLostItemFeeRules >in the codebase >2/ Confirm circulation tests still all pass >3/ Confirm you can still set and unset the lost return rules >4/ Signoff >--- > C4/Circulation.pm | 17 +- > admin/smart-rules.pl | 6 +- > t/db_dependent/RefundLostItemFeeRule.t | 459 ------------------------- > t/lib/TestBuilder.pm | 5 +- > 4 files changed, 11 insertions(+), 476 deletions(-) > delete mode 100755 t/db_dependent/RefundLostItemFeeRule.t > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 7a3b21b903..a0fde11397 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -52,7 +52,6 @@ use Koha::Database; > use Koha::Libraries; > use Koha::Account::Lines; > use Koha::Holds; >-use Koha::RefundLostItemFeeRules; > use Koha::Account::Lines; > use Koha::Account::Offsets; > use Koha::Config::SysPrefs; >@@ -1450,11 +1449,10 @@ sub AddIssue { > ## If item was lost, it has now been found, reverse any list item charges if necessary. > if ( $item_object->itemlost ) { > if ( >- Koha::RefundLostItemFeeRules->should_refund( >+ Koha::CirculationRules->get_lostreturn_policy( > { >- current_branch => C4::Context->userenv->{branch}, >- item_home_branch => $item_object->homebranch, >- item_holding_branch => $item_object->holdingbranch, >+ return_branch => C4::Context->userenv->{branch}, >+ item => $item_object > } > ) > ) >@@ -2034,13 +2032,12 @@ sub AddReturn { > $messages->{'WasLost'} = 1; > unless ( C4::Context->preference("BlockReturnOfLostItems") ) { > if ( >- Koha::RefundLostItemFeeRules->should_refund( >+ Koha::CirculationRules->get_lostreturn_policy( > { >- current_branch => C4::Context->userenv->{branch}, >- item_home_branch => $item->homebranch, >- item_holding_branch => $item_holding_branch >+ return_branch => C4::Context->userenv->{branch}, >+ item => $item, > } >- ) >+ ) > ) > { > _FixAccountForLostAndFound( $item->itemnumber, >diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl >index 195ca818af..d72599faae 100755 >--- a/admin/smart-rules.pl >+++ b/admin/smart-rules.pl >@@ -27,7 +27,6 @@ use C4::Debug; > use Koha::DateUtils; > use Koha::Database; > use Koha::Logger; >-use Koha::RefundLostItemFeeRules; > use Koha::Libraries; > use Koha::CirculationRules; > use Koha::Patron::Categories; >@@ -549,10 +548,11 @@ elsif ( $op eq 'mod-refund-lost-item-fee-rule' ) { > } > } > >-my $refundLostItemFeeRule = Koha::RefundLostItemFeeRules->find({ branchcode => ($branch eq '*') ? undef : $branch }); >+my $refundLostItemFeeRule = Koha::CirculationRules->find({ branchcode => ($branch eq '*') ? undef : $branch, rule_name => 'refund' }); >+my $defaultLostItemFeeRule = Koha::CirculationRules->find({ branchcode => undef, rule_name => 'refund' }); > $template->param( > refundLostItemFeeRule => $refundLostItemFeeRule, >- defaultRefundRule => Koha::RefundLostItemFeeRules->_default_rule >+ defaultRefundRule => $defaultLostItemFeeRule ? $defaultLostItemFeeRule->rule_value : 1 > ); > > my $patron_categories = Koha::Patron::Categories->search({}, { order_by => ['description'] }); >diff --git a/t/db_dependent/RefundLostItemFeeRule.t b/t/db_dependent/RefundLostItemFeeRule.t >deleted file mode 100755 >index da53f3ab1b..0000000000 >--- a/t/db_dependent/RefundLostItemFeeRule.t >+++ /dev/null >@@ -1,459 +0,0 @@ >-#!/usr/bin/perl >- >-# This file is part of Koha. >-# >-# Koha is free software; you can redistribute it and/or modify it >-# under the terms of the GNU General Public License as published by >-# the Free Software Foundation; either version 3 of the License, or >-# (at your option) any later version. >-# >-# Koha is distributed in the hope that it will be useful, but >-# WITHOUT ANY WARRANTY; without even the implied warranty of >-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >-# GNU General Public License for more details. >-# >-# You should have received a copy of the GNU General Public License >-# along with Koha; if not, see <http://www.gnu.org/licenses>. >- >-use Modern::Perl; >- >-use Test::More tests => 9; >-use t::lib::Mocks; >-use t::lib::TestBuilder; >- >-use C4::Context; >-use Koha::Database; >- >-BEGIN { >- use_ok('Koha::Object'); >- use_ok('Koha::CirculationRule'); >- use_ok('Koha::RefundLostItemFeeRules'); >-} >- >-my $schema = Koha::Database->new->schema; >-my $builder = t::lib::TestBuilder->new; >- >-subtest 'Koha::RefundLostItemFeeRule::delete() tests' => sub { >- >- plan tests => 5; >- >- # Start transaction >- $schema->storage->txn_begin; >- >- # Clean the table >- $schema->resultset('CirculationRule')->search()->delete; >- >- my $generated_default_rule = $builder->build( >- { >- source => 'CirculationRule', >- value => { >- branchcode => undef, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- } >- } >- ); >- my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; >- my $generated_other_rule = $builder->build( >- { >- source => 'CirculationRule', >- value => { >- branchcode => $branchcode, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- } >- } >- ); >- >- my $default_rule = Koha::CirculationRules->search( >- { >- branchcode => undef, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- } >- )->next(); >- ok( defined $default_rule, 'Default rule created' ); >- ok( $default_rule->_result->in_storage, 'Default rule actually in storage'); >- >- my $other_rule = Koha::CirculationRules->search( >- { >- branchcode => $generated_other_rule->{branchcode}, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- } >- )->next(); >- ok( defined $other_rule, 'Other rule created' ); >- ok( $other_rule->_result->in_storage, 'Other rule actually in storage'); >- >- # deleting the regular rule >- $other_rule->delete; >- ok( !$other_rule->_result->in_storage, 'Other rule deleted from storage' ); >- >- # Rollback transaction >- $schema->storage->txn_rollback; >-}; >- >-subtest 'Koha::RefundLostItemFeeRules::_default_rule() tests' => sub { >- >- plan tests => 6; >- >- # Start transaction >- $schema->storage->txn_begin; >- >- # Clean the table >- $schema->resultset('CirculationRule')->search()->delete; >- >- my $generated_default_rule = $builder->build( >- { >- source => 'CirculationRule', >- value => { >- branchcode => undef, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- rule_value => 1, >- } >- } >- ); >- my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; >- my $generated_other_rule = $builder->build( >- { >- source => 'CirculationRule', >- value => { >- branchcode => $branchcode, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- } >- } >- ); >- >- my $default_rule = Koha::CirculationRules->search( >- { >- branchcode => undef, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- } >- )->next(); >- ok( defined $default_rule, 'Default rule created' ); >- ok( $default_rule->_result->in_storage, 'Default rule actually in storage'); >- is( Koha::RefundLostItemFeeRules->_default_rule, 1, 'Default rule is set to refund' ); >- >- # Change default rule to "Don't refund" >- $default_rule->rule_value(0); >- $default_rule->store; >- # Re-read from DB, to be sure >- $default_rule = Koha::CirculationRules->search( >- { >- branchcode => undef, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- } >- )->next(); >- ok( !Koha::RefundLostItemFeeRules->_default_rule, 'Default rule is set to not refund' ); >- >- $default_rule->delete; >- ok( !$default_rule->_result->in_storage, 'Default rule effectively deleted from storage' ); >- >- ok( Koha::RefundLostItemFeeRules->_default_rule, 'Default rule is set to refund if no default rule is present' ); >- >- # Rollback transaction >- $schema->storage->txn_rollback; >-}; >- >-subtest 'Koha::RefundLostItemFeeRules::_effective_branch_rule() tests' => sub { >- >- plan tests => 3; >- >- # Start transaction >- $schema->storage->txn_begin; >- >- # Clean the table >- $schema->resultset('CirculationRule')->search()->delete; >- >- my $default_rule = $builder->build( >- { >- source => 'CirculationRule', >- value => { >- branchcode => undef, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- rule_value => 1, >- } >- } >- ); >- my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; >- my $specific_rule_false = $builder->build( >- { >- source => 'CirculationRule', >- value => { >- branchcode => $branchcode, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- rule_value => 0, >- } >- } >- ); >- my $branchcode2 = $builder->build( { source => 'Branch' } )->{branchcode}; >- my $specific_rule_true = $builder->build( >- { >- source => 'CirculationRule', >- value => { >- branchcode => $branchcode2, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- rule_value => 1, >- } >- } >- ); >- >- is( Koha::RefundLostItemFeeRules->_effective_branch_rule( $specific_rule_true->{ branchcode } ), >- 1,'Specific rule is applied (true)'); >- is( Koha::RefundLostItemFeeRules->_effective_branch_rule( $specific_rule_false->{ branchcode } ), >- 0,'Specific rule is applied (false)'); >- # Delete specific rules >- Koha::RefundLostItemFeeRules->find({ branchcode => $specific_rule_false->{ branchcode } })->delete; >- is( Koha::RefundLostItemFeeRules->_effective_branch_rule( $specific_rule_false->{ branchcode } ), >- 1,'No specific rule defined, fallback to global (true)'); >- >- # Rollback transaction >- $schema->storage->txn_rollback; >-}; >- >-subtest 'Koha::RefundLostItemFeeRules::_choose_branch() tests' => sub { >- >- plan tests => 9; >- >- # Start transaction >- $schema->storage->txn_begin; >- >- my $params = { >- current_branch => 'current_branch_code', >- item_holding_branch => 'item_holding_branch_code', >- item_home_branch => 'item_home_branch_code' >- }; >- >- t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'CheckinLibrary' ); >- >- is( Koha::RefundLostItemFeeRules->_choose_branch( $params ), >- 'current_branch_code', 'CheckinLibrary is honoured'); >- >- t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'ItemHomeBranch' ); >- is( Koha::RefundLostItemFeeRules->_choose_branch( $params ), >- 'item_home_branch_code', 'ItemHomeBranch is honoured'); >- >- t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'ItemHoldingBranch' ); >- is( Koha::RefundLostItemFeeRules->_choose_branch( $params ), >- 'item_holding_branch_code', 'ItemHoldingBranch is honoured'); >- >- t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'CheckinLibrary' ); >- eval { >- Koha::RefundLostItemFeeRules->_choose_branch(); >- }; >- is( ref($@), 'Koha::Exceptions::MissingParameter', >- 'Missing parameter exception' ); >- is( $@->message, 'CheckinLibrary requires the current_branch param', >- 'Exception message is correct' ); >- >- t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'ItemHomeBranch' ); >- eval { >- Koha::RefundLostItemFeeRules->_choose_branch(); >- }; >- is( ref($@), 'Koha::Exceptions::MissingParameter', >- 'Missing parameter exception' ); >- is( $@->message, 'ItemHomeBranch requires the item_home_branch param', >- 'Exception message is correct' ); >- >- t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'ItemHoldingBranch' ); >- eval { >- Koha::RefundLostItemFeeRules->_choose_branch(); >- }; >- is( ref($@), 'Koha::Exceptions::MissingParameter', >- 'Missing parameter exception' ); >- is( $@->message, 'ItemHoldingBranch requires the item_holding_branch param', >- 'Exception message is correct' ); >- >- # Rollback transaction >- $schema->storage->txn_rollback; >-}; >- >-subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { >- >- plan tests => 3; >- >- # Start transaction >- $schema->storage->txn_begin; >- >- t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'CheckinLibrary' ); >- >- $schema->resultset('CirculationRule')->search()->delete; >- >- my $default_rule = $builder->build( >- { >- source => 'CirculationRule', >- value => { >- branchcode => undef, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- rule_value => 1 >- } >- } >- ); >- my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; >- my $specific_rule_false = $builder->build( >- { >- source => 'CirculationRule', >- value => { >- branchcode => $branchcode, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- rule_value => 0 >- } >- } >- ); >- my $branchcode2 = $builder->build( { source => 'Branch' } )->{branchcode}; >- my $specific_rule_true = $builder->build( >- { >- source => 'CirculationRule', >- value => { >- branchcode => $branchcode2, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- rule_value => 1 >- } >- } >- ); >- # Make sure we have an unused branchcode >- my $branchcode3 = $builder->build( { source => 'Branch' } )->{branchcode}; >- my $specific_rule_dummy = $builder->build( >- { >- source => 'CirculationRule', >- value => { >- branchcode => $branchcode3, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- } >- } >- ); >- my $branch_without_rule = $specific_rule_dummy->{ branchcode }; >- Koha::CirculationRules >- ->search( >- { >- branchcode => $branch_without_rule, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund' >- } >- ) >- ->next >- ->delete; >- >- my $params = { >- current_branch => $specific_rule_true->{ branchcode }, >- # patron_branch => $specific_rule_false->{ branchcode }, >- item_holding_branch => $branch_without_rule, >- item_home_branch => $branch_without_rule >- }; >- >- t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'CheckinLibrary' ); >- is( Koha::RefundLostItemFeeRules->should_refund( $params ), >- 1,'Specific rule is applied (true)'); >- >- t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'ItemHomeBranch' ); >- is( Koha::RefundLostItemFeeRules->should_refund( $params ), >- 1,'No rule for branch, global rule applied (true)'); >- >- # Change the default value just to try >- Koha::CirculationRules->search({ branchcode => undef, rule_name => 'refund' })->next->rule_value(0)->store; >- t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'ItemHoldingBranch' ); >- is( Koha::RefundLostItemFeeRules->should_refund( $params ), >- 0,'No rule for branch, global rule applied (false)'); >- >- # Rollback transaction >- $schema->storage->txn_rollback; >-}; >- >-subtest 'Koha::RefundLostItemFeeRules::find() tests' => sub { >- >- plan tests => 5; >- >- # Start transaction >- $schema->storage->txn_begin; >- >- t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'CheckinLibrary' ); >- >- $schema->resultset('CirculationRule')->search()->delete; >- >- my $default_non_refund = $builder->build( >- { >- source => 'CirculationRule', >- value => { >- branchcode => undef, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'non_refund_rule', >- rule_value => 1 >- } >- } >- ); >- >- ok(defined Koha::RefundLostItemFeeRules->find($default_non_refund->{id}), 'Find should continue to work when passed an id'); >- >- my $specific_non_refund = $builder->build( >- { >- source => 'CirculationRule', >- value => { >- categorycode => undef, >- itemtype => undef, >- rule_name => 'non_refund_rule', >- rule_value => 0 >- } >- } >- ); >- >- ok(!defined Koha::RefundLostItemFeeRules->find({ branchcode => undef }), 'Non refund default rules are not found'); >- ok(!defined Koha::RefundLostItemFeeRules->find({ branchcode => $specific_non_refund->{branchcode} }), 'Non refund specific rules are not found'); >- >- my $default_refund = $builder->build( >- { >- source => 'CirculationRule', >- value => { >- branchcode => undef, >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- rule_value => 1 >- } >- } >- ); >- my $specific_refund = $builder->build( >- { >- source => 'CirculationRule', >- value => { >- categorycode => undef, >- itemtype => undef, >- rule_name => 'refund', >- rule_value => 0 >- } >- } >- ); >- >- ok(defined Koha::RefundLostItemFeeRules->find({ branchcode => undef }), 'Refund default rules are found'); >- ok(defined Koha::RefundLostItemFeeRules->find({ branchcode => $specific_refund->{branchcode} }), 'Refund specific rules are found'); >- >- # Rollback transaction >- $schema->storage->txn_rollback; >-}; >diff --git a/t/lib/TestBuilder.pm b/t/lib/TestBuilder.pm >index 7c9d4e8d1e..5c22c213c2 100644 >--- a/t/lib/TestBuilder.pm >+++ b/t/lib/TestBuilder.pm >@@ -576,10 +576,7 @@ sub _gen_default_values { > }, > AuthHeader => { > marcxml => '', >- }, >- RefundLostItemFeeRules => { >- rule_name => 'refund', >- }, >+ } > }; > } > >-- >2.20.1
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=105526">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 25663
:
105524
|
105525
|
105526
|
105527
|
106105
|
106106
|
106107
|
106108
|
106823
|
106824
|
106825
|
107332
|
107333
|
107334
|
108069
|
108070
|
108071
|
111309