- all items attached to the order are deleted - if there is no more items, then the biblio is deleted as well
Created attachment 3047 [details] [review] Patch
Works as advertised. Tested: - Deleting from order with no remaining items and remaining items - Cancelling order from shipment with no remaining items and remaining items The biblio record was only deleted, when no other items were left. The bibio record remained and only the ordered items were deleted when there were other items attached to the record.
Katrin, chris & me had a strong discussion (by mail) about the proposed behaviour. Chris says Koha should not delete the biblio silently when orders are cancelled (ie: if the librarian has modified the biblio, the deletion can be annoying), I said it can, because it delete only biblio without anymore items, so there's no need to keep the biblio (ie: the booksellers said "can't give you the book" no need to keep it in the catalogue") Can you give your opinion on this question functionnally speaking ?
In most cases you will want to delete the biblio after the order was cancelled/deleted. If you don't do that automatically a lot of biblios without items will be created, because people forget to search and delete them. But I can see there might be circumstances where you want to keep the record to order it from another vendor, order it later, keep it with a note, that it's out of print etc. I think I would like to see a prompt asking the librarian for confirmation before the record is deleted.
I would prefer: Do not delete biblio's when there are items. Warn instead. (Note that we have AcqCreateItem set to cataloging. Normally, there should not be items.) If no items, delete on confirmation. (This could be the first confirmation; in that case the confirmation for deleting an order should be clear about that.) If supplier1 does not have the book, could supplier2 have it? In that case it could be helpful not to delete right-away..
Katrin, I think we (chris & me) can agree on this: I think I would like to see a prompt asking the librarian for confirmation before the record is deleted. Marcel: I don't think the biblio is deleted if you don't have AcqCreateItem on ordering
I also believe it only deletes the items attached to the order. I had tested with a record that had different items, created from acq and cataloging. Only the acq items were deleted.
Yep, all this needs is a follow up patch to check for reserves on the biblio, or subscriptions on the biblio. And then warn the user if this is the case. The scenario of wanting to order the biblio from another supplier was not the one I was concerned about. Its more if a user has placed a hold on it, the hold with either be left dangling or deleted without the user being notified. So not just a confirmation prompt, I want it to check and make sure the biblio has nothing attached to it (another order, a serial subscription a hold etc) and then tell the librarian this.
Christophe will work on a patch for that: when the user clicks on "delete order", an AJAX will return the question that must be asked. 2 possibilities : POSSIBILITY 1 : * no more items on the biblio * no other order (usefull if AcqCreateItems is set to "recieving" or "cataloguing" !) => ask "delete order only / delete order and biblio / cancel" POSSIBILITY 2: otherwise => ask "delete order / cancel"
+1
Created attachment 4255 [details] [review] new ment I take this option after a talk with Paul, in the same column (delete) we have now two links : "Delete order" and "Delete order and biblio". If the biblio exists in other orders or if there is any items, in short if we can't delete the biblio, then, this second link does not appear.
Created attachment 4256 [details] [review] this one is ok sry mistake on the number 5680.
Created attachment 4480 [details] screenshot of basket with test cases 1) TYPO: Wording of confirmation message and link Currently: Are you sure you want to delete this order with his biblio ? I am not sure if we should use 'biblio' or better say record. Also 'his' should be 'its'. Suggestion: Are you sure you want to delete this catalog record and order? Both links have long descriptions and are close to each other - but I have no idea, how to improve that for a clearer display. 2) OK: Link to delete order and biblio is only shown, when there are no additional items on the record. Deleting the order only deletes the on order items. 3) PROBLEM: Deleting a record from the order silently deletes subscription attached to that biblio. You can loose a lot of data this way. 4) PROBLEM: One big reason to create items on orders is to enable holds on these items. If you delete a order and biblio with holds on the on order items, the hold is silently deleted. 5) OK: Deleting order, items and biblio with no holds, subscriptions and other items attached to the record. Works.
Katrin, 1) TYPO: OK, right, sry for my poor english 3) PROBLEM: right I have forforgoten this, I propose to include a test to see if subscription exists. In this case no second link. 4) PROBLEM: it's more complex, I propose to test if there are holds on items in order, and if so, to use notices for emailing to borrower the hold cancelation. I think this option needs to speak about, what is your opinion about my suggest ? Because I think even if we keep biblio we can't keep ghost items, so its better to advise the borrower as we can do when the order is receveid. Waiting suggests.
Hi Christophe, thx for commenting :) 4) Showing no link is ok for me. I am trying to imagine a good way to show the reason why the link is not available, but I have no good idea right now. If you don't show the link for existing subscriptiosn I will sign-off. 5) I like the idea for hold cancellation messages. But to sign-off on this patch I think it would be ok if you check for existing holds, give an alert and refuse to delete the order in that case. So the librarian can cancel them manually and inform the borrower if they want to.
Hi Christophe, can you update on the status?
Created attachment 5813 [details] [review] The lat one I finalize this patch taking in count katrin's comments.
So Katrin, I hope this one is ok, must be tested deeply, because multiple cases. I have taken care of the difference between an hold on catalog record and an hold on a specific item. In some cases you can't delete an order, in other cases you can't delete the order and catalog's record but the order can be deleted. On mouse over you have explanations with count. More, some lines of warnings with count are shown depending of the case. The ergonomy can be discussed, I have chosen these viewing options to have a technical structure steady. Good tests. Waiting your comments.
Created attachment 5814 [details] screenshot with new error messages Testing...
Created attachment 5816 [details] [review] Bug 5680: Order cancelling improvement : delete attached items & biblio if avalaible - all items attached to the order are deleted - if there is no more items, and if the biblio is not in other orders and no subscriptions and no holds then the biblio is proposed to deletion Now whe have 2 links : "delete order" and "delete order and catalog record", the second one appears only if the deletion is possible. Note that if an hold is related to the item or if the item is unique for the biblio the link "Delete order" is canceled due to hold remaining. On mouse over explanations are shown with count. More lines of warnings with count are shown depending of the case. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Configuration: AcqCreateItem = on order Test cases and results: 1) Order new record with 2 items a) From basket - delete order: only deletes items, OK! - delete order and catalog record: deletes record and items, OK b) From shipment/receive - delete order: only deletes items, OK! 2) Order 1 additional item for existing record with 1 item a) From basket: - delete order: works, existing item and record remain, OK - Can't delete order and catalog record, 1 item left, OK! 3) Order new record with 1 item, title level hold on record a) From basket: - delete order: not possible, OK! - delete orer and catalog record: not possible, OK! b) From shipment/receive page - Cancel: Deletes order, record and hold silently. NO WARNING. NOT OK. See note below. 4) Order 1 additional item for existing record with 1 item, item level hold on existing item a) From basket: - delete order: works, hold and existing item remain, OK! - delete order and catalog record: not possible, OK! b) From shipment/receive page - Cancel: on order item is deleted, other item and hold remain. 5) Order new serial record, create subscription a) From basket: - delete order: works, record and subscription remain, OK! - delete order and catalog record: not possible, OK! b) From shipment/receive page: - Cancel: Subscription and record are silently deleted. NOT OK. 6) Order additional item for existing record with other on order items a) From basket: - delete order: works, existing on order items remain, OK! - delete order and catalog record: not possible, OK! b) From shipment: - Cancel: deletes order and ordered item. OK. Changes made: I changed the wording of the error messages a bit in the template. I changed the message 'Can't delete order and catalog record' to not be shown as a link, as the link does nothing. Tooltip still appears. I attached a screenshot to the bug showing some of my changes. Hope that's ok. Necessary enhancements: Cancelling orders when receiving items should work the same as from the basket summary page. We need the same checks and messages there before deleting records and items automatically. I am signing off on this, but to go into Koha it needs a follow-up for the order receive page.
Thx Christophe for your work on this! I think we are almost there!
Katrin, many thx for your tests. Oups, I have forgotten the basket summary page... I'll work on a follow up. Right, it's better with your changes, visualy it's clearer. So with your help, quite to have something.
Created attachment 5904 [details] [review] Bug 5680: Follow Up for Order cancelling improvement : the same for parcel.pl It is the part concerning the parcel.pl page.
Hello Katrin, I hope this part is ok. I have created one more column in parcel.pl. Of course it was not the same script, so I hope I haven't forgotten something. Good tests. Thx
Created attachment 5914 [details] screenshot of messages on basket summary page
Created attachment 5915 [details] screenshot of messages on order receive page
Created attachment 5916 [details] [review] [SIGNED-OFF] Bug 5680: Follow Up for Order cancelling improvement : the same for parcel.pl The same scope than in basket.pl Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Detailed information about possible deletion of orders on parcel page. 1) Delete order for record with 1 other item and item level hold OK. Record, item and hold remain, only on order item is deleted. 2) Delete order for ordered record with 1 item and title level hold OK. Deletion is not possible, before hold is cancelled. 3) Delete order for new record with only items connected to this order OK. Cancelling the order deletes record and item or only order, depending on link used. 4) Delete order for record with subscription. OK. Subscription remains untouched, item is deleted. Also checked: AcqCreateItem = on receive / in cataloging Links show and it's possible to delete the order or only the record using different links. Comments: There is a table sorter option for the last column with the delete links. I am not sure it is useful here, but it does work. Before patch is applied, the links use 'cancel' instead of 'delete'. I am not sure which is better and if it should be different on basket summary and order receive or the same on both.
Created attachment 5964 [details] [review] Passed QA patch Rebased on current HEAD (minor conflict with new Analytics subroutine export)
Created attachment 5965 [details] [review] Passed QA followup patch Followup patch passed QA; minor change to template language. Was showing items remaining, but talking about holds remaining, so I made that consistent.
I've tested and reviewed these patches, and found them sound. Some minor updates to apply to current HEAD, and fix a small language issue in a tooltip. Some new C4::Items subroutines here... could be very helpful in our de-nesting efforts be removing the need to include other modules than C4::Items. Will investigate in the future. Passed QA
Pushed, thank you Katrin and Ian for the good testing, and Christophe for all your hard work writing the card.