From d8a759162cde8f373ea945a7e509102ec81878e0 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Tue, 23 Apr 2019 14:17:58 +0000 Subject: [PATCH] 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 Signed-off-by: Katrin Fischer Signed-off-by: Nick Clemens --- Koha/CirculationRules.pm | 15 ++++++++++ Koha/IssuingRule.pm | 23 ++++++++++++++- admin/smart-rules.pl | 8 +++-- t/db_dependent/Koha/CirculationRules.t | 9 +++++- t/db_dependent/Koha/IssuingRules.t | 41 +++++++++++++++++++++++++- 5 files changed, 91 insertions(+), 5 deletions(-) diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index 82becb1e92..1294eebde1 100644 --- 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 9d81db82ab..dd487299b1 100644 --- 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 d248f4ae48..927533d14d 100755 --- 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 962bf0b149..2da56038d7 100644 --- 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 66e2836759..8dad888c23 100644 --- 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) = @_; -- 2.39.5