Bug 16556 - KohaToMarcMapped columns sharing same field with biblio(item)number are removed.
Summary: KohaToMarcMapped columns sharing same field with biblio(item)number are removed.
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: Olli-Antti Kivilahti
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-19 14:12 UTC by Olli-Antti Kivilahti
Modified: 2019-06-27 09:24 UTC (History)
7 users (show)

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


Attachments
Bug 16556 - KohaToMarcMapped columns sharing same field with biblio(item)number are removed. (7.22 KB, patch)
2016-05-19 14:13 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 16556 - KohaToMarcMapped columns sharing same field with biblio(item)number are removed. (7.27 KB, patch)
2016-08-31 23:34 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 16556 - KohaToMarcMapped columns sharing same field with biblio(item)number are removed. (7.33 KB, patch)
2016-09-02 18:09 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Olli-Antti Kivilahti 2016-05-19 14:12:01 UTC
REPLICATE ISSUE:

1. Map biblio.frameworkcode to 999$b
2. Map biblio.biblionumber to 999$c
3. Add a record with something in 999$b
4. 999$b is removed by C4::Biblio::AddBiblio()

After this patch, the field used by biblio.biblionumber or biblioitems.biblioitemnumber is not removed and created anew, thus dropping all existing additions.

There is no point in dropping the field in any case, since we can just replace the existing subfields in-place with no need to recreate the whole field.
Comment 1 Olli-Antti Kivilahti 2016-05-19 14:13:38 UTC Comment hidden (obsolete)
Comment 2 Olli-Antti Kivilahti 2016-05-19 14:14:10 UTC
C4::Biblio::_koha_marc_update_bib_ids() does the dark deed.
Comment 3 Jonathan Druart 2016-05-20 14:46:06 UTC
This looks like a bug, not an enh :)
Comment 4 Christopher Davis 2016-08-05 22:34:42 UTC
Olli-Antti Kivilahti,

I was taking a look at this bug and I could not find any patch or test plan. Will you please add these to this bug report?

Thank you,

Christopher Davis
Comment 5 Owen Leonard 2016-08-08 13:17:26 UTC
> I was taking a look at this bug and I could not find any patch or test plan.
> Will you please add these to this bug report?

There is one attachment to this bug report and it is the patch. The patch's commit message contains the test plan. See Comment 1.
Comment 6 Chris Cormack 2016-08-31 23:34:47 UTC Comment hidden (obsolete)
Comment 7 Nick Clemens 2016-09-02 18:09:47 UTC
Created attachment 55141 [details] [review]
Bug 16556 - KohaToMarcMapped columns sharing same field with biblio(item)number are removed.

REPLICATE ISSUE:

1. Map biblio.frameworkcode to 999$b
2. Map biblio.biblionumber to 999$c
3. Add a record with something in 999$b
4. 999$b is removed by C4::Biblio::AddBiblio()

After this patch, the field used by biblio.biblionumber or biblioitems.biblioitemnumber
is not removed and created anew, thus dropping all existing additions.

There is no point in dropping the field in any case, since we can just replace
the existing subfields in-place with no need to recreate the whole field.

UNIT TESTS INCLUDED

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Nick Clemens 2016-09-02 18:11:41 UTC
Works nicely, but 999 data loss is a problem Rancor/Adv Cataloging editor  - I don't know if we can require this patchset to fix that as well or if it should be a separate bug
Comment 9 Kyle M Hall 2016-09-15 13:41:58 UTC
(In reply to Nick Clemens from comment #8)
> Works nicely, but 999 data loss is a problem Rancor/Adv Cataloging editor  -
> I don't know if we can require this patchset to fix that as well or if it
> should be a separate bug

I say file a separate bug. Better to have it partially fixed than totally broken I say.
Comment 10 Kyle M Hall 2016-09-15 13:43:16 UTC
Pushed to master for 16.11, thanks Olli!
Comment 11 Frédéric Demians 2016-09-15 15:07:47 UTC
Pushed in 16.05. Will be in 16.05.04.
Comment 12 Julian Maurice 2016-09-16 10:04:29 UTC
Pushed to 3.22.x, will be in 3.22.11