Bug 12672

Summary: GetMarcISBN should return the corresponding subfield
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: bgkriegel, gmc, jcamins, katrin.fischer
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 12554    
Attachments: Bug 12672: (regression test) GetMarcISBN should return the corresponding subfield
Bug 12672: GetMarcISBN should return the corresponding subfield
[SIGNED-OFF] Bug 12672: (regression test) GetMarcISBN should return the corresponding subfield
[SIGNED-OFF] Bug 12672: GetMarcISBN should return the corresponding subfield
[PASSED QA] Bug 12672: (regression test) GetMarcISBN should return the corresponding subfield
[PASSED QA] Bug 12672: GetMarcISBN should return the corresponding subfield

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.