[Bug 14287] New: Preserve valid ISBN-13 identifiers if they start with something else than 978
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14287 Bug ID: 14287 Summary: Preserve valid ISBN-13 identifiers if they start with something else than 978 Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Cataloging Assignee: gmcharlt@gmail.com Reporter: olli-antti.kivilahti@jns.fi QA Contact: testopia@bugs.koha-community.org CC: m.de.rooy@rijksmuseum.nl If Business::ISBN cannot make an ISBN-10 out of a valid ISBN-13 starting with 979, then we should preserve the ISBN-13 instead of getting rid of it. ISBN-13 starting with 979 is valid as well and in a few years starting with 980 might be as well as the ISBN supply is running out. This patch modifies the C4::Koha::NormalizeISBN() to preserve the original ISBN if type conversion fails. Also Koha should support all ISBN-variants interchangeably, see Bug 10192. Some Items have ISBN-10 from some sources and the exact same Item can have ISBN-13 from other valid sources. Maybe the easiest solution is to migrate all current and future imported ISBNs to ISBN-13 -variant since according to the same wikipedia article, the conversion is quite easy. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14287 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |13167 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14287 --- Comment #1 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 39625 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39625&action=edit Bug 14287 - Preserve valid ISBN-13 identifiers if they start with something else than 978 If Business::ISBN cannot make an ISBN-10 out of a valid ISBN-13 starting with 979, then we should preserve the ISBN-13 instead of getting rid of it. ISBN-13 starting with 979 is valid as well and in a few years starting with 980 might be as well as the ISBN supply is running out. This patch modifies the C4::Koha::NormalizeISBN() to preserve the original ISBN if type conversion fails. Use Buug 13167's TEST PLAN. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14287 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #2 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Left a warning to warn if the conversion failed. This can be removed from the patch. I am just curious on how often this problem actually happens. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14287 Indranil Das Gupta <indradg@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |indradg@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14287 --- Comment #3 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 39625 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39625 Bug 14287 - Preserve valid ISBN-13 identifiers if they start with something else than 978 Review of attachment 39625: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=14287&attachment=39625) ----------------------------------------------------------------- ::: C4/Koha.pm @@ +1725,4 @@
elsif ( $format eq 'ISBN-13' ) { $isbn = $isbn->as_isbn13(); } + unless ($isbn) { #If we cannot force the ISBN to another type, atleast prevent Koha from crashing by recovering some ISBN.
unless is hard to read, and if (!$isbn) is shorter. Though, I don't think then as_isbn13() will return undef. Only the as_isbn10() will: "If it is an ISBN-13 with the prefix 978, it returns the ISBN-10 equivalent. For all other cases it returns undef." (CPAN page) So, why not: my $isbn10 = $isbn->as_isbn10(); $isbn = $isbn10 if $isbn10; in the ISBN-10 check? @@ +1725,5 @@
elsif ( $format eq 'ISBN-13' ) { $isbn = $isbn->as_isbn13(); } + unless ($isbn) { #If we cannot force the ISBN to another type, atleast prevent Koha from crashing by recovering some ISBN. + $isbn = Business::ISBN->new($string); #If we fail to force the ISBN to the given format, revert to the format we can infer.
Why rebuild something when we could simplify as I suggested above? @@ +1726,5 @@
$isbn = $isbn->as_isbn13(); } + unless ($isbn) { #If we cannot force the ISBN to another type, atleast prevent Koha from crashing by recovering some ISBN. + $isbn = Business::ISBN->new($string); #If we fail to force the ISBN to the given format, revert to the format we can infer. + warn "C4::Koha::NormalizeISBN():> Couldn't change ISBN '$string' to type '$format'. Using the inferred type '".$isbn->type()."' instead.";
Do we need to flood the error logs? Reports could be used to find those ISBN-13 entries with non-978 beginnings. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14287 --- Comment #4 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 39930 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39930&action=edit Bug 14287: counter-patch to preserve valid ISBN-13 not starting with 978 Since the issue is for ISBN-13 to ISBN-10 of non-978 ISBN-13 values, this patch only modifies the ISBN-10 case. Additionally, since ISBN-13 always returns a value, this patch means the return unless is no longer needed. Flooding the error log seems unnecessary. TEST PLAN --------- 1) prove -v t/Koha.t 2) apply patch 3) prove -v t/Koha.t -- note that test 17 has been changed. 4) run koha qa test tools -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14287 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtompset@hotmail.com --- Comment #5 from M. Tompsett <mtompset@hotmail.com> --- Sign off either the patch or counter-patch, and obsolete the other. See comment #3 as to why my counter patch is better in my opinion. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14287 Indranil Das Gupta <indradg@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39625|0 |1 is obsolete| | Attachment #39930|0 |1 is obsolete| | --- Comment #6 from Indranil Das Gupta <indradg@gmail.com> --- Created attachment 40126 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40126&action=edit Bug 14287: counter-patch to preserve valid ISBN-13 not starting with 978 Since the issue is for ISBN-13 to ISBN-10 of non-978 ISBN-13 values, this patch only modifies the ISBN-10 case. Additionally, since ISBN-13 always returns a value, this patch means the return unless is no longer needed. Flooding the error log seems unnecessary. TEST PLAN --------- 1) prove -v t/Koha.t 2) apply patch 3) prove -v t/Koha.t -- note that test 17 has been changed. 4) run koha qa test tools Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14287 Indranil Das Gupta <indradg@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #7 from Indranil Das Gupta <indradg@gmail.com> --- (In reply to M. Tompsett from comment #5)
Sign off either the patch or counter-patch, and obsolete the other. See comment #3 as to why my counter patch is better in my opinion.
this is a cleaner implementation. so signing off on this one. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14287 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m Status|Signed Off |In Discussion --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Not sure to understand: We have a subroutine to return an isbn in a given format. If there is no isbn for this format, the subroutine should return something to tell "it does not exist" (undef here). Your patch suggests to return another format, IMO it's not relevant. There are 2 other subroutines: GetVariationsOfISBN and GetVariationsOfISBNs, maybe it's more what you want. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14287 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Signed Off --- Comment #9 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Jonathan. The problem is that this subroutine is used by the MARC import scripts. The subroutine is called "NormalizeISBN", so I would expect it to normalize any ISBN to the one Koha uses internally, which should be something else than ISBN10 for numberspace exhaustion reasons. I think we are already seeing duplicate issues with ISBN's (if memory serves me), so we should have the ISBN-13 as the new normal. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14287 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #10 from Jonathan Druart <jonathan.druart@biblibre.com> --- Please provide a test plan. NormalizeISBN is used in some places and it's hard to imagine the side effects this patch could introduce. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org