There's a popup asking for confirmation, but the Yes is ignored. The change to the code is very simple. In intranet-tmpl/prog/en/includes/cat-toolbar.inc: 54 else if ( holdcount > 0 ) { 55 is_confirmed = confirm( _("%s holds(s) for this ... 56 } else { 57 is_confirmed = confirm(_("Are you sure you want ... 58 } 59 if (is_confirmed) { 60 if ( count > 0 || holdcount > 0 ){ <<== but we just confirmed!! 61 return false; <<== Nooooooo!!! 62 } else { 63 window.location="/cgi-bin/koha/cataloguing/addbiblio.pl?op=delete&biblionumber=[% biblionumber %]"; 64 } Our proposed fix is just to remove that extra if.
Created attachment 40918 [details] [review] Bug 14518 - Allow deleting notices with reservations (and no item) after confirmation Cleaning up notices with no item but reservation is impossible since the code does not consider the result from the confirmation popup. This situation happens to our client in their (annual) cleanup. I suppose those are notices for orders that never materialized, but people already made reservations on them. Test: 1) create (or find) a notice with no item. 2) make a reservation on it. 3) try to delete the notice. a) a popup will ask for confirmation b) after accepting, validate if the notice is deleted.
Hi Blou, please be careful - notices are letters in Koha, but I think you are talking about the bibliographic records? From reading the problem description I think we got a different problem here - Koha should not allow to place holds on records without items ever. So maybe we need to find out why this happens first.
Well, the library seems to use it, so in that sense, I think Koha should allow it. :) I think the usage is that they publicize future arrivals before getting them. They created the records, did their orders, told their "clients" and allowed them to reserve them. It does make sense to me as a user, but I'm no librarian.
It should not be possible - because the hold might never be filled. I think they are misusing a bug - and that could turn into a big problem for them on updating. For me it's not possible to place holds for records without items from the OPAC - I have no idea, how they are doing it.
I clarified with Francois: This happened due to a migration to Koha. So you're right, it shouldn't happen in Koha. THAT said, we can argue about handling such case or not, but Koha is clearly misbehaving on the prompt: it asks if we want to do it anyway, then it totally ignores the result.
Yep - but we need a new test plan - I think the only way you can do it is make an item... place a hold and then delete the item using SQL or something like that :)
(In reply to Katrin Fischer from comment #6) > Yep - but we need a new test plan I. Setup Create the following if needed: 1) A patron for placing holds and checkouts 2) A test bibliographic record 3) An item for the record. Set the barcode to "DELETEME" II. Corrupting the Database 1) Checkout the item "DELETEME" 2) Place a record level hold on the test record (next available item) 3) Checkin the item "DELETEME" 4) MySQL: DELETE FROM items WHERE barcode="DELETEME"; III. Deleting the record. 1) On the test record's details page: Edit -> Delete record 2) Click 'yes' in the prompt 3) Nothing happens. IV. Fixing the bug 1) Apply the patch 2) Repeat step III to delete the record 3) Record is deleted
Hi I wasn't able to reproduce this bug, I was able to delete the record just fine
(In reply to Aleisha Amohia from comment #8) > Hi I wasn't able to reproduce this bug, I was able to delete the record just > fine Hi, did you complete step II correctly? After this step. The record should have a hold placed on it, but no items. When placing the hold for the record, did you check the "Place a hold on the next available item" checkbox? The item needs to be deleted directy in the database using a mysql query ( see III.4 )
I probably messed up somewhere, I'll retest now!
I tested again and still couldn't reproduce the bug. Can someone else test this please? I must be testing wrong somewhere but I'm sure I'm following the instructions correctly
Created attachment 43079 [details] [review] [SIGNED OFF] Bug 14518 - Allow deleting recordss with reservations (and no item) after confirmation Cleaning up notices with no item but reservation is impossible since the code does not consider the result from the confirmation popup. This situation happens to our client in their (annual) cleanup. I suppose those are notices for orders that never materialized, but people already made reservations on them. Test: 1) create (or find) a notice with no item. 2) make a reservation on it. 3) try to delete the notice. a) a popup will ask for confirmation b) after accepting, validate if the notice is deleted. Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Please always make sure that the commit message reflects the changes made to the patch - in this case the updated test plan. I have amended the commit message in QA.
Created attachment 43082 [details] [review] [PASSED QA] Bug 14518 - Allow deleting records with holds (and no item) after confirmation Cleaning up records with no item but holds is impossible since the code does not consider the result from the confirmation popup. This situation happens to our client in their (annual) cleanup. I. Setup Create the following if needed: 1) A patron for placing holds and checkouts 2) A test bibliographic record 3) An item for the record. Set the barcode to "DELETEME" II. Corrupting the Database 1) Checkout the item "DELETEME" 2) Place a record level hold on the test record (next available item) 3) Checkin the item "DELETEME" 4) MySQL: DELETE FROM items WHERE barcode="DELETEME"; III. Deleting the record. 1) On the test record's details page: Edit -> Delete record 2) Click 'yes' in the prompt 3) Nothing happens. IV. Fixing the bug 1) Apply the patch 2) Repeat step III to delete the record 3) Record is deleted Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Note: It's not possible in Koha to place a hold on a record with items, so the effect described here is more likely to be the result of a migration issue or another bug. But doesn't hurt to provide a way out :)
Patch pushed to master. Thanks Remi and congrats for your commit!