Bug 30451 - Delete a subscription deletes the linked order
Summary: Delete a subscription deletes the linked order
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: Main
Hardware: All All
: P5 - low blocker
Assignee: Katrin Fischer
QA Contact: Testopia
URL:
Keywords:
Depends on: 5343
Blocks:
  Show dependency tree / graph
 
Reported: 2022-04-04 15:31 UTC by Séverine Queune
Modified: 2023-12-28 20:47 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
When an order had been created using the 'from a subscription' option and the subscription was deleted, the order line would be deleted with it, independent of its status or age. This caused problems with funds and budgets. With this patch, we will unlink order line and subscription on delete, but the order line will remain.
Version(s) released in:
23.11.00,23.05.03,22.11.09
Circulation function:

Attachments
Bug 30451: Update FK constraint on aqorders.subscriptionid (3.48 KB, patch)
2023-07-21 15:11 UTC, Katrin Fischer 		Details | Diff | Splinter Review
Bug 30451: Update FK constraint on aqorders.subscriptionid (3.53 KB, patch)
2023-07-21 21:57 UTC, Biblibre Sandboxes 		Details | Diff | Splinter Review
Bug 30451: Update FK constraint on aqorders.subscriptionid (3.70 KB, patch)
2023-07-27 12:17 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Séverine Queune 2022-04-04 15:31:20 UTC 
Several issues happened when you delete a subscription linked to an order :
  - the warning message doesn't mentioned an existing order
  - the deletion of the subscription deleted the order too. That's annoying... I think the deletion should only empty the aqorder.subscriptionid datafield.
Comment 1 Katrin Fischer 2022-04-04 20:13:55 UTC 
+1

The least would be a warning, but I also think it should just 'unlink' the subscription from the order.
Comment 2 Katrin Fischer 2022-09-12 11:18:57 UTC 
I'd like to give this another push, the current behaviour means that we are losing data that is still needed in acq.
Comment 3 Katrin Fischer 2023-07-20 11:55:34 UTC 
This is a major data loss issue, upgrading severity.

The problem is, that the budgets and fund values will be wrong for the current and past years. Sums don't add up etc.

We just had a library that deleted 2 print serials that have now been bought as electronic versions. All data old data, subscriptions and acquisition data was lost when the record was deleted.
Comment 4 Katrin Fischer 2023-07-20 12:00:00 UTC 
DROP TABLE IF EXISTS `aqorders`;
/*!40101 SET @saved_cs_client     = @@character_set_client */;
/*!40101 SET character_set_client = utf8 */;
CREATE TABLE `aqorders` (
  `ordernumber` int(11) NOT NULL AUTO_INCREMENT COMMENT 'primary key and unique identifier assigned by Koha to each line',
  `biblionumber` int(11) DEFAULT NULL COMMENT 'links the order to the biblio being ordered (biblio.biblionumber)',
  `deleted_biblionumber` int(11) DEFAULT NULL COMMENT 'links the order to the deleted bibliographic record (deletedbiblio.biblionumber)',
  `entrydate` date DEFAULT NULL COMMENT 'the date the bib was added to the basket',
[...]
  CONSTRAINT `aqorders_invoice_currency` FOREIGN KEY (`invoice_currency`) REFERENCES `currency` (`currency`) ON DELETE SET NULL ON UPDATE SET NULL,
  CONSTRAINT `aqorders_subscriptionid` FOREIGN KEY (`subscriptionid`) REFERENCES `subscription` (`subscriptionid`) ON DELETE CASCADE ON UPDATE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
/*!40101 SET character_set_client = @saved_cs_client */;

The constraint needs to be updated to be: ON DELETE SET NULL
Comment 5 Michaela Sieber 2023-07-20 12:25:20 UTC 
+1
Comment 6 Katrin Fischer 2023-07-21 15:11:31 UTC 
Created attachment 153799 [details] [review]
Bug 30451: Update FK constraint on aqorders.subscriptionid

This updates the FK constrant from ON DELETE CASCADE to ON DELETE
SET NULL. This means that if a subscription linked to an order is
deleted, we no longer will also delete the order, but we will just
set subscrptinid in the order to NULL. This will avoid data loss
that can cause the budgets/funds not to add up anymore with the
real espenses of the library.

To test:

Preparation:
* Create 2 subscriptions on different records
* Create a new basket
* Use the "order from subscription" functionality to create order
  lines for both of your subscriptions
* Close basket

Without patch:
* Delete the first subscription
* Verify the order line for this subscription is gone from your basket

Apply patch:
* Run database update and restart_all
* Delete the second subscription
* Verify the order line now remained in the basket
Comment 7 Biblibre Sandboxes 2023-07-21 21:57:44 UTC 
Created attachment 153818 [details] [review]
Bug 30451: Update FK constraint on aqorders.subscriptionid

This updates the FK constrant from ON DELETE CASCADE to ON DELETE
SET NULL. This means that if a subscription linked to an order is
deleted, we no longer will also delete the order, but we will just
set subscrptinid in the order to NULL. This will avoid data loss
that can cause the budgets/funds not to add up anymore with the
real espenses of the library.

To test:

Preparation:
* Create 2 subscriptions on different records
* Create a new basket
* Use the "order from subscription" functionality to create order
  lines for both of your subscriptions
* Close basket

Without patch:
* Delete the first subscription
* Verify the order line for this subscription is gone from your basket

Apply patch:
* Run database update and restart_all
* Delete the second subscription
* Verify the order line now remained in the basket

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 8 Jonathan Druart 2023-07-27 12:17:32 UTC 
Created attachment 153966 [details] [review]
Bug 30451: Update FK constraint on aqorders.subscriptionid

This updates the FK constrant from ON DELETE CASCADE to ON DELETE
SET NULL. This means that if a subscription linked to an order is
deleted, we no longer will also delete the order, but we will just
set subscrptinid in the order to NULL. This will avoid data loss
that can cause the budgets/funds not to add up anymore with the
real espenses of the library.

To test:

Preparation:
* Create 2 subscriptions on different records
* Create a new basket
* Use the "order from subscription" functionality to create order
  lines for both of your subscriptions
* Close basket

Without patch:
* Delete the first subscription
* Verify the order line for this subscription is gone from your basket

Apply patch:
* Run database update and restart_all
* Delete the second subscription
* Verify the order line now remained in the basket

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>

JD amended patch: perl tidy

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 9 Jonathan Druart 2023-07-27 12:17:54 UTC 
I would not consider this a blocker, this is an historical behavior.
Comment 10 Katrin Fischer 2023-07-27 19:59:55 UTC 
(In reply to Jonathan Druart from comment #9)
> I would not consider this a blocker, this is an historical behavior.

Historical yes, but we defined blocker by data loss and that's what it does. And I think rather unexpectedly, just often hard to detect right away.
Comment 11 Tomás Cohen Arazi (tcohen) 2023-08-15 08:24:26 UTC 
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 12 Fridolin Somers 2023-08-17 18:50:14 UTC 
Pushed to 23.05.x for 23.05.03
Comment 13 Pedro Amorim 2023-08-18 14:21:41 UTC 
Nice work everyone!

Pushed to 22.11.x for next release
Comment 14 Fridolin Somers 2023-09-08 19:21:36 UTC 
Ah recent push to master :
  9bb23b7d5e Bug 30451: DBIC schema

Pushed to 23.05.x for 23.05.04
Comment 15 Matt Blenkinsop 2023-09-15 09:51:42 UTC 
Follow-up patch also backported to 22.11.x