Bug 12557

Summary: Cannot revert the changes done by AcqItemSetSubfieldsWhenReceived on canceling a receipt
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: AcquisitionsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: katrin.fischer, tomascohen
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:
Bug Depends on: 8307    
Bug Blocks:    
Attachments: Bug 12557: DB changes - Add the AcqItemSetSubfieldsWhenReceiptIsCancelled pref
Bug 12557: Add a way to revert the changes made on items on receving
Bug 12557: DB changes - Add the AcqItemSetSubfieldsWhenReceiptIsCancelled pref
Bug 12557: Add a way to revert the changes made on items on receving
Bug 12557: UT: The item is not created
Bug 12557: Partially received - the change should affect the new order
[PASSED QA] Bug 12557: DB changes - Add the AcqItemSetSubfieldsWhenReceiptIsCancelled pref
[PASSED QA] Bug 12557: Add a way to revert the changes made on items on receving
[PASSED QA] Bug 12557: UT: The item is not created
[PASSED QA] Bug 12557: Partially received - the change should affect the new order

Description Jonathan Druart 2014-07-11 10:05:22 UTC
Bug 8307 introduces the AcqItemSetSubfieldsWhenReceived pref.
You can now update an item field on receiving (if you create items on ordering).
But if the receipt is cancel, there is no way to revert these changes.
Comment 1 Jonathan Druart 2014-07-11 10:14:27 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2014-07-11 10:14:32 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2014-08-17 08:11:27 UTC Comment hidden (obsolete)
Comment 4 Chris Cormack 2014-08-17 08:11:44 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2014-08-18 10:39:43 UTC
I am sorry, but the tests fail for me, could someone else check if they pass for them?

perl t/db_dependent/Acquisition/CancelReceipt.t 
1..3
ok 1 - Create items on receiving: 0 item exist after cancelling a receipt
DBD::mysql::st execute failed: Column 'biblio_auth_number' cannot be null at /home/katrin/kohaclone/C4/Biblio.pm line 2886.
DBD::mysql::st execute failed: Column 'biblio_auth_number' cannot be null at /home/katrin/kohaclone/C4/Biblio.pm line 2886.
# Looks like you planned 3 tests but ran 1.
# Looks like your test exited with 255 just after 1.
Comment 6 Jonathan Druart 2014-08-20 10:16:15 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2014-08-21 07:54:14 UTC
This seems to introduce a new warning in the logs:

[Thu Aug 21 09:44:43.051320 2014] [cgi:error] [pid 18243] [client 127.0.0.1:50730] AH01215: [Thu Aug 21 09:44:43 2014] parcel.pl: Use of uninitialized value $line{"parent_ordernumber"} in numeric eq (==) at /home/katrin/kohaclone/acqui/parcel.pl line 198., referer: http://localhost:8080/cgi-bin/koha/acqui/parcel.pl?invoiceid=8&sticky_filters=1

TEST:
- Order 3 items (none are 'on order', because status was not set)
- Receive 2
- Configure pref to say for MARC21: 7=-1 (set not for loan to 'on order')
- Undo receipt
- Now all 3 items are 'on order' insted of just the 2 the receipt got reverted for
Comment 8 Jonathan Druart 2014-08-21 09:06:03 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2014-08-21 09:08:37 UTC
(In reply to Katrin Fischer from comment #7)
> This seems to introduce a new warning in the logs:
> 
> [Thu Aug 21 09:44:43.051320 2014] [cgi:error] [pid 18243] [client
> 127.0.0.1:50730] AH01215: [Thu Aug 21 09:44:43 2014] parcel.pl: Use of
> uninitialized value $line{"parent_ordernumber"} in numeric eq (==) at
> /home/katrin/kohaclone/acqui/parcel.pl line 198., referer:
> http://localhost:8080/cgi-bin/koha/acqui/parcel.
> pl?invoiceid=8&sticky_filters=1

I didn't get this warning (or did not see it). But it should not be introduced by this patch since it does not touch acqui/parcel.pl.


> TEST:
> - Order 3 items (none are 'on order', because status was not set)
> - Receive 2
> - Configure pref to say for MARC21: 7=-1 (set not for loan to 'on order')
> - Undo receipt
> - Now all 3 items are 'on order' insted of just the 2 the receipt got
> reverted for

Good catch! I stupidly forgot to test partial receipt!
Comment 10 Katrin Fischer 2014-08-21 09:24:57 UTC
Created attachment 31040 [details] [review]
[PASSED QA] Bug 12557: DB changes - Add the AcqItemSetSubfieldsWhenReceiptIsCancelled pref

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, passes tests and QA script.
Comment 11 Katrin Fischer 2014-08-21 09:25:01 UTC
Created attachment 31041 [details] [review]
[PASSED QA] Bug 12557: Add a way to revert the changes made on items on receving

Bug 8307 introduces the AcqItemSetSubfieldsWhenReceived pref.
You can now update an item field on receiving (if you create items on
ordering).
But if the receipt is cancel, there is no way to revert these changes.

This patch adds a new pref AcqItemSetSubfieldsWhenReceiptIsCancelled to
allow to revert changes previously done on receiving

Test plan:
0/ Set the AcqCreateItems to 'ordering'
1/ Fill AcqItemSetSubfieldsWhenReceived with o=1 (UNIMARC) or 7=1
(MARC21).
2/ Fill AcqItemSetSubfieldsWhenReceiptIsCancelled with o=2 (UNIMARC) or
7=2 (MARC21)
3/ Create an order with some items
4/ Receive the order and verify the notforloan value is set to 1
5/ Cancel the receipt and verify the notforloan value is set to 2

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 12 Katrin Fischer 2014-08-21 09:25:04 UTC
Created attachment 31042 [details] [review]
[PASSED QA] Bug 12557: UT: The item is not created

The UT was buggy. I have just fix that in another patch on bug 12555.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 13 Katrin Fischer 2014-08-21 09:25:07 UTC
Created attachment 31043 [details] [review]
[PASSED QA] Bug 12557: Partially received - the change should affect the new order

If the receipt in not on the whole order but only on a part of it, the
change should be done on the itemnumber linked to the "new order", the
one we are reverting.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 14 Tomás Cohen Arazi 2014-09-09 13:56:15 UTC
Patches pushed to master.

Thanks Jonathan!