Bugzilla – Attachment 65243 Details for
Bug 18930
Move 'refund lost item fee rules' into the `circulation_rules` table
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18930 - Move lost item refund rules to circulation_rules table
Bug-18930---Move-lost-item-refund-rules-to-circula.patch (text/plain), 21.67 KB, created by
Kyle M Hall (khall)
on 2017-07-25 15:29:15 UTC
(
hide
)
Description:
Bug 18930 - Move lost item refund rules to circulation_rules table
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2017-07-25 15:29:15 UTC
Size:
21.67 KB
patch
obsolete
>From 0827ad6cde2f1c6d699b2ca222d3d4bec1a1813b Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Wed, 12 Jul 2017 10:55:08 -0400 >Subject: [PATCH] Bug 18930 - Move lost item refund rules to circulation_rules > table > >This patch will move the list item refund rules from a dedicated table to the circulation_rules table. > >Test Plan: >1) Apply this patch >2) Run updatedatabase.pl >3) Verify lost item refund rules remain unchanged >--- > C4/Circulation.pm | 1 - > Koha/RefundLostItemFeeRule.pm | 45 ---- > Koha/RefundLostItemFeeRules.pm | 30 ++- > Koha/Schema/Result/RefundLostItemFeeRule.pm | 66 ------ > admin/smart-rules.pl | 35 +-- > installer/data/mysql/atomicupdate/bug_18930.perl | 14 ++ > .../prog/en/modules/admin/smart-rules.tt | 8 +- > t/db_dependent/RefundLostItemFeeRule.t | 258 ++++++++++++++------- > 8 files changed, 235 insertions(+), 222 deletions(-) > delete mode 100644 Koha/RefundLostItemFeeRule.pm > delete mode 100644 Koha/Schema/Result/RefundLostItemFeeRule.pm > create mode 100644 installer/data/mysql/atomicupdate/bug_18930.perl > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 7ea7815..61e3685 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -51,7 +51,6 @@ use Koha::Patron::Debarments; > use Koha::Database; > use Koha::Libraries; > use Koha::Holds; >-use Koha::RefundLostItemFeeRule; > use Koha::RefundLostItemFeeRules; > use Carp; > use List::MoreUtils qw( uniq ); >diff --git a/Koha/RefundLostItemFeeRule.pm b/Koha/RefundLostItemFeeRule.pm >deleted file mode 100644 >index 56e4d93..0000000 >--- a/Koha/RefundLostItemFeeRule.pm >+++ /dev/null >@@ -1,45 +0,0 @@ >-package Koha::RefundLostItemFeeRule; >- >-# Copyright Theke Solutions 2016 >-# >-# 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, write to the Free Software Foundation, Inc., >-# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. >- >-use Modern::Perl; >- >-use Koha::Database; >-use Koha::Exceptions; >- >-use base qw(Koha::Object); >- >-=head1 NAME >- >-Koha::RefundLostItemFeeRule - Koha RefundLostItemFeeRule object class >- >-=head1 API >- >-=head2 Class Methods >- >-=cut >- >-=head3 type >- >-=cut >- >-sub _type { >- return 'RefundLostItemFeeRule'; >-} >- >-1; >diff --git a/Koha/RefundLostItemFeeRules.pm b/Koha/RefundLostItemFeeRules.pm >index 2e83c76..b08ac11 100644 >--- a/Koha/RefundLostItemFeeRules.pm >+++ b/Koha/RefundLostItemFeeRules.pm >@@ -22,9 +22,9 @@ use Modern::Perl; > use Koha::Database; > use Koha::Exceptions; > >-use Koha::RefundLostItemFeeRule; >+use Koha::CirculationRule; > >-use base qw(Koha::Objects); >+use base qw(Koha::CirculationRules); > > =head1 NAME > >@@ -41,7 +41,7 @@ Koha::RefundLostItemFeeRules - Koha RefundLostItemFeeRules object set class > =cut > > sub _type { >- return 'RefundLostItemFeeRule'; >+ return 'CirculationRule'; > } > > =head3 object_class >@@ -49,7 +49,7 @@ sub _type { > =cut > > sub object_class { >- return 'Koha::RefundLostItemFeeRule'; >+ return 'Koha::CirculationRule'; > } > > =head3 should_refund >@@ -84,10 +84,17 @@ sub _effective_branch_rule { > my $self = shift; > my $branch = shift; > >- my $specific_rule = $self->find({ branchcode => $branch }); >+ my $specific_rule = $self->search( >+ { >+ branchcode => $branch, >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ } >+ )->next(); > > return ( defined $specific_rule ) >- ? $specific_rule->refund >+ ? $specific_rule->rule_value > : $self->_default_rule; > } > >@@ -139,10 +146,17 @@ item fees on return. It defaults to 1 if no rule is defined. > sub _default_rule { > > my $self = shift; >- my $default_rule = $self->find({ branchcode => '*' }); >+ my $default_rule = $self->search( >+ { >+ branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ } >+ )->next(); > > return (defined $default_rule) >- ? $default_rule->refund >+ ? $default_rule->rule_value > : 1; > } > >diff --git a/Koha/Schema/Result/RefundLostItemFeeRule.pm b/Koha/Schema/Result/RefundLostItemFeeRule.pm >deleted file mode 100644 >index 30ae580..0000000 >--- a/Koha/Schema/Result/RefundLostItemFeeRule.pm >+++ /dev/null >@@ -1,66 +0,0 @@ >-use utf8; >-package Koha::Schema::Result::RefundLostItemFeeRule; >- >-# Created by DBIx::Class::Schema::Loader >-# DO NOT MODIFY THE FIRST PART OF THIS FILE >- >-=head1 NAME >- >-Koha::Schema::Result::RefundLostItemFeeRule >- >-=cut >- >-use strict; >-use warnings; >- >-use base 'DBIx::Class::Core'; >- >-=head1 TABLE: C<refund_lost_item_fee_rules> >- >-=cut >- >-__PACKAGE__->table("refund_lost_item_fee_rules"); >- >-=head1 ACCESSORS >- >-=head2 branchcode >- >- data_type: 'varchar' >- default_value: (empty string) >- is_nullable: 0 >- size: 10 >- >-=head2 refund >- >- data_type: 'tinyint' >- default_value: 0 >- is_nullable: 0 >- >-=cut >- >-__PACKAGE__->add_columns( >- "branchcode", >- { data_type => "varchar", default_value => "", is_nullable => 0, size => 10 }, >- "refund", >- { data_type => "tinyint", default_value => 0, is_nullable => 0 }, >-); >- >-=head1 PRIMARY KEY >- >-=over 4 >- >-=item * L</branchcode> >- >-=back >- >-=cut >- >-__PACKAGE__->set_primary_key("branchcode"); >- >- >-# Created by DBIx::Class::Schema::Loader v0.07042 @ 2016-05-31 02:45:35 >-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:K+2D3R+JxrovgvjdqA8xdw >- >- >-# You can replace this text with custom code or comments, and it will be preserved on regeneration >-1; >diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl >index 526c9fd..16e99f4 100755 >--- a/admin/smart-rules.pl >+++ b/admin/smart-rules.pl >@@ -30,7 +30,6 @@ use Koha::Database; > use Koha::IssuingRule; > use Koha::IssuingRules; > use Koha::Logger; >-use Koha::RefundLostItemFeeRule; > use Koha::RefundLostItemFeeRules; > use Koha::Libraries; > use Koha::Patron::Categories; >@@ -479,22 +478,28 @@ elsif ( $op eq 'mod-refund-lost-item-fee-rule' ) { > if ( $refund eq '*' ) { > if ( $branch ne '*' ) { > # only do something for $refund eq '*' if branch-specific >- eval { >- # Delete it so it picks the default >- Koha::RefundLostItemFeeRules->find({ >- branchcode => $branch >- })->delete; >- }; >+ Koha::CirculationRules->set_rules( >+ { >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => $branch, >+ rules => { >+ refund => undef >+ } >+ } >+ ); > } > } else { >- my $refundRule = >- Koha::RefundLostItemFeeRules->find({ >- branchcode => $branch >- }) // Koha::RefundLostItemFeeRule->new; >- $refundRule->set({ >- branchcode => $branch, >- refund => $refund >- })->store; >+ Koha::CirculationRules->set_rules( >+ { >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => $branch, >+ rules => { >+ refund => $refund >+ } >+ } >+ ); > } > } > >diff --git a/installer/data/mysql/atomicupdate/bug_18930.perl b/installer/data/mysql/atomicupdate/bug_18930.perl >new file mode 100644 >index 0000000..3426d99 >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_18930.perl >@@ -0,0 +1,14 @@ >+$DBversion = 'XXX'; # will be replaced by the RM >+if( CheckVersion( $DBversion ) ) { >+ if ( column_exists( 'refund_lost_item_fee_rules', 'refund' ) ) { >+ $dbh->do(" >+ INSERT INTO circulation_rules ( categorycode, branchcode, itemtype, rule_name, rule_value ) >+ SELECT NULL, branchcode, NULL, 'refund', refund >+ FROM refund_lost_item_fee_rules >+ "); >+ $dbh->do("DROP TABLE refund_lost_item_fee_rules"); >+ } >+ >+ SetVersion( $DBversion ); >+ print "Upgrade to $DBversion done (Bug 18930 - Move lost item refund rules to circulation_rules table)\n"; >+} >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 7552709..90f164f 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 >@@ -679,14 +679,14 @@ $(document).ready(function() { > <select name="refund"> > [#% Default branch %#] > [% IF ( current_branch == '*' ) %] >- [% IF ( refundLostItemFeeRule.refund ) %] >+ [% IF ( refundLostItemFeeRule.rule_value ) %] > <option value="1" selected="selected"> > [% ELSE %] > <option value="1"> > [% END %] > Yes > </option> >- [% IF ( not refundLostItemFeeRule.refund ) %] >+ [% IF ( not refundLostItemFeeRule.rule_value ) %] > <option value="0" selected="selected"> > [% ELSE %] > <option value="0"> >@@ -710,14 +710,14 @@ $(document).ready(function() { > <option value="1">Yes</option> > <option value="0">No</option> > [% ELSE %] >- [% IF ( refundLostItemFeeRule.refund ) %] >+ [% IF ( refundLostItemFeeRule.rule_value ) %] > <option value="1" selected="selected"> > [% ELSE %] > <option value="1"> > [% END %] > Yes > </option> >- [% IF ( not refundLostItemFeeRule.refund ) %] >+ [% IF ( not refundLostItemFeeRule.rule_value ) %] > <option value="0" selected="selected"> > [% ELSE %] > <option value="0"> >diff --git a/t/db_dependent/RefundLostItemFeeRule.t b/t/db_dependent/RefundLostItemFeeRule.t >index 2a8a759..a2a1290 100755 >--- a/t/db_dependent/RefundLostItemFeeRule.t >+++ b/t/db_dependent/RefundLostItemFeeRule.t >@@ -26,7 +26,7 @@ use Koha::Database; > > BEGIN { > use_ok('Koha::Object'); >- use_ok('Koha::RefundLostItemFeeRule'); >+ use_ok('Koha::CirculationRule'); > use_ok('Koha::RefundLostItemFeeRules'); > } > >@@ -41,32 +41,55 @@ subtest 'Koha::RefundLostItemFeeRule::delete() tests' => sub { > $schema->storage->txn_begin; > > # Clean the table >- $schema->resultset('RefundLostItemFeeRule')->search()->delete; >- >- my $generated_default_rule = $builder->build({ >- source => 'RefundLostItemFeeRule', >- value => { >- branchcode => '*' >+ $schema->resultset('CirculationRule')->search()->delete; >+ >+ my $generated_default_rule = $builder->build( >+ { >+ source => 'CirculationRule', >+ value => { >+ branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ } > } >- }); >- my $generated_other_rule = $builder->build({ >- source => 'RefundLostItemFeeRule' >- }); >- >- my $default_rule = Koha::RefundLostItemFeeRules->find({ >- branchcode => '*' }); >+ ); >+ my $generated_other_rule = $builder->build( >+ { >+ source => 'CirculationRule', >+ value => { >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ } >+ } >+ ); >+ >+ my $default_rule = Koha::CirculationRules->search( >+ { >+ branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ } >+ )->next(); > ok( defined $default_rule, 'Default rule created' ); >- ok( $default_rule->in_storage, 'Default rule actually in storage'); >- >- my $other_rule = Koha::RefundLostItemFeeRules->find({ >- branchcode => $generated_other_rule->{ branchcode } >- }); >+ 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->in_storage, 'Other rule actually in storage'); >+ ok( $other_rule->_result->in_storage, 'Other rule actually in storage'); > > # deleting the regular rule > $other_rule->delete; >- ok( !$other_rule->in_storage, 'Other rule deleted from storage' ); >+ ok( !$other_rule->_result->in_storage, 'Other rule deleted from storage' ); > > # Rollback transaction > $schema->storage->txn_rollback; >@@ -80,35 +103,59 @@ subtest 'Koha::RefundLostItemFeeRules::_default_rule() tests' => sub { > $schema->storage->txn_begin; > > # Clean the table >- $schema->resultset('RefundLostItemFeeRule')->search()->delete; >- >- my $generated_default_rule = $builder->build({ >- source => 'RefundLostItemFeeRule', >- value => { >- branchcode => '*', >- refund => 1 >+ $schema->resultset('CirculationRule')->search()->delete; >+ >+ my $generated_default_rule = $builder->build( >+ { >+ source => 'CirculationRule', >+ value => { >+ branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ rule_value => 1, >+ } > } >- }); >- my $generated_other_rule = $builder->build({ >- source => 'RefundLostItemFeeRule' >- }); >- >- my $default_rule = Koha::RefundLostItemFeeRules->find({ >- branchcode => '*' }); >+ ); >+ my $generated_other_rule = $builder->build( >+ { >+ source => 'CirculationRule', >+ value => { >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ } >+ } >+ ); >+ >+ my $default_rule = Koha::CirculationRules->search( >+ { >+ branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ } >+ )->next(); > ok( defined $default_rule, 'Default rule created' ); >- ok( $default_rule->in_storage, 'Default rule actually in storage'); >- ok( Koha::RefundLostItemFeeRules->_default_rule, 'Default rule is set to refund' ); >+ 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->refund(0); >+ $default_rule->rule_value(0); > $default_rule->store; > # Re-read from DB, to be sure >- $default_rule = Koha::RefundLostItemFeeRules->find({ >- branchcode => '*' }); >+ $default_rule = Koha::CirculationRules->search( >+ { >+ branchcode => '*', >+ 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->in_storage, 'Default rule effectively deleted from storage' ); >+ 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' ); > >@@ -124,27 +171,42 @@ subtest 'Koha::RefundLostItemFeeRules::_effective_branch_rule() tests' => sub { > $schema->storage->txn_begin; > > # Clean the table >- $schema->resultset('RefundLostItemFeeRule')->search()->delete; >- >- my $default_rule = $builder->build({ >- source => 'RefundLostItemFeeRule', >- value => { >- branchcode => '*', >- refund => 1 >+ $schema->resultset('CirculationRule')->search()->delete; >+ >+ my $default_rule = $builder->build( >+ { >+ source => 'CirculationRule', >+ value => { >+ branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ rule_value => 1, >+ } > } >- }); >- my $specific_rule_false = $builder->build({ >- source => 'RefundLostItemFeeRule', >- value => { >- refund => 0 >+ ); >+ my $specific_rule_false = $builder->build( >+ { >+ source => 'CirculationRule', >+ value => { >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ rule_value => 0, >+ } > } >- }); >- my $specific_rule_true = $builder->build({ >- source => 'RefundLostItemFeeRule', >- value => { >- refund => 1 >+ ); >+ my $specific_rule_true = $builder->build( >+ { >+ source => 'CirculationRule', >+ value => { >+ 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)'); >@@ -225,34 +287,64 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { > > t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'CheckinLibrary' ); > >- $schema->resultset('RefundLostItemFeeRule')->search()->delete; >- >- my $default_rule = $builder->build({ >- source => 'RefundLostItemFeeRule', >- value => { >- branchcode => '*', >- refund => 1 >+ $schema->resultset('CirculationRule')->search()->delete; >+ >+ my $default_rule = $builder->build( >+ { >+ source => 'CirculationRule', >+ value => { >+ branchcode => '*', >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ rule_value => 1 >+ } > } >- }); >- my $specific_rule_false = $builder->build({ >- source => 'RefundLostItemFeeRule', >- value => { >- refund => 0 >+ ); >+ my $specific_rule_false = $builder->build( >+ { >+ source => 'CirculationRule', >+ value => { >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ rule_value => 0 >+ } > } >- }); >- my $specific_rule_true = $builder->build({ >- source => 'RefundLostItemFeeRule', >- value => { >- refund => 1 >+ ); >+ my $specific_rule_true = $builder->build( >+ { >+ source => 'CirculationRule', >+ value => { >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ rule_value => 1 >+ } > } >- }); >+ ); > # Make sure we have an unused branchcode >- my $specific_rule_dummy = $builder->build({ >- source => 'RefundLostItemFeeRule' >- }); >+ my $specific_rule_dummy = $builder->build( >+ { >+ source => 'CirculationRule', >+ value => { >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ } >+ } >+ ); > my $branch_without_rule = $specific_rule_dummy->{ branchcode }; >- Koha::RefundLostItemFeeRules >- ->find({ branchcode => $branch_without_rule }) >+ Koha::CirculationRules >+ ->search( >+ { >+ branchcode => $branch_without_rule, >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund' >+ } >+ ) >+ ->next > ->delete; > > my $params = { >@@ -271,7 +363,7 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { > 1,'No rule for branch, global rule applied (true)'); > > # Change the default value just to try >- Koha::RefundLostItemFeeRules->find({ branchcode => '*' })->refund(0)->store; >+ Koha::CirculationRules->search({ branchcode => '*', 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)'); >-- >2.10.2
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=65243">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 18930
:
65243
|
71007
|
81101
|
81148
|
82937
|
82938
|
85492
|
85493
|
86169
|
86170
|
86171
|
89404
|
89405
|
89406
|
90280
|
90281
|
90282
|
90283
|
90284
|
90285
|
90541
|
90542
|
90543
|
90544
|
90545
|
90546
|
90547
|
90548
|
90549
|
91254
|
91255
|
91256
|
91257
|
91258
|
91259
|
91260