Description
Fridolin Somers
2021-12-02 22:21:15 UTC
Created attachment 128183 [details] [review] Bug 29625: Fix var name in Koha::BiblioUtils get_all_biblios_iterator In Koha::BiblioUtils, get_all_biblios_iterator method is a class level method like get_from_biblionumber. So its first arg should be named $class instead of $self. Even if this var in not used. Test plan : 1) Run Elasticsearch full rebuild with and without patch 2) Check all biblio records are in index Created attachment 129629 [details] [review] Bug 29625: Fix var name in Koha::BiblioUtils get_all_biblios_iterator In Koha::BiblioUtils, get_all_biblios_iterator method is a class level method like get_from_biblionumber. So its first arg should be named $class instead of $self. Even if this var in not used. Test plan : 1) Run Elasticsearch full rebuild with and without patch 2) Check all biblio records are in index Signed-off-by: David Nind <david@davidnind.com> Testing notes (koha-testing-docker): - 436 records both before and after the patch is applied and index regenerated using Elasticsearch. - Searched in the staff interface catalogue using * - Same number of records as indexed when using Zebra. Then I would also change the following line: 156 __PACKAGE__->new($marc, $row->biblionumber); Created attachment 129839 [details] [review] Bug 29625: Fix var name in Koha::BiblioUtils get_all_biblios_iterator In Koha::BiblioUtils, get_all_biblios_iterator method is a class level method like get_from_biblionumber. So its first arg should be named $class instead of $self. Even if this var in not used. Test plan : 1) Run Elasticsearch full rebuild with and without patch 2) Check all biblio records are in index (In reply to Jonathan Druart from comment #4) > Then I would also change the following line: > 156 __PACKAGE__->new($marc, $row->biblionumber); Good catch ;) Why are there no tests for this routine btw ? (In reply to Marcel de Rooy from comment #7) > Why are there no tests for this routine btw ? Oh indeed we should have. Created attachment 130774 [details] [review] Bug 29625: Fix var name in Koha::BiblioUtils get_all_biblios_iterator In Koha::BiblioUtils, get_all_biblios_iterator method is a class level method like get_from_biblionumber. So its first arg should be named $class instead of $self. Even if this var in not used. Test plan : 1) Run Elasticsearch full rebuild with and without patch 2) Check all biblio records are in index Signed-off-by: Michal Denar <black23@gmail.com> Created attachment 130815 [details] [review] Bug 29625: Fix var name in Koha::BiblioUtils get_all_biblios_iterator In Koha::BiblioUtils, get_all_biblios_iterator method is a class level method like get_from_biblionumber. So its first arg should be named $class instead of $self. Even if this var in not used. Test plan : 1) Run Elasticsearch full rebuild with and without patch 2) Check all biblio records are in index Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 130816 [details] [review] Bug 29625: Add test for get_all_biblios_iterator Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to master for 22.05, thanks to everybody involved 🦄 Pushed to 21.11.x for 21.11.03 Pushed to 21.05.x for 21.05.12 Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed. |