Bug 36345

Summary:	Deleting related things causes data loss for old_reserves
Product:	Koha	Reporter:	Kyle M Hall (khall) <kyle>
Component:	Architecture, internals, and plumbing	Assignee:	Tomás Cohen Arazi (tcohen) <tomascohen>
Status:	NEW ---	QA Contact:	Testopia <testopia>
Severity:	normal
Priority:	P5 - low	CC:	andrew, kebliss, kyle
Version:	Main
Hardware:	All
OS:	All
See Also:	https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36101
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Bug Depends on:	30648
Bug Blocks:

Description Kyle M Hall (khall) 2024-03-18 14:46:21 UTC

The old_reserves table is meant to be a historical preservation of data. Currently it has foreign keys a number of tables where that also have a delete/old table for preservation.

For example, we may have an "old reserve" with an associated item. When we delete that item, the item is moved to the deleteditems, and old_reserves.itemnumber is set to NULL thus losing the associated item even though we still have the item data! This is true for borrowers, bibs and items.

To enable saving this data, we should add a non FK column for each related table that has a delete/old version of that table. Then, when the associated item is deleted, copy the FK value to the new column before is is nullified. In fact, if we populated those columns when moving the hold from reserves to old_reserves, we wouldn't even have to add code to each of the delete methods!

Comment 1 Katrin Fischer 2024-03-18 21:09:00 UTC

I think this is the solution for bug 36101, but I am not sure about doubling up the data. 
We could imagine having a FK on deleteditems if we add the information on delete for example.

Comment 2 Kyle M Hall (khall) 2024-03-19 10:27:06 UTC

(In reply to Katrin Fischer from comment #1)
> I think this is the solution for bug 36101, but I am not sure about doubling
> up the data. 
> We could imagine having a FK on deleteditems if we add the information on
> delete for example.

Comment 3 Katrin Fischer 2024-03-19 16:40:46 UTC

Kyle? :)

Comment 4 Kyle M Hall (khall) 2024-03-20 10:59:28 UTC

(In reply to Katrin Fischer from comment #3)
> Kyle? :)

I was ready to write a against your suggestion when I decided you were correct and i just changed the see also (iirc) while forgetting about the reply box being populated. We'd need a hook of some kind to handle this either way. If we do it my way, it's a minimum of one hook firing. Doing it at "deletion" time, it's also a maximum of one hook firing :)

Comment 5 Katrin Fischer 2024-04-01 11:05:51 UTC

(In reply to Kyle M Hall from comment #4)
> (In reply to Katrin Fischer from comment #3)
> > Kyle? :)
> 
> I was ready to write a against your suggestion when I decided you were
> correct and i just changed the see also (iirc) while forgetting about the
> reply box being populated. We'd need a hook of some kind to handle this
> either way. If we do it my way, it's a minimum of one hook firing. Doing it
> at "deletion" time, it's also a maximum of one hook firing :)

Ah! But hope you now I would not have minded if you had disagreed - it's about to find the best way forward :)