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

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 59626b4..a622049 100644 (file)
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -54,7 +54,6 @@ use Koha::Database;
 use Koha::Libraries;
 use Koha::Account::Lines;
 use Koha::Holds;
-use Koha::RefundLostItemFeeRule;
 use Koha::RefundLostItemFeeRules;
 use Koha::Account::Lines;
 use Koha::Account::Offsets;

diff --git a/Koha/RefundLostItemFeeRule.pm b/Koha/RefundLostItemFeeRule.pm
deleted file mode 100644 (file)
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 (file)
--- 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 (file)
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 2bdebc9..1b03aed 100755 (executable)
--- a/admin/smart-rules.pl
+++ b/admin/smart-rules.pl
@@ -29,7 +29,6 @@ use Koha::Database;
 use Koha::IssuingRule;
 use Koha::IssuingRules;
 use Koha::Logger;
-use Koha::RefundLostItemFeeRule;
 use Koha::RefundLostItemFeeRules;
 use Koha::Libraries;
 use Koha::CirculationRules;
@@ -515,22 +514,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 (file)
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 17e77ba..17ee8ce 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
@@ -633,14 +633,14 @@
                         <select name="refund">
                           [#% Default branch %#]
                           [% IF ( current_branch == '*' ) %]
-                            [% IF ( ( not refundLostItemFeeRule.refund.defined ) || refundLostItemFeeRule.refund ) %]
+                            [% IF ( refundLostItemFeeRule.rule_value ) %]
                             <option value="1" selected="selected">
                             [% ELSE %]
                             <option value="1">
                             [% END %]
                                 Yes
                             </option>
-                            [% IF ( refundLostItemFeeRule.refund.defined && ( not refundLostItemFeeRule.refund ) ) %]
+                            [% IF ( not refundLostItemFeeRule.rule_value ) %]
                             <option value="0" selected="selected">
                             [% ELSE %]
                             <option value="0">
@@ -664,14 +664,14 @@
                                 <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 84a373a..3c7b235 100755 (executable)
--- 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)');