Bug 32630 - Don't delete ILL requests when patron is deleted
Summary: Don't delete ILL requests when patron is deleted
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Bugs List
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-01-13 14:38 UTC by Katrin Fischer
Modified: 2024-03-19 16:36 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2023-01-13 14:38:00 UTC
There is a FK constraint, that makes it so that when a patron is deleted, the ILL requests linked to the account are deleted as well.

This can potentially cause a number of problems:
* ILL requests deleted could be pending, in shipment etc. You will have no audit track if there are any problems with transport etc.
* You won't be able to do reliable statistics on number of ILL requests etc. especially since adding a request is not even logged.


I propose that instead of removing the borrowernumber, we either
* leave it
* set it to NULL

Would love to get some input on this one!
Comment 1 Katrin Fischer 2023-01-13 14:38:23 UTC
illrequests:

[...]
  `orderid` varchar(50) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `backend` varchar(20) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  PRIMARY KEY (`illrequest_id`),
  UNIQUE KEY `illrequest_id` (`illrequest_id`),
  KEY `illrequests_bnfk` (`borrowernumber`),
  KEY `illrequests_bcfk_2` (`branchcode`),
  KEY `illrequests_safk` (`status_alias`),
  CONSTRAINT `illrequests_bcfk_2` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE,

  CONSTRAINT `illrequests_bnfk` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE,

  CONSTRAINT `illrequests_safk` FOREIGN KEY (`status_alias`) REFERENCES `authorised_values` (`authorised_value`) ON DELETE SET NULL ON UPDATE CASCADE
) ENGINE=InnoDB AUTO_INCREMENT=71 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
Comment 2 Martin Renvoize 2023-06-06 07:01:33 UTC
I think 'SET NULL' is the right thing to do here.. if you want the audit trail for what deleted borrower we should follow the pattern and add a deleted_borrower_id field to the table and trigger a move at patron delete.