Bug 22679 - circulation_rules are not deleted when accompanying issuingrules are deleted
Summary: circulation_rules are not deleted when accompanying issuingrules are deleted
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 18925
Blocks:
  Show dependency treegraph
 
Reported: 2019-04-10 13:49 UTC by Nick Clemens
Modified: 2019-05-24 21:42 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.05.00


Attachments
Bug 22679: Delete related CirculationRules when Removing IssuingRule (6.36 KB, patch)
2019-04-23 14:22 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22679: Delete related CirculationRules when Removing IssuingRule (6.40 KB, patch)
2019-04-23 16:09 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22679: Delete related CirculationRules when Removing IssuingRule (6.46 KB, patch)
2019-04-23 21:13 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2019-04-10 13:49:56 UTC
To recreate:
1 - Browse to Administration ->Circulation and fines
2 - Add a rule with a number for 'Total checkouts allowed'
3 - check the db:
    select * from circulation_rules
4 - The rule should be there
5 - Delete the rule from the staff interface
6 - Check the DB
7 - The rule is not deleted
Comment 1 Nick Clemens 2019-04-23 14:22:06 UTC
Created attachment 88489 [details] [review]
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
Comment 2 Tomás Cohen Arazi 2019-04-23 14:58:08 UTC
I think this is a good workaround. As we are removing issuingrules (I think Jonathan submitted patches to complete that already) I think we can live with this for a little.
Comment 3 Liz Rea 2019-04-23 16:09:34 UTC
Created attachment 88529 [details] [review]
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>
Comment 4 Katrin Fischer 2019-04-23 21:13:49 UTC
Created attachment 88567 [details] [review]
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>
Comment 5 Nick Clemens 2019-04-24 11:43:10 UTC
Awesome work all!

Pushed to master for 19.05
Comment 6 Martin Renvoize 2019-04-26 14:47:05 UTC
Bug with enhancement not in 18.11.x series.
Comment 7 Jonathan Druart 2019-05-03 21:15:16 UTC
See bug 18928 comment 37 for a follow-up on this, we should not delete all rules.
Comment 8 Jonathan Druart 2019-05-24 21:42:00 UTC
We should delete in a transaction and handle the returned value.