Bug 18153 - UNIMARC bib records exported with invalid 'a' char in label pos.9
Summary: UNIMARC bib records exported with invalid 'a' char in label pos.9
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P1 - high major (vote)
Assignee: stephane.delaune
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-02-22 11:08 UTC by stephane.delaune
Modified: 2017-10-26 07:42 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 18153 : fix unimarc label in export tool (1.28 KB, patch)
2017-02-22 11:54 UTC, stephane.delaune
Details | Diff | Splinter Review
unimarc-withbad-a-char-in-label-pos9.mrc (1.21 KB, text/plain)
2017-02-22 11:55 UTC, stephane.delaune
Details
Bug 18153 : fix unimarc label in export tool (1.28 KB, patch)
2017-04-18 00:24 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18153 : fix unimarc label in export tool (1.34 KB, patch)
2017-04-18 00:49 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18153 : fix unimarc label in export tool (1.35 KB, patch)
2017-05-03 09:06 UTC, stephane.delaune
Details | Diff | Splinter Review
Bug 18153 : fix unimarc label in export tool (1.25 KB, patch)
2017-10-25 13:48 UTC, stephane.delaune
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description stephane.delaune 2017-02-22 11:08:42 UTC
The standard UNIMARC requires than the 9th character (starting from 0) in labels must be blank (while it may be 'a' in marc21), but under certain conditions (e.g. when a iso2709 file is imported) Koha's unimarc records can have an invalid char. 'a' in label pos.9

So when we want to export them with export's tool, if marcflavour syspref is UNIMARC, we need to delete this bad 'a' char from label pos.9 (to have blank)
Comment 1 stephane.delaune 2017-02-22 11:54:54 UTC
Created attachment 60552 [details] [review]
Bug 18153 : fix unimarc label in export tool

The standard UNIMARC requires than the 9th character (starting from 0) in
labels must be blank (while it may be 'a' in marc21), but under certain
conditions (e.g. when a iso2709 file is imported) Koha's unimarc records can
have an invalid char. 'a' in label pos.9

So when we want to export them with export's tool, if marcflavour syspref is
UNIMARC, we need to delete this bad 'a' char from label pos.9 (to have blank) :
this patch make this
Comment 2 stephane.delaune 2017-02-22 11:55:52 UTC
Created attachment 60553 [details]
unimarc-withbad-a-char-in-label-pos9.mrc
Comment 3 stephane.delaune 2017-02-22 11:56:31 UTC
to test :
before applying the patch : 
- change marcflavour syspref in UNIMARC
- import unimarc-withbad-a-char-in-label-pos9.mrc in koha (see below how to) : this record have invalid char 'a' in label pos.9
- export your record (Home › Tools › Export data, choose your biblionumber in "from biblionumber:" and "To biblionumber:", Clear all Library) : open file in text editor and you will see 9th char (from0) is 'a'
- apply patch
- export your record (Home › Tools › Export data, choose your biblionumber in "from biblionumber:" and "To biblionumber:", Clear all Library) : open file in text editor and you will see 9th char (from0) is blank (bug is fixed)

Note : how to import a record in Koha :
Home › Tools › Stage MARC records for import, find file, click on "upload",verify character encoding is UTF-8, click on "stage for import" ; Home › Tools › Manage staged MARC records : choose your file and clic on "import" button ; then you can click below on file name to see it (and note (or not) the presence of char 'a' in label pos.9)
Comment 4 Alex Buckley 2017-04-18 00:24:28 UTC
Created attachment 62244 [details] [review]
Bug 18153 : fix unimarc label in export tool

The standard UNIMARC requires than the 9th character (starting from 0) in
labels must be blank (while it may be 'a' in marc21), but under certain
conditions (e.g. when a iso2709 file is imported) Koha's unimarc records can
have an invalid char. 'a' in label pos.9

So when we want to export them with export's tool, if marcflavour syspref is
UNIMARC, we need to delete this bad 'a' char from label pos.9 (to have blank) :
this patch make this
Comment 5 Alex Buckley 2017-04-18 00:49:53 UTC
Created attachment 62245 [details] [review]
Bug 18153 : fix unimarc label in export tool

The standard UNIMARC requires than the 9th character (starting from 0) in
labels must be blank (while it may be 'a' in marc21), but under certain
conditions (e.g. when a iso2709 file is imported) Koha's unimarc records can
have an invalid char. 'a' in label pos.9

So when we want to export them with export's tool, if marcflavour syspref is
UNIMARC, we need to delete this bad 'a' char from label pos.9 (to have blank) :
this patch make this

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 6 stephane.delaune 2017-05-03 09:06:47 UTC
Created attachment 63009 [details] [review]
Bug 18153 : fix unimarc label in export tool

only fix author's name of the patch
Comment 7 Jonathan Druart 2017-05-03 18:33:50 UTC
Must it be done when importing as well?
Comment 8 Jonathan Druart 2017-05-08 15:29:45 UTC
The question is:
Should not we call SetMarcUnicodeFlag from ModBiblioMarc to replace the whole "if ( $encoding eq "UNIMARC" ) {" block?

(read s/encoding/marcflavour ...)
Comment 9 stephane.delaune 2017-05-11 13:09:59 UTC
(In reply to Jonathan Druart from comment #8)
> The question is:
> Should not we call SetMarcUnicodeFlag from ModBiblioMarc to replace the
> whole "if ( $encoding eq "UNIMARC" ) {" block?
> 
> (read s/encoding/marcflavour ...)

The purpose of this patch is not to fix the wrong call of «MARC::Record->encoding('utf-8')» to import (it's the goal of bug 18152), nor even everywhere (it's the goal of bug 18533) ; the goal is to fix exported records. Indeed, a lot of existing unimarc records in a lot of Koha are erroneous because of this bug, the goal is that they do not remain erroneous when exported. I don't see the relationship with ModBiblioMarc who has been corrected to not add the bad character, not to delete it (and export don't call ModBiblioMarc)
Comment 10 Jonathan Druart 2017-05-11 15:43:47 UTC
I have asked Julian to take a look.
Comment 11 Julian Maurice 2017-05-12 09:15:30 UTC
QAing this now
Comment 12 Julian Maurice 2017-05-12 10:26:01 UTC
QA:

1. With the patch, the export tool also modifies the leader of exported authorities (position 9 is for authority type, so this is a problem)
2. The code is poorly formatted and is missing a comment to explain why we need to do this.
3. It only fixes exported records from the export tool and only for iso2709 format. If I export in marcxml or if I use the 'Save' button from the detail page, the leader is still invalid.
4. This patch states clearly that the leader may be invalid in Koha but tries to fix it at export-time instead of fixing it directly in Koha. I think it is possible to fix the leader at save-time, so that every biblio created/modified (whether it's by import or manually) will be saved with a correct leader. And maybe at read-time too, for already existing invalid records. @QAers, any opinions ?

Failing QA for 1 and 2.
Comment 13 stephane.delaune 2017-10-25 13:32:27 UTC
Regarding the fourth point: a patch to remove this invalid character (to fix the leader) at save-time records was developed with the bug 18152; however, while this bz was pushed master, it introduced a regression (bug 18910) that required a revert. Indeed, MARC::Record->encoding is initially planned for marc21 and therefore considers that the absence of this invalid character makes that it is not encoded in utf8 (even in unimarc). Koha often uses this erroneous (in UNIMARC) function so, waiting to have a viable alternative, it is best to keep this invalid character for the moment in Koha
Comment 14 stephane.delaune 2017-10-25 13:48:15 UTC
Created attachment 68532 [details] [review]
Bug 18153 : fix unimarc label in export tool

new patch for QA
Comment 15 M. Tompsett 2017-10-26 05:00:16 UTC
(In reply to Julian Maurice from comment #12)
> 1. With the patch, the export tool also modifies the leader of exported
> authorities (position 9 is for authority type, so this is a problem)
> 2. The code is poorly formatted and is missing a comment to explain why we
> need to do this.

The new patch seems to address this, but I haven't tested it yet.


> 3. It only fixes exported records from the export tool and only for iso2709
> format. If I export in marcxml or if I use the 'Save' button from the detail
> page, the leader is still invalid.

I think this is a valid reason to keep it Failed QA, though I'm changing it
to in discussion. The summary doesn't say only iso2709 export format.
It suggests UNIMARC records in general.


> 4. This patch states clearly that the leader may be invalid in Koha but
> tries to fix it at export-time instead of fixing it directly in Koha. I
> think it is possible to fix the leader at save-time, so that every biblio
> created/modified (whether it's by import or manually) will be saved with a
> correct leader. And maybe at read-time too, for already existing invalid
> records. @QAers, any opinions ?

While I am not a QA person, I think this is a really good point. The leader record should be saved correctly in the first place, not need correction on export.
Comment 16 stephane.delaune 2017-10-26 07:42:02 UTC
 
> > 4. This patch states clearly that the leader may be invalid in Koha but
> > tries to fix it at export-time instead of fixing it directly in Koha. I
> > think it is possible to fix the leader at save-time, so that every biblio
> > created/modified (whether it's by import or manually) will be saved with a
> > correct leader. And maybe at read-time too, for already existing invalid
> > records. @QAers, any opinions ?
> 
> While I am not a QA person, I think this is a really good point. The leader
> record should be saved correctly in the first place, not need correction on
> export.


already answered, copy of my previous comment:
Regarding the fourth point: a patch to remove this invalid character (to fix the leader) at save-time records was developed with the bug 18152; however, while this bz was pushed master, it introduced a regression (bug 18910) that required a revert. Indeed, MARC::Record->encoding is initially planned for marc21 and therefore considers that the absence of this invalid character makes that it is not encoded in utf8 (even in unimarc). Koha often uses this erroneous (in UNIMARC) function so, waiting to have a viable alternative, it is best to keep this invalid character for the moment in Koha