Bug 29625

Summary: Wrong var name in Koha::BiblioUtils get_all_biblios_iterator
Product: Koha Reporter: Fridolin Somers <fridolin.somers>
Component: Architecture, internals, and plumbingAssignee: Fridolin Somers <fridolin.somers>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: trivial    
Priority: P5 - low CC: andrewfh, david, jonathan.druart, kyle, m.de.rooy, victor
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.03,21.05.13
Attachments: Bug 29625: Fix var name in Koha::BiblioUtils get_all_biblios_iterator
Bug 29625: Fix var name in Koha::BiblioUtils get_all_biblios_iterator
Bug 29625: Fix var name in Koha::BiblioUtils get_all_biblios_iterator
Bug 29625: Fix var name in Koha::BiblioUtils get_all_biblios_iterator
Bug 29625: Fix var name in Koha::BiblioUtils get_all_biblios_iterator
Bug 29625: Add test for get_all_biblios_iterator

Description Fridolin Somers 2021-12-02 22:21:15 UTC
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.
Comment 1 Fridolin Somers 2021-12-02 22:23:42 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
Comment 2 David Nind 2022-01-19 23:06:28 UTC
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>
Comment 3 David Nind 2022-01-19 23:08:44 UTC
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.
Comment 4 Jonathan Druart 2022-01-26 14:01:48 UTC
Then I would also change the following line:
156                 __PACKAGE__->new($marc, $row->biblionumber);
Comment 5 Fridolin Somers 2022-01-27 06:25:18 UTC
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
Comment 6 Fridolin Somers 2022-01-27 06:25:46 UTC
(In reply to Jonathan Druart from comment #4)
> Then I would also change the following line:
> 156                 __PACKAGE__->new($marc, $row->biblionumber);

Good catch ;)
Comment 7 Marcel de Rooy 2022-01-27 07:43:50 UTC
Why are there no tests for this routine btw ?
Comment 8 Fridolin Somers 2022-01-28 06:28:45 UTC
(In reply to Marcel de Rooy from comment #7)
> Why are there no tests for this routine btw ?

Oh indeed we should have.
Comment 9 Michal Denar 2022-02-17 21:47:12 UTC
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>
Comment 10 Jonathan Druart 2022-02-18 10:03:32 UTC
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>
Comment 11 Jonathan Druart 2022-02-18 10:03:36 UTC
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>
Comment 12 Fridolin Somers 2022-02-22 01:14:08 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 13 Kyle M Hall 2022-02-25 13:26:29 UTC
Pushed to 21.11.x for 21.11.03
Comment 14 Andrew Fuerste-Henry 2022-03-07 20:58:59 UTC
Pushed to 21.05.x for 21.05.12
Comment 15 Victor Grousset/tuxayo 2022-03-18 02:38:58 UTC
Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed.