Bug 9500 - GetNormalizedISBN only ever checks the first populated ISBN field
Summary: GetNormalizedISBN only ever checks the first populated ISBN field
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-28 13:16 UTC by Kyle M Hall
Modified: 2016-12-01 21:58 UTC (History)
5 users (show)

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


Attachments
MARCXML file with multiple ISBN numbers -- ISBN-13 first (5.45 KB, application/xml)
2014-10-22 21:10 UTC, Barton Chittenden
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2013-01-28 13:16:00 UTC
If you take a look at C4::Koha::GetNormalizedISBN you will see code like this:

    if ($marcflavour eq 'UNIMARC') {
        @fields = $record->field('010');
        foreach my $field (@fields) {
            my $isbn = $field->subfield('a');
            if ($isbn) {
                return _isbn_cleanup($isbn);
            } else {
                return undef;
            }
        }
    }

If I read this correctly, if we have say two ISBNs in a record, with the first being invalid and the second being valid, GetNormalizedISBN will still return undef because it never bothers to check the second isbn!
Comment 1 Barton Chittenden 2014-10-22 21:10:10 UTC
Created attachment 32608 [details]
MARCXML file with multiple ISBN numbers -- ISBN-13 first

Stage and import this record.
Comment 2 Barton Chittenden 2014-10-22 21:14:17 UTC
Oh, the badness goes *so* much deeper: GetNormalizedISBN calls _isbn_cleanup():

sub _isbn_cleanup {
    my ($isbn) = @_;
    return NormalizeISBN(
        {
            isbn          => $isbn,
            format        => 'ISBN-10',
            strip_hyphens => 1,
        }
    ) if $isbn;
}

... So if the first ISBN isn't a valid *10 digit* isbn, GetNormalizedISBN will fail.

I've attached a MARCXML record which will trigger the bug.
Comment 3 Jonathan Druart 2015-04-08 09:51:47 UTC
Could you describe an issue visible on the interface?