Bug 7329 - The "undo import into catalog" command deletes items onloan without checking
Summary: The "undo import into catalog" command deletes items onloan without checking
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Bibliographic record staging/import (show other bugs)
Version: 3.8
Hardware: All All
: P3 critical (vote)
Target Milestone: Oldversions
Assignee: Christophe Croullebois
QA Contact: Ian Walls
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 8149
  Show dependency treegraph
 
Reported: 2011-12-07 10:47 UTC by Christophe Croullebois
Modified: 2018-12-16 19:34 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
new proposed patch (1.68 KB, patch)
2011-12-07 11:08 UTC, Christophe Croullebois
Details | Diff | Splinter Review
screenshot after step 1) (89.04 KB, image/png)
2011-12-16 07:23 UTC, Katrin Fischer
Details
Bug 7329 [Followup] : when reverting an import, keep matching biblionumber in batch table, if items are not deleted beacause on loan. (1.16 KB, patch)
2012-06-10 10:29 UTC, Sophie MEYNIEUX
Details | Diff | Splinter Review
Bug 7329 [Followup] : when reverting an import, keep matching biblionumber in batch table, if items are not deleted because they are issued. (1.38 KB, patch)
2012-06-10 15:06 UTC, Sophie MEYNIEUX
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7329: Items onloan are deleted when undoing import into catalog (1.75 KB, patch)
2012-06-11 08:58 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7329 [Followup] : when reverting an import, keep matching biblionumber in batch table, if items are not deleted beacause on loan. (1.23 KB, patch)
2012-06-11 08:58 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7329 [Followup] : when reverting an import, keep matching biblionumber in batch table, if items are not deleted because they are issued. (1.45 KB, patch)
2012-06-11 08:58 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 Christophe Croullebois 2011-12-07 10:47:43 UTC
To test, stage MARC records and import with items created.
Next, check out one or more items.
Then undo import into catalog, and you'll see that record and item(s) will be deleted.
I have made a patch that checks the "on loan" status before deleting item, and do noting in this case, so the record can't be deleted due to existing item.
Comment 1 Christophe Croullebois 2011-12-07 11:08:43 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2011-12-16 07:23:12 UTC
I noticed 2 some problems for this patch:

1) I had one item checked out and 1 available item with missing home and holding branch (not sure that was the reason). Both were not removed from the catalog.

2) After 'undoing' the import the bib# for the 2 records remaining in the catalog due to errors were gone from the list. 

3) I reimported and the bib# were still missing.

4) I undid the import again and now it reports to have removed all records. My 2 problematic records show up in the list as reverted, but they are still available in my catalog.

I think 3) and 4) are perhaps side effects of 2)?
Comment 3 Katrin Fischer 2011-12-16 07:23:37 UTC
Created attachment 6829 [details]
screenshot after step 1)
Comment 4 Sophie MEYNIEUX 2012-06-10 10:29:36 UTC Comment hidden (obsolete)
Comment 5 Sophie MEYNIEUX 2012-06-10 10:33:16 UTC
First patch was ok but uncomplete. This follow-up keep tracks in import tool itself, that some biblios remain in the catalog because corresponding items are issued and can't be deleted.
Comment 6 Sophie MEYNIEUX 2012-06-10 15:06:43 UTC Comment hidden (obsolete)
Comment 7 Sophie MEYNIEUX 2012-06-10 15:08:04 UTC
Followup corrected to take care of biblios with several items
Comment 8 Katrin Fischer 2012-06-11 08:56:30 UTC
I am ready to sign-off on the first three patches, but there is a small thing I have to note.

If you have a record with more than one item and one of the items is checked out, undoing the import will delete all items except for the checked out one. So this is perfectly correct.

However, if you repeat the import after that, the additional items do not get readded to the record. 

I think this is only a very small issue and the import works a lot nicer with the patches. so signing off.
Comment 9 Katrin Fischer 2012-06-11 08:58:12 UTC
Created attachment 10155 [details] [review]
[SIGNED-OFF] Bug 7329: Items onloan are deleted when undoing import into catalog

this patch checks the "on loan" and "reserved" status before deleting item, and
do noting in this case, so the record can't be deleted due to existing item.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 10 Katrin Fischer 2012-06-11 08:58:19 UTC
Created attachment 10156 [details] [review]
[SIGNED-OFF] Bug 7329 [Followup] : when reverting an import, keep matching biblionumber in batch table, if items are not deleted beacause on loan.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 11 Katrin Fischer 2012-06-11 08:58:26 UTC
Created attachment 10157 [details] [review]
[SIGNED-OFF] Bug 7329 [Followup] : when reverting an import, keep matching biblionumber in batch table, if items are not deleted because they are issued.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 12 Sophie MEYNIEUX 2012-06-11 09:55:08 UTC
Katrin ; the problem is that the record is not totally reverted (one item left) and not totally imported (some items have been deleted). Should we add a status for this situation ("partially reverted" or "partially imported" ) ?
Comment 13 Katrin Fischer 2012-06-11 10:51:31 UTC
I think it would be nice, but perhaps could be a different bug and enhancement :)
Comment 14 Ian Walls 2012-06-15 17:40:51 UTC
Patch 1:  uses DelItemCheck instead of DelItem, with logic to handle when an item cannot be deleted.  Good use of existing subroutine.  Passed QA.

Patch 2 + 3:  Work together to prevent the biblio from being NULLifed if the item isn't deleted.  SQL only change, with a comment, marking Passed QA.
Comment 15 Chris Cormack 2012-06-20 09:02:13 UTC
No string changes, pushed to 3.8.x will be in 3.8.2