Bug 29625 - Wrong var name in Koha::BiblioUtils get_all_biblios_iterator
Summary: Wrong var name in Koha::BiblioUtils get_all_biblios_iterator
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low trivial (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-12-02 22:21 UTC by Fridolin Somers
Modified: 2022-12-12 21:24 UTC (History)
6 users (show)

See Also:
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 (1006 bytes, patch)
2021-12-02 22:23 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 29625: Fix var name in Koha::BiblioUtils get_all_biblios_iterator (1.03 KB, patch)
2022-01-19 23:06 UTC, David Nind
Details | Diff | Splinter Review
Bug 29625: Fix var name in Koha::BiblioUtils get_all_biblios_iterator (1.41 KB, patch)
2022-01-27 06:25 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 29625: Fix var name in Koha::BiblioUtils get_all_biblios_iterator (1.45 KB, patch)
2022-02-17 21:47 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 29625: Fix var name in Koha::BiblioUtils get_all_biblios_iterator (1.53 KB, patch)
2022-02-18 10:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29625: Add test for get_all_biblios_iterator (1.52 KB, patch)
2022-02-18 10:03 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.