Bug 12672 - GetMarcISBN should return the corresponding subfield
Summary: GetMarcISBN should return the corresponding subfield
Status: CLOSED FIXED
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: Testopia
URL:
Keywords:
Depends on:
Blocks: 12554
  Show dependency treegraph
 
Reported: 2014-07-29 16:31 UTC by Tomás Cohen Arazi
Modified: 2015-06-04 23:33 UTC (History)
4 users (show)

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


Attachments
Bug 12672: (regression test) GetMarcISBN should return the corresponding subfield (1.69 KB, patch)
2014-07-29 17:00 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 12672: GetMarcISBN should return the corresponding subfield (1.05 KB, patch)
2014-07-29 17:00 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12672: (regression test) GetMarcISBN should return the corresponding subfield (1.80 KB, patch)
2014-07-30 15:06 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12672: GetMarcISBN should return the corresponding subfield (1.16 KB, patch)
2014-07-30 15:06 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[PASSED QA] Bug 12672: (regression test) GetMarcISBN should return the corresponding subfield (1.85 KB, patch)
2014-08-02 21:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12672: GetMarcISBN should return the corresponding subfield (1.21 KB, patch)
2014-08-02 21:37 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2014-07-29 16:31:59 UTC
Currently, if you call GetMARCISBN for a record that contains another subfield than $a (for example 020$c in MARC21) GetMARCISN returns all the subfields appended.

As the function is used for visualization purposes, it should be fixed to only show the ISBN subfield.
Comment 1 Tomás Cohen Arazi 2014-07-29 17:00:45 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 2014-07-29 17:00:48 UTC Comment hidden (obsolete)
Comment 3 Bernardo Gonzalez Kriegel 2014-07-30 15:06:15 UTC Comment hidden (obsolete)
Comment 4 Bernardo Gonzalez Kriegel 2014-07-30 15:06:24 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2014-07-30 21:27:39 UTC
Hm, I am not sure about removing $z - invalid/cancelled ISBN.

Adding Jared and Galen in cc, looking for opinions.
Comment 6 Jared Camins-Esakov 2014-08-01 14:47:32 UTC
(In reply to Katrin Fischer from comment #5)
> Hm, I am not sure about removing $z - invalid/cancelled ISBN.
> 
> Adding Jared and Galen in cc, looking for opinions.

My inclination is that $z shouldn't appear, because it doesn't provide useful
Comment 7 Jared Camins-Esakov 2014-08-01 14:48:56 UTC
(In reply to Jared Camins-Esakov from comment #6)
> My inclination is that $z shouldn't appear, because it doesn't provide useful

[whoops, somehow submitted before I finished]
... doesn't provide useful information in the majority of situations, but could easily confuse someone looking for the ISBN.
Comment 8 Katrin Fischer 2014-08-02 21:37:46 UTC
Created attachment 30502 [details] [review]
[PASSED QA] Bug 12672: (regression test) GetMarcISBN should return the corresponding subfield

Run:
 $ prove -v t/db_dependent/Biblio.t
=> FAIL: GetMarcISBN doesn't return the expected value for ISBN
         in MARC21, NORMARC and UNIMARC

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Test fails, no koha-qa errors.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 9 Katrin Fischer 2014-08-02 21:37:59 UTC
Created attachment 30503 [details] [review]
[PASSED QA] Bug 12672: GetMarcISBN should return the corresponding subfield

The current GetMarcISBN implementation returns an array of ISBN
in which all subfields of a ISBN field occurence are appended.

For example, in MARC21, if you have $a and $c defined, they will
be appended for output. This happens for $z.

To reproduce:
- Run the regression tests attached to this bug.

To test:
- Apply the patch, regression tests pass.
- Sign off

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Now test pass, no koha-qa errors

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 10 Tomás Cohen Arazi 2014-08-03 15:31:21 UTC
Patches pushed to master.