| Summary: | borrowers.borrowernumber should be a foreign key for 8 tables | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Christophe Croullebois <christophe.croullebois> |
| Component: | Database | Assignee: | Martin Renvoize (ashimema) <martin.renvoize> |
| Status: | RESOLVED MOVED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | dcook, martin.renvoize, veron |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | 22008, 22368 | ||
| Bug Blocks: | 5334 | ||
(In reply to comment #0) > reviews.borrowernumber (with ON DELETE CASCADE) Was this not already "ON DELETE SET NULL?" My fix for Bug 7493 should have set it as such so that reviews would be kept even if the patron had been deleted. Templates were modified so that display of the patron name (or lack thereof) would be handled gracefully. Still valid? I'm upset that this still appears to apply.. writing a patch to make these all foreign keys is pretty trivial.. but I'm not sure what side effects may ensue... personally, I feel we should enforce this at the database level.. especially as we're moving to enforce strict modes at the db and opting to fix bug highlighted by such modes.. these logical relationships should be enforced too to enable us to catch existing mishandling. I've split some of these off into their own bugs as they're already covered or it's easier to test them per table. >alert.borrowernumber (with ON DELETE CASCADE) CONSTRAINT `alert_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE >statistics.borrowernumber (with ON DELETE SET NULL) I think this could be debatable. I am pretty sure libraries look-up in deletedborrowers for statistical reports regularly and this is a statistics table. I consider it WONTFIX. >messages.borrowernumber (with ON DELETE SET NULL) CONSTRAINT `messages_borrowernumber` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE, I believe the ON DELETE CASCADE is correct, you don't want to keep those messages without the patrons as they might contain sensitive information. >reserveconstraints.borrowernumber (with ON DELETE CASCADE) Table no longer exists. >reviews.borrowernumber (with ON DELETE CASCADE) CONSTRAINT `reviews_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE, I believe the ON DELETE SET NULL is what most libraries would want. >saved_sql.borrowernumber (with ON DELETE SET NULL) NOT FIXED, but already filed separately and stuck in discussion: Bug 13668 - Creator of an SQL report shoudl be NULL when the staff patron is deleted >search_history.borrowernumber (with ON DELETE CASCADE) -the borrowernumber field is called userid atm, see below- NOT FIXED Now filed separately: Bug 32590 - Add FK constraint for search_history.userid >suggestions.suggestedby, suggestion.managedby, acceptedby, rejectedby (with ON DELETE SET NULL) CONSTRAINT `suggestions_ibfk_managedby` FOREIGN KEY (`managedby`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE, CONSTRAINT `suggestions_ibfk_rejectedby` FOREIGN KEY (`rejectedby`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE, CONSTRAINT `suggestions_ibfk_suggestedby` FOREIGN KEY (`suggestedby`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE CONSTRAINT `suggestions_ibfk_acceptedby` FOREIGN KEY (`acceptedby`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE SET NULL ON UPDATE CASCADE, |
I just add constraints : alert.borrowernumber (with ON DELETE CASCADE) statistics.borrowernumber (with ON DELETE SET NULL) messages.borrowernumber (with ON DELETE SET NULL) reserveconstraints.borrowernumber (with ON DELETE CASCADE) reviews.borrowernumber (with ON DELETE CASCADE) saved_sql.borrowernumber (with ON DELETE SET NULL) search_history.borrowernumber (with ON DELETE CASCADE) -the borrowernumber field is called userid atm, see below- suggestions.suggestedby, suggestion.managedby, acceptedby, rejectedby (with ON DELETE SET NULL)