Bug 14200 - GetNormalizedISBN failing to normalize valid ISBNs in some cases
Summary: GetNormalizedISBN failing to normalize valid ISBNs in some cases
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-14 10:19 UTC by Kyle M Hall
Modified: 2015-12-03 22:11 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 14200 - Unit Tests (2.24 KB, patch)
2015-05-14 10:21 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14200 - GetNormalizedISBN failing to normalize valid ISBNs in some cases (1.17 KB, patch)
2015-05-14 10:21 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14200 - Unit Tests (2.35 KB, patch)
2015-05-14 17:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14200 - GetNormalizedISBN failing to normalize valid ISBNs in some cases (1.28 KB, patch)
2015-05-14 17:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14200 - Unit Tests (2.44 KB, patch)
2015-05-19 12:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14200 - GetNormalizedISBN failing to normalize valid ISBNs in some cases (1.36 KB, patch)
2015-05-19 12:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2015-05-14 10:19:19 UTC
For some reason the regular expression used to split the multiple ISBN fields can fail which keeps Business::ISBN from being able to parse out and return a valid isbn. Using split() instead of the regex fixes the issue.
Comment 1 Kyle M Hall 2015-05-14 10:21:32 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2015-05-14 10:21:38 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2015-05-14 17:44:12 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2015-05-14 17:44:41 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2015-05-19 12:41:43 UTC
Created attachment 39296 [details] [review]
Bug 14200 - Unit Tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Karen Bryant <kbryant@ccpl-fl.net>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2015-05-19 12:41:49 UTC
Created attachment 39297 [details] [review]
Bug 14200 - GetNormalizedISBN failing to normalize valid ISBNs in some cases

For some reason the regular expression used to split the multiple ISBN
fields can fail which keeps Business::ISBN from being able to parse out
and return a valid isbn. Using split() instead of the regex fixes the
issue.

Test Plan:
1) Apply the unit test patch first
2) prove t/Koha.t
3) Note the failures
4) Apply the second patch
5) prove t/Koha.t
6) Note all tests pass

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Karen Bryant <kbryant@ccpl-fl.net>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Tomás Cohen Arazi 2015-05-19 12:47:54 UTC
Patches pushed to master.

Good work Kyle!