From bdd383358250b6be0032db025db1f27943637d6b Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 20 Dec 2021 15:14:56 +0100 Subject: [PATCH] Bug 29736: Don't return empty clubs There is an error when placing a hold for a club without members: Uncaught TypeError: err.responseJSON.error is undefined It seems that we should remove clubs without members from the search. Test plan: Create 1 club xx with 2 patrons Create 1 club xxx with 1 patron and cancel their enrolment Create 1 club xxxx without patron Place a hold for club "x", only the first one should be returned with this patch. Signed-off-by: Tomas Cohen Arazi --- Koha/Clubs.pm | 26 ++++++++++++++++ reserve/request.pl | 6 ++-- t/db_dependent/Clubs.t | 70 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 97 insertions(+), 5 deletions(-) diff --git a/Koha/Clubs.pm b/Koha/Clubs.pm index d206b9023a..4acb64c9d9 100644 --- a/Koha/Clubs.pm +++ b/Koha/Clubs.pm @@ -85,6 +85,32 @@ sub get_enrollable { } } +=head3 filter_out_empty + + Remove clubs without current enrollments. + +=cut + +sub filter_out_empty { + my ($self) = @_; + return $self->search( + { + -and => [ + [ + { name => { like => '%x%' } }, + { description => { like => '%x%' } }, + ], + { 'club_enrollments.club_id' => { '!=' => undef } }, + { 'club_enrollments.date_canceled' => undef }, + ] + }, + { + join => 'club_enrollments', + distinct => 1, + } + ); +} + =head3 type =cut diff --git a/reserve/request.pl b/reserve/request.pl index 1b6638307d..81dac34ed0 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -165,10 +165,8 @@ if($findclub) { if( $club ) { $club_hold = $club->id; } else { - my @clubs = Koha::Clubs->search( [ - { name => { like => '%'.$findclub.'%' } }, - { description => { like => '%'.$findclub.'%' } } - ] ); + my @clubs = Koha::Clubs->search->filter_out_empty; + if( scalar @clubs == 1 ) { $club_hold = $clubs[0]->id; } elsif ( @clubs ) { diff --git a/t/db_dependent/Clubs.t b/t/db_dependent/Clubs.t index ee0f83b2c5..8d8a0d1951 100755 --- a/t/db_dependent/Clubs.t +++ b/t/db_dependent/Clubs.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 37; +use Test::More tests => 38; use Test::Warn; use C4::Context; @@ -231,3 +231,71 @@ is( $patron->get_enrollable_clubs->count, is( $club->club_enrollments->count, 1, 'There is 1 enrollment for club' ); $schema->storage->txn_rollback(); + +subtest 'filter_out_empty' => sub { + plan tests => 2; + + $schema->storage->txn_begin(); + + Koha::Clubs->delete; + + my $club_template = $builder->build_object({ class => 'Koha::Club::Templates' }); + + # club_1 has 2 patrons + my $club_1 = $builder->build_object( + { + class => 'Koha::Clubs', + value => { club_template_id => $club_template->id } + } + ); + + # club_2 has 1 patron but they canceled enrollment + my $club_2 = $builder->build_object( + { + class => 'Koha::Clubs', + value => { club_template_id => $club_template->id } + } + ); + + # club_3 is empty + my $club_3 = $builder->build_object( + { + class => 'Koha::Clubs', + value => { club_template_id => $club_template->id } + } + ); + + my $patron_1 = $builder->build_object({ class => 'Koha::Patrons' }); + my $patron_2 = $builder->build_object({ class => 'Koha::Patrons' }); + + my $enrollment_1_1 = Koha::Club::Enrollment->new( + { + club_id => $club_1->id, + borrowernumber => $patron_1->borrowernumber, + branchcode => $patron_1->branchcode, + } + )->store(); + my $enrollment_2_1 = Koha::Club::Enrollment->new( + { + club_id => $club_2->id, + borrowernumber => $patron_1->borrowernumber, + branchcode => $patron_1->branchcode, + } + )->store(); + my $enrollment_1_2 = Koha::Club::Enrollment->new( + { + club_id => $club_1->id, + borrowernumber => $patron_2->borrowernumber, + branchcode => $patron_2->branchcode, + } + )->store(); + + + $enrollment_2_1->cancel; + + my $clubs = Koha::Clubs->search->filter_out_empty; + is( $clubs->count, 1, 'Only one club has patron enrolled' ); + is( $clubs->next->id, $club_1->id, 'Correct club is considered non-empty'); + + $schema->storage->txn_rollback(); +} -- 2.32.0