Bug 6852 - Staged import reports wrong success for items with false branchcode
Summary: Staged import reports wrong success for items with false branchcode
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Bibliographic record staging/import (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Bugs List
URL:
Keywords:
: 3529 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-09-07 07:51 UTC by Katrin Fischer
Modified: 2017-12-07 22:16 UTC (History)
6 users (show)

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


Attachments
MARC test file (1.94 KB, application/octet-stream)
2011-09-07 07:51 UTC, Katrin Fischer
Details
screenshot (13.09 KB, image/png)
2011-09-07 07:52 UTC, Katrin Fischer
Details
Bug 6852: Preliminary test (1.69 KB, patch)
2016-09-08 10:09 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 6852: Staged import reports wrong success for items (3.08 KB, patch)
2016-09-08 10:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6852: Staged import reports wrong success for items (3.14 KB, patch)
2016-09-08 16:08 UTC, Owen Leonard
Details | Diff | Splinter Review
[PASSED QA] Bug 6852: Staged import reports wrong success for items (3.16 KB, patch)
2016-09-10 13:01 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 Katrin Fischer 2011-09-07 07:51:51 UTC
Created attachment 5333 [details]
MARC test file

Problem: Staged MARC import reports to have successfully imported items when no items were added.

1) Upload MARC file for staged MARC import, select 'always add' for items and no matching rule. If using the attached MARC file make sure you have an itemtype 'NORMAL' and no branch with brancode 'TEST'
2) Add records and items into catalog using 'manage staged MARC import'. 
! The tool reports to have successfully added 1 record and 1 item. 
3) Check record - no item was added.
4) Create branch with branchcode 'TEST'
5) Import again and check if item was added now.
Comment 1 Katrin Fischer 2011-09-07 07:52:29 UTC
Created attachment 5334 [details]
screenshot
Comment 2 Sophie MEYNIEUX 2016-04-22 08:51:53 UTC
*** Bug 3529 has been marked as a duplicate of this bug. ***
Comment 3 Owen Leonard 2016-08-10 19:10:54 UTC
This is still valid in master. In my test I had a record with 7 items, the first of which was invalid. No items were imported.
Comment 4 Marcel de Rooy 2016-09-08 10:09:12 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2016-09-08 10:24:44 UTC
(In reply to Owen Leonard from comment #3)
> This is still valid in master. In my test I had a record with 7 items, the
> first of which was invalid. No items were imported.

This is not what I experienced. I tested one record with two items. One item had a wrong branchcode and was not imported. But the other item was imported.
Comment 6 Marcel de Rooy 2016-09-08 10:57:45 UTC Comment hidden (obsolete)
Comment 7 Owen Leonard 2016-09-08 16:08:24 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2016-09-10 13:01:04 UTC
Created attachment 55477 [details] [review]
[PASSED QA] Bug 6852: Staged import reports wrong success for items

If we import items that have a wrong branch code, the items will not
be imported but manage-marc-import reports them as imported. (A wrong
branch code probably occurs most, but other causes are possible too.)

The underlying cause is that AddItem does not look at the error
returned from _koha_new_item in Items.pm.

This patch deals with that omission in the most economical way. It adjusts
AddItem so that it returns undef if no item was added.
In ImportBatch.pm I check if an item was added and adjust the totals
accordingly instead of just always counting them.

Note: Several scripts like additem.pl use AddItemFromMarc to call
AddItem. They do not report an error, but fail silently. With this patch,
these scripts get undef and will still fail silently. (No change.)
Adding error checks around calls of AddItemFromMarc is outside the scope of
this report. Here we are taking care of correct imported item numbers.

Test plan:
[1] Verify that additem.pl still works by adding a new item.
[2] Run t/db_dependent/Items.t
[3] Add a new branchcode, say XXX.
[4] Pick a biblio record with a few items (n) and set one item to branch XXX.
[5] Export this biblio with items to a MARC file.
[6] Change the XXX item to the original branch and remove branch XXX.
[7] Import the MARC file. Verify that one item was not imported and that
    the number of imported items reflects that (equals n-1).

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 9 Kyle M Hall 2016-09-12 09:57:43 UTC
Pushed to master for 16.11, thanks Marcel!
Comment 10 Frédéric Demians 2016-09-13 07:42:45 UTC
Pushed in 16.05. Will be in 16.05.04.
Comment 11 Julian Maurice 2016-09-16 09:11:44 UTC
Pushed to 3.22.x, will be in 3.22.11