Summary: | Adjust foreign key for parent item type | ||
---|---|---|---|
Product: | Koha | Reporter: | Marcel de Rooy <m.de.rooy> |
Component: | Database | Assignee: | Marcel de Rooy <m.de.rooy> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | ||
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
22.11.00
|
|
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 30486 | ||
Attachments: |
Bug 30490: Adjust constraint itemtypes.itemtypes_ibfk_1
Bug 30490: Adjust constraint itemtypes.itemtypes_ibfk_1 |
Description
Marcel de Rooy
2022-04-07 14:32:54 UTC
My feeling would be that if you delete a parent itemtype, it should set the field to NULL in the 'child itemtypes'. Does that make sense? IIUC quietly deleting the 'children' seems a dangerous move, although it should hopefully not be possible while items exist for them. (In reply to Katrin Fischer from comment #1) > My feeling would be that if you delete a parent itemtype, it should set the > field to NULL in the 'child itemtypes'. Does that make sense? Yes. That is the same as RESTRICT. > IIUC quietly deleting the 'children' seems a dangerous move, although it > should hopefully not be possible while items exist for them. Looking closer, the items table has no FK on itype. So you could delete itemtypes but the items will not be deleted. But note that the interface will not allow you to delete the itemtype. But you could in a script or via sql etc, ending up with items that have not existing itemtype. But that is outside the narrower scope of this report. And probably already reported somewhere else.. (In reply to Marcel de Rooy from comment #2) > (In reply to Katrin Fischer from comment #1) > > My feeling would be that if you delete a parent itemtype, it should set the > > field to NULL in the 'child itemtypes'. Does that make sense? > > Yes. That is the same as RESTRICT. Actually no :) You describe SET NULL but I opt for RESTRICT. You cannot delete the parent type if there still is a child itemtype. Created attachment 139678 [details] [review] Bug 30490: Adjust constraint itemtypes.itemtypes_ibfk_1 Change from CASCADE to restrict. In harmony with dbrev 20.06.00.022. Test plan: Run the dbrev. Bonus: update itemtypes set parent_type='VM' where itemtype='CF'; delete from itemtypes where itemtype='VM'; => ERROR 1451 (23000): Cannot delete or update a parent row: a foreign key constraint fails (`koha_myclone`.`itemtypes`, CONSTRAINT `itemtypes_ibfk_1` FOREIGN KEY (`parent_type`) REFERENCES `itemtypes` (`itemtype`)) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Self signoff RM: Dont forget DBIx update please, not included here (In reply to Marcel de Rooy from comment #3) > (In reply to Marcel de Rooy from comment #2) > > (In reply to Katrin Fischer from comment #1) > > > My feeling would be that if you delete a parent itemtype, it should set the > > > field to NULL in the 'child itemtypes'. Does that make sense? > > > > Yes. That is the same as RESTRICT. > > Actually no :) You describe SET NULL but I opt for RESTRICT. You cannot > delete the parent type if there still is a child itemtype. Hm, I not sure about this one. You might just add a parent itemtype for testing, the only purpose right now is the max checkouts, later decide to remove it... It's not a required/mandatory field, so I'd day NULL would be ok, maybe with a confirmation warning in the GUI. This way you need to remove it in all itemtypes first, more clicks, but I can live it, just think the other is nicer :) (In reply to Katrin Fischer from comment #7) > (In reply to Marcel de Rooy from comment #3) > > (In reply to Marcel de Rooy from comment #2) > > > (In reply to Katrin Fischer from comment #1) > > > > My feeling would be that if you delete a parent itemtype, it should set the > > > > field to NULL in the 'child itemtypes'. Does that make sense? > > > > > > Yes. That is the same as RESTRICT. > > > > Actually no :) You describe SET NULL but I opt for RESTRICT. You cannot > > delete the parent type if there still is a child itemtype. > > Hm, I not sure about this one. > > You might just add a parent itemtype for testing, the only purpose right now > is the max checkouts, later decide to remove it... It's not a > required/mandatory field, so I'd day NULL would be ok, maybe with a > confirmation warning in the GUI. > > This way you need to remove it in all itemtypes first, more clicks, but I > can live it, just think the other is nicer :) This would be an enhancment actually. Now I restore the sync gap between dbrev and structure. You are invited to open a new report? :) Created attachment 140072 [details] [review] Bug 30490: Adjust constraint itemtypes.itemtypes_ibfk_1 Change from CASCADE to restrict. In harmony with dbrev 20.06.00.022. Test plan: Run the dbrev. Bonus: update itemtypes set parent_type='VM' where itemtype='CF'; delete from itemtypes where itemtype='VM'; => ERROR 1451 (23000): Cannot delete or update a parent row: a foreign key constraint fails (`koha_myclone`.`itemtypes`, CONSTRAINT `itemtypes_ibfk_1` FOREIGN KEY (`parent_type`) REFERENCES `itemtypes` (`itemtype`)) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed to master for 22.11. Nice work everyone, thanks! |