http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=11549 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 25445 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25445&action=edit Bug 11744: Cancel a receipt does not delete items created on receiving If items is created on receiving, cancel a receipt should delete them. The code only manage the case if the order is a child of another order (partial). To reproduce: 1/ Set AcqCreateItem to receiving 2/ Order one or more item(s) 3/ Receive the order and verify the item is created 4/ Cancel the receipt 5/ The item is not deleted Test plan: 1/ Apply this patch and do again previous steps. The item should not be deleted at step 5. 2/ Set AcqCreateItem to ordering and verify the item is not deleted. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 --- Comment #5 from Marjorie Barry-Vila <marjorie.barry-vila@ccsr.qc.ca> --- Hi, I tested the patch on 3.12.7 and it works fine. Regards, Marjorie -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 Marjorie Barry-Vila <marjorie.barry-vila@ccsr.qc.ca> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #7 from Marjorie Barry-Vila <marjorie.barry-vila@ccsr.qc.ca> --- (In reply to Jonathan Druart from comment #6)

(In reply to Marjorie Barry-Vila from comment #5) Hi Marjorie, Could you test this patch against the master branch using a sandbox?

It's the first time i am testing a patch in a sandbox. I tested on a sandbox (http://pro.test6.biblibre.com) and it works fine too. Marjorie -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 sandboxes@biblibre.com <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #25444|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 --- Comment #9 from sandboxes@biblibre.com <sandboxes@biblibre.com> --- Created attachment 25682 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25682&action=edit Bug 11744: Add regression tests Before the patch: prove t/db_dependent/Acquisition/CancelReceipt.t returns 1 failure. After the patch, the 2 tests pass. Signed-off-by: marjorie barry-vila <marjorie.barry-vila@ccsr.qc.ca> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think I found a problem with the way the item is deleted - it seems it's missing some of the checks it shoudl do before deleting. What I did: - received 1 out of 2 items ordered - placed a hold on the record - undid the receipt - item was deleted, hold didn't show up any longer... but: - repeated the receipt - the hold showed up again as it hadn't been cancelled before, but was invisible from the GUI. Next I tried with a checkout: - checked out the created item - undid the receipt - the item was not deleted (which is good), but there was also no warning I wonder if this is more like a missing feature than a real bug, as it might need some more thought to make it completely work. My thought is, that some of the same checks for deleting an order with items (when using AcqCreateItem = on order is used) might also apply here. I can think of holds and issues for now. For a hold we could raise a warning, that it will be cancelled if there ar no other items on the record. For an issue the item should be left and maybe undoing the receipt be denied? Not sure what would happen if you undid it and then later received it again - would it create a second item? I also tested for AcqCreateItem = on order that the items would not be touched, which is good :) But something for another bug - maybe we shoudl put a note, that the changes made to the items on receive won't be undone by undoing the receipt? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- The problem I see here is that right now the feature is not there, but no damage is done. The item is not deleted and you can do it manually, the data will remain consistent. With the patch, you can delete the item, but the hold is not cancelled and invisible from the interface - you will never know that there is a lost hold on a record with no items - I think that is a data inconsistency we should avoid. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 --- Comment #14 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 26447 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26447&action=edit Bug 11744: A receipt should not be cancelled if holds exist Test plan: 1/ Set acqCreateItem pref to on receiving 2/ Receive 1 of 2 items ordered 3/ Place a hold on the record 4/ Verify you are not able to cancel the receipt. This is not the best way to do that but it is the easy one. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 --- Comment #16 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Katrin Fischer from comment #15)

Could showing an alert: 'There is at least 1 hold on this record, please delete items manually' be an option?

Did you see I added a 'title' on the "Can't cancel receipt" span? It displays "Cannot cancel receipt of this order line because at least one reservation exists on the records." on mouse over. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |kyle@bywatersolutions.com --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jacek Ablewicz from comment #1)

(In reply to Marjorie Barry-Vila from comment #0)

...

Someone can test it?

Yes, I can confirm there is a problem with CancelReceipt(). It will delete (one or more, it doesn't matter) items associated with received order only if the given order is a partial receive (and the parent order is not fully received yet).

Plus, in those rare cases when it is actually deleting the items, it's currently not doing a very good job - it is removing item records directly from mysql table, but it does NOT flag biblio record for Zebra reindexing after the deletion.

Until recently, it also used to delete the items if given order was a transfer to another basket - but it was deleting the order record itself too, which was not all that good (see bug 11549).

This patch set does not appear to update zebra on deletion. Also, I wonder if it would be best to call DelItem from the item deletion to take care of that. Using DelItem would also cause the item to be moved to deleteditems rather than being irrevocably destroyed. If these are not issues or I am missing something, please reset the status. Other than that everything looks good! -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 --- Comment #20 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Kyle M Hall from comment #18)

This patch set does not appear to update zebra on deletion. Also, I wonder if it would be best to call DelItem from the item deletion to take care of that. Using DelItem would also cause the item to be moved to deleteditems rather than being irrevocably destroyed. If these are not issues or I am missing something, please reset the status.

I am not sure this should be done on this report. It is a major one and I tried to change the code as few as possible. Note that I totally agree with your remark, we should call DelItem here. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |12555 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 --- Comment #23 from Jonathan Druart <jonathan.druart@biblibre.com> --- The UT patch is rebased (conflict with commit Bug 11699: ModReceiveOrder should take a hashref in parameter), -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #25683|0 |1 is obsolete| | Attachment #26447|0 |1 is obsolete| | Attachment #29622|0 |1 is obsolete| | --- Comment #24 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 29626 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29626&action=edit [PASSED QA] Bug 11744: Cancel a receipt does not delete items created on receiving If items is created on receiving, cancel a receipt should delete them. The code only manage the case if the order is a child of another order (partial). To reproduce: 1/ Set AcqCreateItem to receiving 2/ Order one or more item(s) 3/ Receive the order and verify the item is created 4/ Cancel the receipt 5/ The item is not deleted Test plan: 1/ Apply this patch and do again previous steps. The item should not be deleted at step 5. 2/ Set AcqCreateItem to ordering and verify the item is not deleted. Signed-off-by: marjorie barry-vila <marjorie.barry-vila@ccsr.qc.ca> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 --- Comment #26 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 29628 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29628&action=edit [PASSED QA] Bug 11744: Add regression tests Before the patch: prove t/db_dependent/Acquisition/CancelReceipt.t returns 1 failure. After the patch, the 2 tests pass. Signed-off-by: marjorie barry-vila <marjorie.barry-vila@ccsr.qc.ca> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 --- Comment #28 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Katrin Fischer from comment #27)

I lost track of this a bit, should we have a new bug report for comment 18-20?

Bug 12555 already opened and linked ;) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11744 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #30 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.