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.
Created attachment 30295 [details] [review] 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
Created attachment 30296 [details] [review] 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
Created attachment 30346 [details] [review] [SIGNED-OFF] 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.
Created attachment 30347 [details] [review] [SIGNED-OFF] 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
Hm, I am not sure about removing $z - invalid/cancelled ISBN. Adding Jared and Galen in cc, looking for opinions.
(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
(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.
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>
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>
Patches pushed to master.