Bug 22679: Delete related CirculationRules when Removing IssuingRule

Unfortunately, the tables here can't use a foreign key as one table uses null where the other uses
'*' This patchset alters the delete method so delete related rules. It is somewhat of a workaround until
all the columns in issuingrules are moved to circulation_rules

To test:
1 - Add some issuing rules in koha, making sure to set maxissueqty
2 - Check the DB: SELECT * FROM circulation_rules;
3 - note some circulation rules were created
4 - Delete your rules via the staff interface
5 - Check the DB, the circulation rules remain
6 - Apply patch
7 - Repeat
8 - Huzzah! The rules delete!
9 - Prove the tests, read the code

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm
index 82becb1..1294eeb 100644 (file)
--- a/Koha/CirculationRules.pm
+++ b/Koha/CirculationRules.pm
@@ -162,6 +162,21 @@ sub set_rules {
     return $rule_objects;
 }
 
+=head3 delete
+
+Delete a set of circulation rules, needed for cleaning up when deleting issuingrules
+
+=cut
+
+sub delete {
+    my ( $self ) = @_;
+
+    while ( my $rule = $self->next ){
+        $rule->delete;
+    }
+
+}
+
 =head3 type
 
 =cut

diff --git a/Koha/IssuingRule.pm b/Koha/IssuingRule.pm
index 9d81db8..dd48729 100644 (file)
--- a/Koha/IssuingRule.pm
+++ b/Koha/IssuingRule.pm
@@ -31,6 +31,27 @@ Koha::Hold - Koha Hold object class
 
 =cut
 
+=head3 delete
+
+=cut
+
+sub delete {
+    my ($self) = @_;
+
+    my $branchcode = $self->branchcode eq '*' ? undef : $self->branchcode;
+    my $categorycode = $self->categorycode eq '*' ? undef : $self->categorycode;
+    my $itemtype = $self->itemtype eq '*' ? undef : $self->itemtype;
+
+    Koha::CirculationRules->search({
+        branchcode   => $branchcode,
+        itemtype     => $itemtype,
+        categorycode => $categorycode,
+    })->delete;
+
+    $self->SUPER::delete;
+
+}
+
 =head3 type
 
 =cut
@@ -39,4 +60,4 @@ sub _type {
     return 'Issuingrule';
 }
 
-1;
\ No newline at end of file
+1;

diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl
index d248f4a..927533d 100755 (executable)
--- a/admin/smart-rules.pl
+++ b/admin/smart-rules.pl
@@ -82,8 +82,12 @@ if ($op eq 'delete') {
     my $categorycode = $input->param('categorycode');
     $debug and warn "deleting $1 $2 $branch";
 
-    my $sth_Idelete = $dbh->prepare("delete from issuingrules where branchcode=? and categorycode=? and itemtype=?");
-    $sth_Idelete->execute($branch, $categorycode, $itemtype);
+    Koha::IssuingRules->find({
+        branchcode   => $branch,
+        categorycode => $categorycode,
+        itemtype     => $itemtype
+    })->delete;
+
 }
 elsif ($op eq 'delete-branch-cat') {
     my $categorycode  = $input->param('categorycode');

diff --git a/t/db_dependent/Koha/CirculationRules.t b/t/db_dependent/Koha/CirculationRules.t
index 962bf0b..2da5603 100644 (file)
--- a/t/db_dependent/Koha/CirculationRules.t
+++ b/t/db_dependent/Koha/CirculationRules.t
@@ -33,7 +33,7 @@ $schema->storage->txn_begin;
 my $builder = t::lib::TestBuilder->new;
 
 subtest 'set_rule + get_effective_rule' => sub {
-    plan tests => 11;
+    plan tests => 13;
 
     my $categorycode = $builder->build_object( { class => 'Koha::Patron::Categories' } )->categorycode;
     my $itemtype     = $builder->build_object( { class => 'Koha::ItemTypes' } )->itemtype;
@@ -232,6 +232,13 @@ subtest 'set_rule + get_effective_rule' => sub {
     is( $rule->rule_value, 8,
         'More specific rule is returned when branchcode, categorycode and itemtype exist'
     );
+
+    my $our_branch_rules = Koha::CirculationRules->search({branchcode => $branchcode});
+    is( $our_branch_rules->count, 4, "We added 8 rules");
+    $our_branch_rules->delete;
+    is( $our_branch_rules->count, 0, "We deleted 8 rules");
+
+
 };
 
 $schema->storage->txn_rollback;

diff --git a/t/db_dependent/Koha/IssuingRules.t b/t/db_dependent/Koha/IssuingRules.t
index 66e2836..8dad888 100644 (file)
--- a/t/db_dependent/Koha/IssuingRules.t
+++ b/t/db_dependent/Koha/IssuingRules.t
@@ -19,11 +19,12 @@
 
 use Modern::Perl;
 
-use Test::More tests => 3;
+use Test::More tests => 4;
 
 use Benchmark;
 
 use Koha::IssuingRules;
+use Koha::CirculationRules;
 
 use t::lib::TestBuilder;
 use t::lib::Mocks;
@@ -306,6 +307,44 @@ subtest 'get_onshelfholds_policy' => sub {
     is( Koha::IssuingRules->get_onshelfholds_policy({ item => $item, patron => $patron }), 2, 'Should be two now' );
 };
 
+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
+        }
+    });
+    $builder->build_object({ class => 'Koha::CirculationRules', value => {
+            categorycode => $category->categorycode,
+            itemtype     => $itemtype->itemtype,
+            branchcode   => $library->branchcode
+        }
+    });
+
+    # Now we delete the issuing rule
+    $issue_rule->delete;
+    is( Koha::CirculationRules->search()->count ,$count, "We remove related circ rules with our issuing rule");
+
+};
+
 sub _row_match {
     my ($rule, $branchcode, $categorycode, $itemtype) = @_;