|
Description
Tomás Cohen Arazi (tcohen)
2014-03-09 21:28:26 UTC
Created attachment 25968 [details] [review] 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+ Created attachment 25969 [details] [review] 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 Created attachment 26018 [details] [review] 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 Created attachment 26019 [details] [review] 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 Created attachment 26277 [details] [review] 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 Created attachment 26286 [details] [review] [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> Created attachment 26287 [details] [review] [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> Created attachment 26288 [details] [review] [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> Created attachment 26471 [details] [review] 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> Created attachment 26472 [details] [review] 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> Created attachment 26473 [details] [review] 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> Created attachment 26474 [details] [review] 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> Created attachment 26475 [details] [review] 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> Pushed to master. Thanks, Tomás! Pushed to 3.14.x, will be in 3.14.10 Picked for 3.12.15! |