Bug 12557 - Cannot revert the changes done by AcqItemSetSubfieldsWhenReceived on canceling a receipt
Summary: Cannot revert the changes done by AcqItemSetSubfieldsWhenReceived on cancelin...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 8307
Blocks:
  Show dependency treegraph
 
Reported: 2014-07-11 10:05 UTC by Jonathan Druart
Modified: 2015-06-04 23:33 UTC (History)
2 users (show)

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


Attachments
Bug 12557: DB changes - Add the AcqItemSetSubfieldsWhenReceiptIsCancelled pref (3.40 KB, patch)
2014-07-11 10:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12557: Add a way to revert the changes made on items on receving (4.53 KB, patch)
2014-07-11 10:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12557: DB changes - Add the AcqItemSetSubfieldsWhenReceiptIsCancelled pref (3.45 KB, patch)
2014-08-17 08:11 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 12557: Add a way to revert the changes made on items on receving (3.59 KB, patch)
2014-08-17 08:11 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 12557: UT: The item is not created (1.09 KB, patch)
2014-08-20 10:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12557: Partially received - the change should affect the new order (3.90 KB, patch)
2014-08-21 09:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 12557: DB changes - Add the AcqItemSetSubfieldsWhenReceiptIsCancelled pref (3.57 KB, patch)
2014-08-21 09:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12557: Add a way to revert the changes made on items on receving (3.66 KB, patch)
2014-08-21 09:25 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12557: UT: The item is not created (1.16 KB, patch)
2014-08-21 09:25 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12557: Partially received - the change should affect the new order (3.96 KB, patch)
2014-08-21 09:25 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!