From 9e28725966466579fb5c1c1274de7d133994138e Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Mon, 11 Jul 2022 17:35:27 -0300
Subject: [PATCH] Bug 22456: (QA follow-up) Consider cancellation requested as
 cancelled in OPAC

This patch adds a helper method for filtering out cancellation
requested-holds from resultsets, and makes use of it in the OPAC to
filter out those in the current holds tab. Holds history now shows
'Waiting (cancellation requested)' on those.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
---
 Koha/Holds.pm                                     | 15 +++++++++++++++
 .../bootstrap/en/includes/holds-table.inc         |  4 +---
 .../bootstrap/en/modules/opac-holdshistory.tt     |  6 +++++-
 opac/opac-user.pl                                 |  2 +-
 t/db_dependent/Koha/Holds.t                       | 13 +++++++++++--
 5 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/Koha/Holds.pm b/Koha/Holds.pm
index 7bedab1a03..4ff90e679e 100644
--- a/Koha/Holds.pm
+++ b/Koha/Holds.pm
@@ -163,6 +163,21 @@ sub filter_by_has_cancellation_requests {
         { join => 'cancellation_requests' } );
 }
 
+=head3 filter_out_has_cancellation_requests
+
+    my $holds_without_cancellation_requests = $holds->filter_out_has_cancellation_requests;
+
+Returns a filtered resultset without holds with cancellation requests.
+
+=cut
+
+sub filter_out_has_cancellation_requests {
+    my ($self) = @_;
+
+    return $self->search( { 'hold_cancellation_request_id' => { '=' => undef } },
+        { join => 'cancellation_requests' } );
+}
+
 =head2 Internal methods
 
 =head3 _type
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc
index c659300642..70b6901034 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc
@@ -156,14 +156,12 @@
                                         <input type="hidden" name="reserve_id" value="[% HOLD.reserve_id | html %]" />
                                         <button data-title="[% INCLUDE 'biblio-title-head.inc' biblio=HOLD.biblio %]" data-reserve_id="[% HOLD.reserve_id | html %]" type="submit" class="btn btn-sm btn-danger btn-delete-hold"><i class="fa fa-remove" aria-hidden="true"></i> [% tp('Cancel hold button', 'Cancel') | html %]</button>
                                     </form>
-                                [% ELSIF HOLD.cancellation_requests.count > 0 %]
-                                    <button type="submit" class="btn btn-sm btn-danger disabled"><i class="fa fa-remove" aria-hidden="true"></i> [% t('Cancellation requested') | html %]</button>
                                 [% ELSIF HOLD.is_waiting && HOLD.cancellation_requestable_from_opac %]
                                     <form id="req_cancel_hold_[% HOLD.reserve_id | html %]" action="/cgi-bin/koha/opac-modrequest.pl" method="post">
                                         <input type="hidden" name="biblionumber" value="[% HOLD.biblionumber | html %]" />
                                         <input type="hidden" name="reserve_id" value="[% HOLD.reserve_id | html %]" />
                                         <input type="hidden" name="cancellation_request" value="1" />
-                                        <button data-title="[% INCLUDE 'biblio-title-head.inc' biblio=HOLD.biblio %]" data-reserve_id="[% HOLD.reserve_id | html %]" type="submit" class="btn btn-sm btn-danger btn-req-delete-hold"><i class="fa fa-remove" aria-hidden="true"></i> [% tp('Cancellation request hold button', 'Request cancellation') | html %]</button>
+                                        <button data-title="[% INCLUDE 'biblio-title-head.inc' biblio=HOLD.biblio %]" data-reserve_id="[% HOLD.reserve_id | html %]" type="submit" class="btn btn-sm btn-danger btn-req-delete-hold"><i class="fa fa-remove" aria-hidden="true"></i> [% tp('Cancel hold button', 'Cancel') | html %]</button>
                                     </form>
                                 [% END %]
                             </td>
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-holdshistory.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-holdshistory.tt
index d42eb7f18f..8171142628 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-holdshistory.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-holdshistory.tt
@@ -153,7 +153,11 @@
                                         [% ELSIF hold.cancellationdate %]
                                             <span>Cancelled</span>
                                         [% ELSIF hold.found == 'W' %]
-                                            <span>Waiting</span>
+                                            [% IF hold.cancellation_requests.count == 0 %]
+                                                <span>Waiting</span>
+                                            [% ELSE %]
+                                                <span>Cancelled</span>
+                                            [% END %]
                                         [% ELSIF hold.found == 'T' %]
                                             <span>In transit</span>
                                         [% ELSE %]
diff --git a/opac/opac-user.pl b/opac/opac-user.pl
index 147e7d9109..ba45ddd237 100755
--- a/opac/opac-user.pl
+++ b/opac/opac-user.pl
@@ -326,7 +326,7 @@ if ($show_barcode) {
 $template->param( show_barcode => 1 ) if $show_barcode;
 
 # now the reserved items....
-my $reserves = Koha::Holds->search( { borrowernumber => $borrowernumber } );
+my $reserves = $patron->holds->filter_out_has_cancellation_requests;
 
 $template->param(
     RESERVES       => $reserves,
diff --git a/t/db_dependent/Koha/Holds.t b/t/db_dependent/Koha/Holds.t
index 188ffd51c6..aec261dd1f 100755
--- a/t/db_dependent/Koha/Holds.t
+++ b/t/db_dependent/Koha/Holds.t
@@ -644,9 +644,9 @@ subtest 'set_waiting+patron_expiration_date' => sub {
 
 $schema->storage->txn_rollback;
 
-subtest 'filter_by_has_cancellation_requests() tests' => sub {
+subtest 'filter_by_has_cancellation_requests() and filter_out_has_cancellation_requests() tests' => sub {
 
-    plan tests => 4;
+    plan tests => 7;
 
     $schema->storage->txn_begin;
 
@@ -699,6 +699,10 @@ subtest 'filter_by_has_cancellation_requests() tests' => sub {
 
     is( $filtered_rs->count, 0 );
 
+    my $filtered_out_rs = $rs->filter_out_has_cancellation_requests;
+
+    is( $filtered_out_rs->count, 3 );
+
     $hold_2->add_cancellation_request;
 
     $filtered_rs = $rs->filter_by_has_cancellation_requests;
@@ -706,5 +710,10 @@ subtest 'filter_by_has_cancellation_requests() tests' => sub {
     is( $filtered_rs->count,    1 );
     is( $filtered_rs->next->id, $hold_2->id );
 
+    $filtered_out_rs = $rs->filter_out_has_cancellation_requests;
+
+    is( $filtered_out_rs->count,    2 );
+    is( $filtered_out_rs->next->id, $hold_1->id );
+
     $schema->storage->txn_rollback;
 };
-- 
2.39.5