Bug 34469 - Modifying an order line of a standing order will delete linked invoice ID
Summary: Modifying an order line of a standing order will delete linked invoice ID
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low blocker (vote)
Assignee: Katrin Fischer
QA Contact: Testopia
URL:
Keywords:
: 34254 (view as bug list)
Depends on:
Blocks:
 
Reported: 2023-08-03 13:30 UTC by Katrin Fischer
Modified: 2023-12-01 16:46 UTC (History)
4 users (show)

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


Attachments
Bug 34469: Block editing for received order lines (5.06 KB, patch)
2023-08-04 15:21 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34469: Block editing for received order lines (5.10 KB, patch)
2023-08-04 15:40 UTC, Sam Lau
Details | Diff | Splinter Review
Error Message order cannot be edited, the order was already received. (17.79 KB, image/png)
2023-08-04 16:12 UTC, Michaela Sieber
Details
Bug 34469: Block editing for received order lines (5.16 KB, patch)
2023-08-04 16:14 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 34469: Block editing for received order lines (5.22 KB, patch)
2023-08-11 00:57 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2023-08-03 13:30:21 UTC
When modifying a received order line from a standing order, the invoice id in the order line will be emptied out, leaving you with an order line that is complete, but not linked to an invoice (= inconsistent state, data loss)

This cannot be resolved in the GUI, since you can't 'cancel receipt' for this order line, since it's not linked to an invoice any more. The only way to fix things is with SQL. 

And there is no way to find out which invoice the order line was originally received in.

To test:
* Create basket as standing order with items added in cataloguing
* Add an order line
* Receive shipment
* Go to the basket summary page, click on 'Modify'
* Change the actual cost
* Save
* Verify that the invoice is now empty
* In the database aqorders.invoiceid will be NULL
Comment 1 Katrin Fischer 2023-08-03 13:34:00 UTC
Solution: it should not be possible to modify received order lines in any basket.
Comment 2 Laura Escamilla 2023-08-03 14:01:18 UTC
*** Bug 34254 has been marked as a duplicate of this bug. ***
Comment 3 Katrin Fischer 2023-08-04 15:21:11 UTC
Created attachment 154287 [details] [review]
Bug 34469: Block editing for received order lines

When an already received order line is modified/edited,
we experience data loss. Most prominently, we will lose the
linked invoiceid. Therefore we should not allow editing an
order line that was already received. If something needs to be
changed, the receipt should be cancelled first.

To test:
* Create basket as standing order with items added in cataloguing
* Add an order line
* Receive shipment
* Go to the basket summary page, click on 'Modify'
* Change the actual cost
* Save
* Verify that the invoice is now empty
* In the database aqorders.invoiceid will be NULL
* Apply patch
* Receive another shipment for the standing order
* Go back to basket summary page
* Verify the 'Modify' link is no longer present
* Click 'Modify' on the unreceived order line
* Edit the URL parameter odernumber to have the number of an
  already received order line
* Verify you see a nice error message:
  This order cannot be edited, the basket is closed or the order was already received.
Comment 4 Sam Lau 2023-08-04 15:40:41 UTC
Created attachment 154288 [details] [review]
Bug 34469: Block editing for received order lines

When an already received order line is modified/edited,
we experience data loss. Most prominently, we will lose the
linked invoiceid. Therefore we should not allow editing an
order line that was already received. If something needs to be
changed, the receipt should be cancelled first.

To test:
* Create basket as standing order with items added in cataloguing
* Add an order line
* Receive shipment
* Go to the basket summary page, click on 'Modify'
* Change the actual cost
* Save
* Verify that the invoice is now empty
* In the database aqorders.invoiceid will be NULL
* Apply patch
* Receive another shipment for the standing order
* Go back to basket summary page
* Verify the 'Modify' link is no longer present
* Click 'Modify' on the unreceived order line
* Edit the URL parameter odernumber to have the number of an
  already received order line
* Verify you see a nice error message:
  This order cannot be edited, the basket is closed or the order was already received.

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 5 Laura Escamilla 2023-08-04 15:41:48 UTC
Hi Katrin, 

Thank you so much for working on this patch! 

In testing, everything was working as described until these steps:
* Click 'Modify' on the unreceived order line
* Edit the URL parameter ordernumber to have the number of an
  already received order line.

When I edited the URL parameter with an order number that was already received, it let me make modifications without receiving an error message. I then confirmed that the received item line was removed from the invoice.
Comment 6 Michaela Sieber 2023-08-04 16:09:04 UTC
(In reply to Laura Escamilla from comment #5)
> 
> When I edited the URL parameter with an order number that was already
> received, it let me make modifications without receiving an error message. I
> then confirmed that the received item line was removed from the invoice.

Hi Laura,

I tried to reproduce that in a sandbox and there is an error message :
"This order cannot be edited, the basket is closed or the order was already received."

Please have a look here: 
http://staff-b34469.sandboxes.ptfs-europe.co.uk/cgi-bin/koha/acqui/neworderempty.pl?ordernumber=3&booksellerid=1&basketno=2

Acq Log is active and there is a SQL-Report for aqorders.

I also checked the use case described in bug 34254
Comment 7 Katrin Fischer 2023-08-04 16:09:32 UTC
(In reply to Laura Escamilla from comment #5)
> Hi Katrin, 
> 
> Thank you so much for working on this patch! 
> 
> In testing, everything was working as described until these steps:
> * Click 'Modify' on the unreceived order line
> * Edit the URL parameter ordernumber to have the number of an
>   already received order line.
> 
> When I edited the URL parameter with an order number that was already
> received, it let me make modifications without receiving an error message. I
> then confirmed that the received item line was removed from the invoice.

Hi Laura, that's not nice! It looks like I committed all changes and it worked for me (on the other computer) - I'll retest, but maybe you could confirm that you did a restart_all before testing?
Comment 8 Katrin Fischer 2023-08-04 16:10:47 UTC
Now I overlapped with Michaela also testing - glad to see the acq bugs getting so much attention :) Hope we can resolve!
Comment 9 Michaela Sieber 2023-08-04 16:12:59 UTC
Created attachment 154293 [details]
Error Message order cannot be edited, the order was already received.

Message: 
order cannot be edited, the basket is closed or the order was already received.
Comment 10 PTFS Europe Sandboxes 2023-08-04 16:14:49 UTC
Created attachment 154294 [details] [review]
Bug 34469: Block editing for received order lines

When an already received order line is modified/edited,
we experience data loss. Most prominently, we will lose the
linked invoiceid. Therefore we should not allow editing an
order line that was already received. If something needs to be
changed, the receipt should be cancelled first.

To test:
* Create basket as standing order with items added in cataloguing
* Add an order line
* Receive shipment
* Go to the basket summary page, click on 'Modify'
* Change the actual cost
* Save
* Verify that the invoice is now empty
* In the database aqorders.invoiceid will be NULL
* Apply patch
* Receive another shipment for the standing order
* Go back to basket summary page
* Verify the 'Modify' link is no longer present
* Click 'Modify' on the unreceived order line
* Edit the URL parameter odernumber to have the number of an
  already received order line
* Verify you see a nice error message:
  This order cannot be edited, the basket is closed or the order was already received.

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 11 Michaela Sieber 2023-08-04 16:19:23 UTC
status back to signed Off
Comment 12 Laura Escamilla 2023-08-04 17:03:11 UTC
(In reply to Michaela Sieber from comment #11)
> status back to signed Off

My apologies, Katrin! I missed that pesky restart_all step! It works as expected and I did in fact get the error message. 

Thank you again for working on this patch so quickly!
Comment 13 Katrin Fischer 2023-08-04 17:09:03 UTC
(In reply to Laura Escamilla from comment #12)
> (In reply to Michaela Sieber from comment #11)
> > status back to signed Off
> 
> My apologies, Katrin! I missed that pesky restart_all step! It works as
> expected and I did in fact get the error message. 
> 
> Thank you again for working on this patch so quickly!

Glad that resolved it :) And very happy to be able to kill an evil bug!
Comment 14 Victor Grousset/tuxayo 2023-08-11 00:57:57 UTC
Created attachment 154356 [details] [review]
Bug 34469: Block editing for received order lines

When an already received order line is modified/edited,
we experience data loss. Most prominently, we will lose the
linked invoiceid. Therefore we should not allow editing an
order line that was already received. If something needs to be
changed, the receipt should be cancelled first.

To test:
* Create basket as standing order with items added in cataloguing
* Add an order line
* Receive shipment
* Go to the basket summary page, click on 'Modify'
* Change the actual cost
* Save
* Verify that the invoice is now empty
* In the database aqorders.invoiceid will be NULL
* Apply patch
* Receive another shipment for the standing order
* Go back to basket summary page
* Verify the 'Modify' link is no longer present
* Click 'Modify' on the unreceived order line
* Edit the URL parameter odernumber to have the number of an
  already received order line
* Verify you see a nice error message:
  This order cannot be edited, the basket is closed or the order was already received.

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Comment 15 Victor Grousset/tuxayo 2023-08-11 00:58:37 UTC
Added Laura's signoff :)
Comment 16 Victor Grousset/tuxayo 2023-08-11 01:27:27 UTC
Works, makes sense, QA script happy, code looks good, passing QA :)
Comment 17 Tomás Cohen Arazi 2023-08-15 09:14:35 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 18 Fridolin Somers 2023-08-17 19:42:50 UTC
Pushed to 23.05.x for 23.05.03
Comment 19 Pedro Amorim 2023-08-18 13:51:45 UTC
Nice work everyone!

Pushed to 22.11.x for next release