[Bug 11912] New: GetMarcISBN wrongly prepends a space to ISBNs
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 Bug ID: 11912 Summary: GetMarcISBN wrongly prepends a space to ISBNs Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: MARC Bibliographic data support Assignee: gmcharlt@gmail.com Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 25968 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25968&action=edit Bug 11912: (regression tests) GetMarcISBN wrongly prepends a space to ISBNs This patch makes the tests run in both MARC21 and UNIMARC contexts. It previously run only for MARC21. It mocks what needs to be mocked. To test, run - prove t/db_dependent/Biblio.t => Notice the first ISBN has a space in front of it and those tests fails. Thanks for any feedback To+ -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 25969 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25969&action=edit Bug 11912: GetMarcISBN wrongly prepends a space to ISBNs This patch makes the logic inside GetMarcISBN simpler and fixes the issue. To test: - Run the regression tests: prove -v t/db_dependent/Biblio.t => FAIL - Apply the patch - Run: prove -v t/db_dependent/Biblio.t => SUCCESS - Verify that opac-detail.pl and catalogue/detail.pl look as usual regarding ISBN - Sign off Regards To+ Sponsored-by: Universidad Nacional de Cordoba -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 26018 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26018&action=edit Bug 11912: (refactoring followup) GetMarcISBN should implement its advertised API The current implementation of GetMarcISBN contradicts the documented API. It currently returns an array of hashes with only one key (marcisbn) which doesn't add any value to it. I chose to fix GetMarcISBN to honour the API instead of changing the docs, because it seems a really silly change. To test: - Run: prove t/db_dependent/Biblio.t => SUCCESS - catalogue/detail.pl should correctly show ISBNs. - opac/opac-detail.pl should correctly show ISBNs in both prog and bootstrap. - opac-opac-sendshelf.pl should correctly show ISBNs in the email. - Sign off :-D Regards To+ Sponsored-by: Universidad Nacional de Cordoba -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26018|0 |1 is obsolete| | --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 26019 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26019&action=edit Bug 11912: (refactoring followup) GetMarcISBN should implement its advertised API The current implementation of GetMarcISBN contradicts the documented API. It currently returns an array of hashes with only one key (marcisbn) which doesn't add any value to it. I chose to fix GetMarcISBN to honour the API instead of changing the docs, because it seems a really silly change. To test: - Run: prove t/db_dependent/Biblio.t => SUCCESS - catalogue/detail.pl should correctly show ISBNs. - opac/opac-detail.pl should correctly show ISBNs in both prog and bootstrap. - opac-opac-sendshelf.pl should correctly show ISBNs in the email. - Sign off :-D Edit: forgot to git add some templates. Regards To+ Sponsored-by: Universidad Nacional de Cordoba -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26019|0 |1 is obsolete| | --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 26277 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26277&action=edit Bug 11912: (refactoring followup) GetMarcISBN should implement its advertised API The current implementation of GetMarcISBN contradicts the documented API. It currently returns an array of hashes with only one key (marcisbn) which doesn't add any value to it. I chose to fix GetMarcISBN to honour the API instead of changing the docs, because it seems a really silly change. To test: - Run: prove t/db_dependent/Biblio.t => SUCCESS - catalogue/detail.pl should correctly show ISBNs. - opac/opac-detail.pl should correctly show ISBNs in both prog and bootstrap. - opac-opac-sendshelf.pl should correctly show ISBNs in the email. - Sign off :-D Edit: forgot to git add some templates and fixing the unit tests. Regards To+ Sponsored-by: Universidad Nacional de Cordoba -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 Nicolas Legrand <nicolas.legrand@bulac.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 Nicolas Legrand <nicolas.legrand@bulac.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #25968|0 |1 is obsolete| | --- Comment #6 from Nicolas Legrand <nicolas.legrand@bulac.fr> --- Created attachment 26286 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26286&action=edit [SIGNED-OFF] Bug 11912: (regression tests) GetMarcISBN wrongly prepends a space to ISBNs This patch makes the tests run in both MARC21 and UNIMARC contexts. It previously run only for MARC21. It mocks what needs to be mocked. To test, run - prove t/db_dependent/Biblio.t => Notice the first ISBN has a space in front of it and those tests fails. Thanks for any feedback To+ Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 Nicolas Legrand <nicolas.legrand@bulac.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #25969|0 |1 is obsolete| | --- Comment #7 from Nicolas Legrand <nicolas.legrand@bulac.fr> --- Created attachment 26287 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26287&action=edit [SIGNED-OFF] Bug 11912: GetMarcISBN wrongly prepends a space to ISBNs This patch makes the logic inside GetMarcISBN simpler and fixes the issue. To test: - Run the regression tests: prove -v t/db_dependent/Biblio.t => FAIL - Apply the patch - Run: prove -v t/db_dependent/Biblio.t => SUCCESS - Verify that opac-detail.pl and catalogue/detail.pl look as usual regarding ISBN - Sign off Regards To+ Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 Nicolas Legrand <nicolas.legrand@bulac.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26277|0 |1 is obsolete| | --- Comment #8 from Nicolas Legrand <nicolas.legrand@bulac.fr> --- Created attachment 26288 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26288&action=edit [SIGNED-OFF] Bug 11912: (refactoring followup) GetMarcISBN should implement its advertised API The current implementation of GetMarcISBN contradicts the documented API. It currently returns an array of hashes with only one key (marcisbn) which doesn't add any value to it. I chose to fix GetMarcISBN to honour the API instead of changing the docs, because it seems a really silly change. To test: - Run: prove t/db_dependent/Biblio.t => SUCCESS - catalogue/detail.pl should correctly show ISBNs. - opac/opac-detail.pl should correctly show ISBNs in both prog and bootstrap. - opac-opac-sendshelf.pl should correctly show ISBNs in the email. - Sign off :-D Edit: forgot to git add some templates and fixing the unit tests. Regards To+ Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26286|0 |1 is obsolete| | Attachment #26287|0 |1 is obsolete| | Attachment #26288|0 |1 is obsolete| | --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 26471 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26471&action=edit Bug 11912: (regression tests) GetMarcISBN wrongly prepends a space to ISBNs This patch makes the tests run in both MARC21 and UNIMARC contexts. It previously run only for MARC21. It mocks what needs to be mocked. To test, run - prove t/db_dependent/Biblio.t => Notice the first ISBN has a space in front of it and those tests fails. Thanks for any feedback To+ Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 --- Comment #10 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 26472 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26472&action=edit Bug 11912: GetMarcISBN wrongly prepends a space to ISBNs This patch makes the logic inside GetMarcISBN simpler and fixes the issue. To test: - Run the regression tests: prove -v t/db_dependent/Biblio.t => FAIL - Apply the patch - Run: prove -v t/db_dependent/Biblio.t => SUCCESS - Verify that opac-detail.pl and catalogue/detail.pl look as usual regarding ISBN - Sign off Regards To+ Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 26473 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26473&action=edit Bug 11912: (refactoring followup) GetMarcISBN should implement its advertised API The current implementation of GetMarcISBN contradicts the documented API. It currently returns an array of hashes with only one key (marcisbn) which doesn't add any value to it. I chose to fix GetMarcISBN to honour the API instead of changing the docs, because it seems a really silly change. To test: - Run: prove t/db_dependent/Biblio.t => SUCCESS - catalogue/detail.pl should correctly show ISBNs. - opac/opac-detail.pl should correctly show ISBNs in both prog and bootstrap. - opac-opac-sendshelf.pl should correctly show ISBNs in the email. - Sign off :-D Edit: forgot to git add some templates and fixing the unit tests. Regards To+ Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 --- Comment #12 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 26474 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26474&action=edit Bug 11912: Use the common way to mock syspref t::lib::Mocks::mock_preference provides a simple way to mock syspref. Test plan: Verify that t/db_dependent/Biblio.t returns green. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26474|0 |1 is obsolete| | --- Comment #13 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 26475 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26475&action=edit Bug 11912: Use the common way to mock syspref t::lib::Mocks::mock_preference provides a simple way to mock syspref. Test plan: Verify that t/db_dependent/Biblio.t returns green. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |gmcharlt@gmail.com --- Comment #14 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. Thanks, Tomás! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |fridolyn.somers@biblibre.co | |m --- Comment #15 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Pushed to 3.14.x, will be in 3.14.10 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11912 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Picked for 3.12.15! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org