|
Description
Tomás Cohen Arazi (tcohen)
2022-01-10 19:02:30 UTC
Created attachment 129285 [details] [review] Bug 29844: Remove use of wantarray from Koha::Objects This patch removes the use of `wantarray` from the following methods in the Koha::Objects class: - search - search_related In both cases, the change is trivial. And the tests get the 'list context' portion removed as well. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Caveat: we broke many things by removing the feature. Check follow-up patches as well. Created attachment 129286 [details] [review] Bug 29844: Fix t/db_dependent/Koha/* Ok, I did the easy part :-D Team work is welcome on this one. Created attachment 129287 [details] [review] Bug 29844: Remove use of wantarray from Koha::Objects This patch removes the use of `wantarray` from the following methods in the Koha::Objects class: - search - search_related In both cases, the change is trivial. And the tests get the 'list context' portion removed as well. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Caveat: we broke many things by removing the feature. Check follow-up patches as well. Created attachment 129290 [details] [review] Bug 29844: Fix ->search occurrences and some more... There are lot of inconsistencies in our ->search calls. We could simplify some of them, but not in this patch. Here we want to prevent regressions as much as possible and so don't add unecessary changes. Created attachment 129291 [details] [review] Bug 29844: Fix ->search occurrences and some more... There are lot of inconsistencies in our ->search calls. We could simplify some of them, but not in this patch. Here we want to prevent regressions as much as possible and so don't add unecessary changes. Ok so, what do we do now? Tomas, you flexibility is unparalleled :) Created attachment 129292 [details] [review] Bug 29844: Some more uncaught cases Some more cases found using $ git grep '\->search(' | grep -v -e '\->as_list' -e '\->get_column' \ | grep '@' and then manually looking at them. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 129293 [details] [review] Bug 29844: Some more uncaught cases Some more cases found using $ git grep '\->search(' | grep -v -e '\->as_list' -e '\->get_column' \ | grep '@' and then manually looking at them. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> I'm running (again) the test suite on my computer. But we really need Frido here. This is a candidate to get pushed early so we catch any outstanding issues timely. Created attachment 129295 [details] [review] Bug 29844: Some more uncaught cases Some more cases found using $ git grep '\->search(' | grep -v -e '\->as_list' -e '\->get_column' \ | grep '@' and then manually looking at them. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> (In reply to Tomás Cohen Arazi from comment #11) > I'm running (again) the test suite on my computer. But we really need Frido > here. This is a candidate to get pushed early so we catch any outstanding > issues timely. I totally agree with this change. I started programming with Java so I like chained calls this->that->then->other(). Looks like we have catched a lot of occurencies. I agree to pushed early yes. Created attachment 129996 [details] [review] Bug 29844: Fix t/db_dependent/Koha/* Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 129997 [details] [review] Bug 29844: Remove use of wantarray from Koha::Objects This patch removes the use of `wantarray` from the following methods in the Koha::Objects class: - search - search_related In both cases, the change is trivial. And the tests get the 'list context' portion removed as well. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Caveat: we broke many things by removing the feature. Check follow-up patches as well. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 129998 [details] [review] Bug 29844: Fix ->search occurrences and some more... There are lot of inconsistencies in our ->search calls. We could simplify some of them, but not in this patch. Here we want to prevent regressions as much as possible and so don't add unecessary changes. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 129999 [details] [review] Bug 29844: Some more uncaught cases Some more cases found using $ git grep '\->search(' | grep -v -e '\->as_list' -e '\->get_column' \ | grep '@' and then manually looking at them. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 130098 [details] [review] Bug 29844: Fix t/db_dependent/Koha/* Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 130099 [details] [review] Bug 29844: Remove use of wantarray from Koha::Objects This patch removes the use of `wantarray` from the following methods in the Koha::Objects class: - search - search_related In both cases, the change is trivial. And the tests get the 'list context' portion removed as well. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Caveat: we broke many things by removing the feature. Check follow-up patches as well. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 130100 [details] [review] Bug 29844: Fix ->search occurrences and some more... There are lot of inconsistencies in our ->search calls. We could simplify some of them, but not in this patch. Here we want to prevent regressions as much as possible and so don't add unecessary changes. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 130101 [details] [review] Bug 29844: Some more uncaught cases Some more cases found using $ git grep '\->search(' | grep -v -e '\->as_list' -e '\->get_column' \ | grep '@' and then manually looking at them. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Looking here Created attachment 130129 [details] [review] Bug 29844: Fix t/db_dependent/Koha/* Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 130130 [details] [review] Bug 29844: Fix t/db_dependent/Koha/* Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 130131 [details] [review] Bug 29844: Remove use of wantarray from Koha::Objects This patch removes the use of `wantarray` from the following methods in the Koha::Objects class: - search - search_related In both cases, the change is trivial. And the tests get the 'list context' portion removed as well. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Caveat: we broke many things by removing the feature. Check follow-up patches as well. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 130132 [details] [review] Bug 29844: Fix ->search occurrences and some more... There are lot of inconsistencies in our ->search calls. We could simplify some of them, but not in this patch. Here we want to prevent regressions as much as possible and so don't add unecessary changes. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 130133 [details] [review] Bug 29844: Some more uncaught cases Some more cases found using $ git grep '\->search(' | grep -v -e '\->as_list' -e '\->get_column' \ | grep '@' and then manually looking at them. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> oh, oh... Did you run t/db_dependant/Koha/Libraries.t Kyle.. that's failing for me.. looks like a missing change required in Koha::Library::Groups::all_libraries. These patches are failing some unit tests. Created attachment 130134 [details] [review] Bug 29844: (QA follow-up) Fix Koha::Library::Groups->all_libraries I think a rebase perhaps lost a change here.. this fix gets the unit tests passing again. Created attachment 130135 [details] [review] Bug 29844: Fix t/db_dependent/Koha/* Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130136 [details] [review] Bug 29844: Remove use of wantarray from Koha::Objects This patch removes the use of `wantarray` from the following methods in the Koha::Objects class: - search - search_related In both cases, the change is trivial. And the tests get the 'list context' portion removed as well. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Caveat: we broke many things by removing the feature. Check follow-up patches as well. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130137 [details] [review] Bug 29844: Fix ->search occurrences and some more... There are lot of inconsistencies in our ->search calls. We could simplify some of them, but not in this patch. Here we want to prevent regressions as much as possible and so don't add unecessary changes. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130138 [details] [review] Bug 29844: Some more uncaught cases Some more cases found using $ git grep '\->search(' | grep -v -e '\->as_list' -e '\->get_column' \ | grep '@' and then manually looking at them. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130139 [details] [review] Bug 29844: (QA follow-up) Fix Koha::Library::Groups->all_libraries I think a rebase perhaps lost a change here.. this fix gets the unit tests passing again. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> OK, * I ran the whole test suite - Everything now passes * I ran the QA scripts - Everything happy * I tried a bunch of stuff in the UI and nothing failed I'm happy to PQA this.. no doubt there will be a little fallout with such a large change, but this is a great step towards consistency. Go, go, go! Just found an issue with patron clubs whilst I moved on to test bug 29847.. setting back to Signed off whilst Tomas takes a look for me :) Created attachment 130143 [details] [review] Bug 29844: (QA follow-up) Fix Koha::Library::Groups->all_libraries I think a rebase perhaps lost a change here.. this fix gets the unit tests passing again. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 130145 [details] [review] Bug 29844: Simplify Koha::Clubs->get_enrollable This patch removes the wantarray use in Koha::Clubs->get_enrollable and adjusts the callers. Also, reference to some unused params in Koha::Patron clubs-related methods are removed as well. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 130146 [details] [review] Bug 29844: Simplify Koha::Clubs->get_enrollable This patch removes the wantarray use in Koha::Clubs->get_enrollable and adjusts the callers. Also, reference to some unused params in Koha::Patron clubs-related methods are removed as well. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Back to PQA.. I'm sure things will drop out once we're using this in earnest Pushed to master for 22.05, thanks to everybody involved [U+1F984] Created attachment 130481 [details] [review] Bug 29844: Fix selenium/administration_tasks.t > Bug 29844: Fix selenium/administration_tasks.t
Pushed to master
Created attachment 130947 [details] [review] Bug 29844: Fix opac-reserve The method Koha::AuthorisedValues->authorised_value is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Objects.pm line 572 Koha::Objects::AUTOLOAD('Koha::AuthorisedValues=HASH(0x5598ac65a780)') called at /kohadevbox/koha/opac/opac-reserve.pl line 464 (In reply to Jonathan Druart from comment #45) > Created attachment 130947 [details] [review] [review] > Bug 29844: Fix opac-reserve > > The method Koha::AuthorisedValues->authorised_value is not covered by tests! > > Trace begun at /kohadevbox/koha/Koha/Objects.pm line 572 > Koha::Objects::AUTOLOAD('Koha::AuthorisedValues=HASH(0x5598ac65a780)') > called at /kohadevbox/koha/opac/opac-reserve.pl line 464 This is fixed by Bug 30089 Created attachment 131032 [details] [review] Bug 29844: Fix advanced editor The method Koha::Libraries->branchcode is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Objects.pm line 572 Koha::Objects::AUTOLOAD('Koha::Libraries=HASH(0x563bf134b810)') called at /kohadevbox/koha/svc/cataloguing/framework line 33 Patch 'Bug 29844: Fix advanced editor' pushed to master one more lost item (probably aren't converted too):
recalls/recalls_to_pull.pl line 74:
my @items = Koha::Items->search({ biblio_id => $recall->biblio_id });
(In reply to Andrew Nugged from comment #49) > one more lost item (probably aren't converted too): > > recalls/recalls_to_pull.pl line 74: > my @items = Koha::Items->search({ biblio_id => > $recall->biblio_id }); Thanks. A-a-a-and one more:
File: C4/XSLT.pm
464: my %branches = map { $_->branchcode => $_->branchname } Koha::Libraries->search({}, { order_by => 'branchname' });
->
my %branches = map { $_->branchcode => $_->branchname } Koha::Libraries->search({}, { order_by => 'branchname' })->as_list;
pls ignore previous comment - it's the wrong code (another branch :) ). If you like researching code, please have a look at Bug 32472 - [21.11 CRASH] The method Koha::Item->count is not covered by tests. Strongly related to not having this patch set in 21.11. *** Bug 18559 has been marked as a duplicate of this bug. *** |