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.
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 [U+1F984]
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.