From bdd383358250b6be0032db025db1f27943637d6b Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
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 <tomascohen@theke.io>
---
 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